On Sat, Feb 23, 2019 at 2:44 PM Jeff King <peff@xxxxxxxx> wrote: > > On Fri, Feb 22, 2019 at 09:49:44AM -0800, Junio C Hamano wrote: > > > > If we do care about the change in exit code from bisect, then it > > > probably does make sense to go with an external process. Then it can > > > happily die on the corruption, while bisect continues with the rest of > > > the high-level operation. I'm not sure it really matters much, though. > > > Once your repository is corrupted, all bets are off. It's nice that we > > > can bisect in such a state at all. > > > > This is about showing the very final message after finding which one > > is the culprit. Is there any other "clean-up" action we need to do > > after showing the message? I do not care too much about the exit > > code from the bisection, but if dying from diff-tree can interfere > > with such a clean-up, that would bother me a lot more, and at that > > point, given especially that this is not a performance sensitive > > thing at all (it is not even invoked log(n) times---just once at the > > end), moving to external process may make it a lot simpler and > > cleaner. > > Thanks, I had a vague feeling along these lines, but you nicely put it > into words. As far as I can tell, no, we're not missing any important > cleanup in that process; it looks like the only call to show_diff_tree() > then calls exit(10) immediately after. > > However, that does change our exit code, which git-bisect.sh then > propagates instead of writing the entry into the BISECT_LOG. > > I'm still not convinced this is really worth caring about, as it implies > a corrupt repo. I don't care much about what happens in a corrupt repo, but I am adding Jon Seymour in CC who wrote those tests in: d3dfeedf2e (bisect: add tests to document expected behaviour in presence of broken trees., 2011-08-04) b704a8b3fd (bisect: add tests for the --no-checkout option., 2011-08-04)
