Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Junio C Hamano]

Jeff King  writes:

> The outer git wrapper doesn't start the pager, so its stderr still gets
> seen by the user. But the _inner_ git-log does start the pager, and then
> dies of SIGPIPE.
>
> So yeah, I think we want something like this on top of
> nd/clear-gitenv-upon-use-of-alias.

That makes sense to me.

> -- >8 --
> Subject: [PATCH] run-command: don't warn on SIGPIPE deaths
>
> When git executes a sub-command, we print a warning if the
> command dies due to a signal, but make an exception for
> "uninteresting" cases like SIGINT and SIGQUIT (since the
> user presumably just hit ^C).
>
> We should make a similar exception for SIGPIPE, because it's
> an expected and uninteresting return in most cases; it
> generally means the user quit the pager before git had
> finished generating all output.  This used to be very hard
> to trigger in practice, because:
>
>   1. We only complain if we see a real SIGPIPE death, not
>  the shell-induced 141 exit code. This means that
>  anything we run via the shell does not trigger the
>  warning, which includes most non-trivial aliases.
>
>   2. The common case for SIGPIPE is the user quitting the
>  pager before git has finished generating all output.
>  But if the user triggers a pager with "-p", we redirect
>  the git wrapper's stderr to that pager, too.  Since the
>  pager is dead, it means that the message goes nowhere.
>
>   3. You can see it if you run your own pager, like
>  "git foo | head". But that only happens if "foo" is a
>  non-builtin (so it doesn't work with "log", for
>  example).
>
> However, it may become more common after 86d26f2, which
> teaches alias to re-exec builtins rather than running them
> in the same process. This case doesn't trigger (1), as we
> don't need a shell to run a git command. It doesn't trigger
> (2), because the pager is not started by the original git,
> but by the inner re-exec of git. And it doesn't trigger (3),
> because builtins are treated more like non-builtins in this
> case.
>
> Given how flaky this message already is (e.g., you cannot
> even know whether you will see it, as git optimizes out some
> shell invocations behind the scenes based on the contents of
> the command!), and that it is unlikely to ever provide
> useful information, let's suppress it for all cases of
> SIGPIPE.
>
> Signed-off-by: Jeff King 
> ---
>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index 13fa452..694a6ff 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -245,7 +245,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, 
> int in_signal)
>   error("waitpid is confused (%s)", argv0);
>   } else if (WIFSIGNALED(status)) {
>   code = WTERMSIG(status);
> - if (code != SIGINT && code != SIGQUIT)
> + if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
>   error("%s died of signal %d", argv0, code);
>   /*
>* This return value is chosen so that code & 0xff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Jeff King]

On Thu, Dec 24, 2015 at 04:35:33PM +0700, Duy Nguyen wrote:

> On Thu, Dec 24, 2015 at 4:31 AM, Jeff King  wrote:
> >   2. I doubt anybody is actually seeing this in practice anymore. But
> >  maybe I am misunderstanding something in Duy's series that changes
> >  this.
> 
> There are two parts in your patch, one (that you two seemed to focus
> on) about return code with "!" aliases. Another, suppressing SIGPIPE,
> affects more than "!" aliases.

Sorry if I was confusing; most of the examples in my previous message
are about the SIGPIPE thing. I was having trouble triggering the message
in practice, even for externals (because the error message goes to the
pager, too!).

> In my case it's execv_dashed_external(). Non-"!" aliases are now
> forced to use that function.

Thanks, this is the part I was missing.

The outer git wrapper doesn't start the pager, so its stderr still gets
seen by the user. But the _inner_ git-log does start the pager, and then
dies of SIGPIPE.

So yeah, I think we want something like this on top of
nd/clear-gitenv-upon-use-of-alias.

-- >8 --
Subject: [PATCH] run-command: don't warn on SIGPIPE deaths

When git executes a sub-command, we print a warning if the
command dies due to a signal, but make an exception for
"uninteresting" cases like SIGINT and SIGQUIT (since the
user presumably just hit ^C).

We should make a similar exception for SIGPIPE, because it's
an expected and uninteresting return in most cases; it
generally means the user quit the pager before git had
finished generating all output.  This used to be very hard
to trigger in practice, because:

  1. We only complain if we see a real SIGPIPE death, not
 the shell-induced 141 exit code. This means that
 anything we run via the shell does not trigger the
 warning, which includes most non-trivial aliases.

  2. The common case for SIGPIPE is the user quitting the
 pager before git has finished generating all output.
 But if the user triggers a pager with "-p", we redirect
 the git wrapper's stderr to that pager, too.  Since the
 pager is dead, it means that the message goes nowhere.

  3. You can see it if you run your own pager, like
 "git foo | head". But that only happens if "foo" is a
 non-builtin (so it doesn't work with "log", for
 example).

However, it may become more common after 86d26f2, which
teaches alias to re-exec builtins rather than running them
in the same process. This case doesn't trigger (1), as we
don't need a shell to run a git command. It doesn't trigger
(2), because the pager is not started by the original git,
but by the inner re-exec of git. And it doesn't trigger (3),
because builtins are treated more like non-builtins in this
case.

Given how flaky this message already is (e.g., you cannot
even know whether you will see it, as git optimizes out some
shell invocations behind the scenes based on the contents of
the command!), and that it is unlikely to ever provide
useful information, let's suppress it for all cases of
SIGPIPE.

Signed-off-by: Jeff King 
---
 run-command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/run-command.c b/run-command.c
index 13fa452..694a6ff 100644
--- a/run-command.c
+++ b/run-command.c
@@ -245,7 +245,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, int 
in_signal)
error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
code = WTERMSIG(status);
-   if (code != SIGINT && code != SIGQUIT)
+   if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
error("%s died of signal %d", argv0, code);
/*
 * This return value is chosen so that code & 0xff
-- 
2.7.0.rc3.367.g09631da

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Duy Nguyen]

On Thu, Dec 24, 2015 at 4:31 AM, Jeff King  wrote:
>   2. I doubt anybody is actually seeing this in practice anymore. But
>  maybe I am misunderstanding something in Duy's series that changes
>  this.

There are two parts in your patch, one (that you two seemed to focus
on) about return code with "!" aliases. Another, suppressing SIGPIPE,
affects more than "!" aliases. In my case it's
execv_dashed_external(). Non-"!" aliases are now forced to use that
function.

This is the output with 'pu'. Notice git-log is executed separately

> ~/w/git $ GIT_TRACE=2 PAGER=true ./git --exec-path=. l
16:28:19.167040 git.c:567   trace: exec: 'git-l'
16:28:19.167088 run-command.c:345   trace: run_command: 'git-l'
16:28:19.168599 git.c:286   trace: alias expansion: l =>
'log' '--stat'
16:28:19.168633 git.c:567   trace: exec: 'git-log' '--stat'
16:28:19.168649 run-command.c:345   trace: run_command: 'git-log' '--stat'
16:28:19.170420 git.c:350   trace: built-in: git 'log' '--stat'
16:28:19.210074 run-command.c:345   trace: run_command: 'true'
16:28:19.210382 run-command.c:204   trace: exec: 'true'
error: git-log died of signal 13

With your patch on top, "git-log died of.." goes away. And this is
with 'master', where the builtin version of git-log is used

> ~/w/git $ GIT_TRACE=2 PAGER=true ./git --exec-path=. l
16:29:17.546382 git.c:561   trace: exec: 'git-l'
16:29:17.546428 run-command.c:343   trace: run_command: 'git-l'
16:29:17.547485 git.c:282   trace: alias expansion: l =>
'log' '--stat'
16:29:17.548482 git.c:348   trace: built-in: git 'log' '--stat'
16:29:17.586457 run-command.c:343   trace: run_command: 'true'
16:29:17.586770 run-command.c:202   trace: exec: 'true'

So, in practice, people will see "died of signal 13" a lot more often
if my series is merged and released (I assume that people often use
aliases for paged commands like git-log, git-diff...)
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Duy Nguyen]

On Wed, Dec 23, 2015 at 4:37 PM, Jeff King  wrote:
> On Tue, Dec 22, 2015 at 10:13:03AM -0800, Junio C Hamano wrote:
>
>> > Another by the way, this "forcing aliases as external commands" now
>> > shows something like "error: git-log died of signal 13" when the pager
>> > exits early, for an alias like "l1 = log --oneline".
>>
>> ... and we do not show that when we directly call "git log" is...?
>>
>> We do signal this with non-zero exit status like so:
>>
>>   $ GIT_PAGER=true git log --oneline ; echo $?
>> 141
>>
>> and it is not surprising that the one that is catching the exit
>> status of what was spawned and reporting "signal 13".
>
> This sounded very familiar. Apparently I've been running with the patch
> below for almost 3 years and never got around to re-examining it.
>
> The original discussion is in:
>
>   http://thread.gmane.org/gmane.comp.version-control.git/212630
>
> Having skimmed through the arguments there, and given that we applied
> the patch from the middle of the thread, I think this is a good change.

Yep. The new thing here is the "died of" message now also comes from
execv_dashed_external() for non-external aliases, so it can show up a
lot more often. I wasn't sure if excluding SIGPIPE in wait_and_whine
was safe. But I _guess_ it is, based on your having no problem with
the patch for a long time and the past discussion.

>
> -- >8 --
> Date: Fri, 4 Jan 2013 07:19:35 -0500
> Subject: [PATCH] avoid SIGPIPE warnings for aliases
>
> When git executes an alias that specifies an external
> command, it will complain if the alias dies due to a signal.
> This is usually a good thing, as signal deaths are
> unexpected. However, SIGPIPE is not unexpected for many
> commands which produce a lot of output; it is intended that
> the user closing the pager would kill them them via SIGPIPE.
>
> As a result, the user might see annoying messages in a
> scenario like this:
>
>   $ cat ~/.gitconfig
>   [alias]
>   lgbase = log --some-options
>   lg = !git lgbase --more-options
>   lg2 = !git lgbase --other-options
>
>   $ git lg -p
>   [user hits 'q' to exit pager]
>   error: git lgbase --more-options died of signal 13
>   fatal: While expanding alias 'lg': 'git lgbase --more-options': Success
>
> Many users won't see this, because we execute the external
> command with the shell, and a POSIX shell will silently
> rewrite the signal-death exit code into 128+signal, and we
> will treat it like a normal exit code. However, this does
> not always happen:
>
>   1. If the sub-command does not have shell metacharacters,
>  we will skip the shell and exec it directly, getting
>  the signal code.
>
>   2. Some shells do not do this rewriting; for example,
>  setting SHELL_PATH set to zsh will reveal this problem.
>
> This patch squelches the message, and lets git exit silently
> (with the same exit code that a shell would use in case of a
> signal).
>
> The first line of the message comes from run-command's
> wait_or_whine. To silence that message, we remain quiet
> anytime we see SIGPIPE.
>
> The second line comes from handle_alias itself. It calls
> die_errno whenever run_command returns a negative value.
> However, only -1 indicates a syscall error where errno has
> something useful (note that it says the useless "success"
> above). Instead, we treat negative returns from run_command
> (except for -1) as a normal code to be passed to exit.
>
> Signed-off-by: Jeff King 
> ---
>  git.c | 2 +-
>  run-command.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git.c b/git.c
> index 6ed824c..34a18a3 100644
> --- a/git.c
> +++ b/git.c
> @@ -252,7 +252,7 @@ static int handle_alias(int *argcp, const char ***argv)
> alias_argv[argc] = NULL;
>
> ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
> -   if (ret >= 0)   /* normal exit */
> +   if (ret != -1)  /* normal exit */
> exit(ret);
>
> die_errno("While expanding alias '%s': '%s'",
> diff --git a/run-command.c b/run-command.c
> index 13fa452..694a6ff 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -245,7 +245,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, 
> int in_signal)
> error("waitpid is confused (%s)", argv0);
> } else if (WIFSIGNALED(status)) {
> code = WTERMSIG(status);
> -   if (code != SIGINT && code != SIGQUIT)
> +   if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
> error("%s died of signal %d", argv0, code);
> /*
>  * This return value is chosen so that code & 0xff
> --
> 2.7.0.rc2.368.g1cbb535
>



-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Eric Sunshine]

On Wed, Dec 23, 2015 at 4:37 AM, Jeff King  wrote:
> Subject: [PATCH] avoid SIGPIPE warnings for aliases
>
> When git executes an alias that specifies an external
> command, it will complain if the alias dies due to a signal.
> This is usually a good thing, as signal deaths are
> unexpected. However, SIGPIPE is not unexpected for many
> commands which produce a lot of output; it is intended that
> the user closing the pager would kill them them via SIGPIPE.

s/them them/them/

> As a result, the user might see annoying messages in a
> scenario like this:
>
>   $ cat ~/.gitconfig
>   [alias]
>   lgbase = log --some-options
>   lg = !git lgbase --more-options
>   lg2 = !git lgbase --other-options
>
>   $ git lg -p
>   [user hits 'q' to exit pager]
>   error: git lgbase --more-options died of signal 13
>   fatal: While expanding alias 'lg': 'git lgbase --more-options': Success
>
> Many users won't see this, because we execute the external
> command with the shell, and a POSIX shell will silently
> rewrite the signal-death exit code into 128+signal, and we
> will treat it like a normal exit code. However, this does
> not always happen:
>
>   1. If the sub-command does not have shell metacharacters,
>  we will skip the shell and exec it directly, getting
>  the signal code.
>
>   2. Some shells do not do this rewriting; for example,
>  setting SHELL_PATH set to zsh will reveal this problem.
>
> This patch squelches the message, and lets git exit silently
> (with the same exit code that a shell would use in case of a
> signal).
>
> The first line of the message comes from run-command's
> wait_or_whine. To silence that message, we remain quiet
> anytime we see SIGPIPE.
>
> The second line comes from handle_alias itself. It calls
> die_errno whenever run_command returns a negative value.
> However, only -1 indicates a syscall error where errno has
> something useful (note that it says the useless "success"
> above). Instead, we treat negative returns from run_command
> (except for -1) as a normal code to be passed to exit.
>
> Signed-off-by: Jeff King 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Johannes Sixt]

Am 23.12.2015 um 10:37 schrieb Jeff King:

The second line comes from handle_alias itself. It calls
die_errno whenever run_command returns a negative value.
However, only -1 indicates a syscall error where errno has
something useful (note that it says the useless "success"
above). Instead, we treat negative returns from run_command
(except for -1) as a normal code to be passed to exit.

Signed-off-by: Jeff King 
---
  git.c | 2 +-
  run-command.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 6ed824c..34a18a3 100644
--- a/git.c
+++ b/git.c
@@ -252,7 +252,7 @@ static int handle_alias(int *argcp, const char ***argv)
alias_argv[argc] = NULL;

ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
-   if (ret >= 0)   /* normal exit */
+   if (ret != -1)  /* normal exit */


Why does this make a difference? We only ever return -1, zero, or a 
positive value from run_command/finish_command/wait_or_whine, as far as 
I can see.



exit(ret);

die_errno("While expanding alias '%s': '%s'",


-- Hannes

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Jeff King]

On Wed, Dec 23, 2015 at 09:37:04PM +0100, Johannes Sixt wrote:

> >--- a/git.c
> >+++ b/git.c
> >@@ -252,7 +252,7 @@ static int handle_alias(int *argcp, const char ***argv)
> > alias_argv[argc] = NULL;
> >
> > ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
> >-if (ret >= 0)   /* normal exit */
> >+if (ret != -1)  /* normal exit */
> 
> Why does this make a difference? We only ever return -1, zero, or a positive
> value from run_command/finish_command/wait_or_whine, as far as I can see.

Yeah, you're right. This bit predates 709ca73 (run-command: encode
signal death as a positive integer, 2013-01-05), which came out of the
same discussion. So I'd agree this hunk can simply be dropped.

