Re: [PATCH 1/9] pack-objects: allow `--filter` without `--stdout`

[Taylor Blau <me@ttaylorr.com>]

On Wed, Jun 14, 2023 at 09:25:33PM +0200, Christian Couder wrote:
> 9535ce7337 (pack-objects: add list-objects filtering, 2017-11-21)
> taught `git pack-objects` to use `--filter`, but required the use of
> `--stdout` since a partial clone mechanism was not yet in place to
> handle missing objects. Since then, changes like 9e27beaa23
> (promisor-remote: implement promisor_remote_get_direct(), 2019-06-25)
> and others added support to dynamically fetch objects that were missing.
>
> Even without a promisor remote, filtering out objects can also be useful
> if we can put the filtered out objects in a separate pack, and in this
> case it also makes sense for pack-objects to write the packfile directly
> to an actual file rather than on stdout.
>
> Remove the `--stdout` requirement when using `--filter`, so that in a
> follow-up commit, repack can pass `--filter` to pack-objects to omit
> certain objects from the resulting packfile.

Makes sense.

Is there any situation in which using --stdout with --filter would be a
potential foot-gun? I am not as familiar with the partial clone
mechanism as others CC'd, so I have no idea one way or the other.

If it is unsafe in certain situations (or, at the very least, could
produce surprising behavior), it may be worthwhile to only allow
`--filter=<filter> --stdout` with some kind of
`--filter-to-stdout-is-ok` flag to indicate that the caller knows what
they are doing.

Presumably 'git repack --filter' would pass such a flag later on in the
series.

> Signed-off-by: John Cai <johncai86@gmail.com>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/git-pack-objects.txt | 4 ++--
>  builtin/pack-objects.c             | 8 ++------
>  2 files changed, 4 insertions(+), 8 deletions(-)

Should there be a trivial test here? I'm thinking something on the order
of writing a filtered pack to stdout and redirecting it to a file,
moving it into place, and then indexing the pack to make sure that we
got the expected set of objects.

Thanks,
Taylor