<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/git.git/run-command.c, branch jc/pack-objects</title>
<subtitle>github.com: git/git.git
</subtitle>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/'/>
<entry>
<title>hooks: Add function to check if a hook exists</title>
<updated>2013-01-14T17:25:40+00:00</updated>
<author>
<name>Aaron Schrab</name>
<email>aaron@schrab.com</email>
</author>
<published>2013-01-13T05:17:02+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=5a7da2dca166fab74f4514697e26dd80e79933f5'/>
<id>5a7da2dca166fab74f4514697e26dd80e79933f5</id>
<content type='text'>
Create find_hook() function to determine if a given hook exists and is
executable.  If it is, the path to the script will be returned,
otherwise NULL is returned.

This encapsulates the tests that are used to check for the existence of
a hook in one place, making it easier to modify those checks if that is
found to be necessary.  This also makes it simple for places that can
use a hook to check if a hook exists before doing, possibly lengthy,
setup work which would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, immediately added to an argv_array list
which would result in it being duplicated at that point, or used to
actually run the command without much intervening work.  Callers which
need to hold onto the returned value for a longer time are expected to
duplicate the return value themselves.

Signed-off-by: Aaron Schrab &lt;aaron@schrab.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Create find_hook() function to determine if a given hook exists and is
executable.  If it is, the path to the script will be returned,
otherwise NULL is returned.

This encapsulates the tests that are used to check for the existence of
a hook in one place, making it easier to modify those checks if that is
found to be necessary.  This also makes it simple for places that can
use a hook to check if a hook exists before doing, possibly lengthy,
setup work which would be pointless if no such hook is present.

The returned value is left as a static value from get_pathname() rather
than a duplicate because it is anticipated that the return value will
either be used as a boolean, immediately added to an argv_array list
which would result in it being duplicated at that point, or used to
actually run the command without much intervening work.  Callers which
need to hold onto the returned value for a longer time are expected to
duplicate the return value themselves.

Signed-off-by: Aaron Schrab &lt;aaron@schrab.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>run-command: encode signal death as a positive integer</title>
<updated>2013-01-06T19:09:18+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2013-01-05T14:49:49+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=709ca730f8e093005cc882bfb86c0ca9c83d345b'/>
<id>709ca730f8e093005cc882bfb86c0ca9c83d345b</id>
<content type='text'>
When a sub-command dies due to a signal, we encode the
signal number into the numeric exit status as "signal -
128". This is easy to identify (versus a regular positive
error code), and when cast to an unsigned integer (e.g., by
feeding it to exit), matches what a POSIX shell would return
when reporting a signal death in $? or through its own exit
code.

So we have a negative value inside the code, but once it
passes across an exit() barrier, it looks positive (and any
code we receive from a sub-shell will have the positive
form). E.g., death by SIGPIPE (signal 13) will look like
-115 to us in inside git, but will end up as 141 when we
call exit() with it. And a program killed by SIGPIPE but run
via the shell will come to us with an exit code of 141.

Unfortunately, this means that when the "use_shell" option
is set, we need to be on the lookout for _both_ forms. We
might or might not have actually invoked the shell (because
we optimize out some useless shell calls). If we didn't invoke
the shell, we will will see the sub-process's signal death
directly, and run-command converts it into a negative value.
But if we did invoke the shell, we will see the shell's
128+signal exit status. To be thorough, we would need to
check both, or cast the value to an unsigned char (after
checking that it is not -1, which is a magic error value).

Fortunately, most callsites do not care at all whether the
exit was from a code or from a signal; they merely check for
a non-zero status, and sometimes propagate the error via
exit(). But for the callers that do care, we can make life
slightly easier by just using the consistent positive form.

This actually fixes two minor bugs:

  1. In launch_editor, we check whether the editor died from
     SIGINT or SIGQUIT. But we checked only the negative
     form, meaning that we would fail to notice a signal
     death exit code which was propagated through the shell.

  2. In handle_alias, we assume that a negative return value
     from run_command means that errno tells us something
     interesting (like a fork failure, or ENOENT).
     Otherwise, we simply propagate the exit code. Negative
     signal death codes confuse us, and we print a useless
     "unable to run alias 'foo': Success" message. By
     encoding signal deaths using the positive form, the
     existing code just propagates it as it would a normal
     non-zero exit code.

The downside is that callers of run_command can no longer
differentiate between a signal received directly by the
sub-process, and one propagated. However, no caller
currently cares, and since we already optimize out some
calls to the shell under the hood, that distinction is not
something that should be relied upon by callers.

Fix the same logic in t/test-terminal.perl for consistency [jc:
raised by Jonathan in the discussion].

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Acked-by: Johannes Sixt &lt;j6t@kdbg.org&gt;
Reviewed-by: Jonathan Nieder &lt;jrnieder@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When a sub-command dies due to a signal, we encode the
signal number into the numeric exit status as "signal -
128". This is easy to identify (versus a regular positive
error code), and when cast to an unsigned integer (e.g., by
feeding it to exit), matches what a POSIX shell would return
when reporting a signal death in $? or through its own exit
code.

