Re: [BUG] git-new-workdir doesn't understand packed refs

Peter Baumann [off-list ref] writes:

On Wed, Apr 18, 2007 at 11:42:24AM -0700, Junio C Hamano wrote:

quoted

Peter Baumann [off-list ref] writes:

<ot>

Getting more and more annoyed by your stupid Mail-Followup-To...
I do *not* want to bother Julian with a message that points out
a flaw (in my opinion) in YOUR reasoning but you are forcing me
to send my message that way, which I have to waste time
correcting every time.  Grumble.

</ot>

Hm. Sorry. I don't understand. I'm just pressing 'g' for group reply in
mutt which should do the right thing; even your mail has a CC to Julian
set so I _really_ don't understand the problem. I addressed him in the
begining because he was the author of git-new-workdir. But please
forgive me if I'm breaking some netiquette rules but I just started to
hang out activly on mailinglists ...

Because you had Mail-Followup-To: set to point at me and Julian,
when I say "followup", by default I get this in my MUA:

    To: Julian Phillips [off-list ref]
    Cc: git@vger.kernel.org
    Subject: Re: [BUG] git-new-workdir doesn't understand packed refs

Many people prioritize their e-mails depending on where in the
header their name appears (ones that have you on Cc: typically
gets lower priority than the ones addressed specifically to you
by having you on To: line), and if Julian is doing that, sending
my message in which I want to talk to YOU that way would steal
from Julian's time.  So as a general netiquette, I end up hand
fixing it, putting you on To: and demoting Julian to Cc:.

I know why you (or some version of mutt) do so.  It saves you
from filtering incoming duplicates (one addressed to you,
another addressed to the mailing list you subscribe to), but it
is a misfeature.

Anyhow...

quoted

Also, how is the above different from this?

	git init a
        cd a ; git gc ; cd ..	# allowed
	git new-workdir a b
	cd b ; git gc ; cd ..	# NOT ALLOWED

Sorry, you lost me here. Your above sequence _is_ allowed and that was
just the point of the patch. I lightly tested it that it does the right
thing, so perhaps I'm missing something?

What I was getting at was that if you do not allow new-workdir
to be done off of a symlinked one, that was like not allowing gc
in a symlinked one.  Both are limitations we _could_ lift.  But
I'd like to take that back, because...

This is even dissallowed by the code in git-new-workdir (Sorry, I just
saw it now; otherwise I wouldn't spend so much time in arguing this)):

# don't link to a workdir
if test -L "$orig_git/.git/config"
then
        die "\"$orig_git\" is a working directory only, please specify" \
                "a complete repository."
fi

... I missed this one.  People cannot make a symlinked one off
of another by using new-workdir script, which means perhaps
something like this on top of your patch would be safe enough.
Sorry for the confusion.

But with my patch it just works! I really tested it again. The link
in b/.git/packed-refs -> a/.git/packed-refs (using the example from above)
isn't broken up and in the new generated packed-refs are stored inside
the repo a (as they should).

Oh, I never questioned that you made that basic case work.  I
was worried about not making sure the symlink we are looking at
really is the case we are willing to handle, and not erroring
out if that is not the case, perhaps like the attached patch on
top of yours.

An additional test or two in t/t3210 would be nice to accompany
this change.

diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index afa9b5a..1ce4f55 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c

@@ -123,6 +123,9 @@ int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 			die("readlink failed\n");
 		}
 		buf[st.st_size] = '\0';
+		if (!lstat(buf, &st) && S_ISLNK(st.st_mode))
+			die("cannot have doubly symlinked packed-refs file: %s",
+			    ref_file_name);
 		ref_file_name = buf;
 	}