{"thread":{"id":"19520","subject":"[PATCH v2 2/2] bisect: check ancestors without forking a \"git rev-list\" process","startedAt":"2009-05-27T05:09:40Z","lastAt":"2009-05-27T05:09:42Z","messageCount":3,"participants":["Christian Couder"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"114763","messageId":"20090527050656.3694.86787.chriscool@tuxfamily.org","threadId":"19520","inReplyTo":null,"subject":"[PATCH v2 0/2] Avoid forking a process when checking merge bases","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T05:09:40Z","receivedAt":"2009-05-27T05:09:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"The goal of this patch series is to check if good revisions are\nancestor of the bad revision without forking a process to launch\n\"git rev-list $good ^$bad\".\n\nThis new version of this patch series does not use an \"unparse_commit\"\nfunction anymore, we use \"clear_commit_marks\" instead.\n\n  bisect: rework some rev related functions to make them more reusable\n  bisect: check ancestors without forking a \"git rev-list\" process\n\n bisect.c                    |   79 +++++++++++++++++++-----------------------\n t/t6030-bisect-porcelain.sh |   13 +------\n 2 files changed, 37 insertions(+), 55 deletions(-)\n"},{"id":"114765","messageId":"20090527050943.3694.58743.chriscool@tuxfamily.org","threadId":"19520","inReplyTo":"20090527050656.3694.86787.chriscool@tuxfamily.org","subject":"[PATCH v2 1/2] bisect: rework some rev related functions to make them more reusable","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T05:09:41Z","receivedAt":"2009-05-27T05:09:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This patches changes the \"bisect_rev_setup\" and \"bisect_common\"\nfunctions to make it easier to reuse them in a later patch.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c |   25 ++++++++++++++-----------\n 1 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex f57b62c..dc4e1bb 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -553,7 +553,9 @@ struct commit_list *filter_skipped(struct commit_list *list,\n \treturn filtered;\n }\n \n-static void bisect_rev_setup(struct rev_info *revs, const char *prefix)\n+static void bisect_rev_setup(struct rev_info *revs, const char *prefix,\n+\t\t\t     const char *bad_format, const char *good_format,\n+\t\t\t     int read_paths)\n {\n \tstruct argv_array rev_argv = { NULL, 0, 0 };\n \tint i;\n@@ -564,26 +566,24 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix)\n \n \t/* rev_argv.argv[0] will be ignored by setup_revisions */\n \targv_array_push(&rev_argv, xstrdup(\"bisect_rev_setup\"));\n-\targv_array_push_sha1(&rev_argv, current_bad_sha1, \"%s\");\n+\targv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format);\n \tfor (i = 0; i < good_revs.sha1_nr; i++)\n-\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i], \"^%s\");\n+\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i],\n+\t\t\t\t     good_format);\n \targv_array_push(&rev_argv, xstrdup(\"--\"));\n-\tread_bisect_paths(&rev_argv);\n+\tif (read_paths)\n+\t\tread_bisect_paths(&rev_argv);\n \targv_array_push(&rev_argv, NULL);\n \n \tsetup_revisions(rev_argv.argv_nr, rev_argv.argv, revs, NULL);\n-\trevs->limited = 1;\n }\n \n-static void bisect_common(struct rev_info *revs, int *reaches, int *all)\n+static void bisect_common(struct rev_info *revs)\n {\n \tif (prepare_revision_walk(revs))\n \t\tdie(\"revision walk setup failed\");\n \tif (revs->tree_objects)\n \t\tmark_edges_uninteresting(revs->commits, revs, NULL);\n-\n-\trevs->commits = find_bisection(revs->commits, reaches, all,\n-\t\t\t\t       !!skipped_revs.sha1_nr);\n }\n \n static void exit_if_skipped_commits(struct commit_list *tried,\n@@ -843,10 +843,13 @@ int bisect_next_all(const char *prefix)\n \n \tcheck_good_are_ancestors_of_bad(prefix);\n \n-\tbisect_rev_setup(&revs, prefix);\n+\tbisect_rev_setup(&revs, prefix, \"%s\", \"^%s\", 1);\n+\trevs.limited = 1;\n \n-\tbisect_common(&revs, &reaches, &all);\n+\tbisect_common(&revs);\n \n+\trevs.commits = find_bisection(revs.commits, &reaches, &all,\n+\t\t\t\t       !!skipped_revs.sha1_nr);\n \trevs.commits = filter_skipped(revs.commits, &tried, 0);\n \n \tif (!revs.commits) {\n-- \n1.6.3.GIT\n"},{"id":"114762","messageId":"20090527050943.3694.90014.chriscool@tuxfamily.org","threadId":"19520","inReplyTo":"20090527050656.3694.86787.chriscool@tuxfamily.org","subject":"[PATCH v2 2/2] bisect: check ancestors without forking a \"git rev-list\" process","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2009-05-27T05:09:42Z","receivedAt":"2009-05-27T05:09:42Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"We must save the pending commits that will be used during revision\nwalking and clear marks on them after, because we want to leave a\nclean state for the next revision walking that will try to find the\nbest bisection point.\n\nAs we don't fork a process anymore to call \"git rev-list\", we need\nto remove the use of GIT_TRACE to check how \"git rev-list\" is\ncalled from the t6030 test that uses it.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n bisect.c                    |   54 +++++++++++++++++-------------------------\n t/t6030-bisect-porcelain.sh |   13 +---------\n 2 files changed, 23 insertions(+), 44 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex dc4e1bb..18f9fa4 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -750,42 +750,31 @@ static void check_merge_bases(void)\n \tfree_commit_list(result);\n }\n \n-/*\n- * This function runs the command \"git rev-list $_good ^$_bad\"\n- * and returns 1 if it produces some output, 0 otherwise.\n- */\n-static int check_ancestors(void)\n+static int check_ancestors(const char *prefix)\n {\n-\tstruct argv_array rev_argv = { NULL, 0, 0 };\n-\tstruct strbuf str = STRBUF_INIT;\n-\tint i, result = 0;\n-\tstruct child_process rls;\n-\tFILE *rls_fout;\n+\tstruct rev_info revs;\n+\tstruct object_array pending_copy;\n+\tint i, res;\n \n-\targv_array_push(&rev_argv, xstrdup(\"rev-list\"));\n-\targv_array_push_sha1(&rev_argv, current_bad_sha1, \"^%s\");\n-\tfor (i = 0; i < good_revs.sha1_nr; i++)\n-\t\targv_array_push_sha1(&rev_argv, good_revs.sha1[i], \"%s\");\n-\targv_array_push(&rev_argv, NULL);\n+\tbisect_rev_setup(&revs, prefix, \"^%s\", \"%s\", 0);\n \n-\tmemset(&rls, 0, sizeof(rls));\n-\trls.argv = rev_argv.argv;\n-\trls.out = -1;\n-\trls.git_cmd = 1;\n-\tif (start_command(&rls))\n-\t\tdie(\"Could not launch 'git rev-list' command.\");\n-\trls_fout = fdopen(rls.out, \"r\");\n-\twhile (strbuf_getline(&str, rls_fout, '\\n') != EOF) {\n-\t\tstrbuf_trim(&str);\n-\t\tif (*str.buf) {\n-\t\t\tresult = 1;\n-\t\t\tbreak;\n-\t\t}\n+\t/* Save pending objects, so they can be cleaned up later. */\n+\tmemset(&pending_copy, 0, sizeof(pending_copy));\n+\tfor (i = 0; i < revs.pending.nr; i++)\n+\t\tadd_object_array(revs.pending.objects[i].item,\n+\t\t\t\t revs.pending.objects[i].name,\n+\t\t\t\t &pending_copy);\n+\n+\tbisect_common(&revs);\n+\tres = (revs.commits != NULL);\n+\n+\t/* Clean up objects used, as they will be reused. */\n+\tfor (i = 0; i < pending_copy.nr; i++) {\n+\t\tstruct object *o = pending_copy.objects[i].item;\n+\t\tclear_commit_marks((struct commit *)o, ALL_REV_FLAGS);\n \t}\n-\tfclose(rls_fout);\n-\tfinish_command(&rls);\n \n-\treturn result;\n+\treturn res;\n }\n \n /*\n@@ -813,7 +802,8 @@ static void check_good_are_ancestors_of_bad(const char *prefix)\n \tif (good_revs.sha1_nr == 0)\n \t\treturn;\n \n-\tif (check_ancestors())\n+\t/* Check if all good revs are ancestor of the bad rev. */\n+\tif (check_ancestors(prefix))\n \t\tcheck_merge_bases();\n \n \t/* Create file BISECT_ANCESTORS_OK. */\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 54b7ea6..5254b23 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -482,28 +482,17 @@ test_expect_success 'good merge bases when good and bad are siblings' '\n \tgit bisect reset\n '\n \n-check_trace() {\n-\tgrep \"$1\" \"$GIT_TRACE\" | grep \"\\^$2\" | grep \"$3\" >/dev/null\n-}\n-\n test_expect_success 'optimized merge base checks' '\n-\tGIT_TRACE=\"$(pwd)/trace.log\" &&\n-\texport GIT_TRACE &&\n \tgit bisect start \"$HASH7\" \"$SIDE_HASH7\" > my_bisect_log.txt &&\n \tgrep \"merge base must be tested\" my_bisect_log.txt &&\n \tgrep \"$HASH4\" my_bisect_log.txt &&\n-\tcheck_trace \"rev-list\" \"$HASH7\" \"$SIDE_HASH7\" &&\n \tgit bisect good > my_bisect_log2.txt &&\n \ttest -f \".git/BISECT_ANCESTORS_OK\" &&\n \ttest \"$HASH6\" = $(git rev-parse --verify HEAD) &&\n-\t: > \"$GIT_TRACE\" &&\n \tgit bisect bad > my_bisect_log3.txt &&\n-\ttest_must_fail check_trace \"rev-list\" \"$HASH6\" \"$SIDE_HASH7\" &&\n \tgit bisect good \"$A_HASH\" > my_bisect_log4.txt &&\n \tgrep \"merge base must be tested\" my_bisect_log4.txt &&\n-\ttest_must_fail test -f \".git/BISECT_ANCESTORS_OK\" &&\n-\tcheck_trace \"rev-list\" \"$HASH6\" \"$A_HASH\" &&\n-\tunset GIT_TRACE\n+\ttest_must_fail test -f \".git/BISECT_ANCESTORS_OK\"\n '\n \n # This creates another side branch called \"parallel\" with some files\n-- \n1.6.3.GIT\n"}]}