{"thread":{"id":"21383","subject":"Add '--bisect' revision machinery argument","startedAt":"2009-10-27T18:28:07Z","lastAt":"2009-10-31T20:58:08Z","messageCount":5,"participants":["Linus Torvalds","Junio C Hamano","Christian Couder"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"126044","messageId":"alpine.LFD.2.01.0910271124110.31845@localhost.localdomain","threadId":"21383","inReplyTo":null,"subject":"Add '--bisect' revision machinery argument","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-10-27T18:28:07Z","receivedAt":"2009-10-27T18:28:07Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nI personally use \"git bisect visualize\" all the time when I bisect, but it \nturns out that that is not a very flexible model. Sometimes I want to do \nbisection based on all commits (no pathname limiting), but then visualize \nthe current bisection tree with just a few pathnames because I _suspect_ \nthose pathnames are involved in the problem but am not totally sure about \nthem.\n\nAnd at other times, I want to use other revision parsing logic, none of \nwhich is available with \"git bisect visualize\".\n\nSo this adds \"--bisect\" as a revision parsing argument, and as a result it \njust works with all the normal logging tools. So now I can just do\n\n\tgitk --bisect --simplify-by-decoration filename-here\n\netc.\n\nSigned-off-by: Linus Torvalds <torvalds@linux-foundation.org>\n---\n builtin-rev-parse.c |   11 +++++++++++\n revision.c          |   18 +++++++++++++++++-\n 2 files changed, 28 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 45bead6..9526aaf 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -180,6 +180,12 @@ static int show_reference(const char *refname, const unsigned char *sha1, int fl\n \treturn 0;\n }\n \n+static int anti_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)\n+{\n+\tshow_rev(REVERSED, sha1, refname);\n+\treturn 0;\n+}\n+\n static void show_datestring(const char *flag, const char *datestr)\n {\n \tstatic char buffer[100];\n@@ -548,6 +554,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tfor_each_ref(show_reference, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--bisect\")) {\n+\t\t\t\tfor_each_ref_in(\"refs/bisect/bad\", show_reference, NULL);\n+\t\t\t\tfor_each_ref_in(\"refs/bisect/good\", anti_reference, NULL);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--branches\")) {\n \t\t\t\tfor_each_branch_ref(show_reference, NULL);\n \t\t\t\tcontinue;\ndiff --git a/revision.c b/revision.c\nindex 9fc4e8d..80a0528 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -994,7 +994,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \tif (!strcmp(arg, \"--all\") || !strcmp(arg, \"--branches\") ||\n \t    !strcmp(arg, \"--tags\") || !strcmp(arg, \"--remotes\") ||\n \t    !strcmp(arg, \"--reflog\") || !strcmp(arg, \"--not\") ||\n-\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\"))\n+\t    !strcmp(arg, \"--no-walk\") || !strcmp(arg, \"--do-walk\") ||\n+\t    !strcmp(arg, \"--bisect\"))\n \t{\n \t\tunkv[(*unkc)++] = arg;\n \t\treturn 1;\n@@ -1218,6 +1219,16 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,\n \tctx->argc -= n;\n }\n \n+static int for_each_bad_bisect_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn for_each_ref_in(\"refs/bisect/bad\", fn, cb_data);\n+}\n+\n+static int for_each_good_bisect_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn for_each_ref_in(\"refs/bisect/good\", fn, cb_data);\n+}\n+\n /*\n  * Parse revision information, filling in the \"rev_info\" structure,\n  * and removing the used arguments from the argument list.\n@@ -1259,6 +1270,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\t\thandle_refs(revs, flags, for_each_branch_ref);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strcmp(arg, \"--bisect\")) {\n+\t\t\t\thandle_refs(revs, flags, for_each_bad_bisect_ref);\n+\t\t\t\thandle_refs(revs, flags ^ UNINTERESTING, for_each_good_bisect_ref);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--tags\")) {\n \t\t\t\thandle_refs(revs, flags, for_each_tag_ref);\n \t\t\t\tcontinue;\n"},{"id":"126098","messageId":"7vhbtkt29v.fsf@alter.siamese.dyndns.org","threadId":"21383","inReplyTo":"alpine.LFD.2.01.0910271124110.31845@localhost.localdomain","subject":"Re: Add '--bisect' revision machinery argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-28T07:18:20Z","receivedAt":"2009-10-28T07:18:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.\n"},{"id":"126188","messageId":"7viqdzgls9.fsf@alter.siamese.dyndns.org","threadId":"21383","inReplyTo":"alpine.LFD.2.01.0910271124110.31845@localhost.localdomain","subject":"Re: Add '--bisect' revision machinery argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-10-28T23:07:34Z","receivedAt":"2009-10-28T23:07:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> So this adds \"--bisect\" as a revision parsing argument, and as a result it \n> just works with all the normal logging tools. So now I can just do\n>\n> \tgitk --bisect --simplify-by-decoration filename-here\n\nThis shows a very nice direction to evolve, but your patch as-is breaks\n\"rev-list --bisect\", I think. Call to your setup_revisions() eats the\ncommand line \"--bisect\" option but cmd_rev_list() wants to see it to go\ninto the \"bisection\" mode of traversal.\n\nAlso, the helper of \"git bisect\" can and probably should be taught to just\nask this new behaviour from the revision machinery, instead of collecting\ngood and bad refs itself using bisect.c::read_bisect_refs().\n\nHere is a short-term fix that can be squashed in, in order to allow t6022\nto pass again.\n\n builtin-rev-list.c |    2 ++\n revision.c         |    1 +\n revision.h         |    1 +\n 3 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-rev-list.c b/builtin-rev-list.c\nindex 4ba1c12..32bf033 100644\n--- a/builtin-rev-list.c\n+++ b/builtin-rev-list.c\n@@ -319,6 +319,8 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \n \tmemset(&info, 0, sizeof(info));\n \tinfo.revs = &revs;\n+\tif (revs.bisect)\n+\t\tbisect_list = 1;\n \n \tquiet = DIFF_OPT_TST(&revs.diffopt, QUIET);\n \tfor (i = 1 ; i < argc; i++) {\ndiff --git a/revision.c b/revision.c\nindex 80a0528..a36c0d9 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1273,6 +1273,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\tif (!strcmp(arg, \"--bisect\")) {\n \t\t\t\thandle_refs(revs, flags, for_each_bad_bisect_ref);\n \t\t\t\thandle_refs(revs, flags ^ UNINTERESTING, for_each_good_bisect_ref);\n+\t\t\t\trevs->bisect = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--tags\")) {\ndiff --git a/revision.h b/revision.h\nindex b6421a6..921656a 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -63,6 +63,7 @@ struct rev_info {\n \t\t\treverse:1,\n \t\t\treverse_output_stage:1,\n \t\t\tcherry_pick:1,\n+\t\t\tbisect:1,\n \t\t\tfirst_parent_only:1;\n \n \t/* Diff flags */\n"},{"id":"126189","messageId":"alpine.LFD.2.01.0910281631400.31845@localhost.localdomain","threadId":"21383","inReplyTo":"7viqdzgls9.fsf@alter.siamese.dyndns.org","subject":"Re: Add '--bisect' revision machinery argument","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2009-10-28T23:32:59Z","receivedAt":"2009-10-28T23:32:59Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 28 Oct 2009, Junio C Hamano wrote:\n> \n> This shows a very nice direction to evolve, but your patch as-is breaks\n> \"rev-list --bisect\", I think.\n\nI think you're right. I tested git rev-parse, and the 'git log' machinery, \nbut I didn't think about the fact that we already had a meaning \nfor '--bisect' in rev-list.\n\n> Also, the helper of \"git bisect\" can and probably should be taught to just\n> ask this new behaviour from the revision machinery, instead of collecting\n> good and bad refs itself using bisect.c::read_bisect_refs().\n\nYeah. And git-bisect.sh can be simplified too.\n\n\t\tLinus\n"},{"id":"126536","messageId":"200910312158.09049.chriscool@tuxfamily.org","threadId":"21383","inReplyTo":"alpine.LFD.2.01.0910281631400.31845@localhost.localdomain","subject":"Re: Add '--bisect' revision machinery argument","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-10-31T20:58:08Z","receivedAt":"2009-10-31T20:58:08Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thursday 29 October 2009, Linus Torvalds wrote:\n> On Wed, 28 Oct 2009, Junio C Hamano wrote:\n> > This shows a very nice direction to evolve, but your patch as-is breaks\n> > \"rev-list --bisect\", I think.\n>\n> I think you're right. I tested git rev-parse, and the 'git log'\n> machinery, but I didn't think about the fact that we already had a\n> meaning\n> for '--bisect' in rev-list.\n>\n> > Also, the helper of \"git bisect\" can and probably should be taught to\n> > just ask this new behaviour from the revision machinery, instead of\n> > collecting good and bad refs itself using bisect.c::read_bisect_refs().\n>\n> Yeah. And git-bisect.sh can be simplified too.\n\nI will have a look at that.\n\nSorry for not responding earlier but I just came back today from the Linux \nKongress 2009 in Dresden (http://www.linux-kongress.org/2009/).\n\nBest regards,\nChristian.\n"}]}