name-rev: use generation numbers if available
If a commit in a sequence of linear history has a non-monotonically increasing commit timestamp, git name-rev might not properly name the commit. This occurs because name-rev uses a heuristic of the commit date to avoid searching down tags which lead to commits that are older than the named commit. This is intended to avoid work on larger repositories. This heuristic impacts git name-rev, and by extension git describe --contains which is built on top of name-rev. Further more, if --all or --annotate-stdin is used, the heuristic is not enabled because the full history has to be analyzed anyways. This results in some confusion if a user sees that --annotate-stdin works but a normal name-rev does not. If the repository has a commit graph, we can use the generation numbers instead of using the commit dates. This is essentially the same check except that generation numbers make it exact, where the commit date heuristic could be incorrect due to clock errors. Since we're extending the notion of cutoff to more than one variable, create a series of functions for setting and checking the cutoff. This avoids duplication and moves access of the global cutoff and generation_cutoff to as few functions as possible. Add several test cases including a test that covers the new commitGraph behavior, as well as tests for --all and --annotate-stdin with and without commitGraphs. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jacob Keller committed
Mar 11, 2022 at 16:00 UTC
2e8ea40fe3b9befc1420aed6defe8b48f302ca7b
2 files changed
+175
-14
builtin/name-rev.c
+57
-14
@@ -9,6 +9,7 @@
9
#include "prio-queue.h"
10
#include "hash-lookup.h"
11
#include "commit-slab.h"
12
+#include "commit-graph.h"
13
14
/*
15
* One day. See the 'name a rev shortly after epoch' test in t6120 when
@@ -26,9 +27,58 @@ struct rev_name {
27
28
define_commit_slab(commit_rev_name, struct rev_name);
29
30
+static timestamp_t generation_cutoff = GENERATION_NUMBER_INFINITY;
31
static timestamp_t cutoff = TIME_MAX;
32
static struct commit_rev_name rev_names;
33
34
+/* Disable the cutoff checks entirely */
35
+static void disable_cutoff(void)
36
+{
37
+ generation_cutoff = 0;
38
+ cutoff = 0;
39
+}
40
+
41
+/* Cutoff searching any commits older than this one */
42
+static void set_commit_cutoff(struct commit *commit)
43
+{
44
+
45
+ if (cutoff > commit->date)
46
+ cutoff = commit->date;
47
+
48
+ if (generation_cutoff) {
49
+ timestamp_t generation = commit_graph_generation(commit);
50
+
51
+ if (generation_cutoff > generation)
52
+ generation_cutoff = generation;
53
+ }
54
+}
55
+
56
+/* adjust the commit date cutoff with a slop to allow for slightly incorrect
57
+ * commit timestamps in case of clock skew.
58
+ */
59
+static void adjust_cutoff_timestamp_for_slop(void)
60
+{
61
+ if (cutoff) {
62
+ /* check for undeflow */
63
+ if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
64
+ cutoff = cutoff - CUTOFF_DATE_SLOP;
65
+ else
66
+ cutoff = TIME_MIN;
67
+ }
68
+}
69
+
70
+/* Check if a commit is before the cutoff. Prioritize generation numbers
71
+ * first, but use the commit timestamp if we lack generation data.
72
+ */
73
+static int commit_is_before_cutoff(struct commit *commit)
74
+{
75
+ if (generation_cutoff < GENERATION_NUMBER_INFINITY)
76
+ return generation_cutoff &&
77
+ commit_graph_generation(commit) < generation_cutoff;
78
+
79
+ return commit->date < cutoff;
80
+}
81
+
82
/* How many generations are maximally preferred over _one_ merge traversal? */
83
#define MERGE_TRAVERSAL_WEIGHT 65535
84
@@ -151,7 +201,7 @@ static void name_rev(struct commit *start_commit,
201
struct rev_name *start_name;
202
203
parse_commit(start_commit);
154
- if (start_commit->date < cutoff)
204
+ if (commit_is_before_cutoff(start_commit))
205
return;
206
207
start_name = create_or_update_name(start_commit, taggerdate, 0, 0,
@@ -181,7 +231,7 @@ static void name_rev(struct commit *start_commit,
231
int generation, distance;
232
233
parse_commit(parent);
184
- if (parent->date < cutoff)
234
+ if (commit_is_before_cutoff(parent))
235
continue;
236
237
if (parent_number > 1) {
@@ -568,7 +618,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
618
usage_with_options(name_rev_usage, opts);
619
}
620
if (all || annotate_stdin)
571
- cutoff = 0;
621
+ disable_cutoff();
622
623
for (; argc; argc--, argv++) {
624
struct object_id oid;
@@ -596,10 +646,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
646
continue;
647
}
648
599
- if (commit) {
600
- if (cutoff > commit->date)
601
- cutoff = commit->date;
602
- }
649
+ if (commit)
650
+ set_commit_cutoff(commit);
651
652
if (peel_tag) {
653
if (!commit) {
@@ -612,13 +660,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
660
add_object_array(object, *argv, &revs);
661
}
662
615
- if (cutoff) {
616
- /* check for undeflow */
617
- if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
618
- cutoff = cutoff - CUTOFF_DATE_SLOP;
619
- else
620
- cutoff = TIME_MIN;
621
- }
663
+ adjust_cutoff_timestamp_for_slop();
664
+
665
for_each_ref(name_ref, &data);
666
name_tips();
667
t/t6120-describe.sh
+118
@@ -488,6 +488,124 @@ test_expect_success 'name-rev covers all conditions while looking at parents' '
488
)
489
'
490
491
+# A-B-C-D-E-main
492
+#
493
+# Where C has a non-monotonically increasing commit timestamp w.r.t. other
494
+# commits
495
+test_expect_success 'non-monotonic commit dates setup' '
496
+ UNIX_EPOCH_ZERO="@0 +0000" &&
497
+ git init non-monotonic &&
498
+ test_commit -C non-monotonic A &&
499
+ test_commit -C non-monotonic --no-tag B &&
500
+ test_commit -C non-monotonic --no-tag --date "$UNIX_EPOCH_ZERO" C &&
501
+ test_commit -C non-monotonic D &&
502
+ test_commit -C non-monotonic E
503
+'
504
+
505
+test_expect_success 'name-rev with commitGraph handles non-monotonic timestamps' '
506
+ test_config -C non-monotonic core.commitGraph true &&
507
+ (
508
+ cd non-monotonic &&
509
+
510
+ git commit-graph write --reachable &&
511
+
512
+ echo "main~3 tags/D~2" >expect &&
513
+ git name-rev --tags main~3 >actual &&
514
+
515
+ test_cmp expect actual
516
+ )
517
+'
518
+
519
+test_expect_success 'name-rev --all works with non-monotonic timestamps' '
520
+ test_config -C non-monotonic core.commitGraph false &&
521
+ (
522
+ cd non-monotonic &&
523
+
524
+ rm -rf .git/info/commit-graph* &&
525
+
526
+ cat >tags <<-\EOF &&
527
+ tags/E
528
+ tags/D
529
+ tags/D~1
530
+ tags/D~2
531
+ tags/A
532
+ EOF
533
+
534
+ git log --pretty=%H >revs &&
535
+
536
+ paste -d" " revs tags | sort >expect &&
537
+
538
+ git name-rev --tags --all | sort >actual &&
539
+ test_cmp expect actual
540
+ )
541
+'
542
+
543
+test_expect_success 'name-rev --annotate-stdin works with non-monotonic timestamps' '
544
+ test_config -C non-monotonic core.commitGraph false &&
545
+ (
546
+ cd non-monotonic &&
547
+
548
+ rm -rf .git/info/commit-graph* &&
549
+
550
+ cat >expect <<-\EOF &&
551
+ E
552
+ D
553
+ D~1
554
+ D~2
555
+ A
556
+ EOF
557
+
558
+ git log --pretty=%H >revs &&
559
+ git name-rev --tags --annotate-stdin --name-only <revs >actual &&
560
+ test_cmp expect actual
561
+ )
562
+'
563
+
564
+test_expect_success 'name-rev --all works with commitGraph' '
565
+ test_config -C non-monotonic core.commitGraph true &&
566
+ (
567
+ cd non-monotonic &&
568
+
569
+ git commit-graph write --reachable &&
570
+
571
+ cat >tags <<-\EOF &&
572
+ tags/E
573
+ tags/D
574
+ tags/D~1
575
+ tags/D~2
576
+ tags/A
577
+ EOF
578
+
579
+ git log --pretty=%H >revs &&
580
+
581
+ paste -d" " revs tags | sort >expect &&
582
+
583
+ git name-rev --tags --all | sort >actual &&
584
+ test_cmp expect actual
585
+ )
586
+'
587
+
588
+test_expect_success 'name-rev --annotate-stdin works with commitGraph' '
589
+ test_config -C non-monotonic core.commitGraph true &&
590
+ (
591
+ cd non-monotonic &&
592
+
593
+ git commit-graph write --reachable &&
594
+
595
+ cat >expect <<-\EOF &&
596
+ E
597
+ D
598
+ D~1
599
+ D~2
600
+ A
601
+ EOF
602
+
603
+ git log --pretty=%H >revs &&
604
+ git name-rev --tags --annotate-stdin --name-only <revs >actual &&
605
+ test_cmp expect actual
606
+ )
607
+'
608
+
609
# B
610
# o
611
# \