Commit graph

69 commits

Author SHA1 Message Date
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Junio C Hamano
6a5ff7acb5 Merge branch 'jk/pager-in-use'
Code clean-up.

* jk/pager-in-use:
  pager_in_use: use git_env_bool()
2017-03-28 14:05:59 -07:00
Jeff King
df2a6e38b7 pager_in_use: use git_env_bool()
The pager_in_use() function predates git_env_bool(), but
ends up doing the same thing.  Let's make use of the latter,
which is shorter and less repetitive.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-24 12:14:11 -07:00
Johannes Schindelin
0654aa57f3 setup: make read_early_config() reusable
The pager configuration needs to be read early, possibly before
discovering any .git/ directory.

Let's not hide this function in pager.c, but make it available to other
callers.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:16 -07:00
Junio C Hamano
d845d727cb Merge branch 'jk/setup-sequence-update'
There were numerous corner cases in which the configuration files
are read and used or not read at all depending on the directory a
Git command was run, leading to inconsistent behaviour.  The code
to set-up repository access at the beginning of a Git process has
been updated to fix them.

* jk/setup-sequence-update:
  t1007: factor out repeated setup
  init: reset cached config when entering new repo
  init: expand comments explaining config trickery
  config: only read .git/config from configured repos
  test-config: setup git directory
  t1302: use "git -C"
  pager: handle early config
  pager: use callbacks instead of configset
  pager: make pager_program a file-local static
  pager: stop loading git_default_config()
  pager: remove obsolete comment
  diff: always try to set up the repository
  diff: handle --no-index prefixes consistently
  diff: skip implicit no-index check when given --no-index
  patch-id: use RUN_SETUP_GENTLY
  hash-object: always try to set up the git repository
2016-09-21 15:15:24 -07:00
Jeff King
eed2707202 pager: handle early config
The pager code is often run early in the git.c startup,
before we have actually found the repository. When we ask
git_config() to look for values like core.pager, it doesn't
know where to find the repo-level config, and will blindly
examine ".git/config" if it exists. That's why t7006 shows
that many pager-related features happen to work from the
top-level of a repository, but not from a subdirectory.

This patch pulls that ".git/config" hack explicitly into the
pager code. There are two reasons for this:

  1. We'd like to clean up the git_config() behavior, as
     looking at ".git/config" when we do not have a
     configured repository is often the wrong thing to do.
     But we'd prefer not to break the pager config any worse
     than it already is.

  2. It's one very tiny step on the road to ultimately
     making the pager config work consistently. If we
     eventually get an equivalent of setup_git_directory()
     that _just_ finds the directory and doesn't chdir() or
     set up any global state, we could plug it in here
     (instead of blindly looking at ".git/config").

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 15:45:45 -07:00
Jeff King
6a1e1bc0a1 pager: use callbacks instead of configset
While the cached configset interface is more pleasant to
use, it is not appropriate for "early" config like pager
setup, which must sometimes do tricky things like reading
from ".git/config" even when we have not set up the
repository.

As a preparatory step to handling these cases better, let's
switch back to using the callback interface, which gives us
more control.

Note that this is essentially a revert of 586f414 (pager.c:
replace `git_config()` with `git_config_get_value()`,
2014-08-07), but with some minor style fixups and
modernizations.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 15:45:45 -07:00
Jeff King
c0c08897c4 pager: make pager_program a file-local static
This variable is only ever used by the routines in pager.c,
and other parts of the code should always use those routines
(like git_pager()) to make decisions about which pager to
use. Let's reduce its scope to prevent accidents.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 15:45:45 -07:00
Jeff King
4babb839aa pager: stop loading git_default_config()
In git_pager(), we really only care about getting the value
of core.pager. But to do so, we use the git_default_config()
callback, which loads many other values. Ordinarily it
isn't a big deal to load this config an extra time, as it
simply overwrites the values from the previous run. But it's
a bad idea here, for two reasons:

  1. The pager setup may be called very early in the
     program, before we have found the git repository. As a
     result, we may fail to read the correct repo-level
     config file.  This is a problem for core.pager, too,
     but we should at least try to minimize the pollution to
     other configured values.

  2. Because we call setup_pager() from git.c, basically
     every builtin command _may_ end up reading this config
     and getting an implicit git_default_config() setup.

     Which doesn't sound like a terrible thing, except that
     we don't do it consistently; it triggers only when
     stdout is a tty. So if a command forgets to load the
     default config itself (but depends on it anyway), it
     may appear to work, and then mysteriously fail when the
     pager is not in use.

