Skip to content

Commit 50d0470

Browse files
gitstergitster
committed
Merge branch 'lp/repack-propagate-promisor-debugging-info' into seen
* 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 7d54a17 + d2b2f42 commit 50d0470

5 files changed

Lines changed: 253 additions & 15 deletions

File tree

‎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: 137 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,131 @@ 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 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 strbuf **parts;
98+
struct object_id oid;
99+
100+
/* Split line into <oid>, <ref> and <time> (if <time> exists) */
101+
parts = strbuf_split_max(&line, ' ', 3);
102+
103+
/* Ignore the lines where <oid> doesn't appear in the dest_pack */
104+
strbuf_rtrim(parts[0]);
105+
get_oid_hex_algop(parts[0]->buf, &oid, repo->hash_algo);
106+
if (!find_pack_entry_one(&oid, dest_pack))
107+
continue;
108+
109+
/* If <time> doesn't exist, retrieve it and add it to line */
110+
if (!parts[2]) {
111+
struct tm tm;
112+
113+
localtime_r(&source_stat.st_mtim.tv_sec, &tm);
114+
strbuf_addch(&line, ' ');
115+
strbuf_addftime(&line, "%Y/%m/%d-%H:%M:%S", &tm, 0, 0);
116+
}
117+
118+
/*
119+
* Add the finalized line to dest_to_write and dest_content if it
120+
* wasn't already present inside dest_content
121+
*/
122+
if (strset_add(&dest_content, line.buf)) {
123+
strbuf_addbuf(&dest_to_write, &line);
124+
strbuf_addch(&dest_to_write, '\n');
125+
}
126+
127+
strbuf_list_free(parts);
128+
}
129+
130+
err = ferror(source);
131+
err |= fclose(source);
132+
if (err)
133+
die(_("Could not read '%s' promisor file"), source_promisor_name.buf);
134+
}
135+
136+
/* If dest_to_write is not empty, then there are new lines to append */
137+
if (dest_to_write.len) {
138+
if (fseek(dest, 0L, SEEK_END))
139+
die_errno(_("fseek failed"));
140+
fprintf(dest, "%s", dest_to_write.buf);
141+
}
142+
143+
err = ferror(dest);
144+
err |= fclose(dest);
145+
if (err)
146+
die(_("Could not write '%s' promisor file"), dest_promisor_name);
147+
148+
close_pack_index(dest_pack);
149+
free(dest_idx_name);
150+
free(dest_promisor_name);
151+
strset_clear(&dest_content);
152+
strbuf_release(&dest_to_write);
153+
strbuf_release(&source_promisor_name);
154+
strbuf_release(&line);
155+
}
156+
37157
static void finish_repacking_promisor_objects(struct repository *repo,
38158
struct child_process *cmd,
39159
struct string_list *names,
40-
const char *packtmp)
160+
const char *packtmp,
161+
struct strset *not_repacked_basenames)
41162
{
42163
struct strbuf line = STRBUF_INIT;
43164
FILE *out;
@@ -55,19 +176,15 @@ static void finish_repacking_promisor_objects(struct repository *repo,
55176

56177
/*
57178
* 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.
179+
* .promisor file. Create the .promisor file.
66180
*/
67181
promisor_name = mkpathdup("%s-%s.promisor", packtmp,
68182
line.buf);
69183
write_promisor_file(promisor_name, NULL, 0);
70184

185+
/* Now let's fill the content of the newly created .promisor file */
186+
copy_promisor_content(repo, line.buf, packtmp, not_repacked_basenames);
187+
71188
item->util = generated_pack_populate(item->string, packtmp);
72189

73190
free(promisor_name);
@@ -107,7 +224,7 @@ void repack_promisor_objects(struct repository *repo,
107224
return;
108225
}
109226

110-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
227+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp, NULL);
111228
}
112229

113230
void pack_geometry_repack_promisors(struct repository *repo,
@@ -118,6 +235,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
118235
{
119236
struct child_process cmd = CHILD_PROCESS_INIT;
120237
FILE *in;
238+
struct strset not_repacked_basenames = STRSET_INIT;
121239

122240
if (!geometry->promisor_split)
123241
return;
@@ -131,9 +249,15 @@ void pack_geometry_repack_promisors(struct repository *repo,
131249
in = xfdopen(cmd.in, "w");
132250
for (size_t i = 0; i < geometry->promisor_split; i++)
133251
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]));
252+
for (size_t i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++) {
253+
const char *name = pack_basename(geometry->promisor_pack[i]);
254+
fprintf(in, "^%s\n", name);
255+
strset_add(&not_repacked_basenames, name);
256+
}
136257
fclose(in);
137258

138-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
259+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp,
260+
strset_get_size(&not_repacked_basenames) ? &not_repacked_basenames : NULL);
261+
262+
strset_clear(&not_repacked_basenames);
139263
}

