Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.

On Thu, Jun 19, 2008 at 11:11:10AM -0400, Jeff King wrote:

I finally got a chance to look at this [distinguishing stale refs from other non-ff refs]. I was originally planning on doing a multi-patch series:

 1/4 distinguish visually between stale / non-ff refs
 2/4 config option to let stale refs not count towards exit code of 1
 3/4 disallow "push -f" without explicit refspec or option (like --all)
 4/4 config option not to show stale refs unless "-v" is given (which
     is now reasonably safe, since invisible refs won't get pushed when
     you just repeat your "git push" with a "-f")

But I didn't like how it was turning out. Specifically:

  - two config options doesn't really make sense. Either you care about
    stale refs, or you don't. If you do, you should see them and have
    them impact your exit code.

  - I thought 3/4 would introduce a generally useful safety valve. But I
    realized I really _like_ the current behavior. Most of the time I
    force a push, it is something like "git push", "oops, that is a
    non-ff, but I know it is OK", "git push -f". It makes sense to me
    that I can repeat my last command and simply say "OK, force this."
    But with such a safety valve, I have to realize the shortcut that
    "git push" was performing (i.e., "git push --matching"), and then
    explicitly retype it. Which is not hard mentally, but it makes the
    interface seem very clunky.

Instead of that, I am considering something more like this:

  - we always visually distinguish stale and other non-ff refs

  - if a config option is set, we treat stale refs as "up to date". That
    is, we don't show them (unless -v is set), and they don't affect the
    exit code. The option is unset by default, giving the same as
    current behavior.

  - If the config option is not set, then forcing works as before.

  - If the config option is set, then we _do not_ force stale refs, but
    only other non-ff. Meaning we are truly treating them like our "up
    to date" refs and saying "there is nothing of interest to push".

This leaves one open issue. If you have the "treat stale as up to date" config option set, how do you force a strict rewind (if the occasion comes up that in some instance, you do want to treat it differently)?

One solution is for the user to unset the config, do the forced push, and then reset it. It makes some sense; the user has said, via the config, that they consider stale refs uninteresting. So to actually perform such a push, they need to "unsay" that temporarily.

Another solution would be an additional flag for forcing strict rewinds (or even "-f -f", though I'm not sure that makes sense). This seems a little hack-ish, since it would _only_ be used if this other config flag is set.

Yet another solution would be to allow "-f" to force a strict rewind, but only if the refspec is mentioned explicitly on the command line, and not part of an automatic match. Reasonably DWIM, but I think it kills the consistency we have now (that "--all" or "--matching" are really just shorthands for spelling out all of the respective refspecs).

Does this seem like a good approach overall? Existing behavior should be identical unless the config option is set, and with it set, I think it should satisfy Pierre and posters from the original thread. If that is sensible, which of the solutions for "no, I really want to force this strict rewind" is the most palatable?

-Peff