Merge pull request #2230 from anuraggup/revwalk-merge-base

No need to find merge base.

src/revwalk.c

@@ -39,8 +39,9 @@ git_commit_list_node *git_revwalk__commit_lookup(
 	return commit;
 }
 
-static int mark_uninteresting(git_commit_list_node *commit)
+static int mark_uninteresting(git_revwalk *walk, git_commit_list_node *commit)
 {
+	int error;
 	unsigned short i;
 	git_array_t(git_commit_list_node *) pending = GIT_ARRAY_INIT;
 	git_commit_list_node **tmp;
@@ -53,12 +54,8 @@ static int mark_uninteresting(git_commit_list_node *commit)
 	do {
 		commit->uninteresting = 1;
 
-		/* This means we've reached a merge base, so there's no need to walk any more */
-		if ((commit->flags & (RESULT | STALE)) == RESULT) {
-			tmp = git_array_pop(pending);
-			commit = tmp ? *tmp : NULL;
-			continue;
-		}
+		if ((error = git_commit_list_parse(walk, commit)) < 0)
+			return error;
 
 		for (i = 0; i < commit->out_degree; ++i)
 			if (!commit->parents[i]->uninteresting) {
@@ -84,7 +81,7 @@ static int process_commit(git_revwalk *walk, git_commit_list_node *commit, int h
 	if (!hide && walk->hide_cb)
 		hide = walk->hide_cb(&commit->oid, walk->hide_cb_payload);
 
-	if (hide && mark_uninteresting(commit) < 0)
+	if (hide && mark_uninteresting(walk, commit) < 0)
 		return -1;
 
 	if (commit->seen)
@@ -95,7 +92,10 @@ static int process_commit(git_revwalk *walk, git_commit_list_node *commit, int h
 	if ((error = git_commit_list_parse(walk, commit)) < 0)
 		return error;
 
-	return walk->enqueue(walk, commit);
+	if (!hide)
+		return walk->enqueue(walk, commit);
+
+	return 0;
 }
 
 static int process_commit_parents(git_revwalk *walk, git_commit_list_node *commit)
@@ -144,9 +144,6 @@ static int push_commit(git_revwalk *walk, const git_oid *oid, int uninteresting,
 	if (commit == NULL)
 		return -1; /* error already reported by failed lookup */
 
-	if (uninteresting)
-		walk->did_hide = 1;
-
 	commit->uninteresting = uninteresting;
 	if (walk->one == NULL && !uninteresting) {
 		walk->one = commit;
@@ -298,15 +295,14 @@ static int revwalk_next_timesort(git_commit_list_node **object_out, git_revwalk
 	int error;
 	git_commit_list_node *next;
 
-	while ((next = git_pqueue_pop(&walk->iterator_time)) != NULL) {
-		if ((error = process_commit_parents(walk, next)) < 0)
-			return error;
-
+	while ((next = git_pqueue_pop(&walk->iterator_time)) != NULL)
 		if (!next->uninteresting) {
+			if ((error = process_commit_parents(walk, next)) < 0)
+				return error;
+
 			*object_out = next;
 			return 0;
 		}
-	}
 
 	giterr_clear();
 	return GIT_ITEROVER;
@@ -317,15 +313,14 @@ static int revwalk_next_unsorted(git_commit_list_node **object_out, git_revwalk
 	int error;
 	git_commit_list_node *next;
 
-	while ((next = git_commit_list_pop(&walk->iterator_rand)) != NULL) {
-		if ((error = process_commit_parents(walk, next)) < 0)
-			return error;
-
+	while ((next = git_commit_list_pop(&walk->iterator_rand)) != NULL)
 		if (!next->uninteresting) {
+			if ((error = process_commit_parents(walk, next)) < 0)
+				return error;
+
 			*object_out = next;
 			return 0;
 		}
-	}
 
 	giterr_clear();
 	return GIT_ITEROVER;
@@ -380,7 +375,6 @@ static int prepare_walk(git_revwalk *walk)
 	int error;
 	unsigned int i;
 	git_commit_list_node *next, *two;
-	git_commit_list *bases = NULL;
 
 	/*
 	 * If walk->one is NULL, there were no positive references,
@@ -391,18 +385,6 @@ static int prepare_walk(git_revwalk *walk)
 		return GIT_ITEROVER;
 	}
 
-	/*
-	 * If the user asked to hide commits, we need to figure out
-	 * what the merge bases are so we can know when we can stop
-	 * marking parents uninteresting.
-	 */
-	if (walk->did_hide) {
-		if (git_merge__bases_many(&bases, walk, walk->one, &walk->twos) < 0)
-			return -1;
-
-		git_commit_list_free(&bases);
-	}
-
 	if (process_commit(walk, walk->one, walk->one->uninteresting) < 0)
 		return -1;
 

src/revwalk.h

@@ -32,8 +32,7 @@ struct git_revwalk {
 	int (*enqueue)(git_revwalk *, git_commit_list_node *);
 
 	unsigned walking:1,
-		first_parent: 1,
-		did_hide: 1;
+		first_parent: 1;
 	unsigned int sorting;
 
 	/* merge base calculation */

tests/revwalk/basic.c

@@ -160,7 +160,7 @@ void test_revwalk_basic__glob_heads(void)
 	}
 
 	/* git log --branches --oneline | wc -l => 14 */
-	cl_assert(i == 14);
+	cl_assert_equal_i(i, 14);
 }
 
 void test_revwalk_basic__glob_heads_with_invalid(void)
@@ -194,7 +194,7 @@ void test_revwalk_basic__push_head(void)
 	}
 
 	/* git log HEAD --oneline | wc -l => 7 */
-	cl_assert(i == 7);
+	cl_assert_equal_i(i, 7);
 }
 
 void test_revwalk_basic__push_head_hide_ref(void)
@@ -212,7 +212,7 @@ void test_revwalk_basic__push_head_hide_ref(void)
 	}
 
 	/* git log HEAD --oneline --not refs/heads/packed-test | wc -l => 4 */
-	cl_assert(i == 4);
+	cl_assert_equal_i(i, 4);
 }
 
 void test_revwalk_basic__push_head_hide_ref_nobase(void)
@@ -230,7 +230,78 @@ void test_revwalk_basic__push_head_hide_ref_nobase(void)
 	}
 
 	/* git log HEAD --oneline --not refs/heads/packed | wc -l => 7 */
-	cl_assert(i == 7);
+	cl_assert_equal_i(i, 7);
+}
+
+/*
+* $ git rev-list HEAD 5b5b02 ^refs/heads/packed-test
+* a65fedf39aefe402d3bb6e24df4d4f5fe4547750
+* be3563ae3f795b2b4353bcce3a527ad0a4f7f644
+* c47800c7266a2be04c571c04d5a6614691ea99bd
+* 9fd738e8f7967c078dceed8190330fc8648ee56a
+
+* $ git log HEAD 5b5b02 --oneline --not refs/heads/packed-test | wc -l => 4
+* a65fedf
+* be3563a Merge branch 'br2'
+* c47800c branch commit one
+* 9fd738e a fourth commit
+*/
+void test_revwalk_basic__multiple_push_1(void)
+{
+	int i = 0;
+	git_oid oid;
+
+	revwalk_basic_setup_walk(NULL);
+
+	cl_git_pass(git_revwalk_push_head(_walk));
+
+	cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/packed-test"));
+
+	cl_git_pass(git_oid_fromstr(&oid, "5b5b025afb0b4c913b4c338a42934a3863bf3644"));
+	cl_git_pass(git_revwalk_push(_walk, &oid));
+
+	while (git_revwalk_next(&oid, _walk) == 0)
+		i++;
+
+	cl_assert_equal_i(i, 4);
+}
+
+/*
+* Difference between test_revwalk_basic__multiple_push_1 and 
+* test_revwalk_basic__multiple_push_2 is in the order reference
+* refs/heads/packed-test and commit 5b5b02 are pushed. 
+* revwalk should return same commits in both the tests.
+
+* $ git rev-list 5b5b02 HEAD ^refs/heads/packed-test
+* a65fedf39aefe402d3bb6e24df4d4f5fe4547750
+* be3563ae3f795b2b4353bcce3a527ad0a4f7f644
+* c47800c7266a2be04c571c04d5a6614691ea99bd
+* 9fd738e8f7967c078dceed8190330fc8648ee56a
+
+* $ git log 5b5b02 HEAD --oneline --not refs/heads/packed-test | wc -l => 4
+* a65fedf
+* be3563a Merge branch 'br2'
+* c47800c branch commit one
+* 9fd738e a fourth commit
+*/
+void test_revwalk_basic__multiple_push_2(void)
+{
+	int i = 0;
+	git_oid oid;
+
+	revwalk_basic_setup_walk(NULL);
+
+	cl_git_pass(git_oid_fromstr(&oid, "5b5b025afb0b4c913b4c338a42934a3863bf3644"));
+	cl_git_pass(git_revwalk_push(_walk, &oid));
+
+	cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/packed-test"));
+
+	cl_git_pass(git_revwalk_push_head(_walk));
+
+	while (git_revwalk_next(&oid, _walk) == 0)
+		i++;
+
+	cl_assert_equal_i(i, 4);
 }
 
 void test_revwalk_basic__disallow_non_commit(void)