git-daemon breakage in 1.5.4

git-daemon breakage in 1.5.4

[Wincent Colaiuta]

I just noticed that my copy of git-daemon running from xinetd on Red  
Hat Enterprise Linux 3 has been broken since upgrading to 1.5.4.

On the client side this is what you see ("git clone" used in the  
example but you get the same issue with "git ls-remote"):

   git clone git://git.wincent.com/wikitext.git
   Initialized empty Git repository in /tmp/wikitext/.git/
   fatal: The remote end hung up unexpectedly
   fetch-pack from 'git://git.wincent.com/wikitext.git' failed.

Nothing printed to the logs on the server side: it simply hangs up. By  
connecting via telnet I've confirmed that git-daemon is running and  
does accept the initial connection.

The verdict according to "git bisect" is that  
511707d42b3b3e57d9623493092590546ffeae80 is first bad commit:

commit 511707d42b3b3e57d9623493092590546ffeae80
Author: Scott R Parish <srp@srparish.net>
Date:   Sun Oct 28 04:17:20 2007 -0700

     use only the $PATH for exec'ing git commands

     We need to correctly set up $PATH for non-c based git commands.
     Since we already do this, we can just use that $PATH and execvp,
     instead of looping over the paths with execve.

     This patch adds a setup_path() function to exec_cmd.c, which sets
     the $PATH order correctly for our search order. execv_git_cmd() is
     stripped down to setting up argv and calling execvp(). git.c's
     main() only only needs to call setup_path().

     Signed-off-by: Scott R Parish <srp@srparish.net>
     Signed-off-by: Junio C Hamano <gitster@pobox.com>

:100644 100644 33b17a6b45699e73a9b58f0ff02135eae913b47d  
2d0a75851284392aa8ae44bc486df6a034d0af13 M	exec_cmd.c
:100644 100644 da99287552b5d3eafc495f998a936df81ee3f8b9  
a892355c8212298130fb3925c6cba352ed6999b6 M	exec_cmd.h
:100644 100644 c7cabf5f348118f318d7c3abe55853b576869a98  
4e10581101c26444da5c7c44a80219b11607705b M	git.c

Does that look like it might be the issue? Anyone familiar with that  
part of the code care to comment? Any other info I can provide that  
might shed light on the problem?

Cheers,
Wincent

Re: git-daemon breakage in 1.5.4

[Scott Parish]

On Feb 5, 2008, at 7:39 AM, Wincent Colaiuta wrote:

> I just noticed that my copy of git-daemon running from xinetd on  
> Red Hat Enterprise Linux 3 has been broken since upgrading to 1.5.4.
>
> Nothing printed to the logs on the server side: it simply hangs up.  
> By connecting via telnet I've confirmed that git-daemon is running  
> and does accept the initial connection.
>
> The verdict according to "git bisect" is that  
> 511707d42b3b3e57d9623493092590546ffeae80 is first bad commit:
>
> Does that look like it might be the issue? Anyone familiar with  
> that part of the code care to comment? Any other info I can provide  
> that might shed light on the problem?

Prior to that patch, execv_git_cmd called execve in a loop to find  
the command to run. The above patch added a setup_path() api to setup  
PATH and then called execvp() to do the looping. The problem in this  
case is that daemon is never calling setup_path(), so the builtin  
path (among others) aren't getting included in the PATH.

You should see the problem go away if you run "git daemon" instead of  
"git-daemon". Given that directly using the dash versions of the  
commands are discouraged, it probably wouldn't hurt doing this  
anyway. I'll work up a patch later today.

sRp

Re: git-daemon breakage in 1.5.4

[Wincent Colaiuta]

El 5/2/2008, a las 18:49, Scott Parish escribió:

