Skip to content

Commit d59ecb1

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 6f67314 + 6a709f0 commit d59ecb1

File tree

5 files changed

+249
-15
lines changed

5 files changed

+249
-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: 133 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,127 @@ 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+
strbuf_addf(&line, " %" PRIuMAX,
112+
(uintmax_t)source_stat.st_mtime);
113+
114+
/*
115+
* Add the finalized line to dest_to_write and dest_content if it
116+
* wasn't already present inside dest_content
117+
*/
118+
if (strset_add(&dest_content, line.buf)) {
119+
strbuf_addbuf(&dest_to_write, &line);
120+
strbuf_addch(&dest_to_write, '\n');
121+
}
122+
123+
strbuf_list_free(parts);
124+
}
125+
126+
err = ferror(source);
127+
err |= fclose(source);
128+
if (err)
129+
die(_("Could not read '%s' promisor file"), source_promisor_name.buf);
130+
}
131+
132+
/* If dest_to_write is not empty, then there are new lines to append */
133+
if (dest_to_write.len) {
134+
if (fseek(dest, 0L, SEEK_END))
135+
die_errno(_("fseek failed"));
136+
fprintf(dest, "%s", dest_to_write.buf);
137+
}
138+
139+
err = ferror(dest);
140+
err |= fclose(dest);
141+
if (err)
142+
die(_("Could not write '%s' promisor file"), dest_promisor_name);
143+
144+
close_pack_index(dest_pack);
145+
free(dest_idx_name);
146+
free(dest_promisor_name);
147+
strset_clear(&dest_content);
148+
strbuf_release(&dest_to_write);
149+
strbuf_release(&source_promisor_name);
150+
strbuf_release(&line);
151+
}
152+
37153
static void finish_repacking_promisor_objects(struct repository *repo,
38154
struct child_process *cmd,
39155
struct string_list *names,
40-
const char *packtmp)
156+
const char *packtmp,
157+
struct strset *not_repacked_basenames)
41158
{
42159
struct strbuf line = STRBUF_INIT;
43160
FILE *out;
@@ -55,19 +172,15 @@ static void finish_repacking_promisor_objects(struct repository *repo,
55172

56173
/*
57174
* 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.
175+
* .promisor file. Create the .promisor file.
66176
*/
67177
promisor_name = mkpathdup("%s-%s.promisor", packtmp,
68178
line.buf);
69179
write_promisor_file(promisor_name, NULL, 0);
70180

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

73186
free(promisor_name);
@@ -107,7 +220,7 @@ void repack_promisor_objects(struct repository *repo,
107220
return;
108221
}
109222

110-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
223+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp, NULL);
111224
}
112225

113226
void pack_geometry_repack_promisors(struct repository *repo,
@@ -118,6 +231,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
118231
{
119232
struct child_process cmd = CHILD_PROCESS_INIT;
120233
FILE *in;
234+
struct strset not_repacked_basenames = STRSET_INIT;
121235

122236
if (!geometry->promisor_split)
123237
return;
@@ -131,9 +245,15 @@ void pack_geometry_repack_promisors(struct repository *repo,
131245
in = xfdopen(cmd.in, "w");
132246
for (size_t i = 0; i < geometry->promisor_split; i++)
133247
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]));
248+
for (size_t i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++) {
249+
const char *name = pack_basename(geometry->promisor_pack[i]);
250+
fprintf(in, "^%s\n", name);
251+
strset_add(&not_repacked_basenames, name);
252+
}
136253
fclose(in);
137254

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

‎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)