‎t/t7700-repack.sh‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,4 +904,67 @@ 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 &&
909+
git init prom_test &&
910+
path=prom_test/.git/objects/pack &&
911+
912+
(
913+
test_commit_bulk -C prom_test --start=1 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+
# Save the current .promisor content, repack, and check if correct
921+
prom_before_repack=$(cat $prom) &&
922+
git -C prom_test repack -a -d &&
923+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
924+
# $prom should contain "$prom_before_repack <date>"
925+
test_grep "$prom_before_repack " $prom &&
926+
927+
# Save the current .promisor content, repack, and check if correct
928+
cat $prom >prom_before_repack &&
929+
git -C prom_test repack -a -d &&
930+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
931+
# $prom should be exactly the same as prom_before_repack
932+
test_cmp prom_before_repack $prom
933+
)
934+
'
935+
936+
test_expect_success 'check multiple .promisor file content after repack' '
937+
test_when_finished rm -rf prom_test &&
938+
git init prom_test &&
939+
path=prom_test/.git/objects/pack &&
940+
941+
(
942+
# Create 2 packs and simulate .promisor files by creating them manually
943+
test_commit_bulk -C prom_test --start=1 1 &&
944+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
945+
oid=$(git -C prom_test rev-parse HEAD) &&
946+
echo "$oid ref" >$prom &&
947+
prom_before_repack1=$(cat $prom) &&
948+
test_commit_bulk -C prom_test --start=1 1 &&
949+
prom=$(ls -t $path/*.pack | head -n 1 | sed "s/\.pack/.promisor/") &&
950+
oid=$(git -C prom_test rev-parse HEAD) &&
951+
echo "$oid ref" >$prom &&
952+
prom_before_repack2=$(cat $prom) &&
953+
954+
# Repack, and check if correct compared to previous saved .promisor content
955+
git -C prom_test repack -a -d &&
956+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
957+
# $prom should contain "$prom_before_repack1 <date>" & "$prom_before_repack2 <date>"
958+
test_grep "$prom_before_repack1 " $prom &&
959+
test_grep "$prom_before_repack2 " $prom &&
960+
961+
# Save the current .promisor content, repack, and check if correct
962+
cat $prom >prom_before_repack &&
963+
git -C prom_test repack -a -d &&
964+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
965+
# $prom should be exactly the same as prom_before_repack
966+
test_cmp prom_before_repack $prom
967+
)
968+
'
969+
907970
test_done

‎t/t7703-repack-geometric.sh‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,4 +541,46 @@ 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+
prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
553+
oid=$(git -C prom_test rev-parse HEAD) &&
554+
echo "$oid ref" >$prom &&
555+
prom_before_repack1=$(cat $prom) &&
556+
test_commit_bulk -C prom_test --start=2 1 && # 3 objects
557+
prom=$(ls -t $path/*.pack | head -n 1 | sed "s/\.pack/.promisor/") &&
558+
oid=$(git -C prom_test rev-parse HEAD) &&
559+
echo "$oid ref" >$prom &&
560+
prom_before_repack2=$(cat $prom) &&
561+
562+
# Create 2 packs with 12 and 24 objs, and manually create .promisor files
563+
test_commit_bulk -C prom_test --start=3 4 && # 12 objects
564+
prom=$(ls -t $path/*.pack | head -n 1 | sed "s/\.pack/.promisor/") &&
565+
oid=$(git -C prom_test rev-parse HEAD) &&
566+
echo "$oid ref" >$prom &&
567+
prom_before_repack3=$(cat $prom) &&
568+
test_commit_bulk -C prom_test --start=7 8 && # 24 objects
569+
prom=$(ls -t $path/*.pack | head -n 1 | sed "s/\.pack/.promisor/") &&
570+
oid=$(git -C prom_test rev-parse HEAD) &&
571+
echo "$oid ref" >$prom &&
572+
prom_before_repack4=$(cat $prom) &&
573+
574+
# Geometric repack, and check if correct compared to previous saved .promisor content
575+
git -C prom_test repack --geometric 2 -d &&
576+
prom=$(ls -t $path/*.pack | head -n 1 | sed "s/\.pack/.promisor/") &&
577+
# $prom should have repacked only the first 2 small packs, so it should only contain
578+
# the following: "$prom_before_repack1 <date>" & "$prom_before_repack2 <date>"
579+
test_grep "$prom_before_repack1 " $prom &&
580+
test_grep "$prom_before_repack2 " $prom &&
581+
test_grep ! $prom_before_repack3 $prom &&
582+
test_grep ! $prom_before_repack4 $prom
583+
)
584+
'
585+
544586
test_done

0 commit comments

Comments
 (0)