>
> On Feb 5, 2008, at 7:39 AM, Wincent Colaiuta wrote:
>
>> I just noticed that my copy of git-daemon running from xinetd on  
>> Red Hat Enterprise Linux 3 has been broken since upgrading to 1.5.4.
>>
>> Nothing printed to the logs on the server side: it simply hangs up.  
>> By connecting via telnet I've confirmed that git-daemon is running  
>> and does accept the initial connection.
>>
>> The verdict according to "git bisect" is that  
>> 511707d42b3b3e57d9623493092590546ffeae80 is first bad commit:
>>
>> Does that look like it might be the issue? Anyone familiar with  
>> that part of the code care to comment? Any other info I can provide  
>> that might shed light on the problem?
>
> Prior to that patch, execv_git_cmd called execve in a loop to find  
> the command to run. The above patch added a setup_path() api to  
> setup PATH and then called execvp() to do the looping. The problem  
> in this case is that daemon is never calling setup_path(), so the  
> builtin path (among others) aren't getting included in the PATH.
>
> You should see the problem go away if you run "git daemon" instead  
> of "git-daemon". Given that directly using the dash versions of the  
> commands are discouraged, it probably wouldn't hurt doing this  
> anyway. I'll work up a patch later today.

Interesting. I use the dashless form on my desktop but I hadn't  
thought about using it on the server; I'd think that explicitly  
providing the absolute path should always work anyway (at least, it's  
meant to, isn't it?).

In any case I did a little more investigation.

As I mentioned in my original email, the daemon is being launched by  
xinetd. The xinetd configuration launches it with the full path to the  
executable; eg:

   /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah

Changing that to:

   /usr/local/bin/git daemon --inetd --base-path=/blah -- /blah

We still get the failure.

But dropping the --inetd flag and launching the daemon manually it  
works both as:

   /usr/local/bin/git daemon --inetd --base-path=/blah -- /blah

And:

   /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah

So there's some kind of funky interaction going on. On seeing your  
patch come up in the "git bisect" run I wondered what kind of  
interaction might be happening with the PATH (I imagine the PATH  
environment inherited by processes that xinetd launches must be pretty  
anemic), but seeing as I can reproduce the problem from the command  
line (with a well-stocked PATH) that doesn't seem to be the problem.

Cheers,
Wincent

Re: git-daemon breakage in 1.5.4

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> I just noticed that my copy of git-daemon running from xinetd on Red
> Hat Enterprise Linux 3 has been broken since upgrading to 1.5.4.
>
> On the client side this is what you see ("git clone" used in the
> example but you get the same issue with "git ls-remote"):
>
>   git clone git://git.wincent.com/wikitext.git
>   Initialized empty Git repository in /tmp/wikitext/.git/
>   fatal: The remote end hung up unexpectedly
>   fetch-pack from 'git://git.wincent.com/wikitext.git' failed.
>
> Nothing printed to the logs on the server side: it simply hangs up. By
> connecting via telnet I've confirmed that git-daemon is running and
> does accept the initial connection.
>
> The verdict according to "git bisect" is that
> 511707d42b3b3e57d9623493092590546ffeae80 is first bad commit:
>
> commit 511707d42b3b3e57d9623493092590546ffeae80
> Author: Scott R Parish <srp@srparish.net>
> Date:   Sun Oct 28 04:17:20 2007 -0700
>
>     use only the $PATH for exec'ing git commands

Perhaps you did not install git on the PATH processes launched
by your inetd implementation would use?

Re: git-daemon breakage in 1.5.4

[Wincent Colaiuta]

El 5/2/2008, a las 21:02, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> I just noticed that my copy of git-daemon running from xinetd on Red
>> Hat Enterprise Linux 3 has been broken since upgrading to 1.5.4.
>>
>> On the client side this is what you see ("git clone" used in the
>> example but you get the same issue with "git ls-remote"):
>>
>>  git clone git://git.wincent.com/wikitext.git
>>  Initialized empty Git repository in /tmp/wikitext/.git/
>>  fatal: The remote end hung up unexpectedly
>>  fetch-pack from 'git://git.wincent.com/wikitext.git' failed.
>>
>> Nothing printed to the logs on the server side: it simply hangs up.  
>> By
>> connecting via telnet I've confirmed that git-daemon is running and
>> does accept the initial connection.
>>
>> The verdict according to "git bisect" is that
>> 511707d42b3b3e57d9623493092590546ffeae80 is first bad commit:
>>
>> commit 511707d42b3b3e57d9623493092590546ffeae80
>> Author: Scott R Parish <srp@srparish.net>
>> Date:   Sun Oct 28 04:17:20 2007 -0700
>>
>>    use only the $PATH for exec'ing git commands
>
> Perhaps you did not install git on the PATH processes launched
> by your inetd implementation would use?

I don't know what PATH environment xinetd provides, but I can  
reproduce this directly as follows from the command line without any  
involvement from xientd:

First, set up PATH with all the standard locations, with directories  
under /usr/local specified first. Git 1.5.4 is installed in /usr/local/ 
bin:

   # export PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/ 
sbin:/sbin

This fails with the "remote end hung up unexpectedly" error:

   # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah

Drop the --inetd option and it works with no errors:

   # /usr/local/bin/git-daemon --base-path=/blah -- /blah

Now, if I downgrade to Git 1.5.3.8, it works both with and without the  
--inetd option.

The above behaviour is the same regardless of how I specify the path  
to git-daemon (ie. absolute or relative, dashed or dashless).

Is there anything I can do to get "git daemon" to be more verbose on  
failing?

Cheers,
Wincent

Re: git-daemon breakage in 1.5.4

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> This fails with the "remote end hung up unexpectedly" error:
>
>    # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah
>
> Drop the --inetd option and it works with no errors:

Do you mean you run the above from your command line and it
fails?

I do not think --inetd mode is supposed to work as a daemon that
accepts connections.  Wouldn't it be talking with a single peer
via its stdin/stdout?

Puzzled.

Re: git-daemon breakage in 1.5.4

[Johannes Sixt]

Wincent Colaiuta schrieb:
> El 5/2/2008, a las 21:02, Junio C Hamano escribió:
>> Perhaps you did not install git on the PATH processes launched
>> by your inetd implementation would use?
> 
> I don't know what PATH environment xinetd provides, but I can reproduce
> this directly as follows from the command line without any involvement
> from xientd:
> 
> First, set up PATH with all the standard locations, with directories
> under /usr/local specified first. Git 1.5.4 is installed in /usr/local/bin:
> 
>   # export
> PATH=/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin
> 
> This fails with the "remote end hung up unexpectedly" error:
> 
>   # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah

If you run this from the command line, you can't expect it to do anything
useful: It communicates with the client via stdin and stdout.

> Drop the --inetd option and it works with no errors:
> 
>   # /usr/local/bin/git-daemon --base-path=/blah -- /blah

When I run git-daemon with a reduced path similar to this:

   PATH=/bin:/usr/bin /usr/local/bin/git-daemon ...

i.e. git is installed in /usr/local/bin, but it is not in PATH, then I
also get "hung up unexpectedly" from a client that connects to this server.

Which makes me think that you xinetd doesn't pass a PATH to git-daemon
that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:

    env = PATH=/bin:/usr/bin:/usr/local/bin

(not tested).

-- Hannes

Re: git-daemon breakage in 1.5.4

[Wincent Colaiuta]

El 6/2/2008, a las 9:46, Junio C Hamano escribió:

> Wincent Colaiuta <win@wincent.com> writes:
>
>> This fails with the "remote end hung up unexpectedly" error:
>>
>>   # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah
>>
>> Drop the --inetd option and it works with no errors:
>
> Do you mean you run the above from your command line and it
> fails?

Yes.

> I do not think --inetd mode is supposed to work as a daemon that
> accepts connections.  Wouldn't it be talking with a single peer
> via its stdin/stdout?

I don't know exactly what it does under the covers (will look now).  
Perhaps it detects that its running from an interactive login and  
modifies its behaviour. But all I can confirm is that in 1.5.3.8 (and  
before 511707d42b3b3e57d9623493092590546ffeae80) "git daemon" works  
from the command line both with and without the --inetd option, and in  
1.5.4 (from that commit onwards) "git daemon" doesn't work for me with  
the --inetd option but it does without it. The testing from the  
command line is really only a diagnostic thing (perhaps misguided),  
but it does exhibit the same behaviour as issuing the same command  
within xinetd.

Cheers,
>

Wincent

Re: git-daemon breakage in 1.5.4

[Wincent Colaiuta]

El 6/2/2008, a las 10:40, Johannes Sixt escribió:

>> This fails with the "remote end hung up unexpectedly" error:
>>
>>  # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah
>
> If you run this from the command line, you can't expect it to do  
> anything
> useful: It communicates with the client via stdin and stdout.

Strangely, it worked with 1.5.3.8. But I just tried to reproduce it  
and now I can't, so there must have been some error in my procedure.  
Doh. The bizarre thing is that in preparing these emails I tested it  
at least twice, which means I must have made the exact same mistake at  
least twice...

> Which makes me think that you xinetd doesn't pass a PATH to git-daemon
> that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:
>
>    env = PATH=/bin:/usr/bin:/usr/local/bin
>
> (not tested).

That works. Thanks.

It's an acceptable workaround (the other is installing /usr instead  
of /usr/local). Seeing as it worked in 1.5.3.8, does this qualify as  
breakage, or should we not worry about it?

