Re: [PATCH 1/2] t/t5510: demonstrate failure to fetch when current branch has merge ref

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2010, 21:28 UTC
Message-ID: <7vy6bupeko.fsf@alter.siamese.dyndns.org>
In-Reply-To: <O7UxM6KEqdDAhjJAF7ODSonSLShoyHHhaZNp8vb1mx2_JFqmMUj1Op5xiv_-bSd8xW04rLMul2k@cipher.nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

Show 13 quoted lines

> From: Brandon Casey <drafnel@gmail.com>
>
> When 'git fetch' is supplied just a repository URL (not a remote name),
> and without a fetch refspec, it should fetch from the remote HEAD branch
> and update FETCH_HEAD with the fetched ref.  Currently, when 'git fetch'
> is called like this, it fails to retrieve anything, and does not update
> FETCH_HEAD, if the current checked-out branch has a configured merge ref.
>
> i.e. this fetch fails to retrieve anything nor update FETCH_HEAD:
>
>    git checkout master
>    git config branch.master.merge refs/heads/master
>    git fetch git://git.kernel.org/pub/scm/git/git.git

Hmph, we can call it a regression, since we certainly won't see this failure with versions of git that is unaware of branch.*.merge.

But what should we be expecting?

Just as a datapoint, an ancient git (e.g. v1.4.0), the above command line would have fetched the HEAD from the remote side, no matter what that symref is pointing at. Your [2/2] patch replicates this behaviour, which is fine by me [*1*].

Your test only checks if we leave _anything_ in FETCH_HEAD, and does not check if we only fetch one, if we fetch all the refs, or if we fetch what the configuration branch.*.merge asks for (but without the corresponding branch.*.remote configuration, doing so is pointless).

I think it would be better to have two tests. One arranges the current branch to track the same branch the HEAD at the remote points at, and the other arranges the current branch to track a branch different from the HEAD at the remote points at. In both cases, as "fetch" should ignore the configuration, we should get the object pointed by the HEAD on the remote side.

Thanks.

[Footnote]

*1* A plausible alternative is to match the given URL against list of existing remote.<name>.url (make sure there is only one), and behave as if that the remote name is given. I can be persuaded either way, but not looking at the configuration feels a lot simpler to explain and understand (i.e. "with name, we use the set of configuration variable given to that name; without name, there is no configuration for us to look up").