commit-reach: properly peel tags
The can_all_from_reach_with_flag() algorithm was refactored in 4fbcca4e "commit-reach: make can_all_from_reach... linear" but incorrectly assumed that all objects provided were commits. During a fetch negotiation, ok_to_give_up() in upload-pack.c may provide unpeeled tags to the 'from' array. The current code creates a segfault. Add a direct call to can_all_from_reach_with_flag() in 'test-tool reach' and add a test in t6600-test-reach.sh that demonstrates this segfault. Correct the issue by peeling tags when investigating the initial list of objects in the 'from' array. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Derrick Stolee committed
Sep 21, 2018 at 08:05 UTC
b67f6b26e354813d8cd792dcb3eb1a1b60b98a08
3 files changed
+74
-14
commit-reach.c
+29
-7
@@ -544,20 +544,42 @@ int can_all_from_reach_with_flag(struct object_array *from,
544
{
545
struct commit **list = NULL;
546
int i;
547
+ int nr_commits;
548
int result = 1;
549
550
ALLOC_ARRAY(list, from->nr);
551
+ nr_commits = 0;
552
for (i = 0; i < from->nr; i++) {
551
- list[i] = (struct commit *)from->objects[i].item;
553
+ struct object *from_one = from->objects[i].item;
554
553
- if (parse_commit(list[i]) ||
554
- list[i]->generation < min_generation)
555
- return 0;
555
+ if (!from_one || from_one->flags & assign_flag)
556
+ continue;
557
+
558
+ from_one = deref_tag(the_repository, from_one,
559
+ "a from object", 0);
560
+ if (!from_one || from_one->type != OBJ_COMMIT) {
561
+ /* no way to tell if this is reachable by
562
+ * looking at the ancestry chain alone, so
563
+ * leave a note to ourselves not to worry about
564
+ * this object anymore.
565
+ */
566
+ from->objects[i].item->flags |= assign_flag;
567
+ continue;
568
+ }
569
+
570
+ list[nr_commits] = (struct commit *)from_one;
571
+ if (parse_commit(list[nr_commits]) ||
572
+ list[nr_commits]->generation < min_generation) {
573
+ result = 0;
574
+ goto cleanup;
575
+ }
576
+
577
+ nr_commits++;
578
}
579
558
- QSORT(list, from->nr, compare_commits_by_gen);
580
+ QSORT(list, nr_commits, compare_commits_by_gen);
581
560
- for (i = 0; i < from->nr; i++) {
582
+ for (i = 0; i < nr_commits; i++) {
583
/* DFS from list[i] */
584
struct commit_list *stack = NULL;
585
@@ -600,7 +622,7 @@ int can_all_from_reach_with_flag(struct object_array *from,
622
}
623
624
cleanup:
603
- for (i = 0; i < from->nr; i++) {
625
+ for (i = 0; i < nr_commits; i++) {
626
clear_commit_marks(list[i], RESULT);
627
clear_commit_marks(list[i], assign_flag);
628
}
t/helper/test-reach.c
+17
-5
@@ -31,6 +31,7 @@ int cmd__reach(int ac, const char **av)
31
struct object_id oid_A, oid_B;
32
struct commit *A, *B;
33
struct commit_list *X, *Y;
34
+ struct object_array X_obj = OBJECT_ARRAY_INIT;
35
struct commit **X_array;
36
int X_nr, X_alloc;
37
struct strbuf buf = STRBUF_INIT;
@@ -49,7 +50,8 @@ int cmd__reach(int ac, const char **av)
50
51
while (strbuf_getline(&buf, stdin) != EOF) {
52
struct object_id oid;
52
- struct object *o;
53
+ struct object *orig;
54
+ struct object *peeled;
55
struct commit *c;
56
if (buf.len < 3)
57
continue;
@@ -57,14 +59,14 @@ int cmd__reach(int ac, const char **av)
59
if (get_oid_committish(buf.buf + 2, &oid))
60
die("failed to resolve %s", buf.buf + 2);
61
60
- o = parse_object(r, &oid);
61
- o = deref_tag_noverify(o);
62
+ orig = parse_object(r, &oid);
63
+ peeled = deref_tag_noverify(orig);
64
63
- if (!o)
65
+ if (!peeled)
66
die("failed to load commit for input %s resulting in oid %s\n",
67
buf.buf, oid_to_hex(&oid));
68
67
- c = object_as_type(r, o, OBJ_COMMIT, 0);
69
+ c = object_as_type(r, peeled, OBJ_COMMIT, 0);
70
71
if (!c)
72
die("failed to load commit for input %s resulting in oid %s\n",
@@ -85,6 +87,7 @@ int cmd__reach(int ac, const char **av)
87
commit_list_insert(c, &X);
88
ALLOC_GROW(X_array, X_nr + 1, X_alloc);
89
X_array[X_nr++] = c;
90
+ add_object_array(orig, NULL, &X_obj);
91
break;
92
93
case 'Y':
@@ -113,6 +116,15 @@ int cmd__reach(int ac, const char **av)
116
print_sorted_commit_ids(list);
117
} else if (!strcmp(av[1], "can_all_from_reach")) {
118
printf("%s(X,Y):%d\n", av[1], can_all_from_reach(X, Y, 1));
119
+ } else if (!strcmp(av[1], "can_all_from_reach_with_flag")) {
120
+ struct commit_list *iter = Y;
121
+
122
+ while (iter) {
123
+ iter->item->object.flags |= 2;
124
+ iter = iter->next;
125
+ }
126
+
127
+ printf("%s(X,_,_,0,0):%d\n", av[1], can_all_from_reach_with_flag(&X_obj, 2, 4, 0, 0));
128
} else if (!strcmp(av[1], "commit_contains")) {
129
struct ref_filter filter;
130
struct contains_cache cache;
t/t6600-test-reach.sh
+28
-2
@@ -31,7 +31,8 @@ test_expect_success 'setup' '
31
for i in $(test_seq 1 10)
32
do
33
test_commit "1-$i" &&
34
- git branch -f commit-1-$i
34
+ git branch -f commit-1-$i &&
35
+ git tag -a -m "1-$i" tag-1-$i commit-1-$i
36
done &&
37
for j in $(test_seq 1 9)
38
do
@@ -39,11 +40,13 @@ test_expect_success 'setup' '
40
x=$(($j + 1)) &&
41
test_commit "$x-1" &&
42
git branch -f commit-$x-1 &&
43
+ git tag -a -m "$x-1" tag-$x-1 commit-$x-1 &&
44
45
for i in $(test_seq 2 10)
46
do
47
git merge commit-$j-$i -m "$x-$i" &&
46
- git branch -f commit-$x-$i
48
+ git branch -f commit-$x-$i &&
49
+ git tag -a -m "$x-$i" tag-$x-$i commit-$x-$i
50
done
51
done &&
52
git commit-graph write --reachable &&
@@ -205,6 +208,29 @@ test_expect_success 'can_all_from_reach:miss' '
208
test_three_modes can_all_from_reach
209
'
210
211
+test_expect_success 'can_all_from_reach_with_flag: tags case' '
212
+ cat >input <<-\EOF &&
213
+ X:tag-2-10
214
+ X:tag-3-9
215
+ X:tag-4-8
216
+ X:commit-5-7
217
+ X:commit-6-6
218
+ X:commit-7-5
219
+ X:commit-8-4
220
+ X:commit-9-3
221
+ Y:tag-1-9
222
+ Y:tag-2-8
223
+ Y:tag-3-7
224
+ Y:commit-4-6
225
+ Y:commit-5-5
226
+ Y:commit-6-4
227
+ Y:commit-7-3
228
+ Y:commit-8-1
229
+ EOF
230
+ echo "can_all_from_reach_with_flag(X,_,_,0,0):1" >expect &&
231
+ test_three_modes can_all_from_reach_with_flag
232
+'
233
+
234
test_expect_success 'commit_contains:hit' '
235
cat >input <<-\EOF &&
236
A:commit-7-7