Cheers,
Wincent

Re: git-daemon breakage in 1.5.4

[Johannes Sixt]

Wincent Colaiuta schrieb:
> El 6/2/2008, a las 10:40, Johannes Sixt escribió:
> 
>>> This fails with the "remote end hung up unexpectedly" error:
>>>
>>>  # /usr/local/bin/git-daemon --inetd --base-path=/blah -- /blah
>>
>> If you run this from the command line, you can't expect it to do anything
>> useful: It communicates with the client via stdin and stdout.
> 
> Strangely, it worked with 1.5.3.8. But I just tried to reproduce it and
> now I can't, so there must have been some error in my procedure. Doh.
> The bizarre thing is that in preparing these emails I tested it at least
> twice, which means I must have made the exact same mistake at least
> twice...
> 
>> Which makes me think that you xinetd doesn't pass a PATH to git-daemon
>> that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:
>>
>>    env = PATH=/bin:/usr/bin:/usr/local/bin
>>
>> (not tested).
> 
> That works. Thanks.
> 
> It's an acceptable workaround (the other is installing /usr instead of
> /usr/local). Seeing as it worked in 1.5.3.8, does this qualify as
> breakage, or should we not worry about it?

Does this patch make a difference? (It does for me.)

diff --git a/daemon.c b/daemon.c
index 41a60af..c99285e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1026,6 +1026,7 @@ int main(int argc, char **argv)
 	struct group *group;
 	gid_t gid = 0;
 	int i;