That leaves the ignoring of SIGPIPE in wait_or_whine. I started to
rewrite the commit message to drop the first hunk, but I found I
couldn't replicate the problem in the second either!

Doing:

  GIT_PAGER=false git -c alias.foo='!git log -p' foo

doesn't trigger it. We run the alias through a shell, so we see only the
munged "141" value from the shell's exit code.

Something like:

  GIT_PAGER=false git -p -c alias.foo='!yes' foo

does generate the error message. But we've redirected stderr into the
pager at that point, so by definition it can never be shown.

So I think we would need a case where:

  - the outer git doesn't run the pager that dies; instead the pager is
run inside the alias. But...

  - inside the alias cannot be a shell pipeline, since "foo | less" will
report the exit code of "less", not "foo" (we make special arrangements
in git to propagate the exit code of "foo"). So it pretty much has
to be a git invocation inside the alias. But...

  - The git invocation will convert signal death in the sub-process into
141, like a shell would.

So I'm not sure if this is triggerable at all with an alias.

I did manage to trigger it with an external command, like:

  $ cat $(which git-yes)
  #!/bin/sh
  # This _has_ to be exec, otherwise the shell converts SIGPIPE death
  # into 141.
  exec yes

and then if you run your _own_ pager, like this:

  $ git yes | false
  error: git-yes died of signal 13

