git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 0/5] Introduce --first-parent flag for git bisect

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 4, 2020, 22:19 UTC
Message-ID
<xmqqtuxiqckg.fsf@gitster.c.googlers.com>
In-Reply-To
<20200804220113.5909-1-alipman88@gmail.com>
Aaron Lipman <alipman88@gmail.com> writes:
Show 17 quoted lines
> OK, here's take 4! Responding to Junio's feedback, first:
>
>> That function [cmd_bisect__helper()] is supposed to be a thin shim
>> layer whose only reason of its existence is to serve as an interface
>> with the scripted version of "git bisect".  When everything is
>> migrated from the shell script, cmd_bisect__helper() should disappear.
>> ...
>> This is going backwards, as far as I can tell.  If anything, I'd
>> rather see cmd_bisect__helper() get fixed so that it does not parse
>> "--first-parent" (and similarly, "--no-checkout" that you imitated)
>> into first_parent_only (and no_checkout) variables and passed as
>> parameters to bisect_start().
>
> Thanks for the explanation, Junio. (And for bearing with me while I
> gain familiarity with git's codebase.) Now that I've taken some time
> to examine how git-bisect.sh and cmd_bisect__helper() fit together,
> the correct approach is much more clear.

I did notice that no_checkout variable in the top-level cmd_bisect__helper() was convenient to have in the current code structure, and I didn't think how it should be fixed deeply enough, so for the purpose of this series, I do not mind if it is left untouched. It's just that it was disturbing to see that addition of "--first-parent" wanted to add yet another variable to the top-level, and that the new variable was totally unneeded.

Show 8 quoted lines
> As no_checkout was also passed to bisect_next_all() [via implicitly
> checking for the existence of .git/BISECT_HEAD when calling
> bisect_next() in git-bisect.sh], I've removed that parameter and
> instead check for .git/BISECT_HEAD in bisect_next_all() to determine
> whether no-checkout mode is on.
>
> This means no-checkout mode can no longer be enabled by "git
> bisect--helper --next-all --no-checkout"

Unlike "start", "next-all" is not really a command in the end-user's mind ("git bisect next" is, though). It merely is just an interim "convenience" interface for the remaining part of the scripted "git bisect" to call into. As long as the caller can do the same thing as before, it should be OK, I would think.

Previous: Aaron LipmanNext: Christian Couder
Message 30 of 42 in “Introduce --first-parent flag for git bisect”
  1. 0/3 Introduce --first-parent flag for git bisectAaron Lipman via GitGitGadget, Jul 28, 2020
  2. 1/3 rev-list: allow bisect and first-parent flagsAaron Lipman via GitGitGadget, Jul 28, 2020
  3. Junio C HamanoJul 28, 2020
  4. Junio C HamanoJul 28, 2020
  5. 2/3 bisect: introduce first-parent flagAaron Lipman via GitGitGadget, Jul 28, 2020
  6. 3/3 bisect: combine args passed to find_bisection()Aaron Lipman via GitGitGadget, Jul 28, 2020
  7. 0/3 Introduce --first-parent flag for git bisectAaron Lipman, Jul 30, 2020
  8. 1/3 rev-list: allow bisect and first-parent flagsAaron Lipman, Jul 30, 2020
  9. 2/3 bisect: introduce first-parent flagAaron Lipman, Jul 30, 2020
  10. Junio C HamanoJul 30, 2020
  11. 3/3 bisect: combine args passed to find_bisection()Aaron Lipman, Jul 30, 2020
  12. Junio C HamanoJul 30, 2020
  13. Junio C HamanoJul 30, 2020
  14. 0/4 Introduce --first-parent flag for git bisectAaron Lipman, Aug 1, 2020
  15. 1/4 rev-list: allow bisect and first-parent flagsAaron Lipman, Aug 1, 2020
  16. 3/4 bisect: combine args passed to find_bisection()Aaron Lipman, Aug 1, 2020
  17. Martin ÅgrenAug 1, 2020
  18. 4/4 bisect: consistent style for git bisect run testsAaron Lipman, Aug 1, 2020
  19. Martin ÅgrenAug 1, 2020
  20. 2/4 bisect: introduce first-parent flagAaron Lipman, Aug 1, 2020
  21. Junio C HamanoAug 1, 2020
  22. 0/5 Introduce --first-parent flag for git bisectAaron Lipman, Aug 4, 2020
  23. 1/5 t6030: modernize "git bisect run" testsAaron Lipman, Aug 4, 2020
  24. Christian CouderAug 5, 2020
  25. 2/5 rev-list: allow bisect and first-parent flagsAaron Lipman, Aug 4, 2020
  26. Eric SunshineAug 5, 2020
  27. 3/5 cmd_bisect__helper: defer parsing no-checkout flagAaron Lipman, Aug 4, 2020
  28. 4/5 bisect: introduce first-parent flagAaron Lipman, Aug 4, 2020
  29. 5/5 bisect: combine args passed to find_bisection()Aaron Lipman, Aug 4, 2020
  30. Junio C HamanoAug 4, 2020
  31. Christian CouderAug 5, 2020
  32. Junio C HamanoAug 7, 2020
  33. Eric SunshineAug 7, 2020
  34. Junio C HamanoAug 7, 2020
  35. Eric SunshineAug 7, 2020
  36. Martin ÅgrenAug 5, 2020
  37. 0/5 Introduce --first-parent flag for git bisectAaron Lipman, Aug 7, 2020
  38. 1/5 t6030: modernize "git bisect run" testsAaron Lipman, Aug 7, 2020
  39. 2/5 rev-list: allow bisect and first-parent flagsAaron Lipman, Aug 7, 2020
  40. 3/5 cmd_bisect__helper: defer parsing no-checkout flagAaron Lipman, Aug 7, 2020
  41. 5/5 bisect: combine args passed to find_bisection()Aaron Lipman, Aug 7, 2020
  42. 4/5 bisect: introduce first-parent flagAaron Lipman, Aug 7, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.