{"thread":{"id":"30042","subject":"odd behavior with git-rebase","startedAt":"2012-03-23T18:52:05Z","lastAt":"2012-03-28T17:08:44Z","messageCount":17,"participants":["Neil Horman","Jeff King","Junio C Hamano","Neal Kreitzinger","Phil Hord","Jay Soffian"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"187601","messageId":"20120323185205.GA11916@hmsreliant.think-freely.org","threadId":"30042","inReplyTo":null,"subject":"odd behavior with git-rebase","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-03-23T18:52:05Z","receivedAt":"2012-03-23T18:52:05Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"Hey all-\n\tI hit a strange problem with git rebase and I can't quite decide if its\na design point of the rebase command, or if its happening in error.  When doing\nupstream backports of various kernel components I occasionally run accross\ncommits that, for whatever reason, I don't want/need or can't backport.  When\nthat happens, I insert an empty commit in my history noting the upstream commit\nhash and the reasoning behind why I skipped it (I use git commit -c <hash>\n--allow-empty).  If I later rebase this branch, I note that all my empty commits\nfail indicating the commit cannot be applied.  I can of course do another git\ncommit --allow-empty -c <hash>; git rebase --continue, and everything is fine,\nbut I'd rather it just take the empty commit in the rebase if possible.\n\nI know that git cherry-pick allows for picking of empty commits, and it appears\nthe rebase script uses cherry-picking significantly, so I'm not sure why this\nisn't working, or if its explicitly prevented from working for some reason.\n\nanyone have any insight?\n\nThanks & Regards\nNeil\n"},{"id":"187608","messageId":"20120323195455.GB15063@sigill.intra.peff.net","threadId":"30042","inReplyTo":"20120323185205.GA11916@hmsreliant.think-freely.org","subject":"Re: odd behavior with git-rebase","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-23T19:54:56Z","receivedAt":"2012-03-23T19:54:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 23, 2012 at 02:52:05PM -0400, Neil Horman wrote:\n\n> \tI hit a strange problem with git rebase and I can't quite decide if its\n> a design point of the rebase command, or if its happening in error.  When doing\n> upstream backports of various kernel components I occasionally run accross\n> commits that, for whatever reason, I don't want/need or can't backport.  When\n> that happens, I insert an empty commit in my history noting the upstream commit\n> hash and the reasoning behind why I skipped it (I use git commit -c <hash>\n> --allow-empty).  If I later rebase this branch, I note that all my empty commits\n> fail indicating the commit cannot be applied.  I can of course do another git\n> commit --allow-empty -c <hash>; git rebase --continue, and everything is fine,\n> but I'd rather it just take the empty commit in the rebase if possible.\n\nI think it is even odder than that. If you use plain rebase, the empty\ncommits are silently omitted. If you do an interactive rebase, you get\nthe \"could not apply\" message (and just doing a \"continue\" creates some\nfunny error messages and ends up omitting the commit).\n\nI think both of these are bugs. In the first case, the empty commit\nappears to be already applied, because it does nothing. But if somebody\nbothered to create an empty commit in the first place, they probably\nwant to keep it, and we should special-case it.\n\nAs you've probably guessed, empty commits are not all that common, and I\nthink this area of git is not well-tested.\n\n-Peff\n"},{"id":"187609","messageId":"7vvclvrrad.fsf@alter.siamese.dyndns.org","threadId":"30042","inReplyTo":"20120323185205.GA11916@hmsreliant.think-freely.org","subject":"Re: odd behavior with git-rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-23T20:33:30Z","receivedAt":"2012-03-23T20:33:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neil Horman <nhorman@tuxdriver.com> writes:\n\n> I know that git cherry-pick allows for picking of empty commits, and it appears\n> the rebase script uses cherry-picking significantly, so I'm not sure why this\n> isn't working, or if its explicitly prevented from working for some reason.\n\nThe primary purpose of \"rebase\" is (or at least was when it was conceived)\nto clean up the existing history, and a part of the cleaning up is not to\nreplay a patch that ends up being empty.  Even though we try to omit an\nalready applied patch by using \"git cherry\" internally when choosing which\ncommits to replay, a commit that by itself is *not* empty could end up\nbeing empty when a similar change has already been made to the updated\nbase, and we do want to omit them.\n\nA commit that is empty (i.e. --allow-empty) by itself was a much later\ninvention than the basic rebase logic, and the rebase may want to be\nupdated to special case it, but as the default behaviour it is doing the\nright thing by not letting an empty commit into the cleaned up history.\n"},{"id":"187642","messageId":"20120324165536.GA17932@neilslaptop.think-freely.org","threadId":"30042","inReplyTo":"7vvclvrrad.fsf@alter.siamese.dyndns.org","subject":"Re: odd behavior with git-rebase","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-03-24T16:55:36Z","receivedAt":"2012-03-24T16:55:36Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Fri, Mar 23, 2012 at 01:33:30PM -0700, Junio C Hamano wrote:\n> Neil Horman <nhorman@tuxdriver.com> writes:\n> \n> > I know that git cherry-pick allows for picking of empty commits, and it appears\n> > the rebase script uses cherry-picking significantly, so I'm not sure why this\n> > isn't working, or if its explicitly prevented from working for some reason.\n> \n> The primary purpose of \"rebase\" is (or at least was when it was conceived)\n> to clean up the existing history, and a part of the cleaning up is not to\nI can understand that, although IMHO it seems equally usefull as a tool for\nsimply doing what its name implies, moving a history to a new starting point,\ne.g. to plainly rebase it.  Thats the use that I have for it anyway.\n\n> replay a patch that ends up being empty.  Even though we try to omit an\n> already applied patch by using \"git cherry\" internally when choosing which\n> commits to replay, a commit that by itself is *not* empty could end up\n> being empty when a similar change has already been made to the updated\n> base, and we do want to omit them.\n> \nIs there a way to differentiate a commit that is made empty as the result of a\nprevious patch in the rebase, and a commit that is simply empty?\n\n> A commit that is empty (i.e. --allow-empty) by itself was a much later\n> invention than the basic rebase logic, and the rebase may want to be\n> updated to special case it, but as the default behaviour it is doing the\n> right thing by not letting an empty commit into the cleaned up history.\nI agree, I think perhaps adding an --allow-empty option to the rebase logic, so\nthat empty commits (or perhaps just initially empty, as opposed to commits made\nempty) would be very beneficial.  \n\nThanks all, I'll start trying to pick through the rebase logic this week.\n\nBest\nNeil\n\n> \n> \n> \n"},{"id":"187728","messageId":"4F708AFD.4070402@gmail.com","threadId":"30042","inReplyTo":"20120323185205.GA11916@hmsreliant.think-freely.org","subject":"Re: odd behavior with git-rebase","fromName":"Neal Kreitzinger","fromEmail":"nkreitzinger@gmail.com","sentAt":"2012-03-26T15:27:57Z","receivedAt":"2012-03-26T15:27:57Z","isPatch":false,"sender":{"key":"nkreitzinger@gmail.com","avatar":null},"body":"On 3/23/2012 1:52 PM, Neil Horman wrote:\n> Hey all-\n> \tI hit a strange problem with git rebase and I can't quite decide if its\n> a design point of the rebase command, or if its happening in error.  When doing\n> upstream backports of various kernel components I occasionally run accross\n> commits that, for whatever reason, I don't want/need or can't backport.  When\n> that happens, I insert an empty commit in my history noting the upstream commit\n> hash and the reasoning behind why I skipped it (I use git commit -c<hash>\n> --allow-empty).  If I later rebase this branch, I note that all my empty commits\n> fail indicating the commit cannot be applied.  I can of course do another git\n> commit --allow-empty -c<hash>; git rebase --continue, and everything is fine,\n> but I'd rather it just take the empty commit in the rebase if possible.\n>\n> I know that git cherry-pick allows for picking of empty commits, and it appears\n> the rebase script uses cherry-picking significantly, so I'm not sure why this\n> isn't working, or if its explicitly prevented from working for some reason.\n>\n> anyone have any insight?\n>\nFWIW, I'm not sure what you mean by \"backport\", but IMHO backporting a \ncritical fix to an earlier version seems by nature to be a cherry-pick \noperation as opposed to a rebase operation.  A rebase implies \"I want \neverything\" -- that doesn't sound like a backport.  A cherry-pick \nimplies \"I only want certain things\" -- that sounds like a backport. \nMaybe your really using rebase to cherry-pick several commits.  Using \nyour technique of \"empty commit placeholders\", it seems you could end up \nwith quite a lot of \"empty\" commit placeholders which doesn't seem to \nmake much sense.  Why would you want a bunch of empty commit \nplaceholders in your older version bugfix history saying \"I didn't want \nthis, but its in the newer version.\"  (who cares?).  Isn't having the \nstuff you do want recorded as commits enough to make it clear what you \nbrought over?  You could even edit the \"cherry-picked\" (or rebased) \ncommit messages to document the sha1 of the commit being cherry picked \nfrom the newer version.  That seems to make more sense to document what \nyou did, as opposed to documenting what you didn't do.\n\nI'm not a git expert or a kernel developer, but find this subject \nrelevant and interesting.  Our \"New\" system was forked from our \"old\" \nsystem.  We bring over fixes/features from \"old\" system to \"new\" system. \n  \"New\" system hast limited field testing accumulated, and gets \nfeatures/fixes from \"old\" system which has extensive productional \ntesting ongoing.  When doing so, many \"old\" system changes are not \nwanted as they are irrelevent to the \"new\" system.  Having an empty \ncommit for them makes no sense.\n\nFood for thought.  Maybe my cooking as bad.  Maybe I'll learn a new recipe.\n\nv/r,\nneal\n"},{"id":"187740","messageId":"7v1uofqoa7.fsf@alter.siamese.dyndns.org","threadId":"30042","inReplyTo":"20120324165536.GA17932@neilslaptop.think-freely.org","subject":"Re: odd behavior with git-rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-26T17:12:48Z","receivedAt":"2012-03-26T17:12:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neil Horman <nhorman@tuxdriver.com> writes:\n\n> Is there a way to differentiate a commit that is made empty as the result of a\n> previous patch in the rebase, and a commit that is simply empty?\n\nAn empty commit has the same tree object as its parent commit.\n\n> I agree, I think perhaps adding an --allow-empty option to the rebase logic, so\n> that empty commits (or perhaps just initially empty, as opposed to commits made\n> empty) would be very beneficial.\n\nYeah, that probably may make sense.\n"},{"id":"187741","messageId":"20120326171823.GA12843@hmsreliant.think-freely.org","threadId":"30042","inReplyTo":"4F708AFD.4070402@gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-03-26T17:18:23Z","receivedAt":"2012-03-26T17:18:23Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Mon, Mar 26, 2012 at 10:27:57AM -0500, Neal Kreitzinger wrote:\n> On 3/23/2012 1:52 PM, Neil Horman wrote:\n> >Hey all-\n> >\tI hit a strange problem with git rebase and I can't quite decide if its\n> >a design point of the rebase command, or if its happening in error.  When doing\n> >upstream backports of various kernel components I occasionally run accross\n> >commits that, for whatever reason, I don't want/need or can't backport.  When\n> >that happens, I insert an empty commit in my history noting the upstream commit\n> >hash and the reasoning behind why I skipped it (I use git commit -c<hash>\n> >--allow-empty).  If I later rebase this branch, I note that all my empty commits\n> >fail indicating the commit cannot be applied.  I can of course do another git\n> >commit --allow-empty -c<hash>; git rebase --continue, and everything is fine,\n> >but I'd rather it just take the empty commit in the rebase if possible.\n> >\n> >I know that git cherry-pick allows for picking of empty commits, and it appears\n> >the rebase script uses cherry-picking significantly, so I'm not sure why this\n> >isn't working, or if its explicitly prevented from working for some reason.\n> >\n> >anyone have any insight?\n> >\n> FWIW, I'm not sure what you mean by \"backport\", but IMHO backporting\n> a critical fix to an earlier version seems by nature to be a\n> cherry-pick operation as opposed to a rebase operation.  A rebase\nI work on RHEL, among other things, keeping network drivers up to date with\nupstream bugfixes/features/etc.  When I say 'backport' I mean exactly that,\nbackporting an upstream fix/feature/change to various RHEL kernel versions.\n\nNominally, I would love to be able to do a merge from upstream to just take\neverything, but for various and sundry reasons we can't take all upstream\nchanges (ABI commitments, core API availabliilty, etc).  So In my workflow, I\nreview each commit to the drivers I'm working on, and either cherry-pick them\nback to the relevant RHEL kernel, or I add an empty commit on my private working\nbranch using the upstream changelog entry, referencing the upstream commit hash.\nThat way I have a positive indicator that I've looked at a given upstream\ncommit, and decided not to cherry-pick it.  Prior to integrating with the\nmainline kernel I do an interactive rebase of my private branch, and drop any\ncommits that I've marked as empty.\n\nHowever, during the period that I'm backporting a driver, I occasionally have\nneed to do a rebase where I want to keep my empty commits (perhaps I've made a\nmistake in cleaning up a previous cherry-pick, or want to rebase my branch to\nthe origin/master to pick up a common fix).  At those times, I like to keep my\nhistory in tact, empty commits and all, so I can hold on to my notes about which\ncommits I've reviewed and their dispositions in my work.\n\n> implies \"I want everything\" -- that doesn't sound like a backport.\nRebases aren't equivalent to \"I wan't everything\" (at least not always).  I\nthink what you're thinking of is a merge. A rebase (in the sense that I'm using\nit), is either just a movement of my work branch to a newer base, or a replay of\nmy history to correct an error.\n\n> A cherry-pick implies \"I only want certain things\" -- that sounds\n> like a backport. Maybe your really using rebase to cherry-pick\n> several commits.  Using your technique of \"empty commit\n> placeholders\", it seems you could end up with quite a lot of \"empty\"\n> commit placeholders which doesn't seem to make much sense.  Why\nSee above, the empty commits are really just personal notes.  I expunge them\nprior to integration with master.  The empty commits are just personal notes, an\ninline area to indiate to myself what upstream commits I have (and have yet to)\nconsider.\n\n> would you want a bunch of empty commit placeholders in your older\n> version bugfix history saying \"I didn't want this, but its in the\n> newer version.\"  (who cares?).  Isn't having the stuff you do want\nI care, for the purposes of my backporting work..\n\n> recorded as commits enough to make it clear what you brought over?\n> You could even edit the \"cherry-picked\" (or rebased) commit messages\n> to document the sha1 of the commit being cherry picked from the\n> newer version.  That seems to make more sense to document what you\nYes, I do that, using cherry-pick -x.\n\n> did, as opposed to documenting what you didn't do.\n> \n I document both what I selected to backport, and what I opted not to backport\nand why (using the empty commit logs).  \n\nRegardless, the rational behind my specific case doesn't matter too much.  We\nallow the creation of empty commits (via git commit --allow-empty) because\nexplicitly empty commits are sometimes useful.  It would seems to me that, while\nit makes sense that rebasing cleaning empty commits by default is quite sane, it\nwould make good sense to add an allow-empty option to rebase to let them\nsurvive.  I'm looking into adding that option now.\n\nRegards\nNeil\n"},{"id":"187742","messageId":"20120326172028.GB12843@hmsreliant.think-freely.org","threadId":"30042","inReplyTo":"7v1uofqoa7.fsf@alter.siamese.dyndns.org","subject":"Re: odd behavior with git-rebase","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-03-26T17:20:28Z","receivedAt":"2012-03-26T17:20:28Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Mon, Mar 26, 2012 at 10:12:48AM -0700, Junio C Hamano wrote:\n> Neil Horman <nhorman@tuxdriver.com> writes:\n> \n> > Is there a way to differentiate a commit that is made empty as the result of a\n> > previous patch in the rebase, and a commit that is simply empty?\n> \n> An empty commit has the same tree object as its parent commit.\n> \nGot it, thanks!\n\n> > I agree, I think perhaps adding an --allow-empty option to the rebase logic, so\n> > that empty commits (or perhaps just initially empty, as opposed to commits made\n> > empty) would be very beneficial.\n> \n> Yeah, that probably may make sense.\n> \nOk, cool, I'll have a patch in a few days, thanks!\nNeil\n"},{"id":"187759","messageId":"CABURp0oJwM-KtdBRVHgvOaqFVjA-MEAfJoJH=52Y=QRcgFL+3Q@mail.gmail.com","threadId":"30042","inReplyTo":"7v1uofqoa7.fsf@alter.siamese.dyndns.org","subject":"Re: odd behavior with git-rebase","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-26T18:29:24Z","receivedAt":"2012-03-26T18:29:24Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Mon, Mar 26, 2012 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Neil Horman <nhorman@tuxdriver.com> writes:\n>\n>> Is there a way to differentiate a commit that is made empty as the result of a\n>> previous patch in the rebase, and a commit that is simply empty?\n>\n> An empty commit has the same tree object as its parent commit.\n>\n>> I agree, I think perhaps adding an --allow-empty option to the rebase logic, so\n>> that empty commits (or perhaps just initially empty, as opposed to commits made\n>> empty) would be very beneficial.\n>\n> Yeah, that probably may make sense.\n\n\nCan we have three behaviors?\n\nA: Current mode, stop and error on empty commits\nB: --keep-empty, to retain empty commits without further notice\nC: --purge-empty, to remove empty commits without further notice\n\nPhil\n"},{"id":"187760","messageId":"CABURp0qeJEwELpg_YKxn9Ghb6EMphrwwfueM2XCqua3X_dacdA@mail.gmail.com","threadId":"30042","inReplyTo":"20120323195455.GB15063@sigill.intra.peff.net","subject":"Re: odd behavior with git-rebase","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-26T18:31:05Z","receivedAt":"2012-03-26T18:31:05Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Fri, Mar 23, 2012 at 3:54 PM, Jeff King <peff@peff.net> wrote:\n> On Fri, Mar 23, 2012 at 02:52:05PM -0400, Neil Horman wrote:\n>\n>>       I hit a strange problem with git rebase and I can't quite decide if its\n>> a design point of the rebase command, or if its happening in error.  When doing\n>> upstream backports of various kernel components I occasionally run accross\n>> commits that, for whatever reason, I don't want/need or can't backport.  When\n>> that happens, I insert an empty commit in my history noting the upstream commit\n>> hash and the reasoning behind why I skipped it (I use git commit -c <hash>\n>> --allow-empty).  If I later rebase this branch, I note that all my empty commits\n>> fail indicating the commit cannot be applied.  I can of course do another git\n>> commit --allow-empty -c <hash>; git rebase --continue, and everything is fine,\n>> but I'd rather it just take the empty commit in the rebase if possible.\n>\n> I think it is even odder than that. If you use plain rebase, the empty\n> commits are silently omitted. If you do an interactive rebase, you get\n> the \"could not apply\" message (and just doing a \"continue\" creates some\n> funny error messages and ends up omitting the commit).\n\nCoincidentally I ran into this same behavior this week.  But what\nbothered me about it was the messages git gave me.  The empty commit\ngave me cherry-pick hints instead of rebase ones, including advising\nme to \"use 'git reset'\" to resolve the problem if I don't want this\ncommit after all.\n\n$ git rebase -i HEAD~10\n...\nThe previous cherry-pick is now empty, possibly due to conflict resolution.\nIf you wish to commit it anyway, use:\n\n    git commit --allow-empty\n\nOtherwise, please use 'git reset'\n# Not currently on any branch.\nnothing to commit (working directory clean)\nCould not apply d513504... Some commit message\n\n\nI'm not sure if this is the norm or if it's a result of some other\nthings I did in this sequence.  But I've seen it several times now.\nI've only tested it on 1.7.10 versions, including RC2.\n\nPhil\n"},{"id":"187776","messageId":"20120326195619.GB13098@sigill.intra.peff.net","threadId":"30042","inReplyTo":"CABURp0qeJEwELpg_YKxn9Ghb6EMphrwwfueM2XCqua3X_dacdA@mail.gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-03-26T19:56:19Z","receivedAt":"2012-03-26T19:56:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 26, 2012 at 02:31:05PM -0400, Phil Hord wrote:\n\n> Coincidentally I ran into this same behavior this week.  But what\n> bothered me about it was the messages git gave me.  The empty commit\n> gave me cherry-pick hints instead of rebase ones, including advising\n> me to \"use 'git reset'\" to resolve the problem if I don't want this\n> commit after all.\n> \n> $ git rebase -i HEAD~10\n> ...\n> The previous cherry-pick is now empty, possibly due to conflict resolution.\n> If you wish to commit it anyway, use:\n> \n>     git commit --allow-empty\n> \n> Otherwise, please use 'git reset'\n> # Not currently on any branch.\n> nothing to commit (working directory clean)\n> Could not apply d513504... Some commit message\n> \n> \n> I'm not sure if this is the norm or if it's a result of some other\n> things I did in this sequence.  But I've seen it several times now.\n> I've only tested it on 1.7.10 versions, including RC2.\n\nThis is easily reproducible on a simple test case:\n\n  commit() {\n    echo $1 >$1 && git add $1 && git commit -m $1 && git tag $1\n  }\n\n  git init repo &&\n  cd repo &&\n  commit one &&\n  commit two &&\n  git commit --allow-empty -m empty &&\n  commit three &&\n  git checkout -b fork one &&\n  commit four &&\n  git rebase -i fork master\n  git --no-pager log --oneline\n\n(this is the same test case I used without \"-i\" to check the rebase\nskipping behavior).\n\nI agree the mention of cherry-pick is a little confusing. I think the\nadvice to use \"git commit --allow-empty\" is still the right thing\n(although better still would be to recognize that the commit was empty\nin the first place and not stop at all). I think the message is showing\nthe fact that \"rebase -i\" is cobbled together from other pieces. I\nwonder if the sequencer work would make this a little smoother (I\nconfess I have not paid much attention to what is happening in that\narea).\n\n-Peff\n"},{"id":"187779","messageId":"20120326200418.GC12843@hmsreliant.think-freely.org","threadId":"30042","inReplyTo":"CABURp0oJwM-KtdBRVHgvOaqFVjA-MEAfJoJH=52Y=QRcgFL+3Q@mail.gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-03-26T20:04:18Z","receivedAt":"2012-03-26T20:04:18Z","isPatch":false,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Mon, Mar 26, 2012 at 02:29:24PM -0400, Phil Hord wrote:\n> On Mon, Mar 26, 2012 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> > Neil Horman <nhorman@tuxdriver.com> writes:\n> >\n> >> Is there a way to differentiate a commit that is made empty as the result of a\n> >> previous patch in the rebase, and a commit that is simply empty?\n> >\n> > An empty commit has the same tree object as its parent commit.\n> >\n> >> I agree, I think perhaps adding an --allow-empty option to the rebase logic, so\n> >> that empty commits (or perhaps just initially empty, as opposed to commits made\n> >> empty) would be very beneficial.\n> >\n> > Yeah, that probably may make sense.\n> \n> \n> Can we have three behaviors?\n> \n> A: Current mode, stop and error on empty commits\n> B: --keep-empty, to retain empty commits without further notice\n> C: --purge-empty, to remove empty commits without further notice\n> \nYeah, I've got most of --keep-empty in a private branch here now.  I was calling\nit allow-empty, but given (C) above, I like --keep-empty better.\n\nI'll add --purge-empty to me todo list. and augment the rebase code to pass\nthese options along.\n\nOne more question - The options for cherry-pick are currently mostly merged with\ngit revert.  Are there any opinions on the applicability of\n--keep-empty/--purge-empty to reverts?  \n\nRegards\nNeil\n"},{"id":"187791","messageId":"4F70E53E.6060608@gmail.com","threadId":"30042","inReplyTo":"20120326172028.GB12843@hmsreliant.think-freely.org","subject":"Re: odd behavior with git-rebase","fromName":"Neal Kreitzinger","fromEmail":"nkreitzinger@gmail.com","sentAt":"2012-03-26T21:53:02Z","receivedAt":"2012-03-26T21:53:02Z","isPatch":false,"sender":{"key":"nkreitzinger@gmail.com","avatar":null},"body":"On 3/26/2012 12:20 PM, Neil Horman wrote:\n> On Mon, Mar 26, 2012 at 10:12:48AM -0700, Junio C Hamano wrote:\n>> Neil Horman<nhorman@tuxdriver.com>  writes:\n>>\n>>> I agree, I think perhaps adding an --allow-empty option to the rebase logic, so\n>>> that empty commits (or perhaps just initially empty, as opposed to commits made\n>>> empty) would be very beneficial.\n>>\n>> Yeah, that probably may make sense.\n>>\n> Ok, cool, I'll have a patch in a few days, thanks!\n>\nIMO, it seems like --allow-empty is an appropriate patch for git-rebase \n(non-interactive), and that git-rebase -i would need a command like \n\"k\"eep to distinguish which empty commits are not to be discarded and \nwhich empty commits are ok to discard automatically.  git-rebase -i \nshould allow explicit control on a commit by commit basis as opposed to \nblanket rules like \"discard all empty commits\" or \"keep all empty \ncommits\" that apply to all commits in the rebase-to-do list based on a \nsingle cli option.\n\nMaybe this is what you plan on doing.  Maybe there is a better command \nname than \"k\"eep for this.\n\nv/r,\nneal\n"},{"id":"187792","messageId":"CABURp0oP3YBEhpDrAL-mvt1dR+ZH3av-P_sqDQAdgcN10WS2ig@mail.gmail.com","threadId":"30042","inReplyTo":"4F70E53E.6060608@gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-26T22:53:32Z","receivedAt":"2012-03-26T22:53:32Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Mon, Mar 26, 2012 at 5:53 PM, Neal Kreitzinger\n<nkreitzinger@gmail.com> wrote:\n> On 3/26/2012 12:20 PM, Neil Horman wrote:\n>>\n>> On Mon, Mar 26, 2012 at 10:12:48AM -0700, Junio C Hamano wrote:\n>>>\n>>> Neil Horman<nhorman@tuxdriver.com>  writes:\n>>>\n>>>> I agree, I think perhaps adding an --allow-empty option to the rebase\n>>>> logic, so\n>>>> that empty commits (or perhaps just initially empty, as opposed to\n>>>> commits made\n>>>> empty) would be very beneficial.\n>>>\n>>>\n>>> Yeah, that probably may make sense.\n>>>\n>> Ok, cool, I'll have a patch in a few days, thanks!\n>>\n> IMO, it seems like --allow-empty is an appropriate patch for git-rebase\n> (non-interactive), and that git-rebase -i would need a command like \"k\"eep\n> to distinguish which empty commits are not to be discarded and which empty\n> commits are ok to discard automatically.  git-rebase -i should allow\n> explicit control on a commit by commit basis as opposed to blanket rules\n> like \"discard all empty commits\" or \"keep all empty commits\" that apply to\n> all commits in the rebase-to-do list based on a single cli option.\n\nBut I don't want a 'keep-even-if-empty' option in interactive.  I want\na 'purge-if-empty' option instead.  But I don't want to be bothered\nwith telling git this for every commit.\n\nI recently had a long-running branch to clean up.  It was polluted\nwith commits pulled in by a ham-fisted  developer collaborating on\nthis and another branch.  He's not quite got the git mental model yet\nand he had lots of commits doing things and then undoing them later\non.  Rebase scares him.\n\nSo I did a lot of interactive rebasing on this branch to reorder the\n\"good\" change commits to the front of the line where they could be\npushed to code review sooner.  In the meantime, I wanted to keep the\nrest of the branch in place so I could see what was left to tackle.\n\nI cherry-picked replacements for many of the \"good\" commits -- from\ntheir original topic branches HamFist swiped them from -- so I would\nhave the current, reviewable commit to push. Then I tested the\nlong-running branch on top of these commits.  This involved about 8 or\n10 passes through 'git rebase -i master' for one reason or another.\n\nOn this branch of 40 commits, git interrupted me about 10 times on\neach pass to ask me what to do.  The reason is always one of these:\n\n  1. There is a new conflict I need to resolve\n      examine / mergetool / test / --continue\n\n  2. There is a rerere autoresolved conflict git wants me to approve\n      examine / test / --continue\n\n  3. There are no changes left in this commit because either\n        a. they were introduced into earlier commits, or\n        b. git-rerere-membered that I don't want those changes\n      examine / --skip\n\nI went through this process about 5 or 6 times as I massaged the stink\nout of this branch.  Cases 2 and 3 became more common as I went along.\n But git always wanted to stop and ask my approval before continuing.\nIt was frustrating.\n\nI always had my original branch to go compare to.  This one is really\na trial rework of these commits.  So I wish I could tell git to only\nbother me when he sees a new conflict.  Don't stop and ask me for\nsomething every 3 or 4 commits.\n\nI really wanted something like this:\n\n   $ git rebase --purge-empty --accept-rerere-authority -i master\n\nSo, even though this is an \"interactive\" rebase, I wish git would do\nmore of the busywork for me.  That is, I only want it to be as\ninteractive as it needs to be, and no more so.\n\nPhil\n"},{"id":"187802","messageId":"CAG+J_Dx8o_cpS3zLyRf66dgsRZEzP-yD8nBwLmfZhWkydX_MVA@mail.gmail.com","threadId":"30042","inReplyTo":"CABURp0oJwM-KtdBRVHgvOaqFVjA-MEAfJoJH=52Y=QRcgFL+3Q@mail.gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2012-03-27T01:58:28Z","receivedAt":"2012-03-27T01:58:28Z","isPatch":false,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, Mar 26, 2012 at 2:29 PM, Phil Hord <phil.hord@gmail.com> wrote:\n> Can we have three behaviors?\n>\n> A: Current mode, stop and error on empty commits\n> B: --keep-empty, to retain empty commits without further notice\n> C: --purge-empty, to remove empty commits without further notice\n\nFWIW, filter-branch uses \"--prune-empty\", should we wish to be consistent.\n\nj.\n"},{"id":"187916","messageId":"CABURp0r2_3GdG+iX1BmCbHo5aUoRNLt0WCnDJDcO583qaXT3tQ@mail.gmail.com","threadId":"30042","inReplyTo":"4F72AD25.2090102@gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2012-03-28T06:58:32Z","receivedAt":"2012-03-28T06:58:32Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Wed, Mar 28, 2012 at 2:18 AM, Neal Kreitzinger\n<nkreitzinger@gmail.com> wrote:\n> On 3/26/2012 5:53 PM, Phil Hord wrote:\n>> On Mon, Mar 26, 2012 at 5:53 PM, Neal Kreitzinger\n>> <nkreitzinger@gmail.com> wrote:\n>>> On 3/26/2012 12:20 PM, Neil Horman wrote:\n>>>>\n>>>> On Mon, Mar 26, 2012 at 10:12:48AM -0700, Junio C Hamano wrote:\n>>>>>\n>>>>> Neil Horman<nhorman@tuxdriver.com> writes:\n>>>>>\n>>>>>> I agree, I think perhaps adding an --allow-empty option to\n>>>>>> the rebase logic, so that empty commits (or perhaps just\n>>>>>> initially empty, as opposed to commits made empty) would be\n>>>>>> very beneficial.\n>>>>>\n>>>>>\n>>>>> Yeah, that probably may make sense.\n>>>>>\n>>>> Ok, cool, I'll have a patch in a few days, thanks!\n>>>>\n>>> IMO, it seems like --allow-empty is an appropriate patch for\n>>> git-rebase (non-interactive), and that git-rebase -i would need a\n>>> command like \"k\"eep to distinguish which empty commits are not to\n>>> be discarded and which empty commits are ok to discard\n>>> automatically. git-rebase -i should allow explicit control on a\n>>> commit by commit basis as opposed to blanket rules like \"discard\n>>> all empty commits\" or \"keep all empty commits\" that apply to all\n>>> commits in the rebase-to-do list based on a single cli option.\n>>\n>> But I don't want a 'keep-even-if-empty' option in interactive.\n>\n> That's why its called an option.  Don't use it if you don't want it.  ;-)\n\nYeah, I didn't mean I don't want it to exist in git.  I just meant\nthat it's not what I'm seeking atm.\n\nBut you seem to have forgotten that you were arguing against it at the\nbeginning of the paragraph I was responding to.\n\n>> I want a 'purge-if-empty' option instead.\n>\n> That's the default behavior currently.  We're not proposing to change that.\n\nIt ostensibly is the current behavior.  But Peff and I think we've\nseen it misbehave in --interactive mode.  However, I may have been\nconfused by its doppelgänger, the \"rerere autoresolved to empty\ncommit\" situation, where the behavior is to announce \"Could not apply\"\nand pause the process to wait for human intervention.\n\n>> But I don't want to be bothered with telling git this for every\n>> commit.\n>\n> You only tell it which empty commits you want to \"k\"eep (to not\n> \"auto-purge\").  In your case, you don't want to keep any so you don't have\n> to tell it anything.  The default behavior will purge them all (empty\n> commits) because you didn't mark any as \"k\"eep.\n>\n>\n>> I recently had a long-running branch to clean up.\n>\n> I have users with branches over a year old.\n\nI don't understand this comment.  Is this a pissing contest?\n\n>> It was polluted with commits pulled in by a ham-fisted developer\n>> collaborating on this and another branch.\n>\n> My users cp files from other worktrees and do merges manually without using\n> git-merge, git-rebase, or git-cherry-pick.\n>\n>\n>> He's not quite got the git mental model yet and he had lots of\n>> commits doing things and then undoing them later on.\n>\n> Most of my users think git is cvs 3.0.\n\nI see what you did there.  I like that one.\n\n>\n>> Rebase scares him.\n>\n> git as a whole scares most of my users.\n>\n>> So I did a lot of interactive rebasing on this branch to reorder the\n>> \"good\" change commits to the front of the line where they could be\n>> pushed to code review sooner. In the meantime, I wanted to keep the\n>> rest of the branch in place so I could see what was left to tackle.\n>\n> You should create a \"backup\" branch of the before state so you don't get\n> blamed for breaking their stuff.\n\nApologies for failing to mention that his branch remained \"backed up\"\nin origin/shared/dumbass as well as on a local branch and in my\nreflog.\n\n>> I cherry-picked replacements for many of the \"good\" commits -- from\n>> their original topic branches HamFist swiped them from -- so I would\n>> have the current, reviewable commit to push. Then I tested the\n>> long-running branch on top of these commits. This involved about 8\n>> or 10 passes through 'git rebase -i master' for one reason or\n>> another.\n>\n> Sounds like you may also be a little scared of git-rebase yourself.  Be a\n> man and do it all in one pass.  (You have the backup branch to startover if\n> need be.)  ;-)\n\nOh, it IS a pissing contest then.  Whip it out, boy!\n\nNo, I'm not scared of rebase.  There were 40 commits full of crap and\ngems.  I made several pass through rebase-interactive not because\ngit-rebase is a problem but simply because I was sifting methodically\nfor gems and flushing turds.\n\n>> On this branch of 40 commits, git interrupted me about 10 times on\n>> each pass to ask me what to do. The reason is always one of these:\n>>\n>> 1. There is a new conflict I need to resolve\n>> examine / mergetool / test / --continue\n>>\n>> 2. There is a rerere autoresolved conflict git wants me to approve\n>> examine / test / --continue\n>\n> You trust rerere?  I take back my earlier comment, you are a brave man.\n\nUh huh. That's what I thought!. Hmmph.\n\n> Also, you're starting to sound like some of my users who think that git\n> magically does their work for them.\n\nGit does do my work for me, and it does require more work from me.\nBut in this case, it saved me sifting the same turds over and over.\n\nI do trust rerere to redo what it previously remembered I did for\nresolution.  In that respect, rerere is only as reliable as I am.  But\nI also have a backup that I'll consult periodically, these commits are\nheaded for the CI-Server and human reviewers, and there's plenty of\ntime to catch my stupid mistakes.\n\n\n>> 3. There are no changes left in this commit because either a. they\n>> were introduced into earlier commits, or b. git-rerere-membered that\n>> I don't want those changes examine / --skip\n>\n> git-rerere-membered -- that's a good one.  I felt like\n> git-rerere-dismembered my merge the one time I tried it.  But seriously, I\n> have no experience with this --skip scenario.\n\nI think rerere seemed to butcher my first ugly merge, too.  But it was\nreally my fault.  I knew I was on a throwaway branch previously, so I\nwas not as careful with the merge.  Later when I revisited it to do it\n\"for real\", rerere stepped in and was as un-careful as I had been.\n:-)  I think he was rubbing my nose in it.\n\n\n>> I went through this process about 5 or 6 times as I massaged the\n>> stink out of this branch. Cases 2 and 3 became more common as I\n>> went along. But git always wanted to stop and ask my approval before\n>> continuing. It was frustrating.\n>>\n>> I always had my original branch to go compare to. This one is really\n>> a trial rework of these commits. So I wish I could tell git to only\n>> bother me when he sees a new conflict. Don't stop and ask me for\n>> something every 3 or 4 commits.\n>>\n>> I really wanted something like this:\n>>\n>> $ git rebase --purge-empty --accept-rerere-authority -i master\n>>\n>\n> --accept-rerere-authority sounds like another recent thread (maybe from\n> you).\n\nMaybe.\n\n\n>> So, even though this is an \"interactive\" rebase, I wish git would do\n>> more of the busywork for me. That is, I only want it to be as\n>> interactive as it needs to be, and no more so.\n>>\n> FWIW, I think we may be a little spoiled.  git rebase -i is one of the, if\n> not the, most powerful, flexible, amazing change control commands in\n> existence if you think about it.  I guess we could switch to another SCM\n> that does it better.  Oh, I forgot, there isn't one.\n\nYes, definitely spoiled.  rebase-interactive,  add-patch, and\ncheckout-patch have changed the way I manage my project.  I have a\nmuch better project history because of them, but I'm also much more\nintimate with my SCM than I ever was under Subversion, cvs, SourceSafe\nor tar.  :-)\n\nI never thought I would be excited about an SCM, but I've become the\nresident evangelist at $dayjob.\n\n> Hopefully, my sympathetically-inspired comments based on some shared\n> experiences are helpful at some level.  :-)\n\nYes, thanks.  Sometimes I'm long-winded.  I usually tone it down after\na couple of rewrites, but this time I left most of the elucidating\nback story in.\n\nThanks for reading along.\n\nPhil\n"},{"id":"187952","messageId":"7viphowt43.fsf@alter.siamese.dyndns.org","threadId":"30042","inReplyTo":"4F72AD25.2090102@gmail.com","subject":"Re: odd behavior with git-rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-03-28T17:08:44Z","receivedAt":"2012-03-28T17:08:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Neal Kreitzinger <nkreitzinger@gmail.com> writes:\n\n> On 3/26/2012 5:53 PM, Phil Hord wrote:\n> ...\n>>  2. There is a rerere autoresolved conflict git wants me to approve\n>>  examine / test / --continue\n>\n> You trust rerere?  I take back my earlier comment,...\n\nHe does \"rerere\" and then \"examine\"s, doesn't he?  So it is not \"blindly\ntrust\", but \"trust and verify\".\n\n>>  I really wanted something like this:\n>>\n>>  $ git rebase --purge-empty --accept-rerere-authority -i master\n>>\n> --accept-rerere-authority sounds like another recent thread (maybe\n> from you).\n\nIt was from a different person and on a different command, but I think\nthey are going in the same direction.  The \"--rerere-autoupdate\" option of\n\"git am\", \"git rebase\" and \"git merge\" could learn to be a bool-plus, a\nstronger form than \"yes/no\", such that it makes a commit if everything\nauto-resolves cleanly (and rerere.autoupdate configuration could learn to\nbe a bool-plus as well).\n\nWhile I am hesitant to endorse such a mode of operation, as it can invite\nsloppy users hurt themselves, I do not fundamentally oppose it as an\noption.  It could save time for diligent users who know when to examine\nand verify the result, in order to avoid casting the result with blind\nfaith in mechanical merges.\n"}]}