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

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

[Pierre Habouzit]

It's really painful to have git push error out when it's just that one of
your tracking branches isn't up to date with respect to a remote branch.

Let just add a new status: "lagging", always print it to screen when we're
lagging, but don't exit with a non 0 value, as it really alarms users.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  "lagging" is probably not a very nice name, and anyone is welcomed to use
  a better word for the concept.

  Another little glitch is that with this patch you can see:

	  $ git push
	  To {your-remote}
	   < [lagging]         {local-branch} -> {remote-branch}
	  Everything up-to-date

  The "Everything up-to-date" is slightly confusing, so maybe we should make it
  better like "Everything up-to-date or strict parent" or whatever.



 builtin-send-pack.c |   10 +++++++++-
 cache.h             |    1 +
 2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8d1e7be..cfbc108 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -339,6 +339,9 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count)
 		print_ref_status('=', "[up to date]", ref,
 				ref->peer_ref, NULL);
 		break;
+	case REF_STATUS_LAGGING_BEHIND:
+		print_ref_status('<', "[lagging]", ref, ref->peer_ref, NULL);
+		break;
 	case REF_STATUS_REJECT_NONFASTFORWARD:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				"non-fast forward");
@@ -390,6 +393,7 @@ static int refs_pushed(struct ref *ref)
 		switch(ref->status) {
 		case REF_STATUS_NONE:
 		case REF_STATUS_UPTODATE:
+		case REF_STATUS_LAGGING_BEHIND:
 			break;
 		default:
 			return 1;
@@ -568,7 +572,10 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		      || !ref_newer(new_sha1, ref->old_sha1));
 
 		if (ref->nonfastforward && !ref->force && !args.force_update) {
-			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+			if (ref_newer(ref->old_sha1, new_sha1))
+				ref->status = REF_STATUS_LAGGING_BEHIND;
+			else
+				ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
 			continue;
 		}
 
@@ -628,6 +635,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		switch (ref->status) {
 		case REF_STATUS_NONE:
 		case REF_STATUS_UPTODATE:
+		case REF_STATUS_LAGGING_BEHIND:
 		case REF_STATUS_OK:
 			break;
 		default:
diff --git a/cache.h b/cache.h
index 23f3b92..b9b32eb 100644
--- a/cache.h
+++ b/cache.h
@@ -673,6 +673,7 @@ struct ref {
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
+		REF_STATUS_LAGGING_BEHIND,
 	} status;
 	char *remote_status;
 	struct ref *peer_ref; /* when renaming */
-- 
1.5.6.rc3.158.g1a80c.dirty

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

[Jeff King]

On Thu, Jun 19, 2008 at 12:51:55PM +0200, Pierre Habouzit wrote:

> It's really painful to have git push error out when it's just that one of
> your tracking branches isn't up to date with respect to a remote branch.
> 
> Let just add a new status: "lagging", always print it to screen when we're
> lagging, but don't exit with a non 0 value, as it really alarms users.

This has been discussed before, and the suggested term was "stale".

Check out:

  http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186

which is uncannily identical (the difference is the name, and that I
don't show the lagged branches unless -v is given).

Among the issues that were not sorted out last time:

  - should stale branches be shown without -v?

  - calling ref_newer here is inefficient, since we have already called
    it in the other direction. We should probably do the traversal once
    in such a way as to find out which ref is newer (or if it is
    indeterminate).

  - there is a possible danger with "git push -f", in that you force
    both rejected branches as well as stale branches. Junio and I
    discussed the possibility of disallowing "-f" unless the user
    explicitly requested _what_ to push; i.e., --all, --matching,
    --mirror, or a refspec. See:

      http://thread.gmane.org/gmane.comp.version-control.git/74425/focus=74481

I was considering resurrecting my patch after working up that safety
valve.

-Peff

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

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]

On Thu, Jun 19, 2008 at 01:37:48PM +0000, Jeff King wrote:
> On Thu, Jun 19, 2008 at 12:51:55PM +0200, Pierre Habouzit wrote:
> 
> > It's really painful to have git push error out when it's just that one of
> > your tracking branches isn't up to date with respect to a remote branch.
> > 
> > Let just add a new status: "lagging", always print it to screen when we're
> > lagging, but don't exit with a non 0 value, as it really alarms users.
> 
> This has been discussed before, and the suggested term was "stale".
> 
> Check out:
> 
>   http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186
> 
> which is uncannily identical (the difference is the name, and that I
> don't show the lagged branches unless -v is given).
> 
> Among the issues that were not sorted out last time:
> 
>   - should stale branches be shown without -v?

  I believe so, it's valuable information. It's as valuable as what you
get after a git fetch nowadays (like branches have diverged n and m
commits each or similar) But oh well… I don't care that much.

>   - calling ref_newer here is inefficient, since we have already called
>     it in the other direction. We should probably do the traversal once
>     in such a way as to find out which ref is newer (or if it is
>     indeterminate).

  Well, true, though I don't expect people to have tons of local
branches that match a refspec _and_ lag behind. I suspect this is a very
minor performance loss.

>   - there is a possible danger with "git push -f", in that you force
>     both rejected branches as well as stale branches. Junio and I
>     discussed the possibility of disallowing "-f" unless the user
>     explicitly requested _what_ to push; i.e., --all, --matching,
>     --mirror, or a refspec. See:
> 
>       http://thread.gmane.org/gmane.comp.version-control.git/74425/focus=74481

  Well afaict this is a separate issue, as we're (with such a patch)
only changing what gets printed on the console, not the internal
behavior. So solving this second issue should not really be a
precondition to the inclusion of such a patch.

> I was considering resurrecting my patch after working up that safety
> valve.

  Please please please do :)
  The exit 1 of git-push is really annoying me these days.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Jeff King]

