{"thread":{"id":"19522","subject":"[PATCH] bisect: display first bad commit without forking a new process","startedAt":"2009-05-27T05:23:53Z","lastAt":"2009-05-28T21:20:04Z","messageCount":5,"participants":["Christian Couder","Junio C Hamano","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"114769","messageId":"20090527052354.3824.22018.chriscool@tuxfamily.org","threadId":"19522","inReplyTo":null,"subject":"[PATCH] bisect: display first bad commit without forking a new process","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T05:23:53Z","receivedAt":"2009-05-27T05:23:53Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Previously \"git diff-tree --pretty COMMIT\" was run using\n\"run_command_v_opt\" to display information about the first bad\ncommit.\n\nThe goal of this patch is to avoid a \"fork\" and an \"exec\" call\nwhen displaying that information.\n\nTo do that, we manually setup revision information as\n\"git diff-tree --pretty\" would do it, and then use the\n\"log_tree_commit\" function.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c |   30 +++++++++++++++++++++++++++---\n 1 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex c43c120..e94a77b 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -7,6 +7,7 @@\n #include \"quote.h\"\n #include \"sha1-lookup.h\"\n #include \"run-command.h\"\n+#include \"log-tree.h\"\n #include \"bisect.h\"\n \n struct sha1_array {\n@@ -27,7 +28,6 @@ struct argv_array {\n \tint argv_alloc;\n };\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@@ -816,6 +816,31 @@ static void check_good_are_ancestors_of_bad(const char *prefix)\n }\n \n /*\n+ * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n+ */\n+static void show_diff_tree(const char *prefix, struct commit *commit)\n+{\n+\tstatic struct rev_info opt;\n+\n+\t/* diff-tree init */\n+\tinit_revisions(&opt, prefix);\n+\tgit_config(git_diff_basic_config, NULL); /* no \"diff\" UI options */\n+\topt.abbrev = 0;\n+\topt.diff = 1;\n+\n+\t/* This is what \"--pretty\" does */\n+\topt.verbose_header = 1;\n+\topt.use_terminator = 0;\n+\topt.commit_format = CMIT_FMT_DEFAULT;\n+\n+\t/* diff-tree init */\n+\tif (!opt.diffopt.output_format)\n+\t\topt.diffopt.output_format = DIFF_FORMAT_RAW;\n+\n+\tlog_tree_commit(&opt, commit);\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@@ -860,8 +885,7 @@ int bisect_next_all(const char *prefix)\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\tshow_diff_tree(prefix, revs.commits->item);\n \t\t/* This means the bisection process succeeded. */\n \t\texit(10);\n \t}\n-- \n1.6.3.GIT\n"},{"id":"114828","messageId":"200905272107.52095.chriscool@tuxfamily.org","threadId":"19522","inReplyTo":"20090527052354.3824.22018.chriscool@tuxfamily.org","subject":"Re: [PATCH] bisect: display first bad commit without forking a new process","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T19:07:52Z","receivedAt":"2009-05-27T19:07:52Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le Wednesday 27 May 2009, Christian Couder a écrit :\n> Previously \"git diff-tree --pretty COMMIT\" was run using\n> \"run_command_v_opt\" to display information about the first bad\n> commit.\n>\n> The goal of this patch is to avoid a \"fork\" and an \"exec\" call\n> when displaying that information.\n>\n> To do that, we manually setup revision information as\n> \"git diff-tree --pretty\" would do it, and then use the\n> \"log_tree_commit\" function.\n>\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  bisect.c |   30 +++++++++++++++++++++++++++---\n>  1 files changed, 27 insertions(+), 3 deletions(-)\n>\n> diff --git a/bisect.c b/bisect.c\n> index c43c120..e94a77b 100644\n> --- a/bisect.c\n> +++ b/bisect.c\n> @@ -7,6 +7,7 @@\n>  #include \"quote.h\"\n>  #include \"sha1-lookup.h\"\n>  #include \"run-command.h\"\n> +#include \"log-tree.h\"\n>  #include \"bisect.h\"\n>\n>  struct sha1_array {\n> @@ -27,7 +28,6 @@ struct argv_array {\n>  \tint argv_alloc;\n>  };\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> @@ -816,6 +816,31 @@ static void check_good_are_ancestors_of_bad(const\n> char *prefix) }\n>\n>  /*\n> + * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n> + */\n> +static void show_diff_tree(const char *prefix, struct commit *commit)\n> +{\n> +\tstatic struct rev_info opt;\n\nOops, \"static\" can be removed, it's a copy-paste error, sorry.\n\nThanks,\nChristian.\n"},{"id":"114859","messageId":"7veiuai33m.fsf@alter.siamese.dyndns.org","threadId":"19522","inReplyTo":"200905272107.52095.chriscool@tuxfamily.org","subject":"Re: [PATCH] bisect: display first bad commit without forking a new process","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-27T22:38:05Z","receivedAt":"2009-05-27T22:38:05Z","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> Le Wednesday 27 May 2009, Christian Couder a écrit :\n>> Previously \"git diff-tree --pretty COMMIT\" was run using\n>> \"run_command_v_opt\" to display information about the first bad\n>> commit.\n>>\n>> The goal of this patch is to avoid a \"fork\" and an \"exec\" call\n>> when displaying that information.\n>>\n>> To do that, we manually setup revision information as\n>> \"git diff-tree --pretty\" would do it, and then use the\n>> \"log_tree_commit\" function.\n>>\n>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>> ---\n>>  bisect.c |   30 +++++++++++++++++++++++++++---\n>>  1 files changed, 27 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/bisect.c b/bisect.c\n>> index c43c120..e94a77b 100644\n>> --- a/bisect.c\n>> +++ b/bisect.c\n>> @@ -816,6 +816,31 @@ static void check_good_are_ancestors_of_bad(const\n>> char *prefix) }\n>>\n>>  /*\n>> + * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n>> + */\n>> +static void show_diff_tree(const char *prefix, struct commit *commit)\n>> +{\n>> +\tstatic struct rev_info opt;\n>\n> Oops, \"static\" can be removed, it's a copy-paste error, sorry.\n\nIs that \"can\" or \"must\"?  If the answer is the latter, shouldn't the\nfunction be renamed to make it clear it is a bisect specific thing?\n"},{"id":"114901","messageId":"4A1E458E.6050505@op5.se","threadId":"19522","inReplyTo":"7veiuai33m.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] bisect: display first bad commit without forking a new process","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2009-05-28T08:04:30Z","receivedAt":"2009-05-28T08:04:30Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Christian Couder <chriscool@tuxfamily.org> writes:\n> \n>> Le Wednesday 27 May 2009, Christian Couder a écrit :\n>>> Previously \"git diff-tree --pretty COMMIT\" was run using\n>>> \"run_command_v_opt\" to display information about the first bad\n>>> commit.\n>>>\n>>> The goal of this patch is to avoid a \"fork\" and an \"exec\" call\n>>> when displaying that information.\n>>>\n>>> To do that, we manually setup revision information as\n>>> \"git diff-tree --pretty\" would do it, and then use the\n>>> \"log_tree_commit\" function.\n>>>\n>>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n>>> ---\n>>>  bisect.c |   30 +++++++++++++++++++++++++++---\n>>>  1 files changed, 27 insertions(+), 3 deletions(-)\n>>>\n>>> diff --git a/bisect.c b/bisect.c\n>>> index c43c120..e94a77b 100644\n>>> --- a/bisect.c\n>>> +++ b/bisect.c\n>>> @@ -816,6 +816,31 @@ static void check_good_are_ancestors_of_bad(const\n>>> char *prefix) }\n>>>\n>>>  /*\n>>> + * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n>>> + */\n>>> +static void show_diff_tree(const char *prefix, struct commit *commit)\n>>> +{\n>>> +\tstatic struct rev_info opt;\n>> Oops, \"static\" can be removed, it's a copy-paste error, sorry.\n> \n> Is that \"can\" or \"must\"?  If the answer is the latter, shouldn't the\n> function be renamed to make it clear it is a bisect specific thing?\n\nI think he was referring to the 'static' in the variable declarations.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nRegister now for Nordic Meet on Nagios, June 3-4 in Stockholm\n http://nordicmeetonnagios.op5.org/\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"114966","messageId":"200905282320.05039.chriscool@tuxfamily.org","threadId":"19522","inReplyTo":"4A1E458E.6050505@op5.se","subject":"Re: [PATCH] bisect: display first bad commit without forking a new process","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-28T21:20:04Z","receivedAt":"2009-05-28T21:20:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Le Thursday 28 May 2009, Andreas Ericsson a écrit :\n> Junio C Hamano wrote:\n> > Christian Couder <chriscool@tuxfamily.org> writes:\n> >> Le Wednesday 27 May 2009, Christian Couder a écrit :\n> >>> Previously \"git diff-tree --pretty COMMIT\" was run using\n> >>> \"run_command_v_opt\" to display information about the first bad\n> >>> commit.\n> >>>\n> >>> The goal of this patch is to avoid a \"fork\" and an \"exec\" call\n> >>> when displaying that information.\n> >>>\n> >>> To do that, we manually setup revision information as\n> >>> \"git diff-tree --pretty\" would do it, and then use the\n> >>> \"log_tree_commit\" function.\n> >>>\n> >>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> >>> ---\n> >>>  bisect.c |   30 +++++++++++++++++++++++++++---\n> >>>  1 files changed, 27 insertions(+), 3 deletions(-)\n> >>>\n> >>> diff --git a/bisect.c b/bisect.c\n> >>> index c43c120..e94a77b 100644\n> >>> --- a/bisect.c\n> >>> +++ b/bisect.c\n> >>> @@ -816,6 +816,31 @@ static void\n> >>> check_good_are_ancestors_of_bad(const char *prefix) }\n> >>>\n> >>>  /*\n> >>> + * This does \"git diff-tree --pretty COMMIT\" without one fork+exec.\n> >>> + */\n> >>> +static void show_diff_tree(const char *prefix, struct commit\n> >>> *commit) +{\n> >>> +\tstatic struct rev_info opt;\n> >>\n> >> Oops, \"static\" can be removed, it's a copy-paste error, sorry.\n> >\n> > Is that \"can\" or \"must\"?  If the answer is the latter, shouldn't the\n> > function be renamed to make it clear it is a bisect specific thing?\n>\n> I think he was referring to the 'static' in the variable declarations.\n\nYou are right. I will send a v2 of this patch anyway.\n\nThanks for this clarification,\nChristian.\n"}]}