Skip to content

Commit 37b9dca

Browse files
ttaylorrgitster
authored andcommitted
shallow.c: use '{commit,rollback}_shallow_file'
In bd0b42a (fetch-pack: do not take shallow lock unnecessarily, 2019-01-10), the author noted that 'is_repository_shallow' produces visible side-effect(s) by setting 'is_shallow' and 'shallow_stat'. This is a problem for e.g., fetching with '--update-shallow' in a shallow repository with 'fetch.writeCommitGraph' enabled, since the update to '.git/shallow' will cause Git to think that the repository isn't shallow when it is, thereby circumventing the commit-graph compatibility check. This causes problems in shallow repositories with at least shallow refs that have at least one ancestor (since the client won't have those objects, and therefore can't take the reachability closure over commits when writing a commit-graph). Address this by introducing thin wrappers over 'commit_lock_file' and 'rollback_lock_file' for use specifically when the lock is held over '.git/shallow'. These wrappers (appropriately called 'commit_shallow_file' and 'rollback_shallow_file') call into their respective functions in 'lockfile.h', but additionally reset validity checks used by the shallow machinery. Replace each instance of 'commit_lock_file' and 'rollback_lock_file' with 'commit_shallow_file' and 'rollback_shallow_file' when the lock being held is over the '.git/shallow' file. As a result, 'prune_shallow' can now only be called once (since 'check_shallow_file_for_update' will die after calling 'reset_repository_shallow'). But, this is OK since we only call 'prune_shallow' at most once per process. Helped-by: Jonathan Tan <[email protected]> Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Taylor Blau <[email protected]> Reviewed-by: Jonathan Tan <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8a8da49 commit 37b9dca

File tree

5 files changed

+59
-16
lines changed

5 files changed

+59
-16
lines changed

builtin/receive-pack.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -872,12 +872,12 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
872872
opt.env = tmp_objdir_env(tmp_objdir);
873873
setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
874874
if (check_connected(command_singleton_iterator, cmd, &opt)) {
875-
rollback_lock_file(&shallow_lock);
875+
rollback_shallow_file(the_repository, &shallow_lock);
876876
oid_array_clear(&extra);
877877
return -1;
878878
}
879879

880-
commit_lock_file(&shallow_lock);
880+
commit_shallow_file(the_repository, &shallow_lock);
881881

882882
/*
883883
* Make sure setup_alternate_shallow() for the next ref does

commit.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,8 @@ struct oid_array;
249249
struct ref;
250250
int register_shallow(struct repository *r, const struct object_id *oid);
251251
int unregister_shallow(const struct object_id *oid);
252+
int commit_shallow_file(struct repository *r, struct lock_file *lk);
253+
void rollback_shallow_file(struct repository *r, struct lock_file *lk);
252254
int for_each_commit_graft(each_commit_graft_fn, void *);
253255
int is_repository_shallow(struct repository *r);
254256
struct commit_list *get_shallow_commits(struct object_array *heads,

fetch-pack.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,9 +1629,9 @@ static void update_shallow(struct fetch_pack_args *args,
16291629
if (args->deepen && alternate_shallow_file) {
16301630
if (*alternate_shallow_file == '\0') { /* --unshallow */
16311631
unlink_or_warn(git_path_shallow(the_repository));
1632-
rollback_lock_file(&shallow_lock);
1632+
rollback_shallow_file(the_repository, &shallow_lock);
16331633
} else
1634-
commit_lock_file(&shallow_lock);
1634+
commit_shallow_file(the_repository, &shallow_lock);
16351635
alternate_shallow_file = NULL;
16361636
return;
16371637
}
@@ -1655,7 +1655,7 @@ static void update_shallow(struct fetch_pack_args *args,
16551655
setup_alternate_shallow(&shallow_lock,
16561656
&alternate_shallow_file,
16571657
&extra);
1658-
commit_lock_file(&shallow_lock);
1658+
commit_shallow_file(the_repository, &shallow_lock);
16591659
alternate_shallow_file = NULL;
16601660
}
16611661
oid_array_clear(&extra);
@@ -1693,7 +1693,7 @@ static void update_shallow(struct fetch_pack_args *args,
16931693
setup_alternate_shallow(&shallow_lock,
16941694
&alternate_shallow_file,
16951695
&extra);
1696-
commit_lock_file(&shallow_lock);
1696+
commit_shallow_file(the_repository, &shallow_lock);
16971697
oid_array_clear(&extra);
16981698
oid_array_clear(&ref);
16991699
alternate_shallow_file = NULL;
@@ -1785,7 +1785,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
17851785
error(_("remote did not send all necessary objects"));
17861786
free_refs(ref_cpy);
17871787
ref_cpy = NULL;
1788-
rollback_lock_file(&shallow_lock);
1788+
rollback_shallow_file(the_repository, &shallow_lock);
17891789
goto cleanup;
17901790
}
17911791
args->connectivity_checked = 1;

