Re: [PATCH 2/2] remote: resolve URL-valued push tracking remotes

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

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> A branch may name its push destination with a URL instead of a
> configured remote. This is useful in fork workflows, where the original
> remote is renamed to "upstream", the fork is added as "origin", and an
> existing branch.<name>.pushRemote continues to contain the fork URL.
>
> Git can still push through the anonymous remote created for that URL.
> However, the anonymous remote has no fetch refspec. Git therefore cannot
> resolve @{push} to origin/<branch> or update that remote-tracking branch
> after a push. The push can succeed, or report that everything is up to
> date, while status continues to compare against a stale tracking ref or
> cannot show the push branch at all.

Let me try to think aloud, rephrasing the explanation with a
slightly more concrete illustration, to see whether I understand
what you are trying to achieve.

The current system allows you to set:

     [branch "mytopic"]
        pushRemote = https://hosting.site/users/me/mine.git/
     [remote "notlinked"]
        url = https://hosting.site/users/me/mine.git/
        push = refs/heads/mytopic
        fetch = refs/heads/*:refs/remotes/notlinked/*

but when on the 'mytopic' branch, @{push} cannot determine which
branch at the remote repository to update, so it cannot map it back
to our remote-tracking branch ('refs/remotes/notlinked/mytopic' in
the above illustration).

A question.  Do we currently accept a string that is not a remote
name as the value for 'branch.<name>.pushRemote' by design?

The 'git config --help' output explains that:

 - 'branch.<name>.pushRemote' overrides 'branch.<name>.remote' and
   'remote.pushDefault'; and

 - 'branch.<name>.remote' and 'remote.pushDefault' tell 'git fetch'
   and 'git push' which remote to work with.

It therefore seems clear that setting a string that is not a remote
name (such as a URL) as the value for these three variables is a
misconfiguration in the current system.

I am not saying that it should stay that way forever.  But please
re-read your first sentence and tell me whether it is clear that the
patch extends the current system with a new feature.  It was far
from clear to me and caused significant confusion.  Writing it like
this:

    Under the current system, a branch cannot name its push
    destination using a URL.  If we were to extend the system
    to allow this, such and such benefits would become
    possible.

would have been far less confusing.

If that is what you are doing, that is.

> A uniquely matching configured remote already provides the missing
> mapping.

A very good consideration.  It was the first thing that came to my
mind while I was thinking aloud, constructing an illustration with
'notlinked', wondering "what if there is another remote, with the
same URL, but different 'push' configuration?".

> Use its fetch refspec when resolving the push tracking branch
> and when updating tracking refs after a push.

Is this not needless, and is mentioning it not confusing?  If I
understand correctly, what the change entails is:

 * If the value of 'branch.<name>.pushRemote' (call it X) is 'not' a
   remote name, try to see whether there is a unique remote that
   has either (1) a 'pushurl' whose value matches X, or (2) no
   'pushurl' but a 'url' whose value matches X.  If no such remote
   exists, simply abort and refuse to proceed.

 * If there is such a remote, pretend that the value of
   'branch.<name>.pushRemote' were the name of that remote, and do
   everything else as usual.

And mapping the current branch name to its push destination via
'remote.<name>.push' to find the name of the destination branch at
the remote, and then mapping it back to our remote-tracking branch
using 'remote.<name>.fetch', is not something new that this topic
needs to update, no?

Thanks.  Once I understand what you are trying to achieve, I will
offer further comments on the implementation, as I find this topic
potentially quite interesting.