We can improve this by loading _just_ the core.pager config
from git_pager().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 15:45:45 -07:00
Jeff King
01e7e90359 pager: remove obsolete comment
The comment at the top of pager.c claims that we've split
the code out so that Windows can do something different.
This dates back to f67b45f (Introduce trivial new pager.c
helper infrastructure, 2006-02-28), because the original
implementation used fork(). Later, we ended up sticking the
Windows #ifdefs into this file anyway. And then even later,
in ea27a18 (spawn pager via run_command interface,
2008-07-22) we unified the implementations.

So these days this comment is really saying nothing at all.
Let's drop it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-13 15:45:45 -07:00
Eric Wong
995bc22d7f pager: move pager-specific setup into the build
Allowing PAGER_ENV to be set at build-time allows us to move
pager-specific knowledge out of our build.  This allows us to
set a better default for FreeBSD more(1), which pretends not to
understand ANSI color escapes if the MORE environment variable
is left empty, but accepts the same variables as less(1)

Originally-from:
 https://public-inbox.org/git/xmqq61piw4yf.fsf@gitster.dls.corp.google.com/

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Eric Wong <e@80x24.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-04 13:51:02 -07:00
Junio C Hamano
c3b1e8d851 Merge branch 'jc/am-i-v-fix'
The "v(iew)" subcommand of the interactive "git am -i" command was
broken in 2.6.0 timeframe when the command was rewritten in C.

* jc/am-i-v-fix:
  am -i: fix "v"iew
  pager: factor out a helper to prepare a child process to run the pager
  pager: lose a separate argv[]
2016-02-24 13:26:01 -08:00
Junio C Hamano
3e3a4a41b0 pager: factor out a helper to prepare a child process to run the pager
When running a pager, we need to run the program git_pager() gave
us, but we need to make sure we spawn it via the shell (i.e. it is
valid to say PAGER='less -S', for example) and give default values
to $LESS and $LV environment variables.  Factor out these details
to a separate helper function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-17 09:19:15 -08:00
Junio C Hamano
43b0190224 pager: lose a separate argv[]
These days, using the embedded args array in the child_process
structure is the norm.  Follow that practice.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-16 14:26:40 -08:00
Junio C Hamano
2b72dbbcf3 Merge branch 'ti/glibc-stdio-mutex-from-signal-handler'
Allocation related functions and stdio are unsafe things to call
inside a signal handler, and indeed killing the pager can cause
glibc to deadlock waiting on allocation mutex as our signal handler
tries to free() some data structures in wait_for_pager().  Reduce
these unsafe calls.

* ti/glibc-stdio-mutex-from-signal-handler:
  pager: don't use unsafe functions in signal handlers
2015-10-07 13:38:16 -07:00
Takashi Iwai
507d7804c0 pager: don't use unsafe functions in signal handlers
Since the commit a3da882120 (pager: do wait_for_pager on signal
death), we call wait_for_pager() in the pager's signal handler.  The
recent bug report revealed that this causes a deadlock in glibc at
aborting "git log" [*1*].  When this happens, git process is left
unterminated, and it can't be killed by SIGTERM but only by SIGKILL.

The problem is that wait_for_pager() function does more than waiting
for pager process's termination, but it does cleanups and printing
errors.  Unfortunately, the functions that may be used in a signal
handler are very limited [*2*].  Particularly, malloc(), free() and the
variants can't be used in a signal handler because they take a mutex
internally in glibc.  This was the cause of the deadlock above.  Other
than the direct calls of malloc/free, many functions calling
malloc/free can't be used.  strerror() is such one, either.

Also the usage of fflush() and printf() in a signal handler is bad,
although it seems working so far.  In a safer side, we should avoid
them, too.

This patch tries to reduce the calls of such functions in signal
handlers.  wait_for_signal() takes a flag and avoids the unsafe
calls.   Also, finish_command_in_signal() is introduced for the
same reason.  There the free() calls are removed, and only waits for
the children without whining at errors.

