Re: [PATCH v2 07/13] repack: add --path-walk option

[Taylor Blau <me@ttaylorr.com>]

On Mon, Mar 24, 2025 at 03:22:43PM +0000, Derrick Stolee via GitGitGadget wrote:
> diff --git a/Documentation/git-repack.adoc b/Documentation/git-repack.adoc
> index 5852a5c9736..2199eb3b94f 100644
> --- a/Documentation/git-repack.adoc
> +++ b/Documentation/git-repack.adoc
> @@ -11,7 +11,7 @@ SYNOPSIS
>  [verse]
>  'git repack' [-a] [-A] [-d] [-f] [-F] [-l] [-n] [-q] [-b] [-m]
>  	[--window=<n>] [--depth=<n>] [--threads=<n>] [--keep-pack=<pack-name>]
> -	[--write-midx] [--name-hash-version=<n>]
> +	[--write-midx] [--name-hash-version=<n>] [--path-walk]
>
>  DESCRIPTION
>  -----------
> @@ -255,6 +255,18 @@ linkgit:git-multi-pack-index[1]).
>  	Provide this argument to the underlying `git pack-objects` process.
>  	See linkgit:git-pack-objects[1] for full details.
>
> +--path-walk::
> +	This option passes the `--path-walk` option to the underlying
> +	`git pack-options` process (see linkgit:git-pack-objects[1]).
> +	By default, `git pack-objects` walks objects in an order that
> +	presents trees and blobs in an order unrelated to the path they
> +	appear relative to a commit's root tree. The `--path-walk` option
> +	enables a different walking algorithm that organizes trees and
> +	blobs by path. This has the potential to improve delta compression
> +	especially in the presence of filenames that cause collisions in
> +	Git's default name-hash algorithm. Due to changing how the objects
> +	are walked, this option is not compatible with `--delta-islands`
> +	or `--filter`.

I was going to make a similar comment here as I did in an earlier commit
message about describing the direct effect of a command-line flag
instead of describing it in contrast to the default behavior. But I
think here that I would recommend instead abbreviating the description
to just:

    --path-walk::
    Pass the `--path-walk` option to `git pack-objects`, see
    linkgit:git-pack-objects[1].

, which is consistent with options like `-l`, `-f`, -F`, etc.

Does it makes sense to have a separate `--[no-]cruft-path-walk` here to
let callers determine separate behavior when repack is tasked with
generating cruft packs?

Thanks,
Taylor