Skip to content

Commit

Permalink
name-rev: use generation numbers if available
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jacob-keller authored and gitster committed Mar 13, 2022
1 parent 715d08a commit 2e8ea40
Show file tree
Hide file tree
Showing 2 changed files with 175 additions and 14 deletions.
71 changes: 57 additions & 14 deletions builtin/name-rev.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "prio-queue.h"
#include "hash-lookup.h"
#include "commit-slab.h"
#include "commit-graph.h"

/*
* One day. See the 'name a rev shortly after epoch' test in t6120 when
Expand All @@ -26,9 +27,58 @@ struct rev_name {

define_commit_slab(commit_rev_name, struct rev_name);

static timestamp_t generation_cutoff = GENERATION_NUMBER_INFINITY;
static timestamp_t cutoff = TIME_MAX;
static struct commit_rev_name rev_names;

/* Disable the cutoff checks entirely */
static void disable_cutoff(void)
{
generation_cutoff = 0;
cutoff = 0;
}

/* Cutoff searching any commits older than this one */
static void set_commit_cutoff(struct commit *commit)
{

if (cutoff > commit->date)
cutoff = commit->date;

if (generation_cutoff) {
timestamp_t generation = commit_graph_generation(commit);

if (generation_cutoff > generation)
generation_cutoff = generation;
}
}

/* adjust the commit date cutoff with a slop to allow for slightly incorrect
* commit timestamps in case of clock skew.
*/
static void adjust_cutoff_timestamp_for_slop(void)
{
if (cutoff) {
/* check for undeflow */
if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
cutoff = cutoff - CUTOFF_DATE_SLOP;
else
cutoff = TIME_MIN;
}
}

/* Check if a commit is before the cutoff. Prioritize generation numbers
* first, but use the commit timestamp if we lack generation data.
*/
static int commit_is_before_cutoff(struct commit *commit)
{
if (generation_cutoff < GENERATION_NUMBER_INFINITY)
return generation_cutoff &&
commit_graph_generation(commit) < generation_cutoff;

return commit->date < cutoff;
}

/* How many generations are maximally preferred over _one_ merge traversal? */
#define MERGE_TRAVERSAL_WEIGHT 65535

Expand Down Expand Up @@ -151,7 +201,7 @@ static void name_rev(struct commit *start_commit,
struct rev_name *start_name;

parse_commit(start_commit);
if (start_commit->date < cutoff)
if (commit_is_before_cutoff(start_commit))
return;

start_name = create_or_update_name(start_commit, taggerdate, 0, 0,
Expand Down Expand Up @@ -181,7 +231,7 @@ static void name_rev(struct commit *start_commit,
int generation, distance;

parse_commit(parent);
if (parent->date < cutoff)
if (commit_is_before_cutoff(parent))
continue;

if (parent_number > 1) {
Expand Down Expand Up @@ -568,7 +618,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
usage_with_options(name_rev_usage, opts);
}
if (all || annotate_stdin)
cutoff = 0;
disable_cutoff();

for (; argc; argc--, argv++) {
struct object_id oid;
Expand Down Expand Up @@ -596,10 +646,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
continue;
}

if (commit) {
if (cutoff > commit->date)
cutoff = commit->date;
}
if (commit)
set_commit_cutoff(commit);

if (peel_tag) {
if (!commit) {
Expand All @@ -612,13 +660,8 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
add_object_array(object, *argv, &revs);
}

if (cutoff) {
/* check for undeflow */
if (cutoff > TIME_MIN + CUTOFF_DATE_SLOP)
cutoff = cutoff - CUTOFF_DATE_SLOP;
else
cutoff = TIME_MIN;
}
adjust_cutoff_timestamp_for_slop();

for_each_ref(name_ref, &data);
name_tips();

Expand Down
118 changes: 118 additions & 0 deletions t/t6120-describe.sh
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,124 @@ test_expect_success 'name-rev covers all conditions while looking at parents' '
)
'

# A-B-C-D-E-main
#
# Where C has a non-monotonically increasing commit timestamp w.r.t. other
# commits
test_expect_success 'non-monotonic commit dates setup' '
UNIX_EPOCH_ZERO="@0 +0000" &&
git init non-monotonic &&
test_commit -C non-monotonic A &&
test_commit -C non-monotonic --no-tag B &&
test_commit -C non-monotonic --no-tag --date "$UNIX_EPOCH_ZERO" C &&
test_commit -C non-monotonic D &&
test_commit -C non-monotonic E
'

test_expect_success 'name-rev with commitGraph handles non-monotonic timestamps' '
test_config -C non-monotonic core.commitGraph true &&
(
cd non-monotonic &&
git commit-graph write --reachable &&
echo "main~3 tags/D~2" >expect &&
git name-rev --tags main~3 >actual &&
test_cmp expect actual
)
'

test_expect_success 'name-rev --all works with non-monotonic timestamps' '
test_config -C non-monotonic core.commitGraph false &&
(
cd non-monotonic &&
rm -rf .git/info/commit-graph* &&
cat >tags <<-\EOF &&
tags/E
tags/D
tags/D~1
tags/D~2
tags/A
EOF
git log --pretty=%H >revs &&
paste -d" " revs tags | sort >expect &&
git name-rev --tags --all | sort >actual &&
test_cmp expect actual
)
'

test_expect_success 'name-rev --annotate-stdin works with non-monotonic timestamps' '
test_config -C non-monotonic core.commitGraph false &&
(
cd non-monotonic &&
rm -rf .git/info/commit-graph* &&
cat >expect <<-\EOF &&
E
D
D~1
D~2
A
EOF
git log --pretty=%H >revs &&
git name-rev --tags --annotate-stdin --name-only <revs >actual &&
test_cmp expect actual
)
'

test_expect_success 'name-rev --all works with commitGraph' '
test_config -C non-monotonic core.commitGraph true &&
(
cd non-monotonic &&
git commit-graph write --reachable &&
cat >tags <<-\EOF &&
tags/E
tags/D
tags/D~1
tags/D~2
tags/A
EOF
git log --pretty=%H >revs &&
paste -d" " revs tags | sort >expect &&
git name-rev --tags --all | sort >actual &&
test_cmp expect actual
)
'

test_expect_success 'name-rev --annotate-stdin works with commitGraph' '
test_config -C non-monotonic core.commitGraph true &&
(
cd non-monotonic &&
git commit-graph write --reachable &&
cat >expect <<-\EOF &&
E
D
D~1
D~2
A
EOF
git log --pretty=%H >revs &&
git name-rev --tags --annotate-stdin --name-only <revs >actual &&
test_cmp expect actual
)
'

# B
# o
# \
Expand Down

0 comments on commit 2e8ea40

Please sign in to comment.