Re: [PATCH] request-pull: return the entered branch if more branches are at the same commit

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2010, 01:45 UTC
Message-ID: <7vvdc290ol.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1270678956.27317.34.camel@neo>

Lars Jarnbo Pedersen <lars.jarnbo.pedersen@gmail.com> writes:

Show 12 quoted lines

>  git-request-pull.sh |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index 8fd15f6..787383f 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -49,11 +49,21 @@ merge_base=`git merge-base $baserev $headrev` ||
>  die "fatal: No commits in common between $base and $head"
>  
>  branch=$(git ls-remote "$url" \
> +	| sed -n -e "/^$headrev	refs.heads.$head/{

Isn't $head often omitted, defaulting to HEAD?

Since the original version of this logic was written, git has changed a lot, not in an incompatible way, but simply it got a lot richer. Some assumptions the script made when it was written may need to be revisited, working backwards from the command line to see what we can compute better and how.

    $ git request-pull [options] start url [end]

When "end" is specified, and if that is the name of a branch, we know what branch you are talking about. We can dereference HEAD with symbolic-ref if "end" was missing and we defaulted to HEAD. Either way, in majority of the cases, the user has pushed out the tip of a local branch and that is what "end" would be.

But that "end" branch may not necessarily be the name of the branch your publishing repository has. By looking at configured refspec mapping and the push.default configuration, we can tell which remote ref a push to the url should have updated. The script predates many configurations that control this process, and that is the primary reason it currently guesses from ls-remote output.

You are introducing something better than a guess, but it is not quite there, I suspect. Who says that your branch 'my/topic' will push to your published branch 'my/topic', not 'topic' with "push = my/topic:topic", or "branch.my/topic.merge = topic", for example?

We can take one step at a time, and your patch might be a good first step in the right direction, but I think overhauling this script to be more aware of the ref mapping is worth discussing before moving forward. After such a discussion, it may turn out that majority of people do:

    $ git push $my_public_repo master~3:for-linus

and say "git request-pull origin master~3", in which case the current program output is already correct and the new code may not be adding much value in practice.