[*1*] https://bugzilla.opensuse.org/show_bug.cgi?id=942297
[*2*] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-04 14:57:51 -07:00
Junio C Hamano
7b7c10bf5e Merge branch 'jk/fix-alias-pager-config-key-warnings'
Because the configuration system does not allow "alias.0foo" and
"pager.0foo" as the configuration key, the user cannot use '0foo'
as a custom command name anyway, but "git 0foo" tried to look these
keys up and emitted useless warnings before saying '0foo is not a
git command'.  These warning messages have been squelched.

* jk/fix-alias-pager-config-key-warnings:
  config: silence warnings for command names with invalid keys
2015-08-31 15:38:57 -07:00
Jeff King
9e9de18f1a config: silence warnings for command names with invalid keys
When we are running the git command "foo", we may have to
look up the config keys "pager.foo" and "alias.foo". These
config schemes are mis-designed, as the command names can be
anything, but the config syntax has some restrictions. For
example:

  $ git foo_bar
  error: invalid key: pager.foo_bar
  error: invalid key: alias.foo_bar
  git: 'foo_bar' is not a git command. See 'git --help'.

You cannot name an alias with an underscore. And if you have
an external command with one, you cannot configure its
pager.

In the long run, we may develop a different config scheme
for these features. But in the near term (and because we'll
need to support the existing scheme indefinitely), we should
at least squelch the error messages shown above.

These errors come from git_config_parse_key. Ideally we
would pass a "quiet" flag to the config machinery, but there
are many layers between the pager code and the key parsing.
Passing a flag through all of those would be an invasive
change.

Instead, let's provide a config function to report on
whether a key is syntactically valid, and have the pager and
alias code skip lookup for bogus keys. We can build this
easily around the existing git_config_parse_key, with two
minor modifications:

  1. We now handle a NULL store_key, to validate but not
     write out the normalized key.

  2. We accept a "quiet" flag to avoid writing to stderr.
     This doesn't need to be a full-blown public "flags"
     field, because we can make the existing implementation
     a static helper function, keeping the mess contained
     inside config.c.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-24 08:52:23 -07:00
Junio C Hamano
6cf7eef384 Merge branch 'jc/unexport-git-pager-in-use-in-pager'
When you say "!<ENTER>" while running say "git log", you'd confuse
yourself in the resulting shell, that may look as if you took
control back to the original shell you spawned "git log" from but
that isn't what is happening.  To that new shell, we leaked
GIT_PAGER_IN_USE environment variable that was meant as a local
communication between the original "Git" and subprocesses that was
spawned by it after we launched the pager, which caused many
"interesting" things to happen, e.g. "git diff | cat" still paints
its output in color by default.

Stop leaking that environment variable to the pager's half of the
fork; we only need it on "Git" side when we spawn the pager.

* jc/unexport-git-pager-in-use-in-pager:
  pager: do not leak "GIT_PAGER_IN_USE" to the pager
2015-07-13 14:00:27 -07:00
Junio C Hamano
124b51909d pager: do not leak "GIT_PAGER_IN_USE" to the pager
Since 2e6c012e (setup_pager: set GIT_PAGER_IN_USE, 2011-08-17), we
export GIT_PAGER_IN_USE so that a process that becomes the upstream
of the spawned pager can still tell that we have spawned the pager
and decide to do colored output even when its output no longer goes
to a terminal (i.e. isatty(1)).

But we forgot to clear it from the enviornment of the spawned pager.

This is not a problem in a sane world, but if you have a handful of
thousands Git users in your organization, somebody is bound to do
strange things, e.g. typing "!<ENTER>" instead of 'q' to get control
back from $LESS.  GIT_PAGER_IN_USE is still set in that subshell
spawned by "less", and all sorts of interesting things starts
happening, e.g. "git diff | cat" starts coloring its output.

We can clear the environment variable in the half of the fork that
runs the pager to avoid the confusion.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-03 18:07:21 -07:00
Junio C Hamano
ca00db08da Merge branch 'jk/decimal-width-for-uintmax'
We didn't format an integer that wouldn't fit in "int" but in
"uintmax_t" correctly.

* jk/decimal-width-for-uintmax:
  decimal_width: avoid integer overflow
2015-02-18 11:45:02 -08:00
Jeff King
d306f3d351 decimal_width: avoid integer overflow
The decimal_width function originally appeared in blame.c as
"lineno_width", and was designed for calculating the
print-width of small-ish integer values (line numbers in
text files). In ec7ff5b, it was made into a reusable
function, and in dc801e7, we started using it to align
diffstats.

