commit,shallow: unparse commits if grafts changed

When a commit is parsed, it pretends to have a different (possibly empty) list of parents if there is graft information for that commit. But there is a bug that could occur when a commit is parsed, the graft information is updated (for example, when a shallow file is rewritten), and the same commit is subsequently used: the parents of the commit do not conform to the updated graft information, but the information at the time of parsing. This is usually not an issue, as a commit is usually introduced into the repository at the same time as its graft information. That means that when we try to parse that commit, we already have its graft information. But it is an issue when fetching a shallow point directly into a repository with submodules. The function assign_shallow_commits_to_refs() parses all sought objects (including the shallow point, which we are directly fetching). In update_shallow() in fetch-pack.c, assign_shallow_commits_to_refs() is called before commit_shallow_file(), which means that the shallow point would have been parsed before graft information is updated. Once a commit is parsed, it is no longer sensitive to any graft information updates. This parsed commit is subsequently used when we do a revision walk to search for submodules to fetch, meaning that the commit is considered to have parents even though it is a shallow point (and therefore should be treated as having no parents). Therefore, whenever graft information is updated, mark the commits that were previously grafts and the commits that are newly grafts as unparsed. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jonathan Tan committed Jun 6, 2022 at 10:54 UTC 4d4e49fff17122e0eec185857fa87d526174859f
3 files changed +34 -1
commit.c
+15 -1
@@ -120,6 +120,17 @@ int commit_graft_pos(struct repository *r, const struct object_id *oid)
120 commit_graft_oid_access);
121 }
122
123 +static void unparse_commit(struct repository *r, const struct object_id *oid)
124 +{
125 + struct commit *c = lookup_commit(r, oid);
126 +
127 + if (!c->object.parsed)
128 + return;
129 + free_commit_list(c->parents);
130 + c->parents = NULL;
131 + c->object.parsed = 0;
132 +}
133 +
134 int register_commit_graft(struct repository *r, struct commit_graft *graft,
135 int ignore_dups)
136 {
@@ -145,6 +156,7 @@ int register_commit_graft(struct repository *r, struct commit_graft *graft,
156 (r->parsed_objects->grafts_nr - pos - 1) *
157 sizeof(*r->parsed_objects->grafts));
158 r->parsed_objects->grafts[pos] = graft;
159 + unparse_commit(r, &graft->oid);
160 return 0;
161 }
162
@@ -253,8 +265,10 @@ void reset_commit_grafts(struct repository *r)
265 {
266 int i;
267
256 - for (i = 0; i < r->parsed_objects->grafts_nr; i++)
268 + for (i = 0; i < r->parsed_objects->grafts_nr; i++) {
269 + unparse_commit(r, &r->parsed_objects->grafts[i]->oid);
270 free(r->parsed_objects->grafts[i]);
271 + }
272 r->parsed_objects->grafts_nr = 0;
273 r->parsed_objects->commit_graft_prepared = 0;
274 }
shallow.c
+7
@@ -97,6 +97,13 @@ int commit_shallow_file(struct repository *r, struct shallow_lock *lk)
97 {
98 int res = commit_lock_file(&lk->lock);
99 reset_repository_shallow(r);
100 +
101 + /*
102 + * Update in-memory data structures with the new shallow information,
103 + * including unparsing all commits that now have grafts.
104 + */
105 + is_repository_shallow(r);
106 +
107 return res;
108 }
109
t/t5537-fetch-shallow.sh
+12
@@ -164,12 +164,24 @@ test_expect_success 'fetch --update-shallow' '
164 test_expect_success 'fetch --update-shallow into a repo with submodules' '
165 git init a-submodule &&
166 test_commit -C a-submodule foo &&
167 +
168 + test_when_finished "rm -rf repo-with-sub" &&
169 git init repo-with-sub &&
170 git -C repo-with-sub submodule add ../a-submodule a-submodule &&
171 git -C repo-with-sub commit -m "added submodule" &&
172 git -C repo-with-sub fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/*
173 '
174
175 +test_expect_success 'fetch --update-shallow a commit that is also a shallow point into a repo with submodules' '
176 + test_when_finished "rm -rf repo-with-sub" &&
177 + git init repo-with-sub &&
178 + git -C repo-with-sub submodule add ../a-submodule a-submodule &&
179 + git -C repo-with-sub commit -m "added submodule" &&
180 +
181 + SHALLOW=$(cat shallow/.git/shallow) &&
182 + git -C repo-with-sub fetch --update-shallow ../shallow/.git "$SHALLOW":refs/heads/a-shallow
183 +'
184 +
185 test_expect_success 'fetch --update-shallow (with fetch.writeCommitGraph)' '
186 (
187 cd shallow &&