{"thread":{"id":"40992","subject":"git bisect with temporary commits","startedAt":"2015-12-14T16:37:26Z","lastAt":"2015-12-14T21:26:36Z","messageCount":9,"participants":["Florian Bruhin","Andreas Schwab","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"274386","messageId":"20151214163726.GY13519@tonks","threadId":"40992","inReplyTo":null,"subject":"git bisect with temporary commits","fromName":"Florian Bruhin","fromEmail":"me@the-compiler.org","sentAt":"2015-12-14T16:37:26Z","receivedAt":"2015-12-14T16:37:26Z","isPatch":false,"sender":{"key":"me@the-compiler.org","avatar":null},"body":"Hi!\n\nToday I bisected a bug which required cherry-picking an (unrelated)\ncompile fix later in the history so I could test the commits.\n\nAfter testing a commit, I didn't reset to the commit before the\ncherry-picked one, which seemed to work well, but doesn't in my minimal\nexample:\n\n $ git init\n $ echo 'good and does not compile' > file\n $ git add file && git commit -m 'good and does not compile'\n $ echo again >> file && git commit -am 'still good and does not compile'\n $ echo 'bad and compiles' > file && git commit -am 'bad and compiles'\n $ git log --oneline --decorate\n 97f9214 (HEAD, master) bad and compiles\n 981e109 still good and does not compile\n 753ed25 good and does not compile\n\nNow I start bisecting and cherry-pick the compile fix in master:\n\n $ git bisect start\n $ git bisect bad master\n $ git bisect good master~2 # 753ed25\n Bisecting: 0 revisions left to test after this (roughly 0 steps)\n [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n florian@ws042:~/tmp/bisect1$ git cherry-pick master\n [detached HEAD b49872b] bad and compiles\n Date: Mon Dec 14 17:26:51 2015 +0100\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\nNow when trying to say it's good (and forgetting to remove the\ntemporary commits), I get this:\n\n $ git bisect good\n Bisecting: a merge base must be tested\n [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n\nIs this intended behaviour? Shouldn't git either do a reset to the\ncommit we're currently bisecting, or warn the user as it was probably\nunintended to add new commits?\n\nCurrently it seems bisect just treats the current HEAD as good, and\nthen the bisect algorithm tries to (I think) select a commit between\nthe currently bisected one and the (temporary) HEAD?\n\nWhen I used it today, it actually seemed to work well until I hit an\n*actual* merge base, and then it started to bisect something\nunexpected, which got me a bit confused ;)\n\nFlorian\n\n-- \nhttp://www.the-compiler.org | me@the-compiler.org (Mail/XMPP)\n GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc\n I love long mails! | http://email.is-not-s.ms/\n"},{"id":"274389","messageId":"87si34hphr.fsf@igel.home","threadId":"40992","inReplyTo":"20151214163726.GY13519@tonks","subject":"Re: git bisect with temporary commits","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2015-12-14T18:08:48Z","receivedAt":"2015-12-14T18:08:48Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Florian Bruhin writes:\n\n> Now when trying to say it's good (and forgetting to remove the\n> temporary commits), I get this:\n>\n> $ git bisect good\n> Bisecting: a merge base must be tested\n> [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n>\n> Is this intended behaviour? Shouldn't git either do a reset to the\n> commit we're currently bisecting, or warn the user as it was probably\n> unintended to add new commits?\n\nYou should instead tell git that HEAD^ is good, since that is what git\nasked you to test.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"274391","messageId":"20151214182255.GB13519@tonks","threadId":"40992","inReplyTo":"87si34hphr.fsf@igel.home","subject":"Re: git bisect with temporary commits","fromName":"Florian Bruhin","fromEmail":"me@the-compiler.org","sentAt":"2015-12-14T18:22:55Z","receivedAt":"2015-12-14T18:22:55Z","isPatch":false,"sender":{"key":"me@the-compiler.org","avatar":null},"body":"* Andreas Schwab [2015-12-14 19:08:48 +0100]:\n> Florian Bruhin writes:\n> \n> > Now when trying to say it's good (and forgetting to remove the\n> > temporary commits), I get this:\n> >\n> > $ git bisect good\n> > Bisecting: a merge base must be tested\n> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n> >\n> > Is this intended behaviour? Shouldn't git either do a reset to the\n> > commit we're currently bisecting, or warn the user as it was probably\n> > unintended to add new commits?\n> \n> You should instead tell git that HEAD^ is good, since that is what git\n> asked you to test.\n\nI see - but wouldn't it make more sense for a \"git bisect good\" (or\nbad, respectively) without arguments to assume I mean the commit\nbisect checked out for me, not HEAD?\n\nI don't see any scenario where the current behaviour would make sense,\nbut I might be missing something.\n\nFlorian\n\n-- \nhttp://www.the-compiler.org | me@the-compiler.org (Mail/XMPP)\n GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc\n I love long mails! | http://email.is-not-s.ms/\n"},{"id":"274399","messageId":"xmqqa8pckfa5.fsf@gitster.mtv.corp.google.com","threadId":"40992","inReplyTo":"20151214182255.GB13519@tonks","subject":"Re: git bisect with temporary commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T19:21:06Z","receivedAt":"2015-12-14T19:21:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Bruhin writes:\n\n> * Andreas Schwab [2015-12-14 19:08:48 +0100]:\n>> Florian Bruhin writes:\n>> \n>> > Now when trying to say it's good (and forgetting to remove the\n>> > temporary commits), I get this:\n>> >\n>> > $ git bisect good\n>> > Bisecting: a merge base must be tested\n>> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n>> >\n>> > Is this intended behaviour? Shouldn't git either do a reset to the\n>> > commit we're currently bisecting, or warn the user as it was probably\n>> > unintended to add new commits?\n>> \n>> You should instead tell git that HEAD^ is good, since that is what git\n>> asked you to test.\n>\n> I see - but wouldn't it make more sense for a \"git bisect good\" (or\n> bad, respectively) without arguments to assume I mean the commit\n> bisect checked out for me, not HEAD?\n>\n> I don't see any scenario where the current behaviour would make sense,\n> but I might be missing something.\n\nWhen the commit \"bisect\" checked out is untestable, the user can\nfreely go to another commit, e.g. \"git reset --hard HEAD^\" to go\nback one step, and then test it instead. \"git bisect good\" has\nto mark the then-current HEAD, not the commit that was checked out,\nfor this to work.\n"},{"id":"274410","messageId":"20151214193802.GD13519@tonks","threadId":"40992","inReplyTo":"xmqqa8pckfa5.fsf@gitster.mtv.corp.google.com","subject":"Re: git bisect with temporary commits","fromName":"Florian Bruhin","fromEmail":"me@the-compiler.org","sentAt":"2015-12-14T19:38:02Z","receivedAt":"2015-12-14T19:38:02Z","isPatch":false,"sender":{"key":"me@the-compiler.org","avatar":null},"body":"* Junio C Hamano [2015-12-14 11:21:06 -0800]:\n> Florian Bruhin writes:\n> \n> > * Andreas Schwab [2015-12-14 19:08:48 +0100]:\n> >> Florian Bruhin writes:\n> >> \n> >> > Now when trying to say it's good (and forgetting to remove the\n> >> > temporary commits), I get this:\n> >> >\n> >> > $ git bisect good\n> >> > Bisecting: a merge base must be tested\n> >> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n> >> >\n> >> > Is this intended behaviour? Shouldn't git either do a reset to the\n> >> > commit we're currently bisecting, or warn the user as it was probably\n> >> > unintended to add new commits?\n> >> \n> >> You should instead tell git that HEAD^ is good, since that is what git\n> >> asked you to test.\n> >\n> > I see - but wouldn't it make more sense for a \"git bisect good\" (or\n> > bad, respectively) without arguments to assume I mean the commit\n> > bisect checked out for me, not HEAD?\n> >\n> > I don't see any scenario where the current behaviour would make sense,\n> > but I might be missing something.\n> \n> When the commit \"bisect\" checked out is untestable, the user can\n> freely go to another commit, e.g. \"git reset --hard HEAD^\" to go\n> back one step, and then test it instead. \"git bisect good\" has\n> to mark the then-current HEAD, not the commit that was checked out,\n> for this to work.\n\nThat makes sense - thanks for the explanation!\n\nFlorian\n\n-- \nhttp://www.the-compiler.org | me@the-compiler.org (Mail/XMPP)\n GPG: 916E B0C8 FD55 A072 | http://the-compiler.org/pubkey.asc\n I love long mails! | http://email.is-not-s.ms/\n"},{"id":"274413","messageId":"87lh8whjj2.fsf@igel.home","threadId":"40992","inReplyTo":"20151214182255.GB13519@tonks","subject":"Re: git bisect with temporary commits","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2015-12-14T20:17:37Z","receivedAt":"2015-12-14T20:17:37Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Florian Bruhin writes:\n\n> I see - but wouldn't it make more sense for a \"git bisect good\" (or\n> bad, respectively) without arguments to assume I mean the commit\n> bisect checked out for me, not HEAD?\n\nThe problem is that there is nothing that marks the originally checked\nout commit except by being pointed to by HEAD. Also, testing a\ndifferent commit as the one suggested by git can be useful when skipping\nover commits that are known to fail for unrelated reasons (see \"Avoiding\ntesting a commit\" in git-bisect(1)).\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"274429","messageId":"20151214210936.GD14788@sigill.intra.peff.net","threadId":"40992","inReplyTo":"87si34hphr.fsf@igel.home","subject":"Re: git bisect with temporary commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-14T21:09:36Z","receivedAt":"2015-12-14T21:09:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 14, 2015 at 07:08:48PM +0100, Andreas Schwab wrote:\n\n> Florian Bruhin writes:\n> \n> > Now when trying to say it's good (and forgetting to remove the\n> > temporary commits), I get this:\n> >\n> > $ git bisect good\n> > Bisecting: a merge base must be tested\n> > [981e1093dae24b37189bcba2dd848b0c3388080c] still good and does not compile\n> >\n> > Is this intended behaviour? Shouldn't git either do a reset to the\n> > commit we're currently bisecting, or warn the user as it was probably\n> > unintended to add new commits?\n> \n> You should instead tell git that HEAD^ is good, since that is what git\n> asked you to test.\n\nAnother alternative is to use \"git cherry-pick -n\" to create a working\ntree state that you can test, but leave HEAD at the original commit.\nThen \"git bisect good\" does the right thing.\n\nIt's the same principle and I don't think there is a reason to prefer\none over the other. I just find it harder to screw up, as I usually\nscript the build/test, so I can stick the cherry-pick there and not have\nto remember it on each \"good/bad\" report.\n\n-Peff\n"},{"id":"274434","messageId":"xmqqegeoivcg.fsf@gitster.mtv.corp.google.com","threadId":"40992","inReplyTo":"20151214210936.GD14788@sigill.intra.peff.net","subject":"Re: git bisect with temporary commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-12-14T21:17:03Z","receivedAt":"2015-12-14T21:17:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> You should instead tell git that HEAD^ is good, since that is what git\n>> asked you to test.\n>\n> Another alternative is to use \"git cherry-pick -n\" to create a working\n> tree state that you can test, but leave HEAD at the original commit.\n> Then \"git bisect good\" does the right thing.\n\nI was about to say the same, and \"bisect good\" at that point does\nmark the correct commit, but does it always do the right thing? I\nthink the procedure must be\n\n\tgit cherry-pick -n $the_fixup\n test\n git reset --hard\n git bisect good (or bad)\n\nfor it to always work, which is not all that different from\n\n\tgit cherry-pick $the_fixup\n test\n git reset --hard HEAD^\n git bisect good (or bad)\n"},{"id":"274436","messageId":"20151214212636.GG14788@sigill.intra.peff.net","threadId":"40992","inReplyTo":"xmqqegeoivcg.fsf@gitster.mtv.corp.google.com","subject":"Re: git bisect with temporary commits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-14T21:26:36Z","receivedAt":"2015-12-14T21:26:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 14, 2015 at 01:17:03PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> You should instead tell git that HEAD^ is good, since that is what git\n> >> asked you to test.\n> >\n> > Another alternative is to use \"git cherry-pick -n\" to create a working\n> > tree state that you can test, but leave HEAD at the original commit.\n> > Then \"git bisect good\" does the right thing.\n> \n> I was about to say the same, and \"bisect good\" at that point does\n> mark the correct commit, but does it always do the right thing? I\n> think the procedure must be\n> \n> \tgit cherry-pick -n $the_fixup\n> test\n> git reset --hard\n> git bisect good (or bad)\n\nHmm, you're right. I assumed \"git bisect good\" would do the equivalent\nof \"git checkout -f\", but it doesn't. I guess it has been long enough\nsince I have had to cherry-pick a fix that I completely forgot that bit.\n\nIt might be convenient if bisect did pass \"-f\" to checkout, but I guess\nit would also be destructive if you had hand-tweaks you forgot to save.\n\n-Peff\n"}]}