Re: [PATCH] Make git-clone --use-separate-remote the default

From: Junio C Hamano <junkio@cox.net>
Date: Nov 24, 2006, 05:47 UTC
Message-ID: <7vzmahe6qe.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <7vlkm1hf57.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

Show 9 quoted lines

> I agree --bare should continue to be a "snapshot mirror"; I am
> not advocating for the removal of the internal implementation
> detail such as $use_separate_remote variable.
>
> However, I think having one sane behaviour is the right thing to
> do for a clone that prepares a repository with a working tree
> (including the one made with -n option, which only means "do not
> do the check-out immediately after cloning" for such a
> repository).

Just to let you know, I'll take the patch almost as is (even with the --use-immingled-remote), except with a slight rewording in the documentation to warn people that the backward compatibility option will be removed before the next major release.

However, this simple command fails:

	$ git push $URL master

if the target repository $URL is made with use-separate-remote.

This is because 'master' matches more than one on the remote side (heads/master and remotes/origin/master) which triggers "Hey, that's ambiguous, make yourself clear which one you mean!" check. This breaks t5400 test. We could "fix" the test to make it more explicit, but that is just a workaround.

I think the send-pack/receive-pack pair needs to be taught that an unadorned branch name 'master' never matches anything under refs/remotes. This means that it would require an explicit refspec heads/master:remotes/origin/master in order to pudate refs under refs/remotes on the remote side with a push. I do not think that is a big problem, because the normal patch-flow for shared repository workflow is:

	remote			local

		      (fecth)
	heads/master	--->	remotes/origin/master ---.
							 | (merge)
	heads/master	<---	heads/master	      <--'

and pushing into remotes/origin/* is not a norm.

The function to fix is connect.c::match_explicit_refs() and I _think_ making connect.c::count_refspec_match() not to consider 'foo' to match 'refs/remotes/origin/foo' (but still keeping it to match 'refs/heads/foo' or 'refs/tags/foo') is enough to make this happen.

This brings up two related issues. Currently we automatically prepare "Pull: refs/heads/$branch:refs/remotes/origin/$branch" for all branches that exists at the remote site when a clone happens. Andy Parkins has a patch to allow a glob pattern to be there, like this [*1*]:

	Pull: refs/heads/*:refs/remotes/origin/*

which makes sense, and we might want to have this as the default after the clone [*2*].

Another is if we might want to add "Push: " entry in the default after the clone. I am a bit reluctant to make the default setup too specific to CVS style "central shared repo" workflow, but any stupid default would not suit people with truly distributed workflow anyway, so it might be fine.

[Footnotes]

*1* I rewrote the patch because I wanted to deal with the
    fallout from recent packed-refs work at the same time.  So bugs
    in the counter-proposal patch is mine while the credit for the
    initiative and the idea goes to Andy.

*2* I think the fetch wildcarding has an issue with what remote
    head to merge when used with "git pull".  I think it should
    use the one that is pointed at by refs/remotes/origin/HEAD,
    but there is no code for that yet.  Hints, hints...