Re: [PATCH 1/4] builtin/repack.c: pass "out" to `prepare_pack_objects`

[Junio C Hamano <gitster@pobox.com>]

Taylor Blau <me@ttaylorr.com> writes:

> `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.


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