Re: [PATCH] fixup! repack: exclude cruft pack(s) from the MIDX where possible

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 19, 2025, 23:26 UTC
Message-ID: <aFSctDdIfvum+YtH@nand.local>
In-Reply-To: <xmqqbjqjace5.fsf@gitster.g>

On Thu, Jun 19, 2025 at 10:07:14AM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
>
> > In a previous commit, `grep -o` was introduced as part of t7704.
> >
> > POSIX doesn't have that flag as defined and while it is a popular
> > one is not available at least in the latest release of AIX.
> >
> > Use a sed equivalent that ought to be more portable.
>
> OK.  The patterns are not exactly the same but as long as we know
> $packdir does *not* contain a substring "pack-", it should be OK.

Good catch; thanks, both.

> As the topic is not even in 'next', perhaps a refresh can squash
> this change in?

Yep, I'll squash this in and adjust the other "grep -o" use in this series.

Show 11 quoted lines

> >  		ls $packdir/pack-*.idx | sort >packs.all &&
> > -		grep -o "pack-.*\.idx$" packs.all >in &&
> > +		sed -n "s/.*\(pack-.*\.idx\)$/\1/p" packs.all >in &&
> >  		git multi-pack-index write --stdin-packs --bitmap <in &&
>
> I do not quite see the need for temporary files or "grep/sed" here,
> though.
>
> 		(cd "$packdir" && ls pack-*.idx) |
> 		sort |
> 		git multi-pack-index write --stdin-packs --bitmap &&

The grep/sed is unnecessary (I was just trying to be clever and avoid a sub-shell), but the packs.all temporary file is still needed since we run comm against it later on in the test script.

Show 5 quoted lines

> Tangent to this discussion, but I just noticed that
>
>     $ git multi-pack-index -h
>
> lacks quite a lot of information.  Perhaps it needs updating?

Definitely some good #leftoverbits ;-).

Thanks, Taylor