Re: [PATCH 2/2] push: hint to use push.default=upstream when appropriate

Jeff King [off-list ref] writes:

It seems like the real problem is that they are on branch "foo", but the
matching behavior tried to push "bar", which didn't work. And we are
worried that they may be surprised that we even attempted to push "bar"
at all.

Of we may be seeing a non-fast-forward on 'foo' itself, which is your
1. below.

And that probably happened because of the situation you describe, but we
(and the user) don't have to think about that. We can just think about
the more immediate mistake of "oops, I didn't want to push 'bar'".

Which leads me to two ideas:

  1. This is not good advice to give when pushing the _current_ branch
     failed due to non-ff. Setting push.default to "upstream" would
     probably yield the same error. We should probably tighten the
     condition for showing this message to when a non-HEAD branch failed
     to fast-forward.

  2. The text should focus on the (possible) local misconfiguration, not
     the repo setup.

OK, I think we are in agreement.

So maybe something like:

  By default, git pushes all branches that have a matching counterpart
  on the remote. In this case, some of your local branches were stale
  with respect to their remote counterparts. If you did not intend to
  push these branches, you may want to set the 'push.default'
  configuration variable to 'upstream' to push only the current branch.

I'm not 100% happy with that text, but I think you can see the direction
I am talking about.

As long as we can tighten the condition further to ensure that the advice
above is triggered only when appropriate, I actually am 100% happy with
that text. Seeing others do the real work for me always makes me happy ;-)

In addition to "did we use default-matching?", we should use "did we get
non-fast-forward error on a branch that is _not_ the current one?" instead
of the "did we get any non-fast-forward error?" in my patch, and the text
should match the situation more-or-less exactly.

... If we follow my suggestion above and
only print this message for non-HEAD refs, then these two messages
become an either/or situation, I think. If the HEAD failed to
fast-forward, then the right advice is probably "git pull && git push".
If a non-HEAD failed, then the right advice is either "git checkout $X
&& git pull && git push" or "here's how to avoid accidentally pushing
$X".

So the logic would be something like:

  if (nonfastforward == NONFASTFORWARD_HEAD)
          advise_pull_before_push();
  else if (nonfastforward == NONFASTFORWAD_OTHER) {
          if (default_matching_used)
                  advise_use_upstream();
          else
                  advise_checkout_pull_push();
  }

Sounds good. Spelling things out at this detail would let others who may
be interested in getting their hands dirty try to come up with an updated
patch ;-).

Thanks.