Weird problem with long $PATH and alternates (bisected)

% mkdir 1; cd 1 % echo > a; git add a; git commit -m a % cd .. % git clone -s 1 2 % git push . master:master fatal: Could not switch to '/tmp/a/1/.git/objects/n:/usr/games/bin:/usr/local/ipod-chain' fatal: The remote end hung up unexpectedly % PATH=/bin:/usr/bin git push . master:master Everything up-to-date

The same thing happens if i try to push from 1 to 2: % cd ../1 % git push ../2 fatal: Could not switch to '/tmp/a/1/.git/objects/n:/usr/games/bin:/usr/local/ipod-chain' fatal: The remote end hung up unexpectedly

% echo $#PATH 196 % echo $PATH /home/mikaelh/bin:/opt/bin:/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:/usr/i686-pc-linux-gnu/gcc-bin/4.2.4:/usr/kde/3.5/bin:/usr/qt/3/bin:/usr/games/bin:/usr/local/ipod-chain/bin

d79796bcf05b89774671a75b3285000c43129823 is first bad commit
commit d79796bcf05b89774671a75b3285000c43129823
Author: Junio C Hamano <gitster@pobox.com>
Date:   Tue Sep 9 01:27:10 2008 -0700

    push: receiver end advertises refs from alternate repositories

    Earlier, when pushing into a repository that borrows from alternate object
    stores, we followed the longstanding design decision not to trust refs in
    the alternate repository that houses the object store we are borrowing
    from.  If your public repository is borrowing from Linus's public
    repository, you pushed into it long time ago, and now when you try to push
    your updated history that is in sync with more recent history from Linus,
    you will end up sending not just your own development, but also the
    changes you acquired through Linus's tree, even though the objects needed
    for the latter already exists at the receiving end.  This is because the
    receiving end does not advertise that the objects only reachable from the
    borrowed repository (i.e. Linus's) are already available there.

    This solves the issue by making the receiving end advertise refs from
    borrowed repositories.  They are not sent with their true names but with a
    phoney name ".have" to make sure that the old senders will safely ignore
    them (otherwise, the old senders will misbehave, trying to push matching
    refs, and mirror push that deletes refs that only exist at the receiving
    end).

    Signed-off-by: Junio C Hamano <gitster@pobox.com>

:100644 100644 6d6027ead17b86519d69c3dfc9b98c01e916d277 45e3cd90fd476cdb0a32e5de27739b18e060e031 Mbuiltin-receive-pack.c :100644 100644 98a742122dbacbb39fa104cdfe909a9a884ed7b6 99af83a0479ef472078afcd288b1f6ba6283a2f0 Mcache.h :100644 100644 ae550e89c0942bb9e34b252b653c40e869a074a4 12be17b5dace07a2ca71613e4e9093cdb77492ac Msha1_file.c

"Mikael Magnusson" <mikachu@gmail.com> writes:

I think I see a bug in foreach_alt_odb() to add_refs_from_alternate() callchain, but I cannot explain why the contents of $PATH leaks to the error message.

Can you try this patch?

 sha1_file.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

2008/10/26 Junio C Hamano <gitster@pobox.com>:

Yeah this fixes the issue for me. If I add this
static int add_refs_from_alternate(struct alternate_object_database
*e, void *unused)
{
+	fprintf(stderr, "add: %s\n", e->base);
	char *other = xstrdup(make_absolute_path(e->base));
	size_t len = strlen(other);

and

	prepare_alt_odb();
	for (ent = alt_odb_list; ent; ent = ent->next) {
		fprintf(stderr, "for: %s\n", ent->name);
		if (fn(ent, cb))
			return;

then i get for: in//usr/qt/3/bin:/usr/games/bin:/usr/loca add: /tmp/a/1/.git/objects/in//usr/qt/3/bin:/usr/games/bin:/usr/loca fatal: Could not switch to '/tmp/a/1/.git/objects/in//usr/qt/3/bin:/usr/games/bin:/usr' fatal: The remote end hung up unexpectedly

Junio C Hamano schrieb:

With the following patch, I can no longer reproduce the problem. Does it work fo you, too?

Thanks, René

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

Ah, that would explain the "filled with garbage from $PATH" issue, but I don't think it fixes the fundamental issue.

In the alternate_object_database structure, ent->base[] is a buffer the users can use to form pathnames to loose objects, and ent->name is a pointer into that buffer (it points at one beyond ".git/objects/"). If you get a call to add_refs_from_alternate() after somebody used the entry (has_loose_object() has been called, for example), *ent->name would not be NUL, and ent->base[] won't be the path to the object store.

This caller is expecting to read the path to the object store in ent->base[]; it needs to NUL terminate the buffer if it wants to.

I think the previous patch to sha1_file.c, while it may fix the issue, is not quite nice. Here is a replacement that should work.

 builtin-receive-pack.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

2008/10/26 Junio C Hamano <gitster@pobox.com>:

It does.

"Mikael Magnusson" <mikachu@gmail.com> writes:

Thanks.