git-daemon shallow checkout fail

git-daemon shallow checkout fail

[Bob Proulx]

I am trying to understand a problem with shallow checkouts through the
git-daemon.  The server side fails trying to create a shallow_XXXXXX
file in the repository.  But of course it can't due to no permissions
from the git-daemon user.

However the problem driving me crazy is that this only fails this way
on one machine.  Unfortunately failing on the machine I need to use.
If I try this same setup on any other machine I try then there is no
failure and it works okay.  Therefore I conclude that in the failing
case it is trying to write a shallow_XXXXXX file in the repository but
in all of the passing cases it does not.  I browsed through the
git-daemon source but couldn't deduce the flow yet.

Does anyone know why one system would try to create shallow_XXXXXX
files in the repository while another one would not?

Trying to be unambiguous here is my test case:

  mkdir /run/git
  chmod 755 /run/git
  chown nobody:root /run/git
  cd /run/git && env -i HOME=/run/git PATH=/usr/local/bin:/usr/bin:/bin su -s /bin/sh -c "git-daemon --export-all --base-path=/srv/git --verbose" nobody
  [18340] Ready to rumble

That sets up the test case.  Have any bare git repository in /srv/git
for use for cloning.

  git clone --depth 1 git://localhost/test-project.git
  Cloning into 'test-project'...
  fatal: The remote end hung up unexpectedly
  fatal: early EOF
  fatal: index-pack failed

And the server side says:

  [26071] Request upload-pack for '/test-project.git'
  [26071] fatal: Unable to create temporary file '/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
  [26055] [26071] Disconnected (with error)

Of course git-daemon running as nobody can't create a temporary file
shallow_XXXXXX in the /srv/git/test-project.git because it has no
permissions by design.  But why does this work on other systems and
not work on my target system?

  git --version  # from today's git clone build
  git version 2.11.0.485.g4e59582

Thanks!
Bob

Re: git-daemon shallow checkout fail

[Johannes Schindelin]

Hi Bob,

On Sat, 28 Jan 2017, Bob Proulx wrote:

> And the server side says:
> 
>   [26071] Request upload-pack for '/test-project.git'
>   [26071] fatal: Unable to create temporary file '/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
>   [26055] [26071] Disconnected (with error)

Assuming that you can rebuild your Git with debug symbols and without
optimization (simply remove the -O2 from CFLAGS in the Makefile, I never
had any luck with single-stepping in gdb when compiled with -O2), you
could attach gdb to the git-daemon and/or upload-pack process. Setting a
breakpoint on die_builtin in the failing process should give you a good
idea why things are failing, at least looking at the stacktrace.

A few more tidbits from a cursory look at the Git source code with `git
grep` and the likes:

- that error message comes from shallow.c's setup_temporary_shallow()
  function

- that function is only called from fetch-pack and receive-pack, neither
  of which should be called by upload-pack, so it is a puzzle

- adding a test case to t5570-git-daemon.sh that tests specifically your
  described scenario seems *not* to fail:

-- snip --
diff --git a/t/t5570-git-daemon.sh b/t/t5570-git-daemon.sh
index 225a022e8a..0256c9aded 100755
--- a/t/t5570-git-daemon.sh
+++ b/t/t5570-git-daemon.sh
@@ -186,5 +186,17 @@ test_expect_success 'hostname cannot break out of directory' '
 		git clone --bare "$GIT_DAEMON_URL/escape.git" tmp.git
 '
 
+test_expect_success POSIXPERM 'shallow clone from read-only server' '
+	test_when_finished "rm -rf tmp.git" &&
+	repo="$GIT_DAEMON_DOCUMENT_ROOT_PATH/readonly.git" &&
+	git init --bare "$repo" &&
+	git push "$repo" HEAD &&
+	>"$repo"/git-daemon-export-ok &&
+	chmod a-w "$repo" &&
+	test_must_fail \
+		env GIT_OVERRIDE_VIRTUAL_HOST=.. \
+		git clone --depth 1 "$GIT_DAEMON_URL/readonly.git" tmp.git
+'
+
 stop_git_daemon
 test_done
-- snap --

- I even modified t/lib-git-daemon.sh to start the daemon as `nobody` and
  kill it as `root`, and I won't share that patch because it is as
  ugly, but *even then* the test succeeded.

So my suspicion is that the repository you try to serve may already be
shallow, or something else funky is going on that has not been included in
your report.

The most direct way to get to the bottom of this may be to do something
like this:

-- snip --
diff --git a/shallow.c b/shallow.c
index 11f7dde9d9..30f5c96d50 100644
--- a/shallow.c
+++ b/shallow.c
@@ -288,12 +288,18 @@ int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
 
 static struct tempfile temporary_shallow;
 