So we have a negative value inside the code, but once it
passes across an exit() barrier, it looks positive (and any
code we receive from a sub-shell will have the positive
form). E.g., death by SIGPIPE (signal 13) will look like
-115 to us in inside git, but will end up as 141 when we
call exit() with it. And a program killed by SIGPIPE but run
via the shell will come to us with an exit code of 141.

Unfortunately, this means that when the "use_shell" option
is set, we need to be on the lookout for _both_ forms. We
might or might not have actually invoked the shell (because
we optimize out some useless shell calls). If we didn't invoke
the shell, we will will see the sub-process's signal death
directly, and run-command converts it into a negative value.
But if we did invoke the shell, we will see the shell's
128+signal exit status. To be thorough, we would need to
check both, or cast the value to an unsigned char (after
checking that it is not -1, which is a magic error value).

Fortunately, most callsites do not care at all whether the
exit was from a code or from a signal; they merely check for
a non-zero status, and sometimes propagate the error via
exit(). But for the callers that do care, we can make life
slightly easier by just using the consistent positive form.

This actually fixes two minor bugs:

  1. In launch_editor, we check whether the editor died from
     SIGINT or SIGQUIT. But we checked only the negative
     form, meaning that we would fail to notice a signal
     death exit code which was propagated through the shell.

  2. In handle_alias, we assume that a negative return value
     from run_command means that errno tells us something
     interesting (like a fork failure, or ENOENT).
     Otherwise, we simply propagate the exit code. Negative
     signal death codes confuse us, and we print a useless
     "unable to run alias 'foo': Success" message. By
     encoding signal deaths using the positive form, the
     existing code just propagates it as it would a normal
     non-zero exit code.

The downside is that callers of run_command can no longer
differentiate between a signal received directly by the
sub-process, and one propagated. However, no caller
currently cares, and since we already optimize out some
calls to the shell under the hood, that distinction is not
something that should be relied upon by callers.

Fix the same logic in t/test-terminal.perl for consistency [jc:
raised by Jonathan in the discussion].

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Acked-by: Johannes Sixt &lt;j6t@kdbg.org&gt;
Reviewed-by: Jonathan Nieder &lt;jrnieder@gmail.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>fix compilation with NO_PTHREADS</title>
<updated>2013-01-06T06:47:27+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2013-01-05T14:52:29+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=0398fc349627a8a55ec4727bfb85fb7fad1f4ff0'/>
<id>0398fc349627a8a55ec4727bfb85fb7fad1f4ff0</id>
<content type='text'>
Commit 1327452 cleaned up an unused parameter from
wait_or_whine, but forgot to update a caller that is inside
"#ifdef NO_PTHREADS".

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Commit 1327452 cleaned up an unused parameter from
wait_or_whine, but forgot to update a caller that is inside
"#ifdef NO_PTHREADS".

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>run-command: do not warn about child death from terminal</title>
<updated>2012-12-02T10:06:43+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2012-11-30T22:41:38+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=a2767c5c91ebda3c083419c80a7f64248c5ec175'/>
<id>a2767c5c91ebda3c083419c80a7f64248c5ec175</id>
<content type='text'>
SIGINT and SIGQUIT are not generally interesting signals to
the user, since they are typically caused by them hitting "^C"
or otherwise telling their terminal to send the signal.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
SIGINT and SIGQUIT are not generally interesting signals to
the user, since they are typically caused by them hitting "^C"
or otherwise telling their terminal to send the signal.

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>run-command: drop silent_exec_failure arg from wait_or_whine</title>
<updated>2012-12-02T10:04:50+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2012-11-30T22:40:50+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=13274526c1fc62fd0f15fe2bc188843982f38ec9'/>
<id>13274526c1fc62fd0f15fe2bc188843982f38ec9</id>
<content type='text'>
We do not actually use this parameter; instead we complain
from the child itself (for fork/exec) or from start_command
(if we are using spawn on Windows).

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We do not actually use this parameter; instead we complain
from the child itself (for fork/exec) or from start_command
(if we are using spawn on Windows).

Signed-off-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'jk/no-more-pre-exec-callback'</title>
<updated>2012-10-25T10:41:15+00:00</updated>
<author>
<name>Jeff King</name>
<email>peff@peff.net</email>
</author>
<published>2012-10-25T10:41:15+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=55ff63007509d075d32861ff48cc8bb57d445b2a'/>
<id>55ff63007509d075d32861ff48cc8bb57d445b2a</id>
<content type='text'>
Removes a workaround for buggy version of less older than version
406.

