{"thread":{"id":"18947","subject":"[PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","startedAt":"2009-04-19T09:56:07Z","lastAt":"2009-04-21T05:57:55Z","messageCount":5,"participants":["Christian Couder","Stephen Boyd","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"111637","messageId":"20090419115607.c7089503.chriscool@tuxfamily.org","threadId":"18947","inReplyTo":null,"subject":"[PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-19T09:56:07Z","receivedAt":"2009-04-19T09:56:07Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The goal of this patch is to port more shell code from the \"bisect_next\"\nfunction in \"git-bisect.sh\" to C code in \"builtin-bisect--helper.c\".\n\nSo we port the code that interprets the bisection result and stops or\ncontinues (by checking out the next revision) the bisection process.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c                 |  105 ++++++++++++++++++++++++++++++++++++++++++++++\n bisect.h                 |    1 +\n builtin-bisect--helper.c |   12 ++++-\n 3 files changed, 115 insertions(+), 3 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex 0448eae..f6e1378 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -6,6 +6,7 @@\n #include \"list-objects.h\"\n #include \"quote.h\"\n #include \"sha1-lookup.h\"\n+#include \"run-command.h\"\n #include \"bisect.h\"\n \n static unsigned char (*skipped_sha1)[20];\n@@ -16,6 +17,12 @@ static const char **rev_argv;\n static int rev_argv_nr;\n static int rev_argv_alloc;\n \n+static const unsigned char *current_bad_sha1;\n+\n+static const char *argv_diff_tree[] = {\"diff-tree\", \"--pretty\", NULL, NULL};\n+static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n+static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n+\n /* bits #0-15 in revision.h */\n \n #define COUNTED\t\t(1u<<16)\n@@ -403,6 +410,7 @@ static int register_ref(const char *refname, const unsigned char *sha1,\n {\n \tif (!strcmp(refname, \"bad\")) {\n \t\tALLOC_GROW(rev_argv, rev_argv_nr + 1, rev_argv_alloc);\n+\t\tcurrent_bad_sha1 = sha1;\n \t\trev_argv[rev_argv_nr++] = xstrdup(sha1_to_hex(sha1));\n \t} else if (!prefixcmp(refname, \"good-\")) {\n \t\tconst char *hex = sha1_to_hex(sha1);\n@@ -560,3 +568,100 @@ int bisect_next_vars(const char *prefix)\n \n \treturn show_bisect_vars(&info, reaches, all);\n }\n+\n+static void exit_if_skipped_commits(struct commit_list *tried,\n+\t\t\t\t    const unsigned char *bad)\n+{\n+\tif (!tried)\n+\t\treturn;\n+\n+\tprintf(\"There are only 'skip'ped commit left to test.\\n\"\n+\t       \"The first bad commit could be any of:\\n\");\n+\tprint_commit_list(tried, \"%s\\n\", \"%s\\n\");\n+\tif (bad)\n+\t\tprintf(\"%s\\n\", sha1_to_hex(bad));\n+\tprintf(\"We cannot bisect more!\\n\");\n+\texit(2);\n+}\n+\n+static void mark_expected_rev(char *bisect_rev_hex)\n+{\n+\tint len = strlen(bisect_rev_hex);\n+\tconst char *filename = git_path(\"BISECT_EXPECTED_REV\");\n+\tint fd = open(filename, O_CREAT | O_TRUNC | O_WRONLY, 0600);\n+\n+\tif (fd < 0)\n+\t\tdie(\"could not create file '%s': %s\",\n+\t\t    filename, strerror(errno));\n+\n+\tbisect_rev_hex[len] = '\\n';\n+\twrite_or_die(fd, bisect_rev_hex, len + 1);\n+\tbisect_rev_hex[len] = '\\0';\n+\n+\tif (close(fd) < 0)\n+\t\tdie(\"closing file %s: %s\", filename, strerror(errno));\n+}\n+\n+static int bisect_checkout(char *bisect_rev_hex)\n+{\n+\tint res;\n+\n+\tmark_expected_rev(bisect_rev_hex);\n+\n+\targv_checkout[2] = bisect_rev_hex;\n+\tres = run_command_v_opt(argv_checkout, RUN_GIT_CMD);\n+\tif (res)\n+\t\texit(res);\n+\n+\targv_show_branch[1] = bisect_rev_hex;\n+\treturn run_command_v_opt(argv_show_branch, RUN_GIT_CMD);\n+}\n+\n+/*\n+ * We use the convention that exiting with an exit code 10 means that\n+ * the bisection process finished successfully.\n+ * In this case the calling shell script should exit 0.\n+ */\n+int bisect_next_exit(const char *prefix)\n+{\n+\tstruct rev_info revs;\n+\tstruct commit_list *tried;\n+\tint reaches = 0, all = 0, nr;\n+\tconst unsigned char *bisect_rev;\n+\tchar bisect_rev_hex[41];\n+\n+\tbisect_common(&revs, prefix, &reaches, &all);\n+\n+\trevs.commits = filter_skipped(revs.commits, &tried, 0);\n+\n+\tif (!revs.commits) {\n+\t\t/*\n+\t\t * We should exit here only if the \"bad\"\n+\t\t * commit is also a \"skip\" commit.\n+\t\t */\n+\t\texit_if_skipped_commits(tried, NULL);\n+\n+\t\tprintf(\"%s was both good and bad\\n\",\n+\t\t       sha1_to_hex(current_bad_sha1));\n+\t\texit(1);\n+\t}\n+\n+\tbisect_rev = revs.commits->item->object.sha1;\n+\tmemcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), 41);\n+\n+\tif (!hashcmp(bisect_rev, current_bad_sha1)) {\n+\t\texit_if_skipped_commits(tried, current_bad_sha1);\n+\t\tprintf(\"%s is first bad commit\\n\", bisect_rev_hex);\n+\t\targv_diff_tree[2] = bisect_rev_hex;\n+\t\trun_command_v_opt(argv_diff_tree, RUN_GIT_CMD);\n+\t\t/* This means the bisection process succeeded. */\n+\t\texit(10);\n+\t}\n+\n+\tnr = all - reaches - 1;\n+\tprintf(\"Bisecting: %d revisions left to test after this \"\n+\t       \"(roughly %d steps)\\n\", nr, estimate_bisect_steps(all));\n+\n+\treturn bisect_checkout(bisect_rev_hex);\n+}\n+\ndiff --git a/bisect.h b/bisect.h\nindex 89aa6cb..028eb85 100644\n--- a/bisect.h\n+++ b/bisect.h\n@@ -29,6 +29,7 @@ struct rev_list_info {\n extern int show_bisect_vars(struct rev_list_info *info, int reaches, int all);\n \n extern int bisect_next_vars(const char *prefix);\n+extern int bisect_next_exit(const char *prefix);\n \n extern int estimate_bisect_steps(int all);\n \ndiff --git a/builtin-bisect--helper.c b/builtin-bisect--helper.c\nindex 8fe7787..cb86a9a 100644\n--- a/builtin-bisect--helper.c\n+++ b/builtin-bisect--helper.c\n@@ -5,23 +5,29 @@\n \n static const char * const git_bisect_helper_usage[] = {\n \t\"git bisect--helper --next-vars\",\n+\t\"git bisect--helper --next-exit\",\n \tNULL\n };\n \n int cmd_bisect__helper(int argc, const char **argv, const char *prefix)\n {\n \tint next_vars = 0;\n+\tint next_exit = 0;\n \tstruct option options[] = {\n \t\tOPT_BOOLEAN(0, \"next-vars\", &next_vars,\n \t\t\t    \"output next bisect step variables\"),\n+\t\tOPT_BOOLEAN(0, \"next-exit\", &next_exit,\n+\t\t\t    \"output bisect result and exit instuctions\"),\n \t\tOPT_END()\n \t};\n \n \targc = parse_options(argc, argv, options, git_bisect_helper_usage, 0);\n \n-\tif (!next_vars)\n+\tif ((next_vars && next_exit) || (!next_vars && !next_exit))\n \t\tusage_with_options(git_bisect_helper_usage, options);\n \n-\t/* next-vars */\n-\treturn bisect_next_vars(prefix);\n+\tif (next_vars)\n+\t\treturn bisect_next_vars(prefix);\n+\telse /* next-exit */\n+\t\treturn bisect_next_exit(prefix);\n }\n-- \n1.6.2.2.537.g3b83b\n"},{"id":"111687","messageId":"49EC2353.7080206@gmail.com","threadId":"18947","inReplyTo":"20090419115607.c7089503.chriscool@tuxfamily.org","subject":"Re: [PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-04-20T07:25:07Z","receivedAt":"2009-04-20T07:25:07Z","isPatch":true,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Christian Couder wrote:\n> +\tprintf(\"There are only 'skip'ped commit left to test.\\n\"\n> +\t       \"The first bad commit could be any of:\\n\")\n\nShould this be \"There are only 'skip'ped commits left to test\"?\n"},{"id":"111698","messageId":"7vprf7k93h.fsf@gitster.siamese.dyndns.org","threadId":"18947","inReplyTo":"20090419115607.c7089503.chriscool@tuxfamily.org","subject":"Re: [PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-04-20T08:44:34Z","receivedAt":"2009-04-20T08:44:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> The goal of this patch is to port more shell code from the \"bisect_next\"\n> function in \"git-bisect.sh\" to C code in \"builtin-bisect--helper.c\".\n>\n> So we port the code that interprets the bisection result and stops or\n> continues (by checking out the next revision) the bisection process.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n\nJust to make sure I understand correctly, with this and patch 5/5, we can\nthen drop the --next-vars helper command, right?\n\n> @@ -16,6 +17,12 @@ static const char **rev_argv;\n>  static int rev_argv_nr;\n>  static int rev_argv_alloc;\n>  \n> +static const unsigned char *current_bad_sha1;\n> +\n> +static const char *argv_diff_tree[] = {\"diff-tree\", \"--pretty\", NULL, NULL};\n> +static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n> +static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n\nI do appreciate that you kept this step in \"rewrite in C\" theme a straight\nrewrite to still call out the executables.  We can turn these into\ninternal calls in future steps, but that is a separate issue.\n\nI suspect that the show-branch call outlived its usefulness with this\npatch; it will now be a low hanging fruit to call format_commit_message()\ninternally to reduce one fork+exec.\n\nThanks, queued.\n"},{"id":"111839","messageId":"200904210751.39215.chriscool@tuxfamily.org","threadId":"18947","inReplyTo":"49EC2353.7080206@gmail.com","subject":"Re: [PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-21T05:51:39Z","receivedAt":"2009-04-21T05:51:39Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 20 avril 2009, Stephen Boyd a écrit :\n> Christian Couder wrote:\n> > +\tprintf(\"There are only 'skip'ped commit left to test.\\n\"\n> > +\t       \"The first bad commit could be any of:\\n\")\n>\n> Should this be \"There are only 'skip'ped commits left to test\"?\n\nYes, but the same message was printed previously, so it is a slightly \nincompatible change and I think it should go into another patch. Would you \ncare to send such a patch?\n\nThanks,\nChristian.\n"},{"id":"111843","messageId":"200904210757.55162.chriscool@tuxfamily.org","threadId":"18947","inReplyTo":"7vprf7k93h.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/5] bisect--helper: add \"--next-exit\" to output bisect results","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-04-21T05:57:55Z","receivedAt":"2009-04-21T05:57:55Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le lundi 20 avril 2009, Junio C Hamano a écrit :\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> > The goal of this patch is to port more shell code from the\n> > \"bisect_next\" function in \"git-bisect.sh\" to C code in\n> > \"builtin-bisect--helper.c\".\n> >\n> > So we port the code that interprets the bisection result and stops or\n> > continues (by checking out the next revision) the bisection process.\n> >\n> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>\n> Just to make sure I understand correctly, with this and patch 5/5, we can\n> then drop the --next-vars helper command, right?\n\nYes, that's right. I just sent two patches to remove useless code.\n\n> > @@ -16,6 +17,12 @@ static const char **rev_argv;\n> >  static int rev_argv_nr;\n> >  static int rev_argv_alloc;\n> >\n> > +static const unsigned char *current_bad_sha1;\n> > +\n> > +static const char *argv_diff_tree[] = {\"diff-tree\", \"--pretty\", NULL,\n> > NULL}; +static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL,\n> > \"--\", NULL}; +static const char *argv_show_branch[] = {\"show-branch\",\n> > NULL, NULL};\n>\n> I do appreciate that you kept this step in \"rewrite in C\" theme a\n> straight rewrite to still call out the executables.  We can turn these\n> into internal calls in future steps, but that is a separate issue.\n>\n> I suspect that the show-branch call outlived its usefulness with this\n> patch; it will now be a low hanging fruit to call format_commit_message()\n> internally to reduce one fork+exec.\n\nI will have a look at that.\n\nThanks,\nChristian.\n"}]}