Skip to content

Commit 070fa41

Browse files
gitstergitster
committed
Merge branch 'ps/geometric-repacking-with-promisor-remotes'
"git repack --geometric" did not work with promisor packs, which has been corrected. * ps/geometric-repacking-with-promisor-remotes: builtin/repack: handle promisor packs with geometric repacking repack-promisor: extract function to remove redundant packs repack-promisor: extract function to finalize repacking repack-geometry: extract function to compute repacking split builtin/pack-objects: exclude promisor objects with "--stdin-packs"
2 parents 83a69f1 + dcc9c7e commit 070fa41

7 files changed

Lines changed: 250 additions & 63 deletions

File tree

‎builtin/pack-objects.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3863,8 +3863,11 @@ static void read_packs_list_from_stdin(struct rev_info *revs)
38633863
repo_for_each_pack(the_repository, p) {
38643864
const char *pack_name = pack_basename(p);
38653865

3866-
if ((item = string_list_lookup(&include_packs, pack_name)))
3866+
if ((item = string_list_lookup(&include_packs, pack_name))) {
3867+
if (exclude_promisor_objects && p->pack_promisor)
3868+
die(_("packfile %s is a promisor but --exclude-promisor-objects was given"), p->pack_name);
38673869
item->util = p;
3870+
}
38683871
if ((item = string_list_lookup(&exclude_packs, pack_name)))
38693872
item->util = p;
38703873
}
@@ -3942,6 +3945,7 @@ static void read_stdin_packs(enum stdin_packs_mode mode, int rev_list_unpacked)
39423945
revs.tree_objects = 1;
39433946
revs.tag_objects = 1;
39443947
revs.ignore_missing_links = 1;
3948+
revs.exclude_promisor_objects = exclude_promisor_objects;
39453949

39463950
/* avoids adding objects in excluded packs */
39473951
ignore_packed_keep_in_core = 1;
@@ -5098,9 +5102,13 @@ int cmd_pack_objects(int argc,
50985102
exclude_promisor_objects_best_effort,
50995103
"--exclude-promisor-objects-best-effort");
51005104
if (exclude_promisor_objects) {
5101-
use_internal_rev_list = 1;
51025105
fetch_if_missing = 0;
5103-
strvec_push(&rp, "--exclude-promisor-objects");
5106+
5107+
/* --stdin-packs handles promisor objects separately. */
5108+
if (!stdin_packs) {
5109+
use_internal_rev_list = 1;
5110+
strvec_push(&rp, "--exclude-promisor-objects");
5111+
}
51045112
} else if (exclude_promisor_objects_best_effort) {
51055113
use_internal_rev_list = 1;
51065114
fetch_if_missing = 0;

‎builtin/repack.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,9 @@ int cmd_repack(int argc,
332332
!(pack_everything & PACK_CRUFT))
333333
strvec_push(&cmd.args, "--pack-loose-unreachable");
334334
} else if (geometry.split_factor) {
335+
pack_geometry_repack_promisors(repo, &po_args, &geometry,
336+
&names, packtmp);
337+
335338
if (midx_must_contain_cruft)
336339
strvec_push(&cmd.args, "--stdin-packs");
337340
else

‎repack-geometry.c‎

Lines changed: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -66,45 +66,54 @@ void pack_geometry_init(struct pack_geometry *geometry,
6666
if (p->is_cruft)
6767
continue;
6868

69-
ALLOC_GROW(geometry->pack,
70-
geometry->pack_nr + 1,
71-
geometry->pack_alloc);
72-
73-
geometry->pack[geometry->pack_nr] = p;
74-
geometry->pack_nr++;
69+
if (p->pack_promisor) {
70+
ALLOC_GROW(geometry->promisor_pack,
71+
geometry->promisor_pack_nr + 1,
72+
geometry->promisor_pack_alloc);
73+
74+
geometry->promisor_pack[geometry->promisor_pack_nr] = p;
75+
geometry->promisor_pack_nr++;
76+
} else {
77+
ALLOC_GROW(geometry->pack,
78+
geometry->pack_nr + 1,
79+
geometry->pack_alloc);
80+
81+
geometry->pack[geometry->pack_nr] = p;
82+
geometry->pack_nr++;
83+
}
7584
}
7685

7786
QSORT(geometry->pack, geometry->pack_nr, pack_geometry_cmp);
87+
QSORT(geometry->promisor_pack, geometry->promisor_pack_nr, pack_geometry_cmp);
7888
strbuf_release(&buf);
7989
}
8090

