Skip to content

Commit 85518bf

Browse files
gitstergitster
committed
Merge branch 'lp/repack-propagate-promisor-debugging-info' into seen
When fetching objects into a lazily cloned repository, .promisor files are created with information meant to help debugging. "git repack" has been taught to carry this information forward to packfiles that are newly created. * lp/repack-propagate-promisor-debugging-info: SQUASH??? t7703: test for promisor file content after geometric repack t7700: test for promisor file content after repack repack-promisor: preserve content of promisor files after repack pack-write: add helper to fill promisor file after repack pack-write: add explanation to promisor file content
2 parents 1479a61 + bc3e998 commit 85518bf

File tree

5 files changed

+238
-15
lines changed

5 files changed

+238
-15
lines changed

‎Documentation/git-repack.adoc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ other objects in that pack they already have locally.
4545
+
4646
Promisor packfiles are repacked separately: if there are packfiles that
4747
have an associated ".promisor" file, these packfiles will be repacked
48-
into another separate pack, and an empty ".promisor" file corresponding
49-
to the new separate pack will be written.
48+
into another separate pack, and a ".promisor" file corresponding to the
49+
new separate pack will be written (with arbitrary contents).
5050

5151
-A::
5252
Same as `-a`, unless `-d` is used. Then any unreachable

