Re: [PATCH v5 5/5] builtin/merge-tree.c: implement support for `--write-pack`

[Taylor Blau <me@ttaylorr.com>]

On Wed, Oct 25, 2023 at 09:58:11AM +0200, Patrick Steinhardt wrote:
> > +test_expect_success 'merge-tree can pack its result with --write-pack' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +
> > +	# base has lines [3, 4, 5]
> > +	#   - side adds to the beginning, resulting in [1, 2, 3, 4, 5]
> > +	#   - other adds to the end, resulting in [3, 4, 5, 6, 7]
> > +	#
> > +	# merging the two should result in a new blob object containing
> > +	# [1, 2, 3, 4, 5, 6, 7], along with a new tree.
> > +	test_commit -C repo base file "$(test_seq 3 5)" &&
> > +	git -C repo branch -M main &&
> > +	git -C repo checkout -b side main &&
> > +	test_commit -C repo side file "$(test_seq 1 5)" &&
> > +	git -C repo checkout -b other main &&
> > +	test_commit -C repo other file "$(test_seq 3 7)" &&
> > +
> > +	find repo/$packdir -type f -name "pack-*.idx" >packs.before &&
> > +	tree="$(git -C repo merge-tree --write-pack \
> > +		refs/tags/side refs/tags/other)" &&
> > +	blob="$(git -C repo rev-parse $tree:file)" &&
> > +	find repo/$packdir -type f -name "pack-*.idx" >packs.after &&
>
> While we do assert that we write a new packfile, we don't assert whether
> parts of the written object may have been written as loose objects. Do
> we want to tighten the checks to verify that?

We could, but the tests in t1050 already verify this, so I'm not sure
that we would be significantly hardening our test coverage here. If you
feel strongly about it, though, I'm happy to change it up.

Thanks,
Taylor