git-push branch confusion caused by user mistake

This week a user accidentally did this:

    $ git push origin origin/master
    Total 0 (delta 0), reused 0 (delta 0)
    To parent.git
     * [new branch]      origin/master -> origin/master

He saw his mistake when the "new branch" message appeared, but he was confused about how to fix it and worried he broke something.

It seems reasonable that git expanded the original args into this one:

    git push origin refs/remotes/origin/master

However, since the dest ref was not provided, it was assumed to be the same as the source ref, so it worked as if he typed this:

    git push origin refs/remotes/origin/master:refs/remotes/origin/master

Indeed, git ls-remote origin shows the result:

    $ git ls-remote origin
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master

Also, I verified that this (otherwise valid) command has similar
unexpected results:
    $ git remote add other foo.git && git fetch other && git push
origin other/topic
    $ git ls-remote origin
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master
    d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/other/topic

I think git should be smarter about deducing the dest ref from the source ref if the source ref is in refs/remotes, but I'm not sure how far to take it. It feels like we should translate refspecs something like this for push:

    origin/master
        => refs/remotes/origin/master:refs/heads/master

    refs/remotes/origin/master
         => refs/remotes/origin/master:refs/heads/master

    origin/master:origin/master
         => refs/remotes/origin/master:refs/heads/origin/master

    master:refs/remotes/origin/master
         => refs/heads/master:refs/remotes/origin/master

That is, we should not infer a remote refspec of "refs/remotes/*"; we should only get there if "refs/remotes" was given explicitly by the user.

Does this seem reasonable?  I can try to work up a patch if so.

Phil Hord <phil.hord@gmail.com> writes:

My knee-jerk reaction is "Don't take it anywhere".  

Giving a refspec from the command line is an established way to defeat the default behaviour when you do not give any and only the remote, and making it do things behind user's back, you would be robbing the escape hatch from people.

It often is useful in real-life workflow when "git push $dest origin/master" does exactly the way it works now, which I actually use myself. Imagine that you have two repositories, use one of them primarily to interact with the outside world and do your work, but you then occasionally push from that primary repository to the other one, instead of logging into the host that has the other one and running a fetch on that host from the outside world. Your "trying to be clever when given a colon-less refspec" will force people to type "git push $dest origin/master:origin/master" in such a case.

On Fri, Mar 10, 2017 at 2:13 PM, Junio C Hamano <gitster@pobox.com> wrote:

It might be worth having some warning or something happen here? I've had several co-workers at $DAYJOB get confused by this sort of thing.

Thanks, Jake

On Mon, Mar 13, 2017 at 1:55 AM Jacob Keller <jacob.keller@gmail.com> wrote:

On one very active project at $work, we have 380,000 commits, 4600 branches in refs/heads and 96 branches in refs/remotes. About half of the refs/remotes (43) are obviously user errors. The other half it's not possible for me to know.

I suggested to our admins to block attempts to push to 'refs/remotes/*' so in the future users don't lose track of commits they think they pushed. But I don't know if that will really happen.

Thanks for the counterexample feedback.

W dniu 10.03.2017 o 22:44, Phil Hord pisze:

It is nowadays very easy to delete accidentally created remote branch with

      $ git push origin --delete origin/master
 

This rule depends on push.default setting, but it is a very simple rule. Simple is good. DWIM is usually not worth it, unless program can guess what you meant, and what you meant is always the same.

[...]

Such push doesn't make sense (unless you have a quite unusual situation).

Note that 'origin/master', that is 'refs/remotes/origin/master' is a remote-tracking branch, that is a ref that is meant to track position of the 'master' branch ('refs/heads/master') in the 'origin' remote. Thus it should always be the same as 'master' in 'origin', or be behind if you didn't fetch.

Thus I don't think such complication is reasonable.