Re: [RFC PATCH] Make 'git request-pull' more strict about matching local/remote branches

[Junio C Hamano <gitster@pobox.com>]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> That may be very helpful if your local tree doesn't match the layout of 
> the remote branches, but for the common case it's been a recurring 
> disaster, when "request-pull" is done against a delayed remote update, and 
> it rewrites the target branch randomly to some other branch name that 
> happens to have the same expected SHA1 (or more commonly, leaves it 
> blank).

Thinking about this a bit more...

> Comments? It passes the tests I put it through locally, but I did *not* 
> make it pass the test-suite, since it very much does change the rules. 
> Some of the test suite code literally tests for the old completely broken 
> case (at least t5150, subtests 4 and 5).

I looked at 5150.4 and found that what it attempts to do is halfway
sensible.  The contributor works on the local 'master' branch,
publishes the result to 'for-linus' in its 'origin' repository, and
asks his state to be pulled, with:

	git push origin master:for-linus
        git request-pull initial origin

The contributor could be more explicit in his request-pull and say 

        git request-pull initial origin master

but there is no 'master' on the publishing repository in this case
(or even if there is, it does not match what is being pushed out),
and there is no 'for-linus' branch locally, so there is no way for
him to say

        git request-pull initial origin for-linus

unless he creates it locally first.

I am starting to wonder if it is a better fix to check potentially
ambiguous cases (e.g. the publishing repository does have 'master'
that does not point at the commit local 'master' points at, and
'for-linus' that points at the same commit, and the user asks for
'master' to be pulled) or clearly broken cases (e.g. the user gave
something other than HEAD explicitly from the command line, but we
ended up computing blank) and die loudly, without breaking cases
this test tries to protect.

On the other hand, I tend to think what 5150.5 wants is convoluted
and expects a broken behaviour.  Its publishing repository has
'master' and 'for-upstream', and also has 'tags/full' that is an
annotated tag that points at the commit, runs request-pull with its
local 'master', and expects the resulting message to ask 'tags/full'
to be pulled.  If the contributor wants such a non-commit to be
pulled, I think it should be made more explicit, i.e., not with

	git request-pull initial $origin_url

but with

	git request-pull initial $origin_url tags/full

or something.