Jeff King [off-list ref] writes:
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?
That "strict rewind" workaround alone made me so disgusted beyond words.
And then made me step back and think again.
If it is stale, it is stale. A revert is just a different history that
did not have some things that already happened, while a fork has some
other things that did not happen in the original history on top of that.
They are the same thing -- alternate history that drops some things that
already happened. It does not matter if you are strictly stale or if you
built a few (or hundreds of) commits on top of something that is stale.
You are losing history if you tried to push such a thing.
And if you do not want to trigger "what you are pushing is not up to
date", the solution is simple. You do not push them.
I however think what Pierre wanted to do is different, and you do not have
to solve "force this strict rewind" problem to solve it.
What we need is (with a configuration option) to change the meaning of
"matching refs" from the current definition "branches with the same name
between here and there" to "branches with the same name between here and
there, but excluding the ones that do not fast forward (or strictly
behind)". So if you have tons of stale branches you checked out but
haven't touched most of them, your "matching push" won't even try to push
what you haven't touched.
I do not think we would ever want to allow a configuration that disables
the exit status from push upon a push failure when somebody explicitly
says "git push $there $this" and $this is not non-fast-forward. If you
asked $this to be pushed, and if $this cannot be pushed, we _should_ error
out.
So...
(1) when you say "git push" to trigger the "matching refs" push, with the
new configuration, you do not error out nor even try to push out
things that do not fast forward. Pretend that you did not even
attempt to push them, and do not error out. This may result in not
pushing anything out, but that is what the people who have such a
configuration is asking for.
If you have configured which branches are pushed when you are on your
current branch, and that branch --- most likely it is that current
branch --- does not fast forward, it is your problem.
(2) even with such a configuration, you can "git push $there $this" and
"git push $there +$this" to explicitly ask refs to be pushed. Such a
push won't be interfered by the new configuration and correctly fail
(or force).
On Thu, Jun 26, 2008 at 08:19:02AM +0000, Junio C Hamano wrote:
I however think what Pierre wanted to do is different, and you do not have
to solve "force this strict rewind" problem to solve it.
What we need is (with a configuration option) to change the meaning of
"matching refs" from the current definition "branches with the same name
between here and there" to "branches with the same name between here and
there, but excluding the ones that do not fast forward (or strictly
behind)". So if you have tons of stale branches you checked out but
haven't touched most of them, your "matching push" won't even try to push
what you haven't touched.
I do not think we would ever want to allow a configuration that disables
the exit status from push upon a push failure when somebody explicitly
says "git push $there $this" and $this is not non-fast-forward. If you
asked $this to be pushed, and if $this cannot be pushed, we _should_ error
out.
So...
(1) when you say "git push" to trigger the "matching refs" push, with the
new configuration, you do not error out nor even try to push out
things that do not fast forward. Pretend that you did not even
attempt to push them, and do not error out. This may result in not
pushing anything out, but that is what the people who have such a
configuration is asking for.
If you have configured which branches are pushed when you are on your
current branch, and that branch --- most likely it is that current
branch --- does not fast forward, it is your problem.
(2) even with such a configuration, you can "git push $there $this" and
"git push $there +$this" to explicitly ask refs to be pushed. Such a
push won't be interfered by the new configuration and correctly fail
(or force).
Of course if I: git push $remote $branch, if it's stalled, I want it
to fail (exit with !0). *BUT* if I don't specify branches, I don't want
the stalled ones to generate noise. That's exactly the behaviour I'd
like to see.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
On Thu, Jun 26, 2008 at 01:19:02AM -0700, Junio C Hamano wrote:
That "strict rewind" workaround alone made me so disgusted beyond words.
Heh.
And if you do not want to trigger "what you are pushing is not up to
date", the solution is simple. You do not push them.
I however think what Pierre wanted to do is different, and you do not have
to solve "force this strict rewind" problem to solve it.
What we need is (with a configuration option) to change the meaning of
"matching refs" from the current definition "branches with the same name
between here and there" to "branches with the same name between here and
there, but excluding the ones that do not fast forward (or strictly
behind)". So if you have tons of stale branches you checked out but
haven't touched most of them, your "matching push" won't even try to push
what you haven't touched.
OK. Isn't this basically just another way of restating my third
"solution"? Mine marks strict rewinds as "up to date" instead of
"matching", but in either case, the result is that there is nothing to
even try pushing. And the proposal I gave was "allow -f to force a
strict rewind [which would be as it is now] but only if the refspec is
mentioned explicitly on the command line, and not part of an automatic
match."
So I think the behavior is the same, but the difference is the mental
model.
And not counting stale branches as matching doesn't seem like quite the
right mental model. The matching is about the ref _names_, not about
their contents. So I think the better mental model is that these
branches are simply "up to date" already -- the remote has everything we
have, and more.
Not to mention that ref matching can affect _other_ refs, too, can it
not? I think there are some checks for having two refspec srcs pushing
to the same dest.
I do not think we would ever want to allow a configuration that disables
the exit status from push upon a push failure when somebody explicitly
says "git push $there $this" and $this is not non-fast-forward. If you
asked $this to be pushed, and if $this cannot be pushed, we _should_ error
out.
We do this already for refs that have no changes to push. IOW, we are
not masking an error so much as saying "this condition is _not_ an
error" (and letting the user say that with a config option).
(1) when you say "git push" to trigger the "matching refs" push, with the
new configuration, you do not error out nor even try to push out
things that do not fast forward. Pretend that you did not even
attempt to push them, and do not error out. This may result in not
pushing anything out, but that is what the people who have such a
configuration is asking for.
If you have configured which branches are pushed when you are on your
current branch, and that branch --- most likely it is that current
branch --- does not fast forward, it is your problem.
(2) even with such a configuration, you can "git push $there $this" and
"git push $there +$this" to explicitly ask refs to be pushed. Such a
push won't be interfered by the new configuration and correctly fail
(or force).
I think (1) should be implemented not as "this src ref and this dst ref
do not match" but rather "this pair is up to date". And given that, I
think that is basically the same as my proposal, except that the
"push.rewindIsUpToDate" config variable would only be respected for a
"matching refs" push. Which does feel a little inconsistent, as I noted
in my previous mail, but I think it DWYM.
-Peff
On Sat, Jun 28, 2008 at 12:33:38AM -0400, Jeff King wrote:
I think (1) should be implemented not as "this src ref and this dst ref
do not match" but rather "this pair is up to date". And given that, I
think that is basically the same as my proposal, except that the
"push.rewindIsUpToDate" config variable would only be respected for a
"matching refs" push. Which does feel a little inconsistent, as I noted
in my previous mail, but I think it DWYM.
Where of course if we take "Y" in DWYM to be Pierre, he has already said
that's what he meant. :)
-Peff