* jk/no-more-pre-exec-callback:
  pager: drop "wait for output to run less" hack
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Removes a workaround for buggy version of less older than version
406.

* jk/no-more-pre-exec-callback:
  pager: drop "wait for output to run less" hack
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'dg/run-command-child-cleanup'</title>
<updated>2012-09-15T04:39:37+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2012-09-15T04:39:37+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=5816cc7ca14c476ab34142eb05b817df9a7240cf'/>
<id>5816cc7ca14c476ab34142eb05b817df9a7240cf</id>
<content type='text'>
The code to wait for subprocess and remove it from our internal queue
wasn't quite right.

* dg/run-command-child-cleanup:
  run-command.c: fix broken list iteration in clear_child_for_cleanup
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The code to wait for subprocess and remove it from our internal queue
wasn't quite right.

* dg/run-command-child-cleanup:
  run-command.c: fix broken list iteration in clear_child_for_cleanup
</pre>
</div>
</content>
</entry>
<entry>
<title>run-command.c: fix broken list iteration in clear_child_for_cleanup</title>
<updated>2012-09-11T17:30:31+00:00</updated>
<author>
<name>David Gould</name>
<email>david@optimisefitness.com</email>
</author>
<published>2012-09-11T14:32:47+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=bdee397d7c2345d467548ef9446a2a63b72c5449'/>
<id>bdee397d7c2345d467548ef9446a2a63b72c5449</id>
<content type='text'>
Iterate through children_to_clean using 'next' fields but with an
extra level of indirection. This allows us to update the chain when
we remove a child and saves us managing several variables around
the loop mechanism.

Signed-off-by: David Gould &lt;david@optimisefitness.com&gt;
Acked-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Iterate through children_to_clean using 'next' fields but with an
extra level of indirection. This allows us to update the chain when
we remove a child and saves us managing several variables around
the loop mechanism.

Signed-off-by: David Gould &lt;david@optimisefitness.com&gt;
Acked-by: Jeff King &lt;peff@peff.net&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'jc/maint-sane-execvp-notdir'</title>
<updated>2012-09-03T22:53:26+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2012-09-03T22:53:26+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=12d858aeb45f6eae3eaecb49ccb6f61affca54cd'/>
<id>12d858aeb45f6eae3eaecb49ccb6f61affca54cd</id>
<content type='text'>
"git foo" errored out with "Not a directory" when the user had a non
directory on $PATH, and worse yet it masked an alias "foo" to run.

* jc/maint-sane-execvp-notdir:
  sane_execvp(): ignore non-directory on $PATH
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
"git foo" errored out with "Not a directory" when the user had a non
directory on $PATH, and worse yet it masked an alias "foo" to run.

* jc/maint-sane-execvp-notdir:
  sane_execvp(): ignore non-directory on $PATH
</pre>
</div>
</content>
</entry>
<entry>
<title>sane_execvp(): ignore non-directory on $PATH</title>
<updated>2012-07-31T19:51:30+00:00</updated>
<author>
<name>Junio C Hamano</name>
<email>gitster@pobox.com</email>
</author>
<published>2012-07-31T19:51:30+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/git.git/commit/?id=a78550831a42db6896e598cd2a8bfb441a958fc8'/>
<id>a78550831a42db6896e598cd2a8bfb441a958fc8</id>
<content type='text'>
When you have a non-directory on your PATH, a funny thing happens:

	$ PATH=$PATH:/bin/sh git foo
	fatal: cannot exec 'git-foo': Not a directory?

Worse yet, as real commands always take precedence over aliases,
this behaviour interacts rather badly with them:

	$ PATH=$PATH:/bin/sh git -c alias.foo=show git foo -s
	fatal: cannot exec 'git-foo': Not a directory?

This is because an ENOTDIR error from the underlying execvp(2) is
reported back to the caller of our sane_execvp() wrapper as-is.

Translating it to ENOENT, just like the case where we _might_ have
the command in an unreadable directory, fixes it.  Without an alias,
we would get

	git: 'foo' is not a git command. See 'git --help'.

and we use the 'foo' alias when it is available, of course.

Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When you have a non-directory on your PATH, a funny thing happens:

	$ PATH=$PATH:/bin/sh git foo
	fatal: cannot exec 'git-foo': Not a directory?

Worse yet, as real commands always take precedence over aliases,
this behaviour interacts rather badly with them:

	$ PATH=$PATH:/bin/sh git -c alias.foo=show git foo -s
	fatal: cannot exec 'git-foo': Not a directory?

This is because an ENOTDIR error from the underlying execvp(2) is
reported back to the caller of our sane_execvp() wrapper as-is.

Translating it to ENOENT, just like the case where we _might_ have
the command in an unreadable directory, fixes it.  Without an alias,
we would get

	git: 'foo' is not a git command. See 'git --help'.

and we use the 'foo' alias when it is available, of course.

Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