‎pack-write.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ void write_promisor_file(const char *promisor_name, struct ref **sought, int nr_
603603
int i, err;
604604
FILE *output = xfopen(promisor_name, "w");
605605

606+
/*
607+
* Write in the .promisor file the ref names and associated hashes,
608+
* obtained by fetch-pack, at the point of generation of the
609+
* corresponding packfile. These pieces of info are only used to make
610+
* it easier to debug issues with partial clones, as we can identify
611+
* what refs (and their associated hashes) were fetched at the time
612+
* the packfile was downloaded, and if necessary, compare those hashes
613+
* against what the promisor remote reports now.
614+
*/
606615
for (i = 0; i < nr_sought; i++)
607616
fprintf(output, "%s %s\n", oid_to_hex(&sought[i]->old_oid),
608617
sought[i]->name);

‎repack-promisor.c‎

Lines changed: 134 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,128 @@ static int write_oid(const struct object_id *oid,
3434
return 0;
3535
}
3636

37+
/*
38+
* Go through all .promisor files contained in repo (excluding those whose name
39+
* appears in not_repacked_basenames, which acts as a ignorelist), and copies
40+
* their content inside the destination file "<packtmp>-<dest_hex>.promisor".
41+
* Each line of a never repacked .promisor file is: "<oid> <ref>" (as described
42+
* in the write_promisor_file() function).
43+
* After a repack, the copied lines will be: "<oid> <ref> <time>", where <time>
44+
* is the time (in Unix time) at which the .promisor file was last modified.
45+
* Only the lines whose <oid> is present inside "<packtmp>-<dest_hex>.idx" will
46+
* be copied.
47+
* The contents of all .promisor files are assumed to be correctly formed.
48+
*/
49+
static void copy_promisor_content(struct repository *repo,
50+
const char *dest_hex,
51+
const char *packtmp,
52+
struct strset *not_repacked_basenames)
53+
{
54+
char *dest_idx_name;
55+
char *dest_promisor_name;
56+
FILE *dest;
57+
struct strset dest_content = STRSET_INIT;
58+
struct strbuf dest_to_write = STRBUF_INIT;
59+
struct strbuf source_promisor_name = STRBUF_INIT;
60+
struct strbuf line = STRBUF_INIT;
61+
struct object_id dest_oid;
62+
struct packed_git *dest_pack, *p;
63+
int err;
64+
65+
dest_idx_name = mkpathdup("%s-%s.idx", packtmp, dest_hex);
66+
get_oid_hex_algop(dest_hex, &dest_oid, repo->hash_algo);
67+
dest_pack = parse_pack_index(repo, dest_oid.hash, dest_idx_name);
68+
69+
/* Open the .promisor dest file, and fill dest_content with its content */
70+
dest_promisor_name = mkpathdup("%s-%s.promisor", packtmp, dest_hex);
71+
dest = xfopen(dest_promisor_name, "r+");
72+
while (strbuf_getline(&line, dest) != EOF)
73+
strset_add(&dest_content, line.buf);
74+
75+
repo_for_each_pack(repo, p) {
76+
FILE *source;
77+
struct stat source_stat;
78+
79+
if (!p->pack_promisor)
80+
continue;
81+
82+
if (not_repacked_basenames &&
83+
strset_contains(not_repacked_basenames, pack_basename(p)))
84+
continue;
85+
86+
strbuf_reset(&source_promisor_name);
87+
strbuf_addstr(&source_promisor_name, p->pack_name);
88+
strbuf_strip_suffix(&source_promisor_name, ".pack");
89+
strbuf_addstr(&source_promisor_name, ".promisor");
90+
91+
if (stat(source_promisor_name.buf, &source_stat))
92+
die(_("File not found: %s"), source_promisor_name.buf);
93+
94+
source = xfopen(source_promisor_name.buf, "r");
95+
96+
while (strbuf_getline(&line, source) != EOF) {
97+
struct string_list line_sections = STRING_LIST_INIT_DUP;
98+
struct object_id oid;
99+
100+
/* Split line into <oid>, <ref> and <time> (if <time> exists) */
101+
string_list_split(&line_sections, line.buf, " ", 3);
102+
103+
/* Ignore the lines where <oid> doesn't appear in the dest_pack */
104+
get_oid_hex_algop(line_sections.items[0].string, &oid, repo->hash_algo);
105+
if (!find_pack_entry_one(&oid, dest_pack)) {
106+
string_list_clear(&line_sections, 0);
107+
continue;
108+
}
109+
110+
/* If <time> doesn't exist, retrieve it and add it to line */
111+
if (line_sections.nr < 3)
112+
strbuf_addf(&line, " %" PRItime,
113+
(timestamp_t)source_stat.st_mtime);
114+
115+
/*
116+
* Add the finalized line to dest_to_write and dest_content if it
117+
* wasn't already present inside dest_content
118+
*/
119+
if (strset_add(&dest_content, line.buf)) {
120+
strbuf_addbuf(&dest_to_write, &line);
121+
strbuf_addch(&dest_to_write, '\n');
122+
}
123+
124+
string_list_clear(&line_sections, 0);
125+
}
126+
127+
err = ferror(source);
128+
err |= fclose(source);
129+
if (err)
130+
die(_("Could not read '%s' promisor file"), source_promisor_name.buf);
131+
}
132+
133+
/* If dest_to_write is not empty, then there are new lines to append */
134+
if (dest_to_write.len) {
135+
if (fseek(dest, 0L, SEEK_END))
136+
die_errno(_("fseek failed"));
137+
fprintf(dest, "%s", dest_to_write.buf);
138+
}
139+
140+
err = ferror(dest);
141+
err |= fclose(dest);
142+
if (err)
143+
die(_("Could not write '%s' promisor file"), dest_promisor_name);
144+
145+
close_pack_index(dest_pack);
146+
free(dest_idx_name);
147+
free(dest_promisor_name);
148+
strset_clear(&dest_content);
149+
strbuf_release(&dest_to_write);
150+
strbuf_release(&source_promisor_name);
151+
strbuf_release(&line);
152+
}
153+
37154
static void finish_repacking_promisor_objects(struct repository *repo,
38155
struct child_process *cmd,
39156
struct string_list *names,
40-
const char *packtmp)
157+
const char *packtmp,
158+
struct strset *not_repacked_basenames)
41159
{
42160
struct strbuf line = STRBUF_INIT;
43161
FILE *out;
@@ -55,19 +173,15 @@ static void finish_repacking_promisor_objects(struct repository *repo,
55173

56174
/*
57175
* pack-objects creates the .pack and .idx files, but not the
58-
* .promisor file. Create the .promisor file, which is empty.
59-
*
60-
* NEEDSWORK: fetch-pack sometimes generates non-empty
61-
* .promisor files containing the ref names and associated
62-
* hashes at the point of generation of the corresponding
63-
* packfile, but this would not preserve their contents. Maybe
64-
* concatenate the contents of all .promisor files instead of
65-
* just creating a new empty file.
176+
* .promisor file. Create the .promisor file.
66177
*/
67178
promisor_name = mkpathdup("%s-%s.promisor", packtmp,
68179
line.buf);
69180
write_promisor_file(promisor_name, NULL, 0);
70181

182+
/* Now let's fill the content of the newly created .promisor file */
183+
copy_promisor_content(repo, line.buf, packtmp, not_repacked_basenames);
184+
71185
item->util = generated_pack_populate(item->string, packtmp);
72186

73187
free(promisor_name);
@@ -107,7 +221,7 @@ void repack_promisor_objects(struct repository *repo,
107221
return;
108222
}
109223

110-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
224+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp, NULL);
111225
}
112226

113227
void pack_geometry_repack_promisors(struct repository *repo,
@@ -118,6 +232,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
118232
{
119233
struct child_process cmd = CHILD_PROCESS_INIT;
120234
FILE *in;
235+
struct strset not_repacked_basenames = STRSET_INIT;
121236

122237
if (!geometry->promisor_split)
123238
return;
@@ -131,9 +246,15 @@ void pack_geometry_repack_promisors(struct repository *repo,
131246
in = xfdopen(cmd.in, "w");
132247
for (size_t i = 0; i < geometry->promisor_split; i++)
133248
fprintf(in, "%s\n", pack_basename(geometry->promisor_pack[i]));
134-
for (size_t i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++)
135-
fprintf(in, "^%s\n", pack_basename(geometry->promisor_pack[i]));
249+
for (size_t i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++) {
250+
const char *name = pack_basename(geometry->promisor_pack[i]);
251+
fprintf(in, "^%s\n", name);
252+
strset_add(&not_repacked_basenames, name);
253+
}
136254
fclose(in);
137255

138-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
256+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp,
257+
strset_get_size(&not_repacked_basenames) ? &not_repacked_basenames : NULL);
258+
259+
strset_clear(&not_repacked_basenames);
139260
}

‎t/t7700-repack.sh‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,4 +904,64 @@ test_expect_success 'pending objects are repacked appropriately' '
904904
)
905905
'
906906