+	char *cmd_path = strdup(argv[0]), *slash;

 	/* Without this we cannot rely on waitpid() to tell
 	 * what happened to our children.
@@ -1184,6 +1185,13 @@ int main(int argc, char **argv)
 	if (strict_paths && (!ok_paths || !*ok_paths))
 		die("option --strict-paths requires a whitelist");

+	slash = strrchr(cmd_path, '/');
+	if (slash) {
+		*slash = 0;
+		setup_path(cmd_path);
+	}
+	free(cmd_path);
+
 	if (inetd_mode) {
 		struct sockaddr_storage ss;
 		struct sockaddr *peer = (struct sockaddr *)&ss;

Re: git-daemon breakage in 1.5.4

[Johannes Sixt]

Johannes Sixt schrieb:
> Wincent Colaiuta schrieb:
>> Drop the --inetd option and it works with no errors:
>>
>>   # /usr/local/bin/git-daemon --base-path=/blah -- /blah
> 
> When I run git-daemon with a reduced path similar to this:
> 
>    PATH=/bin:/usr/bin /usr/local/bin/git-daemon ...
> 
> i.e. git is installed in /usr/local/bin, but it is not in PATH, then I
> also get "hung up unexpectedly" from a client that connects to this server.
> 
> Which makes me think that you xinetd doesn't pass a PATH to git-daemon
> that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:
> 
>     env = PATH=/bin:/usr/bin:/usr/local/bin
> 
> (not tested).

And if I run it this way:

    PATH=/bin:/usr/bin /usr/local/bin/git daemon ...

(notice the dash-less form) it works, too. Although I don't know if this
somehow would interfere with --inetd mode.

-- Hannes

Re: git-daemon breakage in 1.5.4

[Wincent Colaiuta]

El 6/2/2008, a las 11:28, Johannes Sixt escribió:

>>> Which makes me think that you xinetd doesn't pass a PATH to git- 
>>> daemon
>>> that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:
>>>
>>>   env = PATH=/bin:/usr/bin:/usr/local/bin
>>>
>>> (not tested).
>>
>> That works. Thanks.
>>
>> It's an acceptable workaround (the other is installing /usr instead  
>> of
>> /usr/local). Seeing as it worked in 1.5.3.8, does this qualify as
>> breakage, or should we not worry about it?
>
> Does this patch make a difference? (It does for me.)

Nope. I applied this on top of "maint", removed the PATH setup from / 
etc/xinetd.d/git, and get the "remote end hung up unexpectedly" again.  
If I restore the PATH setup then it works, but then, so does does 1.5.4.

Cheers,
Wincent

> diff --git a/daemon.c b/daemon.c
> index 41a60af..c99285e 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -1026,6 +1026,7 @@ int main(int argc, char **argv)
> 	struct group *group;
> 	gid_t gid = 0;
> 	int i;
> +	char *cmd_path = strdup(argv[0]), *slash;
>
> 	/* Without this we cannot rely on waitpid() to tell
> 	 * what happened to our children.
> @@ -1184,6 +1185,13 @@ int main(int argc, char **argv)
> 	if (strict_paths && (!ok_paths || !*ok_paths))
> 		die("option --strict-paths requires a whitelist");
>
> +	slash = strrchr(cmd_path, '/');
> +	if (slash) {
> +		*slash = 0;
> +		setup_path(cmd_path);
> +	}
> +	free(cmd_path);
> +
> 	if (inetd_mode) {
> 		struct sockaddr_storage ss;
> 		struct sockaddr *peer = (struct sockaddr *)&ss;

Re: git-daemon breakage in 1.5.4

[Adam Piatyszek]

* Johannes Sixt [6 II 2008 10:40]:
> When I run git-daemon with a reduced path similar to this:
> 
>    PATH=/bin:/usr/bin /usr/local/bin/git-daemon ...
> 
> i.e. git is installed in /usr/local/bin, but it is not in PATH, then I
> also get "hung up unexpectedly" from a client that connects to this server.
> 
> Which makes me think that you xinetd doesn't pass a PATH to git-daemon
> that includes /usr/local/bin. Add this to your /etc/xinetd.d/git:
> 
>     env = PATH=/bin:/usr/bin:/usr/local/bin

You can also run "git daemon" passing --exec-path as a git argument. 
This should help.  For instance, I use the following configuration in 
inetd.conf (SunOS 5.9):

   git   stream   tcp   nowait   gituser   /usr/local/bin/git \
     git --exec-path=/usr/local/bin daemon --inetd \
     --base-path=/export/home/gituser/git /export/home/gituser/git

BR,
/Adam


-- 
.:.  Adam Piatyszek (ediap)  .:.....................................:.
.:.  ediap@users.sourceforge.net  .:................................:.