Binary files in a diffstat show byte counts rather than line
numbers, meaning they can be quite large (e.g., consider
adding or removing a 2GB file). decimal_width is not up to
the challenge for two reasons:

  1. It takes the value as an "int", whereas large files may
     easily surpass this. The value may be truncated, in
     which case we will produce an incorrect value.

  2. It counts "up" by repeatedly multiplying another
     integer by 10 until it surpasses the value.  This can
     cause an infinite loop when the value is close to the
     largest representable integer.

     For example, consider using a 32-bit signed integer,
     and a value of 2,140,000,000 (just shy of 2^31-1).
     We will count up and eventually see that 1,000,000,000
     is smaller than our value. The next step would be to
     multiply by 10 and see that 10,000,000,000 is too
     large, ending the loop. But we can't represent that
     value, and we have signed overflow.

     This is technically undefined behavior, but a common
     behavior is to lose the high bits, in which case our
     iterator will certainly be less than the number. So
     we'll keep multiplying, overflow again, and so on.

This patch changes the argument to a uintmax_t (the same
type we use to store the diffstat information for binary
filese), and counts "down" by repeatedly dividing our value
by 10.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-02-05 12:38:35 -08:00
René Scharfe
a915459097 use env_array member of struct child_process
Convert users of struct child_process to using the managed env_array for
specifying environment variables instead of supplying an array on the
stack or bringing their own argv_array.  This shortens and simplifies
the code and ensures automatically that the allocated memory is freed
after use.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-19 15:26:34 -07:00
Junio C Hamano
825fd93767 Merge branch 'rs/child-process-init'
Code clean-up.

* rs/child-process-init:
  run-command: inline prepare_run_command_v_opt()
  run-command: call run_command_v_opt_cd_env() instead of duplicating it
  run-command: introduce child_process_init()
  run-command: introduce CHILD_PROCESS_INIT
2014-09-11 10:33:27 -07:00
René Scharfe
d318027932 run-command: introduce CHILD_PROCESS_INIT
Most struct child_process variables are cleared using memset first after
declaration.  Provide a macro, CHILD_PROCESS_INIT, that can be used to
initialize them statically instead.  That's shorter, doesn't require a
function call and is slightly more readable (especially given that we
already have STRBUF_INIT, ARGV_ARRAY_INIT etc.).

Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-20 09:53:37 -07:00
Tanay Abhra
586f414a89 pager.c: replace git_config() with git_config_get_value()
Use `git_config_get_value()` instead of `git_config()` to take advantage of
the config-set API which provides a cleaner control flow.

Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-07 13:33:28 -07:00
Junio C Hamano
db6fbe3770 Merge branch 'je/pager-do-not-recurse'
We used to unconditionally disable the pager in the pager process
we spawn to feed out output, but that prevented people who want to
run "less" within "less" from doing so.

* je/pager-do-not-recurse:
  pager: do allow spawning pager recursively
2014-06-06 11:17:00 -07:00
Matthieu Moy
b3275838d9 pager: remove 'S' from $LESS by default
By default, Git used to set $LESS to -FRSX if $LESS was not set by
the user. The FRX flags actually make sense for Git (F and X because
sometimes the output Git pipes to less is short, and R because Git
pipes colored output). The S flag (chop long lines), on the other
hand, is not related to Git and is a matter of user preference. Git
should not decide for the user to change LESS's default.

More specifically, the S flag harms users who review untrusted code
within a pager, since a patch looking like:

    -old code;
    +new good code; [... lots of tabs ...] malicious code;

would appear identical to:

    -old code;
    +new good code;

Users who prefer the old behavior can still set the $LESS environment
variable to -FRSX explicitly, or set core.pager to 'less -S'.

The documentation in config.txt is made a bit longer to keep both an
example setting the 'S' flag (needed to recover the old behavior)
and an example showing how to unset a flag set by Git.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-05-07 13:41:04 -07:00
Jörn Engel
c0459ca4dc pager: do allow spawning pager recursively
This reverts commit 88e8f908f2, which
tried to allow

    GIT_PAGER="git -p column --mode='dense color'" git -p branch

and still wanted to avoid "git -p column" to invoke itself.  However,
this falls into "don't do that -p then" category.

In particular, inside "git log", with results going through less, a
potentially interesting commit may be found and from there inside
"less", the user may want to execute "git show <commit>".  Before
the commit being reverted, this used to show the patch in less but
it no longer does.