On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:

> >   http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186
> [...]
> >   - should stale branches be shown without -v?
> 
>   I believe so, it's valuable information. It's as valuable as what you
> get after a git fetch nowadays (like branches have diverged n and m
> commits each or similar) But oh well… I don't care that much.

If you read the beginning of that thread, the original impetus was
people cloning repos that had dozens of branches, then doing a push.
If they hadn't recently done a fetch, they got dozens of lines of
"rejected".

> >   - calling ref_newer here is inefficient, since we have already called
> >     it in the other direction. We should probably do the traversal once
> >     in such a way as to find out which ref is newer (or if it is
> >     indeterminate).
> 
>   Well, true, though I don't expect people to have tons of local
> branches that match a refspec _and_ lag behind. I suspect this is a very
> minor performance loss.

Yeah, maybe it is not worth worrying about; I haven't actually measured
any performance issue. I'll try to look and see how painful it is to
combine the traversals.

> >   - there is a possible danger with "git push -f", in that you force
> >     both rejected branches as well as stale branches. Junio and I
>   Well afaict this is a separate issue, as we're (with such a patch)
> only changing what gets printed on the console, not the internal
> behavior. So solving this second issue should not really be a
> precondition to the inclusion of such a patch.

It is a separate issue, but it is exacerbated by hiding stale refs.
Imagine:

$ git push
To /path/to/repo
   ! [rejected]        master -> master (non-fast forward)

$ git push -f
To /path/to/repo
   + 0abfa88...c1ed93b master -> master (forced update)
   + 0329485...3498576 stale_branch -> stale_branch (forced update)

I think that is a nasty surprise to spring on an unsuspecting user.
Another solution might be "-f" not pushing rewound branches, but then we
need a way to specify "no, really, push this rewound branch". Perhaps
"-f -f"?

>   Please please please do :)
>   The exit 1 of git-push is really annoying me these days.

OK, I will try to take a look in the next few days.

-Peff

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

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]

On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:
> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:
> > >   - there is a possible danger with "git push -f", in that you force
> > >     both rejected branches as well as stale branches. Junio and I
> >   Well afaict this is a separate issue, as we're (with such a patch)
> > only changing what gets printed on the console, not the internal
> > behavior. So solving this second issue should not really be a
> > precondition to the inclusion of such a patch.
> 
> It is a separate issue, but it is exacerbated by hiding stale refs.
> Imagine:
> 
> $ git push
> To /path/to/repo
>    ! [rejected]        master -> master (non-fast forward)
> 
> $ git push -f
> To /path/to/repo
>    + 0abfa88...c1ed93b master -> master (forced update)
>    + 0329485...3498576 stale_branch -> stale_branch (forced update)
> 
> I think that is a nasty surprise to spring on an unsuspecting user.
> Another solution might be "-f" not pushing rewound branches, but then we
> need a way to specify "no, really, push this rewound branch". Perhaps
> "-f -f"?

  Well then we could keep the [stalled] lines for now until this issue
