From a22347c6c866c94e9e941b1b39d57ea43218af96 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 17 May 2009 17:36:44 +0200 Subject: [PATCH 1/3] bisect: rework some rev related functions to make them more reusable This patches changes the "bisect_rev_setup" and "bisect_common" functions to make it easier to reuse them in a later patch. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/bisect.c b/bisect.c index f57b62cdd..dc4e1bb3c 100644 --- a/bisect.c +++ b/bisect.c @@ -553,7 +553,9 @@ struct commit_list *filter_skipped(struct commit_list *list, return filtered; } -static void bisect_rev_setup(struct rev_info *revs, const char *prefix) +static void bisect_rev_setup(struct rev_info *revs, const char *prefix, + const char *bad_format, const char *good_format, + int read_paths) { struct argv_array rev_argv = { NULL, 0, 0 }; int i; @@ -564,26 +566,24 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix) /* rev_argv.argv[0] will be ignored by setup_revisions */ argv_array_push(&rev_argv, xstrdup("bisect_rev_setup")); - argv_array_push_sha1(&rev_argv, current_bad_sha1, "%s"); + argv_array_push_sha1(&rev_argv, current_bad_sha1, bad_format); for (i = 0; i < good_revs.sha1_nr; i++) - argv_array_push_sha1(&rev_argv, good_revs.sha1[i], "^%s"); + argv_array_push_sha1(&rev_argv, good_revs.sha1[i], + good_format); argv_array_push(&rev_argv, xstrdup("--")); - read_bisect_paths(&rev_argv); + if (read_paths) + read_bisect_paths(&rev_argv); argv_array_push(&rev_argv, NULL); setup_revisions(rev_argv.argv_nr, rev_argv.argv, revs, NULL); - revs->limited = 1; } -static void bisect_common(struct rev_info *revs, int *reaches, int *all) +static void bisect_common(struct rev_info *revs) { if (prepare_revision_walk(revs)) die("revision walk setup failed"); if (revs->tree_objects) mark_edges_uninteresting(revs->commits, revs, NULL); - - revs->commits = find_bisection(revs->commits, reaches, all, - !!skipped_revs.sha1_nr); } static void exit_if_skipped_commits(struct commit_list *tried, @@ -843,10 +843,13 @@ int bisect_next_all(const char *prefix) check_good_are_ancestors_of_bad(prefix); - bisect_rev_setup(&revs, prefix); + bisect_rev_setup(&revs, prefix, "%s", "^%s", 1); + revs.limited = 1; - bisect_common(&revs, &reaches, &all); + bisect_common(&revs); + revs.commits = find_bisection(revs.commits, &reaches, &all, + !!skipped_revs.sha1_nr); revs.commits = filter_skipped(revs.commits, &tried, 0); if (!revs.commits) { From 836a3fd5b0c439642268fd1299cd16729f15e614 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 17 May 2009 17:36:45 +0200 Subject: [PATCH 2/3] commit: add function to unparse a commit and its parents This patch adds the "unparse_commit" function that returns a commit into an unparsed state by freeing its data and resetting its fields to 0. Its parents are recursively unparsed too, because they might have been changed. But its tree is not unparsed as it should not have been modifed. Note that as the "flags" and "used" fields may be used even if the object is not parsed, we have to reset them anyway. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- commit.c | 20 ++++++++++++++++++++ commit.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/commit.c b/commit.c index aa3b35b6a..8f6b703c5 100644 --- a/commit.c +++ b/commit.c @@ -316,6 +316,26 @@ int parse_commit(struct commit *item) return ret; } +static void unparse_commit_list(struct commit_list *list) +{ + for (; list; list = list->next) + unparse_commit(list->item); +} + +void unparse_commit(struct commit *item) +{ + item->object.flags = 0; + item->object.used = 0; + if (item->object.parsed) { + item->object.parsed = 0; + if (item->parents) { + unparse_commit_list(item->parents); + free_commit_list(item->parents); + item->parents = NULL; + } + } +} + struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p) { struct commit_list *new_list = xmalloc(sizeof(struct commit_list)); diff --git a/commit.h b/commit.h index ba9f63813..f3eaf1d04 100644 --- a/commit.h +++ b/commit.h @@ -40,6 +40,8 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size); int parse_commit(struct commit *item); +void unparse_commit(struct commit *item); + struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p); unsigned commit_list_count(const struct commit_list *l); struct commit_list * insert_by_date(struct commit *item, struct commit_list **list); From 2d938fc7bcf2c8ce314c44955db5a4dd2e9b6adb Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sun, 17 May 2009 17:36:46 +0200 Subject: [PATCH 3/3] bisect: check ancestors without forking a "git rev-list" process We must save the pending commits that will be used during revision walking and unparse them after, because we want to leave a clean state for the next revision walking that will try to find the best bisection point. As we don't fork a process anymore to call "git rev-list", we need to remove the use of GIT_TRACE to check how "git rev-list" is called from the t6030 test that uses it. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 54 +++++++++++++++---------------------- t/t6030-bisect-porcelain.sh | 13 +-------- 2 files changed, 23 insertions(+), 44 deletions(-) diff --git a/bisect.c b/bisect.c index dc4e1bb3c..c43c120bd 100644 --- a/bisect.c +++ b/bisect.c @@ -750,42 +750,31 @@ static void check_merge_bases(void) free_commit_list(result); } -/* - * This function runs the command "git rev-list $_good ^$_bad" - * and returns 1 if it produces some output, 0 otherwise. - */ -static int check_ancestors(void) +static int check_ancestors(const char *prefix) { - struct argv_array rev_argv = { NULL, 0, 0 }; - struct strbuf str = STRBUF_INIT; - int i, result = 0; - struct child_process rls; - FILE *rls_fout; + struct rev_info revs; + struct object_array pending_copy; + int i, res; - argv_array_push(&rev_argv, xstrdup("rev-list")); - argv_array_push_sha1(&rev_argv, current_bad_sha1, "^%s"); - for (i = 0; i < good_revs.sha1_nr; i++) - argv_array_push_sha1(&rev_argv, good_revs.sha1[i], "%s"); - argv_array_push(&rev_argv, NULL); + bisect_rev_setup(&revs, prefix, "^%s", "%s", 0); - memset(&rls, 0, sizeof(rls)); - rls.argv = rev_argv.argv; - rls.out = -1; - rls.git_cmd = 1; - if (start_command(&rls)) - die("Could not launch 'git rev-list' command."); - rls_fout = fdopen(rls.out, "r"); - while (strbuf_getline(&str, rls_fout, '\n') != EOF) { - strbuf_trim(&str); - if (*str.buf) { - result = 1; - break; - } + /* Save pending objects, so they can be cleaned up later. */ + memset(&pending_copy, 0, sizeof(pending_copy)); + for (i = 0; i < revs.pending.nr; i++) + add_object_array(revs.pending.objects[i].item, + revs.pending.objects[i].name, + &pending_copy); + + bisect_common(&revs); + res = (revs.commits != NULL); + + /* Clean up objects used, as they will be reused. */ + for (i = 0; i < pending_copy.nr; i++) { + struct object *o = pending_copy.objects[i].item; + unparse_commit((struct commit *)o); } - fclose(rls_fout); - finish_command(&rls); - return result; + return res; } /* @@ -813,7 +802,8 @@ static void check_good_are_ancestors_of_bad(const char *prefix) if (good_revs.sha1_nr == 0) return; - if (check_ancestors()) + /* Check if all good revs are ancestor of the bad rev. */ + if (check_ancestors(prefix)) check_merge_bases(); /* Create file BISECT_ANCESTORS_OK. */ diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 54b7ea650..5254b2351 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -482,28 +482,17 @@ test_expect_success 'good merge bases when good and bad are siblings' ' git bisect reset ' -check_trace() { - grep "$1" "$GIT_TRACE" | grep "\^$2" | grep "$3" >/dev/null -} - test_expect_success 'optimized merge base checks' ' - GIT_TRACE="$(pwd)/trace.log" && - export GIT_TRACE && git bisect start "$HASH7" "$SIDE_HASH7" > my_bisect_log.txt && grep "merge base must be tested" my_bisect_log.txt && grep "$HASH4" my_bisect_log.txt && - check_trace "rev-list" "$HASH7" "$SIDE_HASH7" && git bisect good > my_bisect_log2.txt && test -f ".git/BISECT_ANCESTORS_OK" && test "$HASH6" = $(git rev-parse --verify HEAD) && - : > "$GIT_TRACE" && git bisect bad > my_bisect_log3.txt && - test_must_fail check_trace "rev-list" "$HASH6" "$SIDE_HASH7" && git bisect good "$A_HASH" > my_bisect_log4.txt && grep "merge base must be tested" my_bisect_log4.txt && - test_must_fail test -f ".git/BISECT_ANCESTORS_OK" && - check_trace "rev-list" "$HASH6" "$A_HASH" && - unset GIT_TRACE + test_must_fail test -f ".git/BISECT_ANCESTORS_OK" ' # This creates another side branch called "parallel" with some files