Re: [PATCH] p5332: drop "+" from --stdin-packs input

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>
> -- >8 --
> Subject: [PATCH] p5332: drop "+" from --stdin-packs input
>
> This perf script creates a midx by running "git multi-pack-index write"
> with the "--stdin-packs" option. We feed that stdin by running "find" on
> .git/objects/pack, using sed to strip off everything but the basename.
>
> But that sed invocation also does something peculiar: it adds a "+" to
> the start of each pack name. This causes the multi-pack-index command to
> barf. The modified name does not match any pack it knows about, so it
> ends up with an empty list of packs to put in the midx. And thus nothing
> matches the --preferred-pack option we pass, which causes it die().
>
> The fix is to remove the extra "+" (which also lets us simplify the sed
> invocation a bit, as it is now just stripping the leading directories).
>
> But that leaves the mystery of why it was ever there in the first place.
> The answer is that an earlier iteration of the patch series had a
> concept of "disjoint" packs in the midx. And one of its patches here:
>
>   https://lore.kernel.org/git/c52d7e7b27a9add4f58b8334db4fe4498af1c90f.1701198172.git.me@ttaylorr.com/
>
> taught read_packs_from_stdin() to treat a leading "+" as marking a
> disjoint pack. But in the second version of the series, which was
> ultimately merged, that disjoint concept went away, and the code to
> parse "+" did likewise. The regular regression tests were adjusted to
> match, but this case in t/perf was forgotten.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/perf/p5332-multi-pack-reuse.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks.  I wonder if we had some tools and mechanisms people were
discussing to track changes on changsets, such a mishap could have
been caught more easily.  [jc: random folks from that discussion
CC'ed, just in case they are interested].

>
> diff --git a/t/perf/p5332-multi-pack-reuse.sh b/t/perf/p5332-multi-pack-reuse.sh
> index d1c89a8b7d..0a2525db44 100755
> --- a/t/perf/p5332-multi-pack-reuse.sh
> +++ b/t/perf/p5332-multi-pack-reuse.sh
> @@ -58,7 +58,7 @@ do
>  	'
>  
>  	test_expect_success "setup bitmaps for $nr_packs-pack scenario" '
> -		find $packdir -type f -name "*.idx" | sed -e "s/.*\/\(.*\)$/+\1/g" |
> +		find $packdir -type f -name "*.idx" | sed -e "s/.*\///" |
>  		git multi-pack-index write --stdin-packs --bitmap \
>  			--preferred-pack="$(find_pack $(git rev-parse HEAD))"
>  	'