you see it. But if git starts the pager, you don't:

  $ GIT_PAGER=false git -p yes

Because the stderr of the outer git process is going to the same dead
pipe.

So my takeaways are:

  1. Complaining about signal death in general is going to be flaky,
 because it's so easy for shells or git to rewrite the exit code and
 not trigger WIFSIGNALED() in the first place.

  2. I doubt anybody is actually seeing this in practice anymore. But
 maybe I am misunderstanding something in Duy's series that changes
 this.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Jeff King]

On Tue, Dec 22, 2015 at 10:13:03AM -0800, Junio C Hamano wrote:

> > Another by the way, this "forcing aliases as external commands" now
> > shows something like "error: git-log died of signal 13" when the pager
> > exits early, for an alias like "l1 = log --oneline".
> 
> ... and we do not show that when we directly call "git log" is...?
> 
> We do signal this with non-zero exit status like so:
> 
>   $ GIT_PAGER=true git log --oneline ; echo $?
> 141
> 
> and it is not surprising that the one that is catching the exit
> status of what was spawned and reporting "signal 13".

This sounded very familiar. Apparently I've been running with the patch
below for almost 3 years and never got around to re-examining it.

The original discussion is in:

  http://thread.gmane.org/gmane.comp.version-control.git/212630

Having skimmed through the arguments there, and given that we applied
the patch from the middle of the thread, I think this is a good change.