is resolved then, despite what the people at the beginning of the other
thread complained about. My real issue is that I have my shell
configured so that my prompt becomes inverted if the last command
failed. So do many people I know, and well, git push for stalled
references should just not generate an error. _this_ is my sole concern
:)

> >   Please please please do :)
> >   The exit 1 of git-push is really annoying me these days.
> 
> OK, I will try to take a look in the next few days.
> 
> -Peff

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

> On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:
>> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:
>> > >   - there is a possible danger with "git push -f", in that you force
>> > >     both rejected branches as well as stale branches. Junio and I
>> >   Well afaict this is a separate issue, as we're (with such a patch)
>> > only changing what gets printed on the console, not the internal
>> > behavior. So solving this second issue should not really be a
>> > precondition to the inclusion of such a patch.
>> 
>> It is a separate issue, but it is exacerbated by hiding stale refs.
>> Imagine:
>> 
>> $ git push
>> To /path/to/repo
>>    ! [rejected]        master -> master (non-fast forward)
>> 
>> $ git push -f
>> To /path/to/repo
>>    + 0abfa88...c1ed93b master -> master (forced update)
>>    + 0329485...3498576 stale_branch -> stale_branch (forced update)
>> 
>> I think that is a nasty surprise to spring on an unsuspecting user.
>> Another solution might be "-f" not pushing rewound branches, but then we
>> need a way to specify "no, really, push this rewound branch". Perhaps
>> "-f -f"?
>
>   Well then we could keep the [stalled] lines for now until this issue
> is resolved then, despite what the people at the beginning of the other
> thread complained about. My real issue is that I have my shell
> configured so that my prompt becomes inverted if the last command
> failed. So do many people I know, and well, git push for stalled
> references should just not generate an error. _this_ is my sole concern
> :)

There are two cases the push does not fast forward.  The case where you
are truly behind (aka "stale") and you and the pushed-to repository have
diverged history.  Reporting success when you did not push due to the
latter is unacceptable.  I personally rely on the fast-forward safety in
my push-out scripts, but I do not think it is just me.  The exit status from
commands are designed to be used that way.

The issue of "many refs in the repo but I work only on a few" has already
been resolved by being able to say "I push only the current branch" in the
previous thread, I think, but I am too busy to go back to re-study the
history, so could you kindly do that for us?

The thing is, the user asked to push certain refs, and some did not get
updated.  The user has the right to expect a failure indication from the
command.  If you choose to _ignore_ the failure, that is _your_ choice,
like:

	$ git push || :

You might argue that the case where you are truly behind _could_ be
ignored and pretend as if the user did not even _ask_ to push it (hence,
return success without doing anything to that branch), but I am not
convinced even that is a good idea.

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

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 4122 bytes --]

