Commit 07011e60cc2ea473bf47dfb5f0c9a1a2898dd6b4
2018-04-12T13:32:27
revwalk: fix uninteresting revs sometimes not limiting graphwalk When we want to limit our graphwalk, we use the heuristic of checking whether the newest limiting (uninteresting) revision is newer than the oldest interesting revision. We do so by inspecting whether the first item's commit time of the user-supplied list of revisions is newer than the last added interesting revision. This is wrong though, as the user supplied list is in no way guaranteed to be sorted by increasing commit dates. This could lead us to abort the revwalk early before applying all relevant limiting revisions, outputting revisions which should in fact have been hidden. Fix the heuristic by instead checking whether _any_ of the limiting commits was made earlier than the last interesting commit. Add a test.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146
diff --git a/src/revwalk.c b/src/revwalk.c
index b1aa8f9..eb228a5 100644
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -375,20 +375,6 @@ static int add_parents_to_list(git_revwalk *walk, git_commit_list_node *commit,
return 0;
}
-static int everybody_uninteresting(git_commit_list *orig)
-{
- git_commit_list *list = orig;
-
- while (list) {
- git_commit_list_node *commit = list->item;
- list = list->next;
- if (!commit->uninteresting)
- return 0;
- }
-
- return 1;
-}
-
/* How many unintersting commits we want to look at after we run out of interesting ones */
#define SLOP 5
@@ -398,16 +384,15 @@ static int still_interesting(git_commit_list *list, int64_t time, int slop)
if (!list)
return 0;
- /*
- * If the destination list has commits with an earlier date
- * than our source we want to continue looking.
- */
- if (time <= list->item->time)
- return SLOP;
-
- /* If we find interesting commits, we reset the slop count */
- if (!everybody_uninteresting(list))
- return SLOP;
+ for (; list; list = list->next) {
+ /*
+ * If the destination list has commits with an earlier date than
+ * our source or if it still contains interesting commits we
+ * want to continue looking.
+ */
+ if (!list->item->uninteresting || list->item->time > time)
+ return SLOP;
+ }
/* Everything's uninteresting, reduce the count */
return slop - 1;
diff --git a/tests/resources/revwalk.git/HEAD b/tests/resources/revwalk.git/HEAD
new file mode 100644
index 0000000..cb089cd
--- /dev/null
+++ b/tests/resources/revwalk.git/HEAD
@@ -0,0 +1 @@
+ref: refs/heads/master
diff --git a/tests/resources/revwalk.git/config b/tests/resources/revwalk.git/config
new file mode 100644
index 0000000..7c968c3
--- /dev/null
+++ b/tests/resources/revwalk.git/config
@@ -0,0 +1,6 @@
+[core]
+ bare = true
+ repositoryformatversion = 0
+ filemode = false
+ symlinks = false
+ ignorecase = true
diff --git a/tests/resources/revwalk.git/description b/tests/resources/revwalk.git/description
new file mode 100644
index 0000000..498b267
--- /dev/null
+++ b/tests/resources/revwalk.git/description
@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.
diff --git a/tests/resources/revwalk.git/objects/info/packs b/tests/resources/revwalk.git/objects/info/packs
new file mode 100644
index 0000000..d8d85b8
--- /dev/null
+++ b/tests/resources/revwalk.git/objects/info/packs
@@ -0,0 +1,2 @@
+P pack-9adacb9971981a1a3264fd473da5b800f2715959.pack
+
diff --git a/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx
new file mode 100644
index 0000000..e157b38
Binary files /dev/null and b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.idx differ
diff --git a/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack
new file mode 100644
index 0000000..2a61f94
Binary files /dev/null and b/tests/resources/revwalk.git/objects/pack/pack-9adacb9971981a1a3264fd473da5b800f2715959.pack differ
diff --git a/tests/resources/revwalk.git/packed-refs b/tests/resources/revwalk.git/packed-refs
new file mode 100644
index 0000000..905a3db
--- /dev/null
+++ b/tests/resources/revwalk.git/packed-refs
@@ -0,0 +1,7 @@
+# pack-refs with: peeled fully-peeled sorted
+3ae0f53011bdb7e68f99bde4943449f36c1c318a refs/heads/A
+061978578d7c9ff2ba92dd36d31fd8d809871030 refs/heads/B
+743398b425d6c216d6cfaae3786b5bc436393ae5 refs/heads/C
+790ba0facf6fd103699a5c40cd19dad277ff49cd refs/heads/D
+d3d783066cf7d95def6844b9c5118c1e7bcce7df refs/heads/E
+d3d783066cf7d95def6844b9c5118c1e7bcce7df refs/heads/master
diff --git a/tests/resources/revwalk.git/refs/.gitkeep b/tests/resources/revwalk.git/refs/.gitkeep
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/resources/revwalk.git/refs/.gitkeep
diff --git a/tests/revwalk/basic.c b/tests/revwalk/basic.c
index 6a23701..1106bf4 100644
--- a/tests/revwalk/basic.c
+++ b/tests/revwalk/basic.c
@@ -555,3 +555,30 @@ void test_revwalk_basic__old_hidden_commit_two(void)
cl_git_fail_with(GIT_ITEROVER, git_revwalk_next(&oid, _walk));
}
+
+/*
+ * Ensure that we correctly hide all parent commits of a newer
+ * commit when first hiding older commits.
+ *
+ * % git rev-list D ^B ^A ^E
+ * 790ba0facf6fd103699a5c40cd19dad277ff49cd
+ * b82cee5004151ae0c4f82b69fb71b87477664b6f
+ */
+void test_revwalk_basic__newer_hidden_commit_hides_old_commits(void)
+{
+ git_oid oid;
+
+ revwalk_basic_setup_walk("revwalk.git");
+
+ cl_git_pass(git_revwalk_push_ref(_walk, "refs/heads/D"));
+ cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/B"));
+ cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/A"));
+ cl_git_pass(git_revwalk_hide_ref(_walk, "refs/heads/E"));
+
+ cl_git_pass(git_revwalk_next(&oid, _walk));
+ cl_assert(git_oid_streq(&oid, "b82cee5004151ae0c4f82b69fb71b87477664b6f"));
+ cl_git_pass(git_revwalk_next(&oid, _walk));
+ cl_assert(git_oid_streq(&oid, "790ba0facf6fd103699a5c40cd19dad277ff49cd"));
+
+ cl_git_fail_with(GIT_ITEROVER, git_revwalk_next(&oid, _walk));
+}