-- >8 --
Date: Fri, 4 Jan 2013 07:19:35 -0500
Subject: [PATCH] avoid SIGPIPE warnings for aliases

When git executes an alias that specifies an external
command, it will complain if the alias dies due to a signal.
This is usually a good thing, as signal deaths are
unexpected. However, SIGPIPE is not unexpected for many
commands which produce a lot of output; it is intended that
the user closing the pager would kill them them via SIGPIPE.

As a result, the user might see annoying messages in a
scenario like this:

  $ cat ~/.gitconfig
  [alias]
  lgbase = log --some-options
  lg = !git lgbase --more-options
  lg2 = !git lgbase --other-options

  $ git lg -p
  [user hits 'q' to exit pager]
  error: git lgbase --more-options died of signal 13
  fatal: While expanding alias 'lg': 'git lgbase --more-options': Success

Many users won't see this, because we execute the external
command with the shell, and a POSIX shell will silently
rewrite the signal-death exit code into 128+signal, and we
will treat it like a normal exit code. However, this does
not always happen:

  1. If the sub-command does not have shell metacharacters,
 we will skip the shell and exec it directly, getting
 the signal code.

  2. Some shells do not do this rewriting; for example,
 setting SHELL_PATH set to zsh will reveal this problem.

This patch squelches the message, and lets git exit silently
(with the same exit code that a shell would use in case of a
signal).

