@matheusbsbs / matheus-bagliano / commits / 98fa0d50a7

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 <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Taylor Blau committed Oct 15, 2025 at 18:29 UTC 98fa0d50a75099df3f2d62f9181e4c1bbf70f063

3 files changed +17 -17

builtin/repack.c

+4 -16

@@ -149,6 +149,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,

149 const char *caret;

150 const char *scratch;

151 int local = skip_prefix(opts->destination, opts->packdir, &scratch);

152 + const char *pack_prefix = write_pack_opts_pack_prefix(opts);

153

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

155

@@ -173,7 +174,7 @@ static int write_filtered_pack(const struct write_pack_opts *opts,

174 */

175 in = xfdopen(cmd.in, "w");

176 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);

178 for_each_string_list_item(item, &existing->non_kept_packs)

179 fprintf(in, "%s.pack\n", item->string);

180 for_each_string_list_item(item, &existing->cruft_packs)

@@ -233,6 +234,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,

234 int ret;

235 const char *scratch;

236 int local = skip_prefix(opts->destination, opts->packdir, &scratch);

237 + const char *pack_prefix = write_pack_opts_pack_prefix(opts);

238

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

240

@@ -265,7 +267,7 @@ static int write_cruft_pack(const struct write_pack_opts *opts,

267 */

268 in = xfdopen(cmd.in, "w");

269 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);

271 if (combine_cruft_below_size && !cruft_expiration) {

272 combine_small_cruft_packs(in, combine_cruft_below_size,

273 existing);

@@ -283,17 +285,6 @@ static int write_cruft_pack(const struct write_pack_opts *opts,

285 local);

286 }

287

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 -

288 int cmd_repack(int argc,

289 const char **argv,

290 const char *prefix,

@@ -596,11 +587,9 @@ int cmd_repack(int argc,

587 }

588

589 if (pack_everything & PACK_CRUFT) {

599 - const char *pack_prefix = find_pack_prefix(packdir, packtmp);

590 struct write_pack_opts opts = {

591 .po_args = &cruft_po_args,

592 .destination = packtmp,

603 - .pack_prefix = pack_prefix,

593 .packtmp = packtmp,

594 .packdir = packdir,

595 };

@@ -667,7 +656,6 @@ int cmd_repack(int argc,

656 struct write_pack_opts opts = {

657 .po_args = &po_args,

658 .destination = filter_to,

670 - .pack_prefix = find_pack_prefix(packdir, packtmp),

659 .packdir = packdir,

660 .packtmp = packtmp,

661 };

repack.c

+11

@@ -66,6 +66,17 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,

66 strbuf_release(&buf);

67 }

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 +

80 #define DELETE_PACK 1

81 #define RETAIN_PACK 2

repack.h

+2 -1

@@ -35,11 +35,12 @@ void repack_remove_redundant_pack(struct repository *repo, const char *dir_name,

35 struct write_pack_opts {

36 struct pack_objects_args *po_args;

37 const char *destination;

38 - const char *pack_prefix;

38 const char *packdir;

39 const char *packtmp;

40 };

42 +const char *write_pack_opts_pack_prefix(const struct write_pack_opts *opts);

43 +

44 struct repository;

45 struct packed_git;