From: ZheNing Hu <adlternative@gmail.com>
Date: Wed, 15 Jan 2025 08:08:54 GMT
Subject: Re: [PATCH] builtin/repack.c: prune unreachable objects with `--expire-to`
Message-ID: <CAOLTT8R3UULA9xrv8FZcTsTE1qvsToU=WgOEWMfuO0vq5ztUAw@mail.gmail.com>
In-Reply-To: <20241201213439.GA145938@coredump.intra.peff.net>

> OK, so we are adding the containing directory as an alternate. That
> gives me two concerns:
>
>   1. The expire-to string is something like "path/to/objects/pack/pack",
>      and we'll have created "path/to/objects/pack/pack-<hash>.pack"
>      Using dirname() strips that down to "path/to/objects/pack". OK. And
>      then we manually strip "pack/" off the end, which we have to do to
>      get the "base" objects/ directory.
>
>      But what if the path given by the user via --expire-to doesn't look
>      like an object directory? I.e., does not end in "pack/"? Then this
>      feature would not work at all.
>
>      Should we be mentioning this in the git-repack docs?
>
>      As an aside, I think the current --expire-to docs are misleading.
>      They say:
>
>        --expire-to=<dir>
>            Write a cruft pack containing pruned objects (if any) to the
>            directory <dir>. [...]
>
>      But that isn't right. It is not a <dir> but a <base-name> similar
>      to the one that pack-objects takes. If you do --expire-to=some/dir,
>      then you'll get some/dir-<hash>.pack.
>

I agree. The `--expire-to=<dir>` option can easily cause confusion for users.
For example, using `--expire-to=xxx.git/objects/pack` will actually generate
files like xxx.git/objects/pack-*.{mtimes,idx,pack} instead of placing them
in xxx.git/objects/pack/pack-*.{mtimes,idx,pack}.

--
ZheNing Hu