The first line of the message comes from run-command's
wait_or_whine. To silence that message, we remain quiet
anytime we see SIGPIPE.

The second line comes from handle_alias itself. It calls
die_errno whenever run_command returns a negative value.
However, only -1 indicates a syscall error where errno has
something useful (note that it says the useless "success"
above). Instead, we treat negative returns from run_command
(except for -1) as a normal code to be passed to exit.

Signed-off-by: Jeff King 
---
 git.c | 2 +-
 run-command.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 6ed824c..34a18a3 100644
--- a/git.c
+++ b/git.c
@@ -252,7 +252,7 @@ static int handle_alias(int *argcp, const char ***argv)
alias_argv[argc] = NULL;
 
ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
-   if (ret >= 0)   /* normal exit */
+   if (ret != -1)  /* normal exit */
exit(ret);
 
die_errno("While expanding alias '%s': '%s'",
diff --git a/run-command.c b/run-command.c
index 13fa452..694a6ff 100644
--- a/run-command.c
+++ b/run-command.c
@@ -245,7 +245,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, int 
in_signal)
error("waitpid is confused (%s)", argv0);
} else if (WIFSIGNALED(status)) {
code = WTERMSIG(status);
-   if (code != SIGINT && code != SIGQUIT)
+   if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
error("%s died of signal %d", argv0, code);
/*
 * This return value is chosen so that code & 0xff
-- 
2.7.0.rc2.368.g1cbb535

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Junio C Hamano]

Duy Nguyen  writes:

> On Tue, Dec 22, 2015 at 5:57 PM, Duy Nguyen  wrote:
>> On Tue, Dec 22, 2015 at 4:18 AM, Junio C Hamano  wrote:
>>> Thanks.  I wiggled these three on top of the "Revert the earlier
>>> one"; while I think the result is correct, I'd appreciate if you can
>>> double check the result when I push the topic out later today.
>>
>> Looks good. "prove" passed too by the way.
>
> Another by the way, this "forcing aliases as external commands" now
> shows something like "error: git-log died of signal 13" when the pager
> exits early, for an alias like "l1 = log --oneline".

... and we do not show that when we directly call "git log" is...?

We do signal this with non-zero exit status like so:

$ GIT_PAGER=true git log --oneline ; echo $?
141

and it is not surprising that the one that is catching the exit
status of what was spawned and reporting "signal 13".

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Duy Nguyen]

On Tue, Dec 22, 2015 at 4:18 AM, Junio C Hamano  wrote:
> Thanks.  I wiggled these three on top of the "Revert the earlier
> one"; while I think the result is correct, I'd appreciate if you can
> double check the result when I push the topic out later today.

Looks good. "prove" passed too by the way.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Duy Nguyen]

On Tue, Dec 22, 2015 at 5:57 PM, Duy Nguyen  wrote:
> On Tue, Dec 22, 2015 at 4:18 AM, Junio C Hamano  wrote:
>> Thanks.  I wiggled these three on top of the "Revert the earlier
>> one"; while I think the result is correct, I'd appreciate if you can
>> double check the result when I push the topic out later today.
>
> Looks good. "prove" passed too by the way.

Another by the way, this "forcing aliases as external commands" now
shows something like "error: git-log died of signal 13" when the pager
exits early, for an alias like "l1 = log --oneline". It's probably not
a regression because other external git commands with pager should
show the same message. But I haven't checked thoroughly yet.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Junio C Hamano]

Thanks.  I wiggled these three on top of the "Revert the earlier
one"; while I think the result is correct, I'd appreciate if you can
double check the result when I push the topic out later today.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html