Skip to content

Commit 3300809

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??? repack-promisor: add missing headers 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 repack-promisor add helper to fill promisor file after repack pack-write: add explanation to promisor file content
2 parents 1479a61 + 1e288cc commit 3300809

File tree

5 files changed

+247
-15
lines changed

5 files changed

+247
-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: 143 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
11
#include "git-compat-util.h"
22
#include "repack.h"
3+
#include "hash.h"
34
#include "hex.h"
5+
#include "odb.h"
46
#include "pack.h"
57
#include "packfile.h"
68
#include "path.h"
79
#include "repository.h"
810
#include "run-command.h"
11+
#include "strbuf.h"
12+
#include "string-list.h"
13+
#include "strmap.h"
14+
#include "strvec.h"
915

1016
struct write_oid_context {
1117
struct child_process *cmd;
@@ -34,10 +40,131 @@ static int write_oid(const struct object_id *oid,
3440
return 0;
3541
}
3642

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

56183
/*
57184
* 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.
185+
* .promisor file. Create the .promisor file.
66186
*/
67187
promisor_name = mkpathdup("%s-%s.promisor", packtmp,
68188
line.buf);
69189
write_promisor_file(promisor_name, NULL, 0);
70190

191+
/* Now let's fill the content of the newly created .promisor file */
192+
copy_promisor_content(repo, line.buf, packtmp, not_repacked_basenames);
193+
71194
item->util = generated_pack_populate(item->string, packtmp);
72195

73196
free(promisor_name);
@@ -107,7 +230,7 @@ void repack_promisor_objects(struct repository *repo,
107230
return;
108231
}
109232

110-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
233+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp, NULL);
111234
}
112235

113236
void pack_geometry_repack_promisors(struct repository *repo,
@@ -118,6 +241,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
118241
{
119242
struct child_process cmd = CHILD_PROCESS_INIT;
120243
FILE *in;
244+
struct strset not_repacked_basenames = STRSET_INIT;
121245

122246
if (!geometry->promisor_split)
123247
return;
@@ -131,9 +255,15 @@ void pack_geometry_repack_promisors(struct repository *repo,
131255
in = xfdopen(cmd.in, "w");
132256
for (size_t i = 0; i < geometry->promisor_split; i++)
133257
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]));
258+
for (size_t i = geometry->promisor_split; i < geometry->promisor_pack_nr; i++) {
259+
const char *name = pack_basename(geometry->promisor_pack[i]);
260+
fprintf(in, "^%s\n", name);
261+
strset_add(&not_repacked_basenames, name);
262+
}
136263
fclose(in);
137264

138-
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
265+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp,
266+
strset_get_size(&not_repacked_basenames) ? &not_repacked_basenames : NULL);
267+
268+
strset_clear(&not_repacked_basenames);
139269
}

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