git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 2/3] builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE

From
Patrick Steinhardt <ps@pks.im>
Date
Oct 5, 2023, 11:31 UTC
Message-ID
<ZR6ednOcuK6qizch@tanuki>
In-Reply-To
<9ec999882d790aa770aba8c0916b9260661af9be.1696293862.git.me@ttaylorr.com>
On Mon, Oct 02, 2023 at 08:44:29PM -0400, Taylor Blau wrote:
Show 12 quoted lines
> The repack builtin takes a `--max-pack-size` command-line argument which
> it uses to feed into any of the pack-objects children that it may spawn
> when generating a new pack.
> 
> This option is parsed with OPT_STRING, meaning that we'll accept
> anything as input, punting on more fine-grained validation until we get
> down into pack-objects.
> 
> This is fine, but it's wasteful to spend an entire sub-process just to
> figure out that one of its option is bogus. Instead, parse the value of
> `--max-pack-size` with OPT_MAGNITUDE in 'git repack', and then pass the
> knonw-good result down to pack-objects.
Tiny nit: s/knonw/known.
Other than that this patch looks good to me.
Patrick
Show 40 quoted lines
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
>  builtin/repack.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/builtin/repack.c b/builtin/repack.c
> index 529e13120d..8a5bbb9cba 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -51,7 +51,7 @@ struct pack_objects_args {
>  	const char *window_memory;
>  	const char *depth;
>  	const char *threads;
> -	const char *max_pack_size;
> +	unsigned long max_pack_size;
>  	int no_reuse_delta;
>  	int no_reuse_object;
>  	int quiet;
> @@ -242,7 +242,7 @@ static void prepare_pack_objects(struct child_process *cmd,
>  	if (args->threads)
>  		strvec_pushf(&cmd->args, "--threads=%s", args->threads);
>  	if (args->max_pack_size)
> -		strvec_pushf(&cmd->args, "--max-pack-size=%s", args->max_pack_size);
> +		strvec_pushf(&cmd->args, "--max-pack-size=%lu", args->max_pack_size);
>  	if (args->no_reuse_delta)
>  		strvec_pushf(&cmd->args, "--no-reuse-delta");
>  	if (args->no_reuse_object)
> @@ -946,7 +946,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
>  				N_("limits the maximum delta depth")),
>  		OPT_STRING(0, "threads", &po_args.threads, N_("n"),
>  				N_("limits the maximum number of threads")),
> -		OPT_STRING(0, "max-pack-size", &po_args.max_pack_size, N_("bytes"),
> +		OPT_MAGNITUDE(0, "max-pack-size", &po_args.max_pack_size,
>  				N_("maximum size of each packfile")),
>  		OPT_BOOL(0, "pack-kept-objects", &pack_kept_objects,
>  				N_("repack objects in packs marked with .keep")),
> -- 
> 2.42.0.310.g9604b54f73.dirty
> 
Previous: Taylor BlauNext: Taylor Blau
Message 12 of 21 in “repack: implement `--cruft-max-size`”
  1. 0/2 repack: implement `--cruft-max-size`Taylor Blau, Sep 7, 2023
  2. 1/2 t7700: split cruft-related tests to t7704Taylor Blau, Sep 7, 2023
  3. Eric SunshineSep 8, 2023
  4. 2/2 builtin/repack.c: implement support for `--cruft-max-size`Taylor Blau, Sep 7, 2023
  5. Junio C HamanoSep 7, 2023
  6. Taylor BlauSep 25, 2023
  7. Patrick SteinhardtSep 8, 2023
  8. Taylor BlauOct 2, 2023
  9. 0/3 repack: implement `--cruft-max-size`Taylor Blau, Oct 3, 2023
  10. 1/3 t7700: split cruft-related tests to t7704Taylor Blau, Oct 3, 2023
  11. 2/3 builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDETaylor Blau, Oct 3, 2023
  12. Patrick SteinhardtOct 5, 2023
  13. Taylor BlauOct 5, 2023
  14. Junio C HamanoOct 5, 2023
  15. 3/3 builtin/repack.c: implement support for `--max-cruft-size`Taylor Blau, Oct 3, 2023
  16. Patrick SteinhardtOct 5, 2023
  17. Taylor BlauOct 5, 2023
  18. Junio C HamanoOct 5, 2023
  19. repack: free existing_cruft array after useJeff King, Oct 7, 2023
  20. Taylor BlauOct 9, 2023
  21. Junio C HamanoOct 9, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.