Signed-off-by: Jörn Engel <joern@logfs.org>
Reviewed-by: Jeff King <peff@peff.net>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Acked-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-28 16:03:22 -07:00
Junio C Hamano
9fac0777e1 Merge branch 'jn/pager-lv-default-env'
Just like we give a reasonable default for "less" via the LESS
environment variable, specify a reasonable default for "lv" via the
"LV" environment variable when spawning the pager.

* jn/pager-lv-default-env:
  pager: set LV=-c alongside LESS=FRSX
2014-01-13 11:33:35 -08:00
Jonathan Nieder
e54c1f2d25 pager: set LV=-c alongside LESS=FRSX
On systems with lv configured as the preferred pager (i.e.,
DEFAULT_PAGER=lv at build time, or PAGER=lv exported in the
environment) git commands that use color show control codes instead of
color in the pager:

	$ git diff
	^[[1mdiff --git a/.mailfilter b/.mailfilter^[[m
	^[[1mindex aa4f0b2..17e113e 100644^[[m
	^[[1m--- a/.mailfilter^[[m
	^[[1m+++ b/.mailfilter^[[m
	^[[36m@@ -1,11 +1,58 @@^[[m

"less" avoids this problem because git uses the LESS environment
variable to pass the -R option ('output ANSI color escapes in raw
form') by default.  Use the LV environment variable to pass 'lv' the
-c option ('allow ANSI escape sequences for text decoration / color')
to fix it for lv, too.

Noticed when the default value for color.ui flipped to 'auto' in
v1.8.4-rc0~36^2~1 (2013-06-10).

Reported-by: Olaf Meeuwissen <olaf.meeuwissen@avasys.jp>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-01-07 09:23:41 -08:00
Christian Couder
5955654823 replace {pre,suf}fixcmp() with {starts,ends}_with()
Leaving only the function definitions and declarations so that any
new topic in flight can still make use of the old functions, replace
existing uses of the prefixcmp() and suffixcmp() with new API
functions.

The change can be recreated by mechanically applying this:

    $ git grep -l -e prefixcmp -e suffixcmp -- \*.c |
      grep -v strbuf\\.c |
      xargs perl -pi -e '
        s|!prefixcmp\(|starts_with\(|g;
        s|prefixcmp\(|!starts_with\(|g;
        s|!suffixcmp\(|ends_with\(|g;
        s|suffixcmp\(|!ends_with\(|g;
      '

on the result of preparatory changes in this series.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-05 14:13:21 -08:00
Jeff King
ed016612e6 pager: turn on "cat" optimization for DEFAULT_PAGER
If the user specifies a pager of "cat" (or the empty
string), whether it is in the environment or from config, we
automagically optimize it out to mean "no pager" and avoid
forking at all. We treat an empty pager variable similary.

However, we did not apply this optimization when
DEFAULT_PAGER was set to "cat" (or the empty string). There
is no reason to treat DEFAULT_PAGER any differently. The
optimization should not be user-visible (unless the user has
a bizarre "cat" in their PATH). And even if it is, we are
better off behaving consistently between the compile-time
default and the environment and config settings.

The stray "else" we are removing from this code was
introduced by 402461a (pager: do not fork a pager if PAGER
is set to empty., 2006-04-16). At that time, the line
directly above used:

   if (!pager)
	   pager = "less";

as a fallback, meaning that it could not possibly trigger
the optimization. Later, a3d023d (Provide a build time
default-pager setting, 2009-10-30) turned that constant into
a build-time setting which could be anything, but didn't
loosen the "else" to let DEFAULT_PAGER use the optimization.

Noticed-by: Dale R. Worley <worley@alum.mit.edu>
Suggested-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-09-03 10:36:12 -07:00
Nguyễn Thái Ngọc Duy
4914c9629c Move setup_diff_pager to libgit.a
This is used by diff-no-index.c, part of libgit.a while it stays in
builtin/diff.c. Move it to diff.c so that we won't get undefined
reference if a program that uses libgit.a happens to pull it in.

While at it, move check_pager from git.c to pager.c. It makes more
sense there and pager.c is also part of libgit.a

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
2012-10-29 03:08:30 -04:00
Jeff King
e8320f350f pager: drop "wait for output to run less" hack
Commit 35ce862 (pager: Work around window resizing bug in
'less', 2007-01-24) causes git's pager sub-process to wait
to receive input after forking but before exec-ing the
pager. To handle this, run-command had to grow a "pre-exec
callback" feature. Unfortunately, this feature does not work
at all on Windows (where we do not fork), and interacts
poorly with run-command's parent notification system. Its
use should be discouraged.

The bug in less was fixed in version 406, which was released
in June 2007. It is probably safe at this point to remove
our workaround. That lets us rip out the preexec_cb feature
entirely.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-05 09:38:00 -07:00
Nguyễn Thái Ngọc Duy
88e8f908f2 Stop starting pager recursively
git-column can be used as a pager for other git commands, something
like this:

    GIT_PAGER="git -p column --mode='dense color'" git -p branch

The problem with this is that "git -p column" also has $GIT_PAGER set so
the pager runs itself again as another pager. The end result is an
infinite loop of forking. Other git commands have the same problem if
being abused this way.

Check if $GIT_PAGER is already set and stop launching another pager.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-27 09:26:38 -07:00
Junio C Hamano
4d9e079e82 Merge branch 'zj/decimal-width'
* zj/decimal-width:
  make lineno_width() from blame reusable for others

Conflicts:
	cache.h
	pager.c
2012-02-20 00:15:11 -08:00
Zbigniew Jędrzejewski-Szmek
ec7ff5ba27 make lineno_width() from blame reusable for others
builtin/blame.c has a helper function to compute how many columns
we need to show a line-number, whose implementation is reusable as
a more generic helper function to count the number of columns
necessary to show any cardinal number.

Rename it to decimal_width(), move it to pager.c and export it for
use by future callers.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-02-14 16:16:19 -08:00
Zbigniew Jędrzejewski-Szmek
ad6c3739a3 pager: find out the terminal width before spawning the pager
term_columns() checks for terminal width via ioctl(2) on the standard
output, but we spawn the pager too early for this check to be useful.

The effect of this buglet can be observed by opening a wide terminal and
running "git -p help --all", which still shows 80-column output, while
"git help --all" uses the full terminal width. Run the check before we
spawn the pager to fix this.

While at it, move term_columns() to pager.c and export it from cache.h so
that callers other than the help subsystem can use it.

Signed-off-by: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-02-13 15:08:47 -08:00
Jeff King
2e6c012e10 setup_pager: set GIT_PAGER_IN_USE
We have always set a global "spawned_pager" variable when we
start the pager. This lets us make the auto-color decision
later in the program as as "we are outputting to a terminal,
or to a pager which can handle colors".

Commit 6e9af86 added support for the GIT_PAGER_IN_USE
environment variable. An external program calling git (e.g.,
git-svn) could set this variable to indicate that it had
already started the pager, and that the decision about
auto-coloring should take that into account.

However, 6e9af86 failed to do the reverse, which is to tell
external programs when git itself has started the pager.
Thus a git command implemented as an external script that
has the pager turned on (e.g., "git -p stash show") would
not realize it was going to a pager, and would suppress
colors.

This patch remedies that; we always set GIT_PAGER_IN_USE
when we start the pager, and the value is respected by both
this program and any spawned children.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-08-18 14:17:12 -07:00
Jonathan Nieder
64778d24a9 Make 'git var GIT_PAGER' always print the configured pager
Scripted commands that want to use git’s configured pager know better
than ‘git var’ does whether stdout is going to be a tty at the
appropriate time.  Checking isatty(1) as git_pager() does now won’t
cut it, since the output of git var itself is almost never a terminal.
The symptom is that when used by humans, ‘git var GIT_PAGER’ behaves
as it should, but when used by scripts, it always returns ‘cat’!

So avoid tricks with isatty() and just always print the configured
pager.

This does not fix the callers to check isatty(1) themselves yet.
Nevertheless, this patch alone is enough to fix 'am --interactive'.

Thanks to Sebastian Celis for the report and Jeff King for the
analysis.

Reported-by: Sebastian Celis <sebastian@sebastiancelis.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-14 18:23:17 -08:00
Jeff King
ac0ba18df0 run-command: convert simple callsites to use_shell
Now that we have the use_shell feature, these callsites can
all be converted with small changes.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-05 23:41:50 -08:00
Junio C Hamano
a3d023d0a3 Provide a build time default-pager setting
Provide a DEFAULT_PAGER knob so packagers can set the fallback
pager to something appropriate during the build.

Examples:

On (old) solaris systems, /usr/bin/less (typically the first less
found) doesn't understand the default arguments (FXRS), which
forces users to alter their environment (PATH, GIT_PAGER, LESS,
etc) or have a local or global gitconfig before paging works as
expected.

On Debian systems, by policy packages must fall back to the
'pager' command, so that changing the target of the
/usr/bin/pager symlink changes the default pager for all packages
at once.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-13 12:20:56 -08:00
Jonathan Nieder
6361824589 Teach git var about GIT_PAGER
Expose the command found by setup_pager() for scripts to use.
Scripts can use this to avoid repeating the logic to look for a
proper pager in each command.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-11-13 12:20:47 -08:00
Frank Li
71064e3f86 Test for WIN32 instead of __MINGW32_
The code which is conditional on MinGW32 is actually conditional on Windows.
Use the WIN32 symbol, which is defined by the MINGW32 and MSVC environments,
but not by Cygwin.

Define SNPRINTF_SIZE_CORR=1 for MSVC too, as its vsnprintf function does
not add NUL at the end of the buffer if the result fits the buffer size
exactly.

Signed-off-by: Frank Li <lznuaa@gmail.com>
Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
Acked-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-18 20:00:42 -07:00
Johannes Sixt
25fc1786ab pager: set LESS=FRSX also on Windows
Previously, this environment variable was set in the pager_preexec
callback, which is conditionally-compiled only on Unix, because it is not,
and cannot be, called on Windows.

With this patch the env member of struct child_process is used to set
the environment variable, which also works on Windows.

Noticed by Alexey Borzenkov.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-09-11 16:35:08 -07:00
Jeff King
a3da882120 pager: do wait_for_pager on signal death
Since ea27a18 (spawn pager via run_command interface), the
original git process actually does git work, and the pager
is a child process (actually, on Windows it has always been
that way, since Windows lacks fork). After spawning the
pager, we register an atexit() handler that waits for the
pager to finish.

Unfortunately, that handler does not always run. In
particular, if git is killed by a signal, then we exit
immediately. The calling shell then thinks that git is done;
however, the pager is still trying to run and impact the
terminal. The result can be seen by running a long git
process with a pager (e.g., "git log -p") and hitting ^C.
Depending on your config, you should see the shell prompt,
but pressing a key causes the pager to do any terminal
de-initialization sequence.

This patch just intercepts any death-dealing signals and
waits for the pager before dying. Under typical less
configuration, that means hitting ^C will cause git to stop
generating output, but the pager will keep running.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-21 22:46:53 -08:00
Junio C Hamano
90c3302173 Merge branch 'maint'
* maint:
  fast-import: close pack before unlinking it
  pager: do not dup2 stderr if it is already redirected
  git-show: do not segfault when showing a bad tag
2008-12-15 23:06:13 -08:00
Junio C Hamano
a8335024c2 pager: do not dup2 stderr if it is already redirected
An earlier commit 61b8050 (sending errors to stdout under $PAGER,
2008-02-16) avoided losing the error messages that are sent to the
standard error when $PAGER is in effect by dup2'ing fd 2 to the pager.
his way, showing a tag object that points to a bad object:

    $ git show tag-foo

would give the error message to the pager.  However, it was not quite
right if the user did:

    $ git show 2>error.log tag-foo

i.e. use the pager but store the errors in a separate file.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-12-15 01:37:15 -08:00
Jeff King
ea27a18ce2 spawn pager via run_command interface
This has two important effects:

 1. The pager is now the _child_ process, instead of the
    parent. This means that whatever spawned git (e.g., the
    shell) will see the exit code of the git process, and
    not the pager.

 2. The mingw and regular code are now unified, which makes
    the setup_pager function much simpler.

There are two caveats:

 1. We used to call execlp directly on the pager, followed
    by trying to exec it via the shall. We now just use the
    shell (which is what mingw has always done). This may
    have different results for pager names which contain
    shell metacharacters.

    It is also slightly less efficient because we
    unnecessarily run the shell; however, pager spawning is
    by definition an interactive task, so it shouldn't be
    a huge problem.

 2. The git process will remain in memory while the user
    looks through the pager. This is potentially wasteful.
    We could get around this by turning the parent into a
    meta-process which spawns _both_ git and the pager,
    collects the exit status from git, waits for both to
    end, and then exits with git's exit code.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-25 21:29:44 -07:00