Re: [PATCH 4/4] midx docs: clarify tie breaking

From: Taylor Blau <me@ttaylorr.com>
Date: May 20, 2025, 18:07 UTC
Message-ID: <aCzE58GaA0uviPSs@nand.local>
In-Reply-To: <29769df1c601c77031a27f3b3e5b571d5d7d043e.1747753388.git.phillip.wood@dunelm.org.uk>

On Tue, May 20, 2025 at 04:04:27PM +0100, Phillip Wood wrote:

Show 10 quoted lines

> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> Clarify what happens when an object exists in more than one pack, but
> not in the preferred pack. If the user does not pass a preferred pack
> then the pack with the lowest mtime is chosen as the preferred pack. For
> objects that are not in the preferred pack the pack with the highest
> mtime is used. "git multi-pack-index repack" relies on this behavior. If
> ties were resolved in favor of the oldest pack as the current
> documentation suggests the multi-pack index would not reference any of
> the objects in the pack created by "git multi-pack-index repack".

This commit message could likely be shortened since it is repeating some information from the patch content itself, but I don't have a strong opinion here.

Show 19 quoted lines

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  Documentation/git-multi-pack-index.adoc | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-multi-pack-index.adoc b/Documentation/git-multi-pack-index.adoc
> index 631d5c7d15c..1f016b2f682 100644
> --- a/Documentation/git-multi-pack-index.adoc
> +++ b/Documentation/git-multi-pack-index.adoc
> @@ -40,8 +40,10 @@ write::
>  	--preferred-pack=<pack>::
>  		Optionally specify the tie-breaking pack used when
>  		multiple packs contain the same object. `<pack>` must
> -		contain at least one object. If not given, ties are
> -		broken in favor of the pack with the lowest mtime.
> +		contain at least one object. If not given the pack with
> +		the lowest mtime is used as the preferred pack. Ties
> +		for objects that are not contained in the preferred
> +		are resolved in favor of the pack with the newest mtime.

I think the clarification here is good, but the structure makes it a little difficult to follow. The above reads to me like:

    1. What does --preferred-pack do?
    2. What restrictions are there on the pack?
    3. What happens if --preferred-pack is not given?
    4. What happens if the preferred pack does not contain the object?

But I think it might be clearer to structure this like:

    1. What does --preferred-pack do for objects in the preferred pack?
    2. What happens if the preferred pack does not contain the object?
    3. What happens if --preferred-pack is not given?
    4. What restrictions are there on the pack?

I tried to write something like this below:

    When specified, break ties in favor of this pack when there are
    additional copies of its objects in other packs. Ties for objects
    not found in the preferred pack are resolved in favor of the copy in
    the pack with the highest mtime. If unspecified, the pack with the
    lowest mtime is used by default. The preferred pack must have at
    least one object.

I think that the result here is a little easier to follow than what's proposed above, but I am obviously biased ;-). If you think the two are equivalent or mine is less clear than yours, feel free to ignore this.

Thanks, Taylor