On Wednesday 22 November 2017 at 05:21 pm +0100, Christian Couder wrote: > On Wed, Nov 22, 2017 at 3:39 PM, Adam Dinwoodie <adam@xxxxxxxxxxxxx> wrote: > > In trying to do a bisect on the Git repository, I seem to have come > > across surprising behavior where the order in which `git bisect` appears > > to forget that previous commits were marked as new. > > Yeah, the algorithm uses many "old" commits and only one "new" commit. > > [...] > > > As you can see, in both cases, only the most recent "new" command > > appears to have any effect. I'd have expected that both commits would > > have been marked as "new", and the bisect run would use both facts to > > work out which commit is the target of the bisection. > > I didn't look at your test case, but the algorithm to find another > commit to test is described here: > > https://git-scm.com/docs/git-bisect-lk2009.html#_bisection_algorithm > > and you can see that the first rule of the algorithm is to keep only > the commits that are ancestors of the "new" commit (including the > "new" commit itself). > > The reason for this rule is that other commits cannot have introduced > the behavior we are looking for. The result of this rule is that git > bisect will always ask you to test a commit that is an ancestor of the > "new" commit. > > So if you test a commit that git bisect asks you to test, and it > appears that this commit is "new", then you can just discard the > previous "new" commit because it will give you less information than > the new "new" one. > (The old "new" will not let you discard any commits that the new "new" > commit allows you to discard, because it is a descendant of the new > "new" commit.) > > If you don't test the commit that git bisect asks you to test, then > git bisect assumes that you know better and discards the old "new". Ah, that makes sense, thank you for the explanation. In my case, I knew two commits were "new", but didn't know which would provide more information to the bisect algorithm; I'd assumed if I provided both, Git would work out the appropriate thing to do with the combined information without me needing to work it out. > > This is using v2.15.0. It's possibly relevant that 95a731ce is a > > direct parent of 14c63a9d. > > > > Is this a bug, or intentional behaviour? Am I missing something that > > means it's actually sensible to have Git silently discard some bisect > > commands in this sort of circumstance? > > Well, instead of silently discarding a the old "new" commit when the > new "new" commit is not an ancestor of it, git bisect could perhaps > warn or ask you to confirm that you know what you are doing. Warning the user seems a sensible suggestion to me. I'll add it to my list of things to spend some time on at some point in the future :)
