Re: [PATCH 1/4] builtin/repack.c: pass "out" to `prepare_pack_objects`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 24, 2022, 20:47 UTC
- Message-ID
- <xmqqa65lrltc.fsf@gitster.g>
- In-Reply-To
- <1dd4136f6199ac050cec5eb671c36ae05fbf3bdd.1666636974.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 11 quoted lines
> `builtin/repack.c`'s `prepare_pack_objects()` is used to prepare a set > of arguments to a `pack-objects` process which will generate a desired > pack. > > A future patch will add an `--expire-to` option which allows `git > repack` to write a cruft pack containing the pruned objects out to a > separate repository. Prepare for this by teaching that function to write > packs to an arbitrary location specified by the caller. > > All existing callers of `prepare_pack_objects()` will pass `packtmp` for > `out`, retaining the existing behavior.
It does make sense to allow the caller to specify the name of the temporary file to be used, but is "out" a good name for that? The other two arguments are self explanatory both by their type and the name, but this is of type "const char *" that does not convey what the string is about at all, so giging a good name to the parameter is more important than for others.
The patch text itself is very straight-forward. Thanks.
Show 42 quoted lines
> static void prepare_pack_objects(struct child_process *cmd,
> - const struct pack_objects_args *args)
> + const struct pack_objects_args *args,
> + const char *out)
> {
> strvec_push(&cmd->args, "pack-objects");
> if (args->window)
> @@ -211,7 +212,7 @@ static void prepare_pack_objects(struct child_process *cmd,
> strvec_push(&cmd->args, "--quiet");
> if (delta_base_offset)
> strvec_push(&cmd->args, "--delta-base-offset");
> - strvec_push(&cmd->args, packtmp);
> + strvec_push(&cmd->args, out);
> cmd->git_cmd = 1;
> cmd->out = -1;
> }
> @@ -275,7 +276,7 @@ static void repack_promisor_objects(const struct pack_objects_args *args,
> FILE *out;
> struct strbuf line = STRBUF_INIT;
>
> - prepare_pack_objects(&cmd, args);
> + prepare_pack_objects(&cmd, args, packtmp);
> cmd.in = -1;
>
> /*
> @@ -673,7 +674,7 @@ static int write_cruft_pack(const struct pack_objects_args *args,
> FILE *in, *out;
> int ret;
>
> - prepare_pack_objects(&cmd, args);
> + prepare_pack_objects(&cmd, args, packtmp);
>
> strvec_push(&cmd.args, "--cruft");
> if (cruft_expiration)
> @@ -861,7 +862,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>
> sigchain_push_common(remove_pack_on_signal);
>
> - prepare_pack_objects(&cmd, &po_args);
> + prepare_pack_objects(&cmd, &po_args, packtmp);
>
> show_progress = !po_args.quiet && isatty(2);