shallow: use stat_validity to check for up-to-date file

When we are about to write the shallow file, we check that it has not changed since we last read it. Instead of hand-rolling this, we can use stat_validity. This is built around the index stat-check, so it is more robust than just checking the mtime, as we do now (it uses the same check as we do for index files). The new code also handles the case of a shallow file appearing unexpectedly. With the current code, two simultaneous processes making us shallow (e.g., two "git fetch --depth=1" running at the same time in a non-shallow repository) can race to overwrite each other. As a bonus, we also remove a race in determining the stat information of what we read (we stat and then open, leaving a race window; instead we should open and then fstat the descriptor). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 27, 2014 at 05:56 UTC 0cc77c386cea7afebb54a5e7263ca37569ecfe7a
1 file changed +7 -17
shallow.c
+7 -17
@@ -10,7 +10,7 @@
10 #include "commit-slab.h"
11
12 static int is_shallow = -1;
13 -static struct stat shallow_stat;
13 +static struct stat_validity shallow_stat;
14 static char *alternate_shallow_file;
15
16 void set_alternate_shallow_file(const char *path, int override)
@@ -52,12 +52,12 @@ int is_repository_shallow(void)
52 * shallow file should be used. We could just open it and it
53 * will likely fail. But let's do an explicit check instead.
54 */
55 - if (!*path ||
56 - stat(path, &shallow_stat) ||
57 - (fp = fopen(path, "r")) == NULL) {
55 + if (!*path || (fp = fopen(path, "r")) == NULL) {
56 + stat_validity_clear(&shallow_stat);
57 is_shallow = 0;
58 return is_shallow;
59 }
60 + stat_validity_update(&shallow_stat, fileno(fp));
61 is_shallow = 1;
62
63 while (fgets(buf, sizeof(buf), fp)) {
@@ -137,21 +137,11 @@ struct commit_list *get_shallow_commits(struct object_array *heads, int depth,
137
138 void check_shallow_file_for_update(void)
139 {
140 - struct stat st;
141 -
142 - if (!is_shallow)
143 - return;
144 - else if (is_shallow == -1)
140 + if (is_shallow == -1)
141 die("BUG: shallow must be initialized by now");
142
147 - if (stat(git_path("shallow"), &st))
148 - die("shallow file was removed during fetch");
149 - else if (st.st_mtime != shallow_stat.st_mtime
150 -#ifdef USE_NSEC
151 - || ST_MTIME_NSEC(st) != ST_MTIME_NSEC(shallow_stat)
152 -#endif
153 - )
154 - die("shallow file was changed during fetch");
143 + if (!stat_validity_check(&shallow_stat, git_path("shallow")))
144 + die("shallow file has changed since we read it");
145 }
146
147 #define SEEN_ONLY 1