shallow.c

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,6 @@ int register_shallow(struct repository *r, const struct object_id *oid)
4040

4141
int is_repository_shallow(struct repository *r)
4242
{
43-
/*
44-
* NEEDSWORK: This function updates
45-
* r->parsed_objects->{is_shallow,shallow_stat} as a side effect but
46-
* there is no corresponding function to clear them when the shallow
47-
* file is updated.
48-
*/
49-
5043
FILE *fp;
5144
char buf[1024];
5245
const char *path = r->parsed_objects->alternate_shallow_file;
@@ -79,6 +72,25 @@ int is_repository_shallow(struct repository *r)
7972
return r->parsed_objects->is_shallow;
8073
}
8174

75+
static void reset_repository_shallow(struct repository *r)
76+
{
77+
r->parsed_objects->is_shallow = -1;
78+
stat_validity_clear(r->parsed_objects->shallow_stat);
79+
}
80+
81+
int commit_shallow_file(struct repository *r, struct lock_file *lk)
82+
{
83+
int res = commit_lock_file(lk);
84+
reset_repository_shallow(r);
85+
return res;
86+
}
87+
88+
void rollback_shallow_file(struct repository *r, struct lock_file *lk)
89+
{
90+
rollback_lock_file(lk);
91+
reset_repository_shallow(r);
92+
}
93+
8294
/*
8395
* TODO: use "int" elemtype instead of "int *" when/if commit-slab
8496
* supports a "valid" flag.
@@ -410,10 +422,10 @@ void prune_shallow(unsigned options)
410422
if (write_in_full(fd, sb.buf, sb.len) < 0)
411423
die_errno("failed to write to %s",
412424
get_lock_file_path(&shallow_lock));
413-
commit_lock_file(&shallow_lock);
425+
commit_shallow_file(the_repository, &shallow_lock);
414426
} else {
415427
unlink(git_path_shallow(the_repository));
416-
rollback_lock_file(&shallow_lock);
428+
rollback_shallow_file(the_repository, &shallow_lock);
417429
}
418430
strbuf_release(&sb);
419431
}

t/t5537-fetch-shallow.sh

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,35 @@ test_expect_success 'fetch --update-shallow' '
144144
)
145145
'
146146

147+
test_expect_success 'fetch --update-shallow (with fetch.writeCommitGraph)' '
148+
(
149+
cd shallow &&
150+
git checkout master &&
151+
commit 8 &&
152+
git tag -m foo heavy-tag-for-graph HEAD^ &&
153+
git tag light-tag-for-graph HEAD^:tracked
154+
) &&
155+
test_config -C notshallow fetch.writeCommitGraph true &&
156+
(
157+
cd notshallow &&
158+
git fetch --update-shallow ../shallow/.git refs/heads/*:refs/remotes/shallow/* &&
159+
git fsck &&
160+
git for-each-ref --sort=refname --format="%(refname)" >actual.refs &&
161+
cat <<-EOF >expect.refs &&
162+
refs/remotes/shallow/master
163+
refs/remotes/shallow/no-shallow
164+
refs/tags/heavy-tag
165+
refs/tags/heavy-tag-for-graph
166+
refs/tags/light-tag
167+
refs/tags/light-tag-for-graph
168+
EOF
169+
test_cmp expect.refs actual.refs &&
170+
git log --format=%s shallow/master >actual &&
171+
test_write_lines 8 7 6 5 4 3 >expect &&
172+
test_cmp expect actual
173+
)
174+
'
175+
147176
test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
148177
cp -R .git read-only.git &&
149178
test_when_finished "find read-only.git -type d -print | xargs chmod +w" &&

0 commit comments

Comments
 (0)