81-
void pack_geometry_split(struct pack_geometry *geometry)
91+
static uint32_t compute_pack_geometry_split(struct packed_git **pack, size_t pack_nr,
92+
int split_factor)
8293
{
8394
uint32_t i;
8495
uint32_t split;
8596
off_t total_size = 0;
8697

87-
if (!geometry->pack_nr) {
88-
geometry->split = geometry->pack_nr;
89-
return;
90-
}
98+
if (!pack_nr)
99+
return 0;
91100

92101
/*
93102
* First, count the number of packs (in descending order of size) which
94103
* already form a geometric progression.
95104
*/
96-
for (i = geometry->pack_nr - 1; i > 0; i--) {
97-
struct packed_git *ours = geometry->pack[i];
98-
struct packed_git *prev = geometry->pack[i - 1];
105+
for (i = pack_nr - 1; i > 0; i--) {
106+
struct packed_git *ours = pack[i];
107+
struct packed_git *prev = pack[i - 1];
99108

100-
if (unsigned_mult_overflows(geometry->split_factor,
109+
if (unsigned_mult_overflows(split_factor,
101110
pack_geometry_weight(prev)))
102111
die(_("pack %s too large to consider in geometric "
103112
"progression"),
104113
prev->pack_name);
105114

106115
if (pack_geometry_weight(ours) <
107-
geometry->split_factor * pack_geometry_weight(prev))
116+
split_factor * pack_geometry_weight(prev))
108117
break;
109118
}
110119

@@ -130,21 +139,19 @@ void pack_geometry_split(struct pack_geometry *geometry)
130139
* the geometric progression.
131140
*/
132141
for (i = 0; i < split; i++) {
133-
struct packed_git *p = geometry->pack[i];
142+
struct packed_git *p = pack[i];
134143

135144
if (unsigned_add_overflows(total_size, pack_geometry_weight(p)))
136145
die(_("pack %s too large to roll up"), p->pack_name);
137146
total_size += pack_geometry_weight(p);
138147
}
139-
for (i = split; i < geometry->pack_nr; i++) {
140-
struct packed_git *ours = geometry->pack[i];
148+
for (i = split; i < pack_nr; i++) {
149+
struct packed_git *ours = pack[i];
141150

142-
if (unsigned_mult_overflows(geometry->split_factor,
143-
total_size))
151+
if (unsigned_mult_overflows(split_factor, total_size))
144152
die(_("pack %s too large to roll up"), ours->pack_name);
145153

146-
if (pack_geometry_weight(ours) <
147-
geometry->split_factor * total_size) {
154+
if (pack_geometry_weight(ours) < split_factor * total_size) {
148155
if (unsigned_add_overflows(total_size,
149156
pack_geometry_weight(ours)))
150157
die(_("pack %s too large to roll up"),
@@ -156,7 +163,16 @@ void pack_geometry_split(struct pack_geometry *geometry)
156163
break;
157164
}
158165

159-
geometry->split = split;
166+
return split;
167+
}
168+
169+
void pack_geometry_split(struct pack_geometry *geometry)
170+
{
171+
geometry->split = compute_pack_geometry_split(geometry->pack, geometry->pack_nr,
172+
geometry->split_factor);
173+
geometry->promisor_split = compute_pack_geometry_split(geometry->promisor_pack,
174+
geometry->promisor_pack_nr,
175+
geometry->split_factor);
160176
}
161177

162178
struct packed_git *pack_geometry_preferred_pack(struct pack_geometry *geometry)
@@ -194,17 +210,18 @@ struct packed_git *pack_geometry_preferred_pack(struct pack_geometry *geometry)
194210
return NULL;
195211
}
196212

197-
void pack_geometry_remove_redundant(struct pack_geometry *geometry,
198-
struct string_list *names,
199-
struct existing_packs *existing,
200-
const char *packdir)
213+
static void remove_redundant_packs(struct packed_git **pack,
214+
uint32_t pack_nr,
215+
struct string_list *names,
216+
struct existing_packs *existing,
217+
const char *packdir)
201218
{
202219
const struct git_hash_algo *algop = existing->repo->hash_algo;
203220
struct strbuf buf = STRBUF_INIT;
204221
uint32_t i;
205222

206-
for (i = 0; i < geometry->split; i++) {
207-
struct packed_git *p = geometry->pack[i];
223+
for (i = 0; i < pack_nr; i++) {
224+
struct packed_git *p = pack[i];
208225
if (string_list_has_string(names, hash_to_hex_algop(p->hash,
209226
algop)))
210227
continue;
@@ -223,10 +240,22 @@ void pack_geometry_remove_redundant(struct pack_geometry *geometry,
223240
strbuf_release(&buf);
224241
}
225242

243+
void pack_geometry_remove_redundant(struct pack_geometry *geometry,
244+
struct string_list *names,
245+
struct existing_packs *existing,
246+
const char *packdir)
247+
{
248+
remove_redundant_packs(geometry->pack, geometry->split,
249+
names, existing, packdir);
250+
remove_redundant_packs(geometry->promisor_pack, geometry->promisor_split,
251+
names, existing, packdir);
252+
}
253+
226254
void pack_geometry_release(struct pack_geometry *geometry)
227255
{
228256
if (!geometry)
229257
return;
230258

231259
free(geometry->pack);
260+
free(geometry->promisor_pack);
232261
}

‎repack-promisor.c‎

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

37-
void repack_promisor_objects(struct repository *repo,
38-
const struct pack_objects_args *args,
39-
struct string_list *names, const char *packtmp)
37+
static void finish_repacking_promisor_objects(struct repository *repo,
38+
struct child_process *cmd,
39+
struct string_list *names,
40+
const char *packtmp)
4041
{
41-
struct write_oid_context ctx;
42-
struct child_process cmd = CHILD_PROCESS_INIT;
43-
FILE *out;
4442
struct strbuf line = STRBUF_INIT;
43+
FILE *out;
4544

46-
prepare_pack_objects(&cmd, args, packtmp);
47-
cmd.in = -1;
48-
49-
/*
50-
* NEEDSWORK: Giving pack-objects only the OIDs without any ordering
51-
* hints may result in suboptimal deltas in the resulting pack. See if
52-
* the OIDs can be sent with fake paths such that pack-objects can use a
53-
* {type -> existing pack order} ordering when computing deltas instead
54-
* of a {type -> size} ordering, which may produce better deltas.
55-
*/
56-
ctx.cmd = &cmd;
57-
ctx.algop = repo->hash_algo;
58-
for_each_packed_object(repo, write_oid, &ctx,
59-
FOR_EACH_OBJECT_PROMISOR_ONLY);
60-
61-
if (cmd.in == -1) {
62-
/* No packed objects; cmd was never started */
63-
child_process_clear(&cmd);
64-
return;
65-
}
66-
67-
close(cmd.in);
45+
close(cmd->in);
6846

69-
out = xfdopen(cmd.out, "r");
47+
out = xfdopen(cmd->out, "r");
7048
while (strbuf_getline_lf(&line, out) != EOF) {
7149
struct string_list_item *item;
7250
char *promisor_name;
@@ -96,7 +74,66 @@ void repack_promisor_objects(struct repository *repo,
9674
}
9775

9876
fclose(out);
99-
if (finish_command(&cmd))
77+
if (finish_command(cmd))
10078
die(_("could not finish pack-objects to repack promisor objects"));
10179
strbuf_release(&line);
10280
}
81+
82+
void repack_promisor_objects(struct repository *repo,
83+
const struct pack_objects_args *args,
84+
struct string_list *names, const char *packtmp)
85+
{
86+
struct write_oid_context ctx;
87+
struct child_process cmd = CHILD_PROCESS_INIT;
88+
89+
prepare_pack_objects(&cmd, args, packtmp);
90+
cmd.in = -1;
91+
92+
/*
93+
* NEEDSWORK: Giving pack-objects only the OIDs without any ordering
94+
* hints may result in suboptimal deltas in the resulting pack. See if
95+
* the OIDs can be sent with fake paths such that pack-objects can use a
96+
* {type -> existing pack order} ordering when computing deltas instead
97+
* of a {type -> size} ordering, which may produce better deltas.
98+
*/
99+
ctx.cmd = &cmd;
100+
ctx.algop = repo->hash_algo;
101+
for_each_packed_object(repo, write_oid, &ctx,
102+
FOR_EACH_OBJECT_PROMISOR_ONLY);
103+
104+
if (cmd.in == -1) {
105+
/* No packed objects; cmd was never started */
106+
child_process_clear(&cmd);
107+
return;
108+
}
109+
110+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
111+
}
112+
113+
void pack_geometry_repack_promisors(struct repository *repo,
114+
const struct pack_objects_args *args,
115+
const struct pack_geometry *geometry,
116+
struct string_list *names,
117+
const char *packtmp)
118+
{
119+
struct child_process cmd = CHILD_PROCESS_INIT;
120+
FILE *in;
121+
122+
if (!geometry->promisor_split)
123+
return;
124+
125+
prepare_pack_objects(&cmd, args, packtmp);
126+
strvec_push(&cmd.args, "--stdin-packs");
127+
cmd.in = -1;
128+
if (start_command(&cmd))
129+
die(_("could not start pack-objects to repack promisor packs"));
130+
131+
in = xfdopen(cmd.in, "w");
132+
for (size_t i = 0; i < geometry->promisor_split; i++)
133+
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]));
136+
fclose(in);
137+
138+
finish_repacking_promisor_objects(repo, &cmd, names, packtmp);
139+
}

‎repack.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,19 @@ struct pack_geometry {
103103
uint32_t pack_nr, pack_alloc;
104104
uint32_t split;
105105

106+
struct packed_git **promisor_pack;
107+
uint32_t promisor_pack_nr, promisor_pack_alloc;
108+
uint32_t promisor_split;
109+
106110
int split_factor;
107111
};
108112

113+
void pack_geometry_repack_promisors(struct repository *repo,
114+
const struct pack_objects_args *args,
115+
const struct pack_geometry *geometry,
116+
struct string_list *names,
117+
const char *packtmp);
118+
109119
void pack_geometry_init(struct pack_geometry *geometry,
110120
struct existing_packs *existing,
111121
const struct pack_objects_args *args);

‎t/t5331-pack-objects-stdin.sh‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,45 @@ test_expect_success '--stdin-packs=follow walks into unknown packs' '
319319
)
320320
'
321321

322+
test_expect_success '--stdin-packs with promisors' '
323+
test_when_finished "rm -fr repo" &&
324+
git init repo &&
325+
(
326+
cd repo &&
327+
git config set maintenance.auto false &&
328+
git remote add promisor garbage &&
329+
git config set remote.promisor.promisor true &&
330+
331+
for c in A B C D
332+
do
333+
echo "$c" >file &&
334+
git add file &&
335+
git commit --message "$c" &&
336+
git tag "$c" || return 1
337+
done &&
338+
339+
A="$(echo A | git pack-objects --revs $packdir/pack)" &&
340+
B="$(echo A..B | git pack-objects --revs $packdir/pack --filter=blob:none)" &&
341+
C="$(echo B..C | git pack-objects --revs $packdir/pack)" &&
342+
D="$(echo C..D | git pack-objects --revs $packdir/pack)" &&
343+
touch $packdir/pack-$B.promisor &&
344+
345+
test_must_fail git pack-objects --stdin-packs --exclude-promisor-objects pack- 2>err <<-EOF &&
346+
pack-$B.pack
347+
EOF
348+
test_grep "is a promisor but --exclude-promisor-objects was given" err &&
349+
350+
PACK=$(git pack-objects --stdin-packs=follow --exclude-promisor-objects $packdir/pack <<-EOF
351+
pack-$D.pack
352+
EOF
353+
) &&
354+
objects_in_packs $C $D >expect &&
355+
objects_in_packs $PACK >actual &&
356+
test_cmp expect actual &&
357+
rm -f $packdir/pack-$PACK.*
358+
)
359+
'
360+
322361
stdin_packs__follow_with_only () {
323362
rm -fr stdin_packs__follow_with_only &&
324363
git init stdin_packs__follow_with_only &&

0 commit comments

Comments
 (0)