907+
test_expect_success 'check one .promisor file content after repack' '
908+
test_when_finished rm -rf prom_test prom_before_repack &&
909+
git init prom_test &&
910+
path=prom_test/.git/objects/pack &&
911+
912+
(
913+
test_commit_bulk -C prom_test 1 &&
914+
915+
# Simulate .promisor file by creating it manually
916+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
917+
oid=$(git -C prom_test rev-parse HEAD) &&
918+
echo "$oid ref" >"$prom" &&
919+
920+
# Repack, and check if correct
921+
git -C prom_test repack -a -d -f &&
922+
prom=$(ls $path/*.promisor) &&
923+
# $prom should contain "$oid ref <time>"
924+
test_grep "$prom_before_repack " "$prom" &&
925+
926+
# Save the current .promisor content, repack, and check if correct
927+
cp "$prom" prom_before_repack &&
928+
git -C prom_test repack -a -d -f &&
929+
prom=$(ls $path/*.promisor) &&
930+
# $prom should be exactly the same as prom_before_repack
931+
test_cmp prom_before_repack "$prom"
932+
)
933+
'
934+
935+
test_expect_success 'check multiple .promisor file content after repack' '
936+
test_when_finished rm -rf prom_test prom_before_repack &&
937+
git init prom_test &&
938+
path=prom_test/.git/objects/pack &&
939+
940+
(
941+
# Create 2 packs and simulate .promisor files by creating them manually
942+
test_commit_bulk -C prom_test 1 &&
943+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
944+
oid1=$(git -C prom_test rev-parse HEAD) &&
945+
echo "$oid1 ref1" >"$prom" &&
946+
test_commit_bulk -C prom_test 1 &&
947+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom|d") &&
948+
oid2=$(git -C prom_test rev-parse HEAD) &&
949+
echo "$oid2 ref2" >"$prom" &&
950+
951+
# Repack, and check if correct
952+
git -C prom_test repack -a -d -f &&
953+
prom=$(ls $path/*.promisor) &&
954+
# $prom should contain "$oid1 ref1 <time>" & "$oid2 ref2 <time>"
955+
test_grep "$oid1 ref1 " "$prom" &&
956+
test_grep "$oid2 ref2 " "$prom" &&
957+
958+
# Save the current .promisor content, repack, and check if correct
959+
cp "$prom" prom_before_repack &&
960+
git -C prom_test repack -a -d -f &&
961+
prom=$(ls $path/*.promisor) &&
962+
# $prom should be exactly the same as prom_before_repack
963+
test_cmp prom_before_repack "$prom"
964+
)
965+
'
966+
907967
test_done

‎t/t7703-repack-geometric.sh‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,4 +541,37 @@ test_expect_success 'geometric repack works with promisor packs' '
541541
)
542542
'
543543

544+
test_expect_success 'check .promisor file content after geometric repack' '
545+
test_when_finished rm -rf prom_test &&
546+
git init prom_test &&
547+
path=prom_test/.git/objects/pack &&
548+
549+
(
550+
# Create 2 packs with 3 objs each, and manually create .promisor files
551+
test_commit_bulk -C prom_test --start=1 1 && # 3 objects
552+
prom1=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
553+
oid1=$(git -C prom_test rev-parse HEAD) &&
554+
echo "$oid1 ref1" >"$prom1" &&
555+
test_commit_bulk -C prom_test --start=2 1 && # 3 objects
556+
prom2=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom1|d") &&
557+
oid2=$(git -C prom_test rev-parse HEAD) &&
558+
echo "$oid2 ref2" >"$prom2" &&
559+
560+
# Create 1 pack with 12 objs, and manually create .promisor file
561+
test_commit_bulk -C prom_test --start=3 4 && # 12 objects
562+
prom3=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom1|d; \|$prom2|d") &&
563+
oid3=$(git -C prom_test rev-parse HEAD) &&
564+
echo "$oid3 ref3" >"$prom3" &&
565+
566+
# Geometric repack, and check if correct
567+
git -C prom_test repack --geometric 2 -d &&
568+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/; \|$prom3|d") &&
569+
# $prom should have repacked only the first 2 small packs, so it should only
570+
# contain the following: "$oid1 ref1 <time>" & "$oid2 ref2 <time>"
571+
test_grep "$oid1 ref1 " "$prom" &&
572+
test_grep "$oid2 ref2 " "$prom" &&
573+
test_grep ! "$oid3 ref3" "$prom"
574+
)
575+
'
576+
544577
test_done

0 commit comments

Comments
 (0)