On jeu, jun 19, 2008 at 06:33:12 +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> > On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:
> >> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:
> >> > >   - there is a possible danger with "git push -f", in that you force
> >> > >     both rejected branches as well as stale branches. Junio and I
> >> >   Well afaict this is a separate issue, as we're (with such a patch)
> >> > only changing what gets printed on the console, not the internal
> >> > behavior. So solving this second issue should not really be a
> >> > precondition to the inclusion of such a patch.
> >> 
> >> It is a separate issue, but it is exacerbated by hiding stale refs.
> >> Imagine:
> >> 
> >> $ git push
> >> To /path/to/repo
> >>    ! [rejected]        master -> master (non-fast forward)
> >> 
> >> $ git push -f
> >> To /path/to/repo
> >>    + 0abfa88...c1ed93b master -> master (forced update)
> >>    + 0329485...3498576 stale_branch -> stale_branch (forced update)
> >> 
> >> I think that is a nasty surprise to spring on an unsuspecting user.
> >> Another solution might be "-f" not pushing rewound branches, but then we
> >> need a way to specify "no, really, push this rewound branch". Perhaps
> >> "-f -f"?
> >
> >   Well then we could keep the [stalled] lines for now until this issue
> > is resolved then, despite what the people at the beginning of the other
> > thread complained about. My real issue is that I have my shell
> > configured so that my prompt becomes inverted if the last command
> > failed. So do many people I know, and well, git push for stalled
> > references should just not generate an error. _this_ is my sole concern
> > :)
> 
> There are two cases the push does not fast forward.  The case where you
> are truly behind (aka "stale") and you and the pushed-to repository have
> diverged history.  Reporting success when you did not push due to the
> latter is unacceptable.  I personally rely on the fast-forward safety in
> my push-out scripts, but I do not think it is just me.  The exit status from
> commands are designed to be used that way.

  Sure, but when you use `git push` or `git push <remote>` forms of the
commands, you usually don't mean to push "everything", but rather
"everything that has new stuff". Stalled branches that you didn't
fast-forwarded yet should not generate issues IMHO.

  Of course, for git-push <remote> <branch> this could be different.

> The thing is, the user asked to push certain refs, and some did not get
> updated.  The user has the right to expect a failure indication from the
> command.  If you choose to _ignore_ the failure, that is _your_ choice,
> like:
> 
> 	$ git push || :

  I don't want to ignore push failures. It's just that in my
workflow[0], stalled branches are _not_ a problem, whereas non
fast-forward are. I cannot rely on the exit status, to know if all was
fine, and that's annoying, really (Not a _major_ PITA, but an annoyance
nonetheless).


  [0] we have quite a few branches for our different releases and
      production branches. I rarely do fixes there, but still have
      tracking branches for those, and when I'm pushing my master, I get
      errors each time another coworker pushed a production fix, until I
      git stash, git checkout <stalled-branch>, git merge
      <origin/stalled-branch>, git checkout back, git stash apply
      *PHEEEW* all that for git push[1] ? No thanks.

  [1] I know I can `git update-ref refs/heads/<stalled-branch> $(git
      rev-parse origin/<stalled-branch>)` to fix that in one step but:
        - it's plumbing ;
	- if you forget the refs/heads it don't DWIM ;
	- it's easy to mess things up with such use patterns
	  (sub-problem of `it's plumbing`).
      So I don't really think this is the proper answer to my problem.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[しらいしななこ]

Quoting Junio C Hamano <gitster@pobox.com>:

> The issue of "many refs in the repo but I work only on a few" has already
> been resolved by being able to say "I push only the current branch" in the
> previous thread, I think, but I am too busy to go back to re-study the
> history, so could you kindly do that for us?

I almost agreed with Pierre before I read your response, but as
always you are right.  If the only thing I know is that my
current branch is ready to be pushed and I do not know if other
branches are, I do not have to push all branches (I am not sure
how exactly it works. Do we push all the branches, or do we push
branches that exist on both sides?).  Pushing the one I want to
push is the right thing to do, and I can easily say it.

> You might argue that the case where you are truly behind _could_ be
> ignored and pretend as if the user did not even _ask_ to push it (hence,
> return success without doing anything to that branch), but I am not
> convinced even that is a good idea.

But I think that is a reasonable new feature and is close to
what Pierre is asking for.  He wants to be able to push "all
except for the ones that he does not want to push", wants
git to guess which ones are the ones he does not, and wants to
make the logic of guessing to consider stale ones are unwanted.

But I do agree with you that it is wrong to replace existing
"push all" with such a new feature.  When people want to push
all, they do want to push all and see the command report error
when some branches are not updated.  Even though I think what
Pierre wants to have may be a reasonable new feature, it should
not break "push all" for people who rely on the existing
behavior.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

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

[Jeff King]

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

> >   Please please please do :)
> >   The exit 1 of git-push is really annoying me these days.
> 
> OK, I will try to take a look in the next few days.

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

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

[Junio C Hamano]

Jeff King <peff@peff.net> 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).

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

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 2309 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

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

[Jeff King]

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

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

[Jeff King]

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