+static int debug_me;
+
 const char *setup_temporary_shallow(const struct sha1_array *extra)
 {
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
 
 	if (write_shallow_commits(&sb, 0, extra)) {
+error("About to create shallow_XXXXXX: pid = %d", getpid());
+while (!debug_me) {
+	sleep(1);
+}
 		fd = xmks_tempfile(&temporary_shallow, git_path("shallow_XXXXXX"));
 
 		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
-- snap --

Then let it run, wait for the error message "About to create
shallow_XXXXXX" and then attach with a gdb started as nobody via `attach
<pid>` to see the stack trace.

That should give you an idea where that code path is hit (unexpectedly).

Ciao,
Johannes

Re: git-daemon shallow checkout fail

[Jeff King]

On Sat, Jan 28, 2017 at 05:29:32PM -0700, Bob Proulx wrote:

> However the problem driving me crazy is that this only fails this way
> on one machine.  Unfortunately failing on the machine I need to use.
> If I try this same setup on any other machine I try then there is no
> failure and it works okay.  Therefore I conclude that in the failing
> case it is trying to write a shallow_XXXXXX file in the repository but
> in all of the passing cases it does not.  I browsed through the
> git-daemon source but couldn't deduce the flow yet.
> 
> Does anyone know why one system would try to create shallow_XXXXXX
> files in the repository while another one would not?

It depends on the git version on the server. The interesting code is in
upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
request.

See commit b790e0f67 (upload-pack: send shallow info over stdin to
pack-objects, 2014-03-11), which lays out the history. Since that commit
(in git v2.0.0), there should be no tmpfile needed.

> Of course git-daemon running as nobody can't create a temporary file
> shallow_XXXXXX in the /srv/git/test-project.git because it has no
> permissions by design.  But why does this work on other systems and
> not work on my target system?
> 
>   git --version  # from today's git clone build
>   git version 2.11.0.485.g4e59582

This shouldn't be happening with git v2.11. Are you sure that the "git
daemon" invocation is running that same version? I notice you set up a
restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
older version of git?

-Peff

Re: git-daemon shallow checkout fail

[Duy Nguyen]

On Tue, Jan 31, 2017 at 12:27 AM, Jeff King <peff@peff.net> wrote:
> On Sat, Jan 28, 2017 at 05:29:32PM -0700, Bob Proulx wrote:
>
>> However the problem driving me crazy is that this only fails this way
>> on one machine.  Unfortunately failing on the machine I need to use.
>> If I try this same setup on any other machine I try then there is no
>> failure and it works okay.  Therefore I conclude that in the failing
>> case it is trying to write a shallow_XXXXXX file in the repository but
>> in all of the passing cases it does not.  I browsed through the
>> git-daemon source but couldn't deduce the flow yet.
>>
>> Does anyone know why one system would try to create shallow_XXXXXX
>> files in the repository while another one would not?
>
> It depends on the git version on the server. The interesting code is in
> upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
> request.
>
> See commit b790e0f67 (upload-pack: send shallow info over stdin to
> pack-objects, 2014-03-11), which lays out the history. Since that commit
> (in git v2.0.0), there should be no tmpfile needed.

It must be it. There's nowhere else that upload-pack can create
shallow_XXXXXX. (receive-pack and fetch-pack can).

>> Of course git-daemon running as nobody can't create a temporary file
>> shallow_XXXXXX in the /srv/git/test-project.git because it has no
>> permissions by design.  But why does this work on other systems and
>> not work on my target system?
>>
>>   git --version  # from today's git clone build
>>   git version 2.11.0.485.g4e59582
>
> This shouldn't be happening with git v2.11. Are you sure that the "git
> daemon" invocation is running that same version? I notice you set up a
> restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
> older version of git?

One easy way to verify is clone or fetch again with GIT_TRACE_PACKET=1
since we send the server's version as a capability since 1.8.0
-- 
Duy

Re: git-daemon shallow checkout fail

[Bob Proulx]

Hello Johannes,

Thank you very much for the good hints here.  They are very helpful.

Johannes Schindelin wrote:
> Assuming that you can rebuild your Git with debug symbols and without
> optimization (simply remove the -O2 from CFLAGS in the Makefile, I never
> had any luck with single-stepping in gdb when compiled with -O2), you

I always debug with -g and without -O because otherwise it leads to a
lot of confusion.  But git-daemon is a forking daemon which makes
debugging it with the debugger somewhere more default and requiring
more setup to debug past the fork points.  As you note and hint with
later setting up an attach point for gdb.

> - that error message comes from shallow.c's setup_temporary_shallow()
>   function

Yes.  I find that too.

> - that function is only called from fetch-pack and receive-pack, neither
>   of which should be called by upload-pack, so it is a puzzle

The choice of directionality for upload and download might be
confusing me here too.  Since from the client side of things it is a
download.  But on the server side it is an upload.

> - adding a test case to t5570-git-daemon.sh that tests specifically your
>   described scenario seems *not* to fail:

I have seen some of the patches that seems like should have fixed this
problem.  It is perplexing.  But see my next mail on this too.

> - I even modified t/lib-git-daemon.sh to start the daemon as `nobody` and
>   kill it as `root`, and I won't share that patch because it is as
>   ugly, but *even then* the test succeeded.

It succeeds for me on other systems.  It is only the one (where I need
it) that it is failing.

> So my suspicion is that the repository you try to serve may already be
> shallow, or something else funky is going on that has not been included in
> your report.

It happens to any repository.  I picked a small repository and copied
it verbatim using rsync to my working environment.  The identical bits
of a repository copied by rsync work okay on one system but fail on
the other.

> The most direct way to get to the bottom of this may be to do something
> like this:
...
>  	if (write_shallow_commits(&sb, 0, extra)) {
> +error("About to create shallow_XXXXXX: pid = %d", getpid());
> +while (!debug_me) {
> +	sleep(1);
> +}
>  		fd = xmks_tempfile(&temporary_shallow, git_path("shallow_XXXXXX"));
>  
>  		if (write_in_full(fd, sb.buf, sb.len) != sb.len)
> -- snap --
> 
> Then let it run, wait for the error message "About to create
> shallow_XXXXXX" and then attach with a gdb started as nobody via `attach
> <pid>` to see the stack trace.
> 
> That should give you an idea where that code path is hit (unexpectedly).

That is a good hint.  I will give that a try.  However see my next
email for more (confusing) information.

Bob

Re: git-daemon shallow checkout fail

[Bob Proulx]

Duy Nguyen wrote:
> Jeff King wrote:
> > It depends on the git version on the server. The interesting code is in
> > upload-pack.c, which is spawned by git-daemon to serve a fetch or clone
> > request.
> >
> > See commit b790e0f67 (upload-pack: send shallow info over stdin to
> > pack-objects, 2014-03-11), which lays out the history. Since that commit
> > (in git v2.0.0), there should be no tmpfile needed.
> 
> It must be it. There's nowhere else that upload-pack can create
> shallow_XXXXXX. (receive-pack and fetch-pack can).

I am sure the file creation is there.  Because it isn't being done
anywhere.  But the problem is before that time period.  By then the
paths are already set.

> >Bob Proulx wrote:
> >>   git --version  # from today's git clone build
> >>   git version 2.11.0.485.g4e59582
> >
> > This shouldn't be happening with git v2.11. Are you sure that the "git
> > daemon" invocation is running that same version? I notice you set up a
> > restricted PATH. Is it possible that /usr/local/bin or /usr/bin has an
> > older version of git?

I have been starting it with PATH set to my directory.  But given this
question I start it with a full path.

  PATH=~/src/git-stuff/git:$PATH ~/src/git-stuff/git/git-daemon --export-all --base-path=/srv/git --verbose

But then I worry about the package installed binary still getting
invoked with the fork somehow.  Therefore I disable it.  There should
be no possibility of it invoking the packaged daemon.  I moved it out
of the way.

  mv /usr/lib/git-core/git-daemon /usr/lib/git-core/git-daemon.disabled

I can't trivially remove the packaged version because other things
depend upon it.  I could get more agressive about disabling it in a
non-destructive and reversible way though.

> One easy way to verify is clone or fetch again with GIT_TRACE_PACKET=1
> since we send the server's version as a capability since 1.8.0

And here is the interesting part.  If I read this right the client is
reporting 1.9.1 from the server!  But how?  

  $ GIT_TRACE_PACKET=1 git clone --depth 1 git://git0.savannah.gnu.org/test-project.git
  Cloning into 'test-project'...
  17:20:40.482970 pkt-line.c:46           packet:        clone> git-upload-pack /test-project.git\0host=git0.savannah.gnu.org\0
  17:20:40.590177 pkt-line.c:46           packet:        clone< 34e7202270c381f4e5734180dc19426ce69f2e1e HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master agent=git/1.9.1
  17:20:40.662623 pkt-line.c:46           packet:        clone< 34e7202270c381f4e5734180dc19426ce69f2e1e refs/heads/master
  17:20:40.662653 pkt-line.c:46           packet:        clone< 0000
  17:20:40.663682 pkt-line.c:46           packet:        clone> want 34e7202270c381f4e5734180dc19426ce69f2e1e multi_ack_detailed side-band-64k thin-pack include-tag ofs-delta agent=git/2.1.4
  17:20:40.663701 pkt-line.c:46           packet:        clone> want 34e7202270c381f4e5734180dc19426ce69f2e1e
  17:20:40.663713 pkt-line.c:46           packet:        clone> deepen 1
  17:20:40.663724 pkt-line.c:46           packet:        clone> 0000
  17:20:40.739502 pkt-line.c:46           packet:        clone< shallow 34e7202270c381f4e5734180dc19426ce69f2e1e
  17:20:40.854338 pkt-line.c:46           packet:        clone< 0000
  17:20:40.854360 pkt-line.c:46           packet:        clone> done
  17:20:40.929349 pkt-line.c:46           packet:        clone< NAK
  fatal: The remote end hung up unexpectedly
  fatal: early EOF
  fatal: index-pack failed

I am assuming that the "agent=git/1.9.1" is the evidence that it isn't
running the code that I am trying to make it run?  Of course the
packaged installed git version is 1.9.1 (from Trisquel 7, an Ubuntu
14.04 fork) so that matches.  My client is "agent=git/2.1.4" (from
Debian Jessie) so that matches my client end.

Therefore it looks like it is invoking some other command by a hard
coded path and avoiding PATH to my development bits.  It must be
invoking some other binary.  I will get more agressive about disabling
the packaged version and hopefully find out which one.

Thank you very much for the good hints here.  They are very helpful.

Bob

Re: git-daemon shallow checkout fail

[Bob Proulx]

Bob Proulx wrote:
>   17:20:40.590177 pkt-line.c:46           packet:        clone< 34e7202270c381f4e5734180dc19426ce69f2e1e HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master agent=git/1.9.1

The evidence of it running the wrong version being the 1.9.1 which is
not the bits I built.

> Therefore it looks like it is invoking some other command by a hard
> coded path and avoiding PATH to my development bits.  It must be
> invoking some other binary.  I will get more agressive about disabling
> the packaged version and hopefully find out which one.

I did and of course it is /usr/bin/git-upload-pack and if I disable
that binary then git-daemon no longer operates.

  error: cannot run upload-pack: No such file or directory

But shouldn't it find git-upload-pack from PATH?  Since I have
git-upload-pack installed in PATH?  Apparently not.  At least not when
invoking as /path/to/git-daemon directly.

In any case with the all of your very good help, especially the
version echo print, guiding me to the problem that I was able to make
this work.

  cd /run/git && env -i HOME=/run/git PATH=$HOME/src/git-stuff/git:/usr/bin:/bin $HOME/src/git-stuff/git/git --exec-path=$HOME/src/git-stuff/git daemon --export-all --base-path=/srv/git --verbose

That works for the git-daemon.  It does not require me to modify
anything else on the system.  Good enough.  Problem solved.

Thanks!
Bob

Re: git-daemon shallow checkout fail

[Duy Nguyen]

On Tue, Feb 7, 2017 at 7:56 AM, Bob Proulx <bob@proulx.com> wrote:
> Bob Proulx wrote:
>>   17:20:40.590177 pkt-line.c:46           packet:        clone< 34e7202270c381f4e5734180dc19426ce69f2e1e HEAD\0multi_ack thin-pack side-band side-band-64k ofs-delta shallow no-progress include-tag multi_ack_detailed symref=HEAD:refs/heads/master agent=git/1.9.1
>
> The evidence of it running the wrong version being the 1.9.1 which is
> not the bits I built.

I wonder if we should make this "git/1.9.1" information more visible. We could

1) Always print it when cloning
2) Print it when cloning with -v (printing all capabilities with -v
might not be a bad idea)
3) Include it in error messages when clone/fetch fails
-- 
Duy

Re: git-daemon shallow checkout fail

[Bob Proulx]

Duy Nguyen wrote:
> I wonder if we should make this "git/1.9.1" information more visible. We could
> 
> 1) Always print it when cloning
> 2) Print it when cloning with -v (printing all capabilities with -v
> might not be a bad idea)
> 3) Include it in error messages when clone/fetch fails

I don't think I would want to see it all of the time.  It isn't needed
all of the time.  But having it printable upon demand is nice.  Being
able to use GIT_TRACE_PACKET=1 worked very well.  The only thing I
needed was to know it was available so that I could use it.  I am not
sure where that is documented.

Therefore if and only if a change was made I would vote for printing
only under --verbose or other explicit other information option.  I
would not add it to the normal operation output.

Bob