Skip to content

Commit 37e4bdd

Browse files
committedAug 3, 2022
Merge branch 'tb/commit-graph-genv2-upgrade-fix'
There was a bug in the codepath to upgrade generation information in commit-graph from v1 to v2 format, which has been corrected. * tb/commit-graph-genv2-upgrade-fix: commit-graph: fix corrupt upgrade from generation v1 to v2 commit-graph: introduce `repo_find_commit_pos_in_graph()` t5318: demonstrate commit-graph generation v2 corruption
2 parents f1a0db2 + 9550f6c commit 37e4bdd

File tree

4 files changed

+56
-8
lines changed

4 files changed

+56
-8
lines changed
 

‎bloom.c

+5-5
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,9 @@ static inline unsigned char get_bitmask(uint32_t pos)
3030

3131
static int load_bloom_filter_from_graph(struct commit_graph *g,
3232
struct bloom_filter *filter,
33-
struct commit *c)
33+
uint32_t graph_pos)
3434
{
3535
uint32_t lex_pos, start_index, end_index;
36-
uint32_t graph_pos = commit_graph_position(c);
3736

3837
while (graph_pos < g->num_commits_in_base)
3938
g = g->base_graph;
@@ -203,9 +202,10 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
203202
filter = bloom_filter_slab_at(&bloom_filters, c);
204203

205204
if (!filter->data) {
206-
load_commit_graph_info(r, c);
207-
if (commit_graph_position(c) != COMMIT_NOT_FROM_GRAPH)
208-
load_bloom_filter_from_graph(r->objects->commit_graph, filter, c);
205+
uint32_t graph_pos;
206+
if (repo_find_commit_pos_in_graph(r, c, &graph_pos))
207+
load_bloom_filter_from_graph(r->objects->commit_graph,
208+
filter, graph_pos);
209209
}
210210

211211
if (filter->data && filter->len)

‎commit-graph.c

+9-3
Original file line numberDiff line numberDiff line change
@@ -888,6 +888,14 @@ static int find_commit_pos_in_graph(struct commit *item, struct commit_graph *g,
888888
}
889889
}
890890

891+
int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,
892+
uint32_t *pos)
893+
{
894+
if (!prepare_commit_graph(r))
895+
return 0;
896+
return find_commit_pos_in_graph(c, r->objects->commit_graph, pos);
897+
}
898+
891899
struct commit *lookup_commit_in_graph(struct repository *repo, const struct object_id *id)
892900
{
893901
struct commit *commit;
@@ -945,9 +953,7 @@ int parse_commit_in_graph(struct repository *r, struct commit *item)
945953
void load_commit_graph_info(struct repository *r, struct commit *item)
946954
{
947955
uint32_t pos;
948-
if (!prepare_commit_graph(r))
949-
return;
950-
if (find_commit_pos_in_graph(item, r->objects->commit_graph, &pos))
956+
if (repo_find_commit_pos_in_graph(r, item, &pos))
951957
fill_commit_graph_info(item, r->objects->commit_graph, pos);
952958
}
953959

‎commit-graph.h

+15
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,21 @@ int open_commit_graph(const char *graph_file, int *fd, struct stat *st);
4040
*/
4141
int parse_commit_in_graph(struct repository *r, struct commit *item);
4242

43+
/*
44+
* Fills `*pos` with the graph position of `c`, and returns 1 if `c` is
45+
* found in the commit-graph belonging to `r`, or 0 otherwise.
46+
* Initializes the commit-graph belonging to `r` if it hasn't been
47+
* already.
48+
*
49+
* Note: this is a low-level helper that does not alter any slab data
50+
* associated with `c`. Useful in circumstances where the slab data is
51+
* already being modified (e.g., writing the commit-graph itself).
52+
*
53+
* In most cases, callers should use `parse_commit_in_graph()` instead.
54+
*/
55+
int repo_find_commit_pos_in_graph(struct repository *r, struct commit *c,
56+
uint32_t *pos);
57+
4358
/*
4459
* Look up the given commit ID in the commit-graph. This will only return a
4560
* commit if the ID exists both in the graph and in the object database such

‎t/t5318-commit-graph.sh

+27
Original file line numberDiff line numberDiff line change
@@ -812,4 +812,31 @@ test_expect_success 'set up and verify repo with generation data overflow chunk'
812812

813813
graph_git_behavior 'generation data overflow chunk repo' repo left right
814814

815+
test_expect_success 'overflow during generation version upgrade' '
816+
git init overflow-v2-upgrade &&
817+
(
818+
cd overflow-v2-upgrade &&
819+
820+
# This commit will have a date at two seconds past the Epoch,
821+
# and a (v1) generation number of 1, since it is a root commit.
822+
#
823+
# The offset will then be computed as 1-2, which will underflow
824+
# to 2^31, which is greater than the v2 offset small limit of
825+
# 2^31-1.
826+
#
827+
# This is sufficient to need a large offset table for the v2
828+
# generation numbers.
829+
test_commit --date "@2 +0000" base &&
830+
git repack -d &&
831+
832+
# Test that upgrading from generation v1 to v2 correctly
833+
# produces the overflow table.
834+
git -c commitGraph.generationVersion=1 commit-graph write &&
835+
git -c commitGraph.generationVersion=2 commit-graph write \
836+
--changed-paths &&
837+
838+
git rev-list --all
839+
)
840+
'
841+
815842
test_done

0 commit comments

Comments
 (0)
Please sign in to comment.