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

Re: [PATCH v14 07/27] bisect--helper: `bisect_reset` shell function in C

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
Aug 26, 2016, 13:46 UTC
Message-ID
<CAFZEwPPfXvLUtcBR6cYAP2dT8FAePFPjDSnVm8BhpLN9cfR1uw@mail.gmail.com>
In-Reply-To
<xmqq4m69q3b2.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Thu, Aug 25, 2016 at 2:42 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 43 quoted lines
>
> Pranit Bauva <pranit.bauva@gmail.com> writes:
>
> > +static int bisect_reset(const char *commit)
> > +{
> > +     struct strbuf branch = STRBUF_INIT;
> > +
> > +     if (!commit) {
> > +             if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1) {
>
> Hmm, tricky but correct to do the "< 1" comparison.  If the file
> does not exist, you'd get -1; if it fails to read it, you'd get -1;
> if it turns out to be empty, you'd get 0.
>
> > +                     printf("We are not bisecting.\n");
> > +                     return 0;
> > +             }
> > +             strbuf_rtrim(&branch);
> > +     } else {
> > +             struct object_id oid;
> > +             if (get_oid(commit, &oid))
> > +                     return error(_("'%s' is not a valid commit"), commit);
>
> The original is
>
>         rev-parse --quiet --verify "$1^{commit}"
>
> Unless the caller of this function already appended "^{commit}" to
> whatever the user gave "bisect--helper bisect-reset", this
> conversion is not equivalent.  If you said
>
>     git bisect reset HEAD:
>
> get_oid() would tell you that the top-level tree object of the
> current commit exists in the object store, but the original is
> meant to catch "That's not a commit, it's a tree!" before attempting
> to run "git checkout" on it.
>
> I think get_sha1_committish() is what you want to use here.
>
> > +             strbuf_addstr(&branch, commit);
> > +     }
>
Ya! get_sha1_committish() would be better. Thanks!
>
> Also this version fails to catch "bisect reset a b c" as an error, I
> suspect.

It didn't when I tried it right now. Could you please elaborate on why you think it can fail? There might be a thing which I haven't tested.

Regards, Pranit Bauva

Previous: Junio C HamanoNext: Junio C Hamano
Message 21 of 62 in “bisect--helper: use OPT_CMDMODE instead of OPT_BOOL”
  1. 01/27 bisect--helper: use OPT_CMDMODE instead of OPT_BOOLPranit Bauva, Aug 23, 2016
  2. 25/27 bisect--helper: retire `--bisect-autostart` subcommandPranit Bauva, Aug 23, 2016
  3. 13/27 bisect--helper: `bisect_start` shell function partially in CPranit Bauva, Aug 23, 2016
  4. Junio C HamanoAug 25, 2016
  5. Junio C HamanoAug 25, 2016
  6. Pranit BauvaAug 27, 2016
  7. Junio C HamanoAug 27, 2016
  8. 14/27 bisect--helper: `bisect_next` and `bisect_auto_next` shell function in CPranit Bauva, Aug 23, 2016
  9. Junio C HamanoAug 25, 2016
  10. Pranit BauvaAug 30, 2016
  11. Pranit BauvaAug 30, 2016
  12. Junio C HamanoAug 30, 2016
  13. Pranit BauvaAug 30, 2016
  14. 16/27 bisect--helper: retire `--next-all` subcommandPranit Bauva, Aug 23, 2016
  15. 06/27 wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()Pranit Bauva, Aug 23, 2016
  16. 27/27 bisect--helper: remove the dequote in bisect_start()Pranit Bauva, Aug 23, 2016
  17. 05/27 t6030: explicitly test for bisection cleanupPranit Bauva, Aug 23, 2016
  18. 19/27 bisect--helper: retire `--check-expected-revs` subcommandPranit Bauva, Aug 23, 2016
  19. 07/27 bisect--helper: `bisect_reset` shell function in CPranit Bauva, Aug 23, 2016
  20. Junio C HamanoAug 24, 2016
  21. Pranit BauvaAug 26, 2016
  22. Junio C HamanoAug 26, 2016
  23. Pranit BauvaAug 27, 2016
  24. Junio C HamanoAug 29, 2016
  25. 15/27 bisect--helper: retire `--bisect-clean-state` subcommandPranit Bauva, Aug 23, 2016
  26. Junio C HamanoAug 26, 2016
  27. 09/27 bisect--helper: `bisect_write` shell function in CPranit Bauva, Aug 23, 2016
  28. Junio C HamanoAug 24, 2016
  29. Pranit BauvaAug 27, 2016
  30. Junio C HamanoAug 27, 2016
  31. Pranit BauvaAug 30, 2016
  32. 18/27 bisect--helper: `bisect_state` & `bisect_head` shell function in CPranit Bauva, Aug 23, 2016
  33. 21/27 bisect--helper: `bisect_log` shell function in CPranit Bauva, Aug 23, 2016
  34. Junio C HamanoAug 26, 2016
  35. Pranit BauvaAug 27, 2016
  36. 24/27 bisect--helper: retire `--check-and-set-terms` subcommandPranit Bauva, Aug 23, 2016
  37. 17/27 bisect--helper: `bisect_autostart` shell function in CPranit Bauva, Aug 23, 2016
  38. Junio C HamanoAug 26, 2016
  39. 12/27 bisect--helper: `get_terms` & `bisect_terms` shell function in CPranit Bauva, Aug 23, 2016
  40. Junio C HamanoAug 25, 2016
  41. Pranit BauvaAug 27, 2016
  42. Junio C HamanoAug 29, 2016
  43. 04/27 bisect--helper: `bisect_clean_state` shell function in CPranit Bauva, Aug 23, 2016
  44. Junio C HamanoAug 24, 2016
  45. 20/27 bisect--helper: retire `--write-terms` subcommandPranit Bauva, Aug 23, 2016
  46. 23/27 bisect--helper: retire `--bisect-write` subcommandPranit Bauva, Aug 23, 2016
  47. 10/27 bisect--helper: `check_and_set_terms` shell function in CPranit Bauva, Aug 23, 2016
  48. 03/27 bisect--helper: `write_terms` shell function in CPranit Bauva, Aug 23, 2016
  49. 26/27 bisect--helper: retire `--bisect-auto-next` subcommandPranit Bauva, Aug 23, 2016
  50. 02/27 bisect: rewrite `check_term_format` shell function in CPranit Bauva, Aug 23, 2016
  51. 08/27 bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in CPranit Bauva, Aug 23, 2016
  52. Junio C HamanoAug 24, 2016
  53. Pranit BauvaAug 27, 2016
  54. Junio C HamanoAug 29, 2016
  55. 22/27 bisect--helper: `bisect_replay` shell function in CPranit Bauva, Aug 23, 2016
  56. Junio C HamanoAug 26, 2016
  57. 11/27 bisect--helper: `bisect_next_check` & bisect_voc shell function in CPranit Bauva, Aug 23, 2016
  58. Junio C HamanoAug 24, 2016
  59. Pranit BauvaAug 27, 2016
  60. Pranit BauvaAug 23, 2016
  61. Junio C HamanoAug 23, 2016
  62. Pranit BauvaAug 23, 2016

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.