Skip to content

Commit 98fa0d5

Browse files
ttaylorrgitster
authored andcommitted
repack: move find_pack_prefix() out of the builtin
Both callers within the repack builtin which call functions that take a 'write_pack_opts' structure have the following pattern: struct write_pack_opts opts = { .packdir = packdir, .packtmp = packtmp, .pack_prefix = find_pack_prefix(packdir, packtmp), /* ... */ }; int ret = write_some_kind_of_pack(&opts, /* ... */); , but both "packdir" and "packtmp" are fields within the write_pack_opts struct itself! Instead of also computing the pack_prefix ahead of time, let's have the callees compute it themselves by moving `find_pack_prefix()` out of the repack builtin, and have it take a write_pack_opts pointer instead of the "packdir" and "packtmp" fields directly. This avoids the callers having to do some prep work that is common between the two of them, but also avoids the potential pitfall of accidentally writing: .pack_prefix = find_pack_prefix(packtmp, packdir), (which is well-typed) when the caller meant to instead write: .pack_prefix = find_pack_prefix(packdir, packtmp), Signed-off-by: Taylor Blau <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 3d2ac20 commit 98fa0d5

File tree

3 files changed

+17
-17
lines changed

3 files changed

+17
-17
lines changed

builtin/repack.c

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,
149149
const char *caret;
150150
const char *scratch;
151151
int local = skip_prefix(opts->destination, opts->packdir, &scratch);
152+
const char *pack_prefix = write_pack_opts_pack_prefix(opts);
152153

153154
prepare_pack_objects(&cmd, opts->po_args, opts->destination);
154155

@@ -173,7 +174,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,
173174
*/
174175
in = xfdopen(cmd.in, "w");
175176
for_each_string_list_item(item, names)
176-
fprintf(in, "^%s-%s.pack\n", opts->pack_prefix, item->string);
177+
fprintf(in, "^%s-%s.pack\n", pack_prefix, item->string);
177178
for_each_string_list_item(item, &existing->non_kept_packs)
178179
fprintf(in, "%s.pack\n", item->string);
179180
for_each_string_list_item(item, &existing->cruft_packs)
@@ -233,6 +234,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
233234
int ret;
234235
const char *scratch;
235236
int local = skip_prefix(opts->destination, opts->packdir, &scratch);
237+
const char *pack_prefix = write_pack_opts_pack_prefix(opts);
236238

237239
prepare_pack_objects(&cmd, opts->po_args, opts->destination);
238240

@@ -265,7 +267,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
265267
*/
266268
in = xfdopen(cmd.in, "w");
267269
for_each_string_list_item(item, names)
268-
fprintf(in, "%s-%s.pack\n", opts->pack_prefix, item->string);
270+
fprintf(in, "%s-%s.pack\n", pack_prefix, item->string);
269271
if (combine_cruft_below_size && !cruft_expiration) {
270272
combine_small_cruft_packs(in, combine_cruft_below_size,
271273
existing);
@@ -283,17 +285,6 @@ static int write_cruft_pack(const struct write_pack_opts *opts,
283285
local);
284286
}
285287

286-
static const char *find_pack_prefix(const char *packdir, const char *packtmp)
287-
{
288-
const char *pack_prefix;
289-
if (!skip_prefix(packtmp, packdir, &pack_prefix))
290-
die(_("pack prefix %s does not begin with objdir %s"),
291-
packtmp, packdir);
292-
if (*pack_prefix == '/')
293-
pack_prefix++;
294-
return pack_prefix;
295-
}
296-
297288
int cmd_repack(int argc,
298289
const char **argv,
299290
const char *prefix,
@@ -596,11 +587,9 @@ int cmd_repack(int argc,
596587
}
597588

598589
if (pack_everything & PACK_CRUFT) {
599-
const char *pack_prefix = find_pack_prefix(packdir, packtmp);
600590
struct write_pack_opts opts = {
601591
.po_args = &cruft_po_args,
602592
.destination = packtmp,
603-
.pack_prefix = pack_prefix,
604593
.packtmp = packtmp,
605594
.packdir = packdir,
606595
};
@@ -667,7 +656,6 @@ int cmd_repack(int argc,
667656
struct write_pack_opts opts = {
668657
.po_args = &po_args,
669658
.destination = filter_to,
670-
.pack_prefix = find_pack_prefix(packdir, packtmp),
671659
.packdir = packdir,
672660
.packtmp = packtmp,
673661
};

repack.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,17 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
6666
strbuf_release(&buf);
6767
}
6868

69+
const char *write_pack_opts_pack_prefix(const struct write_pack_opts *opts)
70+
{
71+
const char *pack_prefix;
72+
if (!skip_prefix(opts->packtmp, opts->packdir, &pack_prefix))
73+
die(_("pack prefix %s does not begin with objdir %s"),
74+
opts->packtmp, opts->packdir);
75+
if (*pack_prefix == '/')
76+
pack_prefix++;
77+
return pack_prefix;
78+
}
79+
6980
#define DELETE_PACK 1
7081
#define RETAIN_PACK 2
7182

repack.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,12 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,
3535
struct write_pack_opts {
3636
struct pack_objects_args *po_args;
3737
const char *destination;
38-
const char *pack_prefix;
3938
const char *packdir;
4039
const char *packtmp;
4140
};
4241

42+
const char *write_pack_opts_pack_prefix(const struct write_pack_opts *opts);
43+
4344
struct repository;
4445
struct packed_git;
4546

0 commit comments

Comments
 (0)