On Thu, Apr 12, 2018 at 12:53 PM, Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi Tiago, > > On Thu, 12 Apr 2018, Tiago Botelho wrote: > >> On Thu, Apr 12, 2018 at 9:58 AM, Christian Couder >> <christian.couder@xxxxxxxxx> wrote: >> > On Thu, Apr 12, 2018 at 9:49 AM, Harald Nordgren >> > <haraldnordgren@xxxxxxxxx> wrote: >> >> I think it looks similar. But if I'm reading that thread correctly >> >> then there was never a patch created, right? >> > >> > (It is customary on this mailing list to reply after the sentences we >> > reply to. We don't "top post".) >> > >> > On the GSoC idea pages (like https://git.github.io/SoC-2018-Ideas/) we >> > have been suggesting "Implement git bisect --first-parent" and there >> > are the following related links: >> > >> > https://public-inbox.org/git/20150304053333.GA9584@xxxxxxxx/ >> > https://public-inbox.org/git/4D3CDDF9.6080405@xxxxxxxxx/ >> > >> > Tiago in Cc also tried at a recent London hackathon to implement it >> > and came up with the following: >> > >> > https://github.com/tiagonbotelho/git/pull/1/files >> > >> > I tried to help him by reworking his commit in the following branch: >> > >> > https://github.com/chriscool/git/commits/myfirstparent >> >> Thank you for the cc Christian, I’ve been quite busy and was not able >> to work on the PR for quite some time. >> >> I intended to pick it back up again next week. If it is ok with Harald >> I would love to finish the PR that I started, >> since it is quite close to being finished (I think it was just specs >> missing if I am not mistaken). > > That looks promising. Just like I suggested to Harald in another reply > [*1*] on this thread, you probably want to use `int flags` instead, and > turn `find_all` into BISECT_FIND_ALL in a preparatory commit. > > Also, you will definitely want to add a test. Again, my reply to Harald > [*1*] should give you a head start there... You will want to imitate the > test case I outlined there, maybe something like: > > # A - B - C - F > # \ \ / \ > # D - E - G - H > > [... 'setup' as in my mail to Harald ...] > > test_expect_success '--first-parent' ' > write_script find-e.sh <<-\EOF && > case "$(git show --format=%s -s)" in > B|C|F) ;; # first parent lineage: okay > *) git show -s --oneline HEAD >unexpected;; > esac > # detect whether we are "before" or "after" E > test ! -f E.t > EOF > > git bisect start --first-parent HEAD A && > git bisect run ./find-e.sh >actual && > test_path_is_missing unexpected && > grep "$(git rev-parse F) is the first bad commit" actual > ' > > Also, Tiago, reading through your patch (as on chriscool/git; do you have > your own fork? That would make it much easier to collaborate with you by > offering PRs), it looks more straight-forward than editing the commit_list > after the fact and adding magic weights ;-) > > Except for one thing. I wonder why `bisect_next_all()` does not set > revs.first_parent_only after calling `bisect_rev_setup()`? You would still > need the changes in `count_distance()`, as it performs its own commit > graph traversal, but there is no need to enumerate too many commits in the > first place, right? > > Harald, maybe --merges-only can be implemented on top of --first-parent, > with the `int flags` change I suggested? > > Ciao, > Johannes > > Footnote *1*: > https://public-inbox.org/git/nycvar.QRO.7.76.6.1804121143120.65@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ Hi Johannes, Thank you for the pointers! I do have my own fork, you can see it here: https://github.com/tiagonbotelho/git/pull/1/commits which applies the changes Chris made to my first draft of the solution. I still have to add him as co-author of that commit. Cheers, Tiago Botelho
