# t3512 & t3513 'unexpected passes' 4 messages from 2017-11-20 to 2017-11-22. Participants: Ramsay Jones, Adam Dinwoodie, Phillip Wood, Junio C Hamano. Thread: https://gitlist.dev/t/47254 ## Ramsay Jones, 2017-11-20 20:16 Subject: t3512 & t3513 'unexpected passes' Message-ID: <73f0fb1e-5b55-1049-7706-652f1f9deaed@ramsayjones.plus.com> URL: https://gitlist.dev/e/73f0fb1e-5b55-1049-7706-652f1f9deaed%40ramsayjones.plus.com ``` Hi Junio, For several days, I have been staring at some 'unexpected passes' in the t3512-cherry-pick-submodule.sh and t3513-revert-submodule.sh test files (tests #11-13 in both cases). I finally found time tonight to 'git bisect' the 'problem', and found that bisect fingered commit b5a812b298 ("sequencer: try to commit without forking 'git commit'", 17-11-2017). During the bisection I stumbled across (several times) the following compilation error: CC sequencer.o sequencer.c: In function ‘do_commit’: sequencer.c:1142:9: error: void value not ignored as it ought to be res = print_commit_summary(NULL, &oid, ^ Makefile:2105: recipe for target 'sequencer.o' failed make: *** [sequencer.o] Error 1 which I 'fixed' like so: s/res = // So, given that the 'fingered' commit didn't immediately seem to be related to the problem, along with the above errors, this may well not be the culprit. Just FYI. ATB, Ramsay Jones ``` ## Adam Dinwoodie, 2017-11-21 10:44 Subject: Re: t3512 & t3513 'unexpected passes' Message-ID: <20171121104404.GN20681@dinwoodie.org> URL: https://gitlist.dev/e/20171121104404.GN20681%40dinwoodie.org In-Reply-To: <73f0fb1e-5b55-1049-7706-652f1f9deaed@ramsayjones.plus.com> ``` On Monday 20 November 2017 at 08:16 pm +0000, Ramsay Jones wrote: > For several days, I have been staring at some 'unexpected passes' in > the t3512-cherry-pick-submodule.sh and t3513-revert-submodule.sh test > files (tests #11-13 in both cases). > > I finally found time tonight to 'git bisect' the 'problem', and found > that bisect fingered commit b5a812b298 ("sequencer: try to commit without > forking 'git commit'", 17-11-2017). > > During the bisection I stumbled across (several times) the following > compilation error: > > CC sequencer.o > sequencer.c: In function ‘do_commit’: > sequencer.c:1142:9: error: void value not ignored as it ought to be > res = print_commit_summary(NULL, &oid, > ^ > Makefile:2105: recipe for target 'sequencer.o' failed > make: *** [sequencer.o] Error 1 > > which I 'fixed' like so: s/res = // > > So, given that the 'fingered' commit didn't immediately seem to be > related to the problem, along with the above errors, this may well > not be the culprit. I've seen the same unexpected passes, and had just completed the same bisect run myself, although I fixed the build failure by cherry-picking 82921316a ("SQUASH???", 2017-11-18) onto commits that wouldn't build, given that commit seems to exist entirely to fix that build breakage. I think that adds more weight to b5a812b29 being the culprit for these unexpected passes. It's definitely the case that these unexpected passes exist at 8e4ff0ae1 ("Merge branch 'pw/sequencer-in-process-commit' into pu", 2017-11-21) and do not exist at its immediate left-hand parent, e017a4ccc ("Merge branch 'jn/ssh-wrappers' into jch", 2017-11-21), which means it's clearly _something_ in the branch merged at 8e4ff0ae1. ``` ## Phillip Wood, 2017-11-21 18:19 Subject: Re: t3512 & t3513 'unexpected passes' Message-ID: URL: https://gitlist.dev/e/a3ed15c4-61b7-aae2-d849-4dbea2fd7511%40talktalk.net In-Reply-To: <20171121104404.GN20681@dinwoodie.org> ``` On 21/11/17 10:44, Adam Dinwoodie wrote: > On Monday 20 November 2017 at 08:16 pm +0000, Ramsay Jones wrote: >> For several days, I have been staring at some 'unexpected passes' in >> the t3512-cherry-pick-submodule.sh and t3513-revert-submodule.sh test >> files (tests #11-13 in both cases). Thanks for pointing this out. > I've seen the same unexpected passes, and had just completed the same > bisect run myself, although I fixed the build failure by cherry-picking > 82921316a ("SQUASH???", 2017-11-18) onto commits that wouldn't build, > given that commit seems to exist entirely to fix that build breakage. I > think that adds more weight to b5a812b29 being the culprit for these > unexpected passes. > > It's definitely the case that these unexpected passes exist at 8e4ff0ae1 > ("Merge branch 'pw/sequencer-in-process-commit' into pu", 2017-11-21) > and do not exist at its immediate left-hand parent, e017a4ccc ("Merge > branch 'jn/ssh-wrappers' into jch", 2017-11-21), which means it's > clearly _something_ in the branch merged at 8e4ff0ae1. If I've understood the comments by those tests this has to do with the new code detecting empty commits where the old code does not or vice versa. The new code detects an attempt to create an empty commit by comparing the tree that is about to be committed to HEAD and complains if they are the same. I've had a quick look through the code for 'git commit' but I need more time to figure out exactly what it does in this case (I've a feeling from when I looked a while ago that it does this test before it writes the tree). If anyone has any insight as to why the tests fail with the current commit implementation that would be helpful for understanding what is going on here - unfortunately I've never really used submodules. Best Wishes Phillip ``` ## Junio C Hamano, 2017-11-22 01:40 Subject: Re: t3512 & t3513 'unexpected passes' Message-ID: URL: https://gitlist.dev/e/xmqq1skrytsl.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20171121104404.GN20681@dinwoodie.org> ``` Adam Dinwoodie writes: > I've seen the same unexpected passes, and had just completed the same > bisect run myself, although I fixed the build failure by cherry-picking > 82921316a ("SQUASH???", 2017-11-18) onto commits that wouldn't build, > given that commit seems to exist entirely to fix that build breakage. Your assumption is correct wrt "SQUASH???". When I am in a hurry and cannot spend extra cycles to pinpoint which one of the commits in a multi-patch series breaks a build, I just fix the breakage on top of the tip commit, and leave it to the original author of the series to apply them down to the original breakage in individual commits when the topic is rerolled. It seems that the compilation breakage 82921316a fixes comes from its immediate parent, so I'll squash them together, just in case a rerolled topic does not materialize. Thanks. ```