{"thread":{"id":"14034","subject":"[PATCH] git-send-pack: don't consider branch lagging behind as errors.","startedAt":"2008-06-19T10:51:55Z","lastAt":"2008-06-28T04:34:40Z","messageCount":13,"participants":["Pierre Habouzit","Jeff King","Junio C Hamano","しらいしななこ"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"80309","messageId":"1213872715-11182-1-git-send-email-madcoder@debian.org","threadId":"14034","inReplyTo":null,"subject":"[PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-06-19T10:51:55Z","receivedAt":"2008-06-19T10:51:55Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"It's really painful to have git push error out when it's just that one of\nyour tracking branches isn't up to date with respect to a remote branch.\n\nLet just add a new status: \"lagging\", always print it to screen when we're\nlagging, but don't exit with a non 0 value, as it really alarms users.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n\n  \"lagging\" is probably not a very nice name, and anyone is welcomed to use\n  a better word for the concept.\n\n  Another little glitch is that with this patch you can see:\n\n\t  $ git push\n\t  To {your-remote}\n\t   < [lagging]         {local-branch} -> {remote-branch}\n\t  Everything up-to-date\n\n  The \"Everything up-to-date\" is slightly confusing, so maybe we should make it\n  better like \"Everything up-to-date or strict parent\" or whatever.\n\n\n\n builtin-send-pack.c |   10 +++++++++-\n cache.h             |    1 +\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 8d1e7be..cfbc108 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -339,6 +339,9 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count)\n \t\tprint_ref_status('=', \"[up to date]\", ref,\n \t\t\t\tref->peer_ref, NULL);\n \t\tbreak;\n+\tcase REF_STATUS_LAGGING_BEHIND:\n+\t\tprint_ref_status('<', \"[lagging]\", ref, ref->peer_ref, NULL);\n+\t\tbreak;\n \tcase REF_STATUS_REJECT_NONFASTFORWARD:\n \t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n \t\t\t\t\"non-fast forward\");\n@@ -390,6 +393,7 @@ static int refs_pushed(struct ref *ref)\n \t\tswitch(ref->status) {\n \t\tcase REF_STATUS_NONE:\n \t\tcase REF_STATUS_UPTODATE:\n+\t\tcase REF_STATUS_LAGGING_BEHIND:\n \t\t\tbreak;\n \t\tdefault:\n \t\t\treturn 1;\n@@ -568,7 +572,10 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest\n \t\t      || !ref_newer(new_sha1, ref->old_sha1));\n \n \t\tif (ref->nonfastforward && !ref->force && !args.force_update) {\n-\t\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n+\t\t\tif (ref_newer(ref->old_sha1, new_sha1))\n+\t\t\t\tref->status = REF_STATUS_LAGGING_BEHIND;\n+\t\t\telse\n+\t\t\t\tref->status = REF_STATUS_REJECT_NONFASTFORWARD;\n \t\t\tcontinue;\n \t\t}\n \n@@ -628,6 +635,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest\n \t\tswitch (ref->status) {\n \t\tcase REF_STATUS_NONE:\n \t\tcase REF_STATUS_UPTODATE:\n+\t\tcase REF_STATUS_LAGGING_BEHIND:\n \t\tcase REF_STATUS_OK:\n \t\t\tbreak;\n \t\tdefault:\ndiff --git a/cache.h b/cache.h\nindex 23f3b92..b9b32eb 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -673,6 +673,7 @@ struct ref {\n \t\tREF_STATUS_UPTODATE,\n \t\tREF_STATUS_REMOTE_REJECT,\n \t\tREF_STATUS_EXPECTING_REPORT,\n+\t\tREF_STATUS_LAGGING_BEHIND,\n \t} status;\n \tchar *remote_status;\n \tstruct ref *peer_ref; /* when renaming */\n-- \n1.5.6.rc3.158.g1a80c.dirty\n"},{"id":"80313","messageId":"20080619133747.GA31209@sigill.intra.peff.net","threadId":"14034","inReplyTo":"1213872715-11182-1-git-send-email-madcoder@debian.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-19T13:37:48Z","receivedAt":"2008-06-19T13:37:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 19, 2008 at 12:51:55PM +0200, Pierre Habouzit wrote:\n\n> It's really painful to have git push error out when it's just that one of\n> your tracking branches isn't up to date with respect to a remote branch.\n> \n> Let just add a new status: \"lagging\", always print it to screen when we're\n> lagging, but don't exit with a non 0 value, as it really alarms users.\n\nThis has been discussed before, and the suggested term was \"stale\".\n\nCheck out:\n\n  http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186\n\nwhich is uncannily identical (the difference is the name, and that I\ndon't show the lagged branches unless -v is given).\n\nAmong the issues that were not sorted out last time:\n\n  - should stale branches be shown without -v?\n\n  - calling ref_newer here is inefficient, since we have already called\n    it in the other direction. We should probably do the traversal once\n    in such a way as to find out which ref is newer (or if it is\n    indeterminate).\n\n  - there is a possible danger with \"git push -f\", in that you force\n    both rejected branches as well as stale branches. Junio and I\n    discussed the possibility of disallowing \"-f\" unless the user\n    explicitly requested _what_ to push; i.e., --all, --matching,\n    --mirror, or a refspec. See:\n\n      http://thread.gmane.org/gmane.comp.version-control.git/74425/focus=74481\n\nI was considering resurrecting my patch after working up that safety\nvalve.\n\n-Peff\n"},{"id":"80314","messageId":"20080619135159.GA19560@artemis.madism.org","threadId":"14034","inReplyTo":"20080619133747.GA31209@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-06-19T13:52:00Z","receivedAt":"2008-06-19T13:52:00Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Jun 19, 2008 at 01:37:48PM +0000, Jeff King wrote:\n> On Thu, Jun 19, 2008 at 12:51:55PM +0200, Pierre Habouzit wrote:\n> \n> > It's really painful to have git push error out when it's just that one of\n> > your tracking branches isn't up to date with respect to a remote branch.\n> > \n> > Let just add a new status: \"lagging\", always print it to screen when we're\n> > lagging, but don't exit with a non 0 value, as it really alarms users.\n> \n> This has been discussed before, and the suggested term was \"stale\".\n> \n> Check out:\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186\n> \n> which is uncannily identical (the difference is the name, and that I\n> don't show the lagged branches unless -v is given).\n> \n> Among the issues that were not sorted out last time:\n> \n>   - should stale branches be shown without -v?\n\n  I believe so, it's valuable information. It's as valuable as what you\nget after a git fetch nowadays (like branches have diverged n and m\ncommits each or similar) But oh well… I don't care that much.\n\n>   - calling ref_newer here is inefficient, since we have already called\n>     it in the other direction. We should probably do the traversal once\n>     in such a way as to find out which ref is newer (or if it is\n>     indeterminate).\n\n  Well, true, though I don't expect people to have tons of local\nbranches that match a refspec _and_ lag behind. I suspect this is a very\nminor performance loss.\n\n>   - there is a possible danger with \"git push -f\", in that you force\n>     both rejected branches as well as stale branches. Junio and I\n>     discussed the possibility of disallowing \"-f\" unless the user\n>     explicitly requested _what_ to push; i.e., --all, --matching,\n>     --mirror, or a refspec. See:\n> \n>       http://thread.gmane.org/gmane.comp.version-control.git/74425/focus=74481\n\n  Well afaict this is a separate issue, as we're (with such a patch)\nonly changing what gets printed on the console, not the internal\nbehavior. So solving this second issue should not really be a\nprecondition to the inclusion of such a patch.\n\n> I was considering resurrecting my patch after working up that safety\n> valve.\n\n  Please please please do :)\n  The exit 1 of git-push is really annoying me these days.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"80322","messageId":"20080619151110.GA31654@sigill.intra.peff.net","threadId":"14034","inReplyTo":"20080619135159.GA19560@artemis.madism.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-19T15:11:10Z","receivedAt":"2008-06-19T15:11:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:\n\n> >   http://thread.gmane.org/gmane.comp.version-control.git/73038/focus=73186\n> [...]\n> >   - should stale branches be shown without -v?\n> \n>   I believe so, it's valuable information. It's as valuable as what you\n> get after a git fetch nowadays (like branches have diverged n and m\n> commits each or similar) But oh well… I don't care that much.\n\nIf you read the beginning of that thread, the original impetus was\npeople cloning repos that had dozens of branches, then doing a push.\nIf they hadn't recently done a fetch, they got dozens of lines of\n\"rejected\".\n\n> >   - calling ref_newer here is inefficient, since we have already called\n> >     it in the other direction. We should probably do the traversal once\n> >     in such a way as to find out which ref is newer (or if it is\n> >     indeterminate).\n> \n>   Well, true, though I don't expect people to have tons of local\n> branches that match a refspec _and_ lag behind. I suspect this is a very\n> minor performance loss.\n\nYeah, maybe it is not worth worrying about; I haven't actually measured\nany performance issue. I'll try to look and see how painful it is to\ncombine the traversals.\n\n> >   - there is a possible danger with \"git push -f\", in that you force\n> >     both rejected branches as well as stale branches. Junio and I\n>   Well afaict this is a separate issue, as we're (with such a patch)\n> only changing what gets printed on the console, not the internal\n> behavior. So solving this second issue should not really be a\n> precondition to the inclusion of such a patch.\n\nIt is a separate issue, but it is exacerbated by hiding stale refs.\nImagine:\n\n$ git push\nTo /path/to/repo\n   ! [rejected]        master -> master (non-fast forward)\n\n$ git push -f\nTo /path/to/repo\n   + 0abfa88...c1ed93b master -> master (forced update)\n   + 0329485...3498576 stale_branch -> stale_branch (forced update)\n\nI think that is a nasty surprise to spring on an unsuspecting user.\nAnother solution might be \"-f\" not pushing rewound branches, but then we\nneed a way to specify \"no, really, push this rewound branch\". Perhaps\n\"-f -f\"?\n\n>   Please please please do :)\n>   The exit 1 of git-push is really annoying me these days.\n\nOK, I will try to take a look in the next few days.\n\n-Peff\n"},{"id":"80328","messageId":"20080619162801.GA2468@artemis.madism.org","threadId":"14034","inReplyTo":"20080619151110.GA31654@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-06-19T16:28:01Z","receivedAt":"2008-06-19T16:28:01Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:\n> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:\n> > >   - there is a possible danger with \"git push -f\", in that you force\n> > >     both rejected branches as well as stale branches. Junio and I\n> >   Well afaict this is a separate issue, as we're (with such a patch)\n> > only changing what gets printed on the console, not the internal\n> > behavior. So solving this second issue should not really be a\n> > precondition to the inclusion of such a patch.\n> \n> It is a separate issue, but it is exacerbated by hiding stale refs.\n> Imagine:\n> \n> $ git push\n> To /path/to/repo\n>    ! [rejected]        master -> master (non-fast forward)\n> \n> $ git push -f\n> To /path/to/repo\n>    + 0abfa88...c1ed93b master -> master (forced update)\n>    + 0329485...3498576 stale_branch -> stale_branch (forced update)\n> \n> I think that is a nasty surprise to spring on an unsuspecting user.\n> Another solution might be \"-f\" not pushing rewound branches, but then we\n> need a way to specify \"no, really, push this rewound branch\". Perhaps\n> \"-f -f\"?\n\n  Well then we could keep the [stalled] lines for now until this issue\nis resolved then, despite what the people at the beginning of the other\nthread complained about. My real issue is that I have my shell\nconfigured so that my prompt becomes inverted if the last command\nfailed. So do many people I know, and well, git push for stalled\nreferences should just not generate an error. _this_ is my sole concern\n:)\n\n> >   Please please please do :)\n> >   The exit 1 of git-push is really annoying me these days.\n> \n> OK, I will try to take a look in the next few days.\n> \n> -Peff\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"80348","messageId":"7vhcbpuvfb.fsf@gitster.siamese.dyndns.org","threadId":"14034","inReplyTo":"20080619162801.GA2468@artemis.madism.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-19T18:33:12Z","receivedAt":"2008-06-19T18:33:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:\n>> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:\n>> > >   - there is a possible danger with \"git push -f\", in that you force\n>> > >     both rejected branches as well as stale branches. Junio and I\n>> >   Well afaict this is a separate issue, as we're (with such a patch)\n>> > only changing what gets printed on the console, not the internal\n>> > behavior. So solving this second issue should not really be a\n>> > precondition to the inclusion of such a patch.\n>> \n>> It is a separate issue, but it is exacerbated by hiding stale refs.\n>> Imagine:\n>> \n>> $ git push\n>> To /path/to/repo\n>>    ! [rejected]        master -> master (non-fast forward)\n>> \n>> $ git push -f\n>> To /path/to/repo\n>>    + 0abfa88...c1ed93b master -> master (forced update)\n>>    + 0329485...3498576 stale_branch -> stale_branch (forced update)\n>> \n>> I think that is a nasty surprise to spring on an unsuspecting user.\n>> Another solution might be \"-f\" not pushing rewound branches, but then we\n>> need a way to specify \"no, really, push this rewound branch\". Perhaps\n>> \"-f -f\"?\n>\n>   Well then we could keep the [stalled] lines for now until this issue\n> is resolved then, despite what the people at the beginning of the other\n> thread complained about. My real issue is that I have my shell\n> configured so that my prompt becomes inverted if the last command\n> failed. So do many people I know, and well, git push for stalled\n> references should just not generate an error. _this_ is my sole concern\n> :)\n\nThere are two cases the push does not fast forward.  The case where you\nare truly behind (aka \"stale\") and you and the pushed-to repository have\ndiverged history.  Reporting success when you did not push due to the\nlatter is unacceptable.  I personally rely on the fast-forward safety in\nmy push-out scripts, but I do not think it is just me.  The exit status from\ncommands are designed to be used that way.\n\nThe issue of \"many refs in the repo but I work only on a few\" has already\nbeen resolved by being able to say \"I push only the current branch\" in the\nprevious thread, I think, but I am too busy to go back to re-study the\nhistory, so could you kindly do that for us?\n\nThe thing is, the user asked to push certain refs, and some did not get\nupdated.  The user has the right to expect a failure indication from the\ncommand.  If you choose to _ignore_ the failure, that is _your_ choice,\nlike:\n\n\t$ git push || :\n\nYou might argue that the case where you are truly behind _could_ be\nignored and pretend as if the user did not even _ask_ to push it (hence,\nreturn success without doing anything to that branch), but I am not\nconvinced even that is a good idea.\n"},{"id":"80452","messageId":"20080620095508.GB26765@artemis.madism.org","threadId":"14034","inReplyTo":"7vhcbpuvfb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-06-20T09:55:08Z","receivedAt":"2008-06-20T09:55:08Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On jeu, jun 19, 2008 at 06:33:12 +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > On Thu, Jun 19, 2008 at 03:11:10PM +0000, Jeff King wrote:\n> >> On Thu, Jun 19, 2008 at 03:52:00PM +0200, Pierre Habouzit wrote:\n> >> > >   - there is a possible danger with \"git push -f\", in that you force\n> >> > >     both rejected branches as well as stale branches. Junio and I\n> >> >   Well afaict this is a separate issue, as we're (with such a patch)\n> >> > only changing what gets printed on the console, not the internal\n> >> > behavior. So solving this second issue should not really be a\n> >> > precondition to the inclusion of such a patch.\n> >> \n> >> It is a separate issue, but it is exacerbated by hiding stale refs.\n> >> Imagine:\n> >> \n> >> $ git push\n> >> To /path/to/repo\n> >>    ! [rejected]        master -> master (non-fast forward)\n> >> \n> >> $ git push -f\n> >> To /path/to/repo\n> >>    + 0abfa88...c1ed93b master -> master (forced update)\n> >>    + 0329485...3498576 stale_branch -> stale_branch (forced update)\n> >> \n> >> I think that is a nasty surprise to spring on an unsuspecting user.\n> >> Another solution might be \"-f\" not pushing rewound branches, but then we\n> >> need a way to specify \"no, really, push this rewound branch\". Perhaps\n> >> \"-f -f\"?\n> >\n> >   Well then we could keep the [stalled] lines for now until this issue\n> > is resolved then, despite what the people at the beginning of the other\n> > thread complained about. My real issue is that I have my shell\n> > configured so that my prompt becomes inverted if the last command\n> > failed. So do many people I know, and well, git push for stalled\n> > references should just not generate an error. _this_ is my sole concern\n> > :)\n> \n> There are two cases the push does not fast forward.  The case where you\n> are truly behind (aka \"stale\") and you and the pushed-to repository have\n> diverged history.  Reporting success when you did not push due to the\n> latter is unacceptable.  I personally rely on the fast-forward safety in\n> my push-out scripts, but I do not think it is just me.  The exit status from\n> commands are designed to be used that way.\n\n  Sure, but when you use `git push` or `git push <remote>` forms of the\ncommands, you usually don't mean to push \"everything\", but rather\n\"everything that has new stuff\". Stalled branches that you didn't\nfast-forwarded yet should not generate issues IMHO.\n\n  Of course, for git-push <remote> <branch> this could be different.\n\n> The thing is, the user asked to push certain refs, and some did not get\n> updated.  The user has the right to expect a failure indication from the\n> command.  If you choose to _ignore_ the failure, that is _your_ choice,\n> like:\n> \n> \t$ git push || :\n\n  I don't want to ignore push failures. It's just that in my\nworkflow[0], stalled branches are _not_ a problem, whereas non\nfast-forward are. I cannot rely on the exit status, to know if all was\nfine, and that's annoying, really (Not a _major_ PITA, but an annoyance\nnonetheless).\n\n\n  [0] we have quite a few branches for our different releases and\n      production branches. I rarely do fixes there, but still have\n      tracking branches for those, and when I'm pushing my master, I get\n      errors each time another coworker pushed a production fix, until I\n      git stash, git checkout <stalled-branch>, git merge\n      <origin/stalled-branch>, git checkout back, git stash apply\n      *PHEEEW* all that for git push[1] ? No thanks.\n\n  [1] I know I can `git update-ref refs/heads/<stalled-branch> $(git\n      rev-parse origin/<stalled-branch>)` to fix that in one step but:\n        - it's plumbing ;\n\t- if you forget the refs/heads it don't DWIM ;\n\t- it's easy to mess things up with such use patterns\n\t  (sub-problem of `it's plumbing`).\n      So I don't really think this is the proper answer to my problem.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"80519","messageId":"20080621071440.6117@nanako3.lavabit.com","threadId":"14034","inReplyTo":"7vhcbpuvfb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"しらいしななこ","fromEmail":"nanako3@lavabit.com","sentAt":"2008-06-20T22:14:40Z","receivedAt":"2008-06-20T22:14:40Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>:\n\n> The issue of \"many refs in the repo but I work only on a few\" has already\n> been resolved by being able to say \"I push only the current branch\" in the\n> previous thread, I think, but I am too busy to go back to re-study the\n> history, so could you kindly do that for us?\n\nI almost agreed with Pierre before I read your response, but as\nalways you are right.  If the only thing I know is that my\ncurrent branch is ready to be pushed and I do not know if other\nbranches are, I do not have to push all branches (I am not sure\nhow exactly it works. Do we push all the branches, or do we push\nbranches that exist on both sides?).  Pushing the one I want to\npush is the right thing to do, and I can easily say it.\n\n> You might argue that the case where you are truly behind _could_ be\n> ignored and pretend as if the user did not even _ask_ to push it (hence,\n> return success without doing anything to that branch), but I am not\n> convinced even that is a good idea.\n\nBut I think that is a reasonable new feature and is close to\nwhat Pierre is asking for.  He wants to be able to push \"all\nexcept for the ones that he does not want to push\", wants\ngit to guess which ones are the ones he does not, and wants to\nmake the logic of guessing to consider stale ones are unwanted.\n\nBut I do agree with you that it is wrong to replace existing\n\"push all\" with such a new feature.  When people want to push\nall, they do want to push all and see the command report error\nwhen some branches are not updated.  Even though I think what\nPierre wants to have may be a reasonable new feature, it should\nnot break \"push all\" for people who rely on the existing\nbehavior.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"81247","messageId":"20080626075036.GA25770@sigill.intra.peff.net","threadId":"14034","inReplyTo":"20080619151110.GA31654@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-26T07:50:37Z","receivedAt":"2008-06-26T07:50:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 19, 2008 at 11:11:10AM -0400, Jeff King wrote:\n\n> >   Please please please do :)\n> >   The exit 1 of git-push is really annoying me these days.\n> \n> OK, I will try to take a look in the next few days.\n\nI finally got a chance to look at this [distinguishing stale refs from\nother non-ff refs]. I was originally planning on doing a multi-patch\nseries:\n\n 1/4 distinguish visually between stale / non-ff refs\n 2/4 config option to let stale refs not count towards exit code of 1\n 3/4 disallow \"push -f\" without explicit refspec or option (like --all)\n 4/4 config option not to show stale refs unless \"-v\" is given (which\n     is now reasonably safe, since invisible refs won't get pushed when\n     you just repeat your \"git push\" with a \"-f\")\n\nBut I didn't like how it was turning out. Specifically:\n\n  - two config options doesn't really make sense. Either you care about\n    stale refs, or you don't. If you do, you should see them and have\n    them impact your exit code.\n\n  - I thought 3/4 would introduce a generally useful safety valve. But I\n    realized I really _like_ the current behavior. Most of the time I\n    force a push, it is something like \"git push\", \"oops, that is a\n    non-ff, but I know it is OK\", \"git push -f\". It makes sense to me\n    that I can repeat my last command and simply say \"OK, force this.\"\n    But with such a safety valve, I have to realize the shortcut that\n    \"git push\" was performing (i.e., \"git push --matching\"), and then\n    explicitly retype it. Which is not hard mentally, but it makes the\n    interface seem very clunky.\n\nInstead of that, I am considering something more like this:\n\n  - we always visually distinguish stale and other non-ff refs\n\n  - if a config option is set, we treat stale refs as \"up to date\". That\n    is, we don't show them (unless -v is set), and they don't affect the\n    exit code. The option is unset by default, giving the same as\n    current behavior.\n\n  - If the config option is not set, then forcing works as before.\n\n  - If the config option is set, then we _do not_ force stale refs, but\n    only other non-ff. Meaning we are truly treating them like our \"up\n    to date\" refs and saying \"there is nothing of interest to push\".\n\nThis leaves one open issue. If you have the \"treat stale as up to date\"\nconfig option set, how do you force a strict rewind (if the occasion\ncomes up that in some instance, you do want to treat it differently)?\n\nOne solution is for the user to unset the config, do the forced push,\nand then reset it. It makes some sense; the user has said, via the\nconfig, that they consider stale refs uninteresting. So to actually\nperform such a push, they need to \"unsay\" that temporarily.\n\nAnother solution would be an additional flag for forcing strict rewinds\n(or even \"-f -f\", though I'm not sure that makes sense). This seems a\nlittle hack-ish, since it would _only_ be used if this other config flag\nis set.\n\nYet another solution would be to allow \"-f\" to force a strict rewind,\nbut only if the refspec is mentioned explicitly on the command line, and\nnot part of an automatic match. Reasonably DWIM, but I think it kills\nthe consistency we have now (that \"--all\" or \"--matching\" are really\njust shorthands for spelling out all of the respective refspecs).\n\nDoes this seem like a good approach overall? Existing behavior should be\nidentical unless the config option is set, and with it set, I think it\nshould satisfy Pierre and posters from the original thread. If that is\nsensible, which of the solutions for \"no, I really want to force this\nstrict rewind\" is the most palatable?\n\n-Peff\n"},{"id":"81250","messageId":"7vfxr0sj61.fsf@gitster.siamese.dyndns.org","threadId":"14034","inReplyTo":"20080626075036.GA25770@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-26T08:19:02Z","receivedAt":"2008-06-26T08:19:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Does this seem like a good approach overall? Existing behavior should be\n> identical unless the config option is set, and with it set, I think it\n> should satisfy Pierre and posters from the original thread. If that is\n> sensible, which of the solutions for \"no, I really want to force this\n> strict rewind\" is the most palatable?\n\nThat \"strict rewind\" workaround alone made me so disgusted beyond words.\n\nAnd then made me step back and think again.\n\nIf it is stale, it is stale.  A revert is just a different history that\ndid not have some things that already happened, while a fork has some\nother things that did not happen in the original history on top of that.\nThey are the same thing -- alternate history that drops some things that\nalready happened.  It does not matter if you are strictly stale or if you\nbuilt a few (or hundreds of) commits on top of something that is stale.\nYou are losing history if you tried to push such a thing.\n\nAnd if you do not want to trigger \"what you are pushing is not up to\ndate\", the solution is simple.  You do not push them.\n\nI however think what Pierre wanted to do is different, and you do not have\nto solve \"force this strict rewind\" problem to solve it.\n\nWhat we need is (with a configuration option) to change the meaning of\n\"matching refs\" from the current definition \"branches with the same name\nbetween here and there\" to \"branches with the same name between here and\nthere, but excluding the ones that do not fast forward (or strictly\nbehind)\".  So if you have tons of stale branches you checked out but\nhaven't touched most of them, your \"matching push\" won't even try to push\nwhat you haven't touched.\n\nI do not think we would ever want to allow a configuration that disables\nthe exit status from push upon a push failure when somebody explicitly\nsays \"git push $there $this\" and $this is not non-fast-forward.  If you\nasked $this to be pushed, and if $this cannot be pushed, we _should_ error\nout.\n\nSo...\n\n (1) when you say \"git push\" to trigger the \"matching refs\" push, with the\n     new configuration, you do not error out nor even try to push out\n     things that do not fast forward.  Pretend that you did not even\n     attempt to push them, and do not error out.  This may result in not\n     pushing anything out, but that is what the people who have such a\n     configuration is asking for.\n\n     If you have configured which branches are pushed when you are on your\n     current branch, and that branch --- most likely it is that current\n     branch --- does not fast forward, it is your problem.\n\n (2) even with such a configuration, you can \"git push $there $this\" and\n     \"git push $there +$this\" to explicitly ask refs to be pushed.  Such a\n     push won't be interfered by the new configuration and correctly fail\n     (or force).\n"},{"id":"81287","messageId":"20080626132858.GE7237@artemis.madism.org","threadId":"14034","inReplyTo":"7vfxr0sj61.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-06-26T13:28:58Z","receivedAt":"2008-06-26T13:28:58Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Jun 26, 2008 at 08:19:02AM +0000, Junio C Hamano wrote:\n> I however think what Pierre wanted to do is different, and you do not have\n> to solve \"force this strict rewind\" problem to solve it.\n> \n> What we need is (with a configuration option) to change the meaning of\n> \"matching refs\" from the current definition \"branches with the same name\n> between here and there\" to \"branches with the same name between here and\n> there, but excluding the ones that do not fast forward (or strictly\n> behind)\".  So if you have tons of stale branches you checked out but\n> haven't touched most of them, your \"matching push\" won't even try to push\n> what you haven't touched.\n> \n> I do not think we would ever want to allow a configuration that disables\n> the exit status from push upon a push failure when somebody explicitly\n> says \"git push $there $this\" and $this is not non-fast-forward.  If you\n> asked $this to be pushed, and if $this cannot be pushed, we _should_ error\n> out.\n>\n> So...\n> \n>  (1) when you say \"git push\" to trigger the \"matching refs\" push, with the\n>      new configuration, you do not error out nor even try to push out\n>      things that do not fast forward.  Pretend that you did not even\n>      attempt to push them, and do not error out.  This may result in not\n>      pushing anything out, but that is what the people who have such a\n>      configuration is asking for.\n> \n>      If you have configured which branches are pushed when you are on your\n>      current branch, and that branch --- most likely it is that current\n>      branch --- does not fast forward, it is your problem.\n> \n>  (2) even with such a configuration, you can \"git push $there $this\" and\n>      \"git push $there +$this\" to explicitly ask refs to be pushed.  Such a\n>      push won't be interfered by the new configuration and correctly fail\n>      (or force).\n\n  Of course if I: git push $remote $branch, if it's stalled, I want it\nto fail (exit with !0). *BUT* if I don't specify branches, I don't want\nthe stalled ones to generate noise. That's exactly the behaviour I'd\nlike to see.\n\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"81540","messageId":"20080628043338.GC9451@sigill.intra.peff.net","threadId":"14034","inReplyTo":"7vfxr0sj61.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-28T04:33:38Z","receivedAt":"2008-06-28T04:33:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 26, 2008 at 01:19:02AM -0700, Junio C Hamano wrote:\n\n> That \"strict rewind\" workaround alone made me so disgusted beyond words.\n\nHeh.\n\n> And if you do not want to trigger \"what you are pushing is not up to\n> date\", the solution is simple.  You do not push them.\n> \n> I however think what Pierre wanted to do is different, and you do not have\n> to solve \"force this strict rewind\" problem to solve it.\n> \n> What we need is (with a configuration option) to change the meaning of\n> \"matching refs\" from the current definition \"branches with the same name\n> between here and there\" to \"branches with the same name between here and\n> there, but excluding the ones that do not fast forward (or strictly\n> behind)\".  So if you have tons of stale branches you checked out but\n> haven't touched most of them, your \"matching push\" won't even try to push\n> what you haven't touched.\n\nOK. Isn't this basically just another way of restating my third\n\"solution\"? Mine marks strict rewinds as \"up to date\" instead of\n\"matching\", but in either case, the result is that there is nothing to\neven try pushing. And the proposal I gave was \"allow -f to force a\nstrict rewind [which would be as it is now] but only if the refspec is\nmentioned explicitly on the command line, and not part of an automatic\nmatch.\"\n\nSo I think the behavior is the same, but the difference is the mental\nmodel.\n\nAnd not counting stale branches as matching doesn't seem like quite the\nright mental model. The matching is about the ref _names_, not about\ntheir contents. So I think the better mental model is that these\nbranches are simply \"up to date\" already -- the remote has everything we\nhave, and more.\n\nNot to mention that ref matching can affect _other_ refs, too, can it\nnot? I think there are some checks for having two refspec srcs pushing\nto the same dest.\n\n> I do not think we would ever want to allow a configuration that disables\n> the exit status from push upon a push failure when somebody explicitly\n> says \"git push $there $this\" and $this is not non-fast-forward.  If you\n> asked $this to be pushed, and if $this cannot be pushed, we _should_ error\n> out.\n\nWe do this already for refs that have no changes to push. IOW, we are\nnot masking an error so much as saying \"this condition is _not_ an\nerror\" (and letting the user say that with a config option).\n\n>  (1) when you say \"git push\" to trigger the \"matching refs\" push, with the\n>      new configuration, you do not error out nor even try to push out\n>      things that do not fast forward.  Pretend that you did not even\n>      attempt to push them, and do not error out.  This may result in not\n>      pushing anything out, but that is what the people who have such a\n>      configuration is asking for.\n> \n>      If you have configured which branches are pushed when you are on your\n>      current branch, and that branch --- most likely it is that current\n>      branch --- does not fast forward, it is your problem.\n> \n>  (2) even with such a configuration, you can \"git push $there $this\" and\n>      \"git push $there +$this\" to explicitly ask refs to be pushed.  Such a\n>      push won't be interfered by the new configuration and correctly fail\n>      (or force).\n\nI think (1) should be implemented not as \"this src ref and this dst ref\ndo not match\" but rather \"this pair is up to date\". And given that, I\nthink that is basically the same as my proposal, except that the\n\"push.rewindIsUpToDate\" config variable would only be respected for a\n\"matching refs\" push. Which does feel a little inconsistent, as I noted\nin my previous mail, but I think it DWYM.\n\n-Peff\n"},{"id":"81541","messageId":"20080628043440.GD9451@sigill.intra.peff.net","threadId":"14034","inReplyTo":"20080628043338.GC9451@sigill.intra.peff.net","subject":"Re: [PATCH] git-send-pack: don't consider branch lagging behind as errors.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-06-28T04:34:40Z","receivedAt":"2008-06-28T04:34:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 28, 2008 at 12:33:38AM -0400, Jeff King wrote:\n\n> I think (1) should be implemented not as \"this src ref and this dst ref\n> do not match\" but rather \"this pair is up to date\". And given that, I\n> think that is basically the same as my proposal, except that the\n> \"push.rewindIsUpToDate\" config variable would only be respected for a\n> \"matching refs\" push. Which does feel a little inconsistent, as I noted\n> in my previous mail, but I think it DWYM.\n\nWhere of course if we take \"Y\" in DWYM to be Pierre, he has already said\nthat's what he meant. :)\n\n-Peff\n"}]}