Re: [GSoC PATCH v3 4/5] t7700: test for promisor file content after repack

[Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com>]

On Wed, Apr 08, 2026 at 01:11:42AM +0200, Lorenzo Pegorari wrote:
> On Tue, Apr 07, 2026 at 11:10:02AM -0700, Junio C Hamano wrote:
> > LorenzoPegorari <lorenzo.pegorari2002@gmail.com> writes:
> > 
> > > Add tests that checks if the content of ".promisor" files are correctly
> > > copied inside the ".promisor" files created by a repack.
> > >
> > > Signed-off-by: LorenzoPegorari <lorenzo.pegorari2002@gmail.com>
> > > ---
> > >  t/t7700-repack.sh | 63 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 63 insertions(+)
> > >
> > > diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
> > > index 63ef63fc50..89a2116641 100755
> > > --- a/t/t7700-repack.sh
> > > +++ b/t/t7700-repack.sh
> > > @@ -904,4 +904,67 @@ test_expect_success 'pending objects are repacked appropriately' '
> > >  	)
> > >  '
> > >  
> > > +test_expect_success 'check one .promisor file content after repack' '
> > > +	test_when_finished rm -rf prom_test &&
> > > +	git init prom_test &&
> > > +	path=prom_test/.git/objects/pack &&
> > > +
> > > +	(
> > > +		test_commit_bulk -C prom_test --start=1 1 &&
> > > +		
> > > +		# Simulate .promisor file by creating it manually
> > > +		prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
> > > +		oid=$(git -C prom_test rev-parse HEAD) &&
> > > +		echo "$oid ref" >$prom &&
> > > +
> > > +		# Save the current .promisor content, repack, and check if correct
> > > +		prom_before_repack=$(cat $prom) &&
> > > +		git -C prom_test repack -a -d &&
> > > +		prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
> > > +		# $prom should contain "$prom_before_repack <date>"
> > > +		test_grep "$prom_before_repack " $prom &&
> > > +
> > > +		# Save the current .promisor content, repack, and check if correct
> > > +		cat $prom >prom_before_repack &&
> > > +		git -C prom_test repack -a -d &&
> > > +		prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
> > > +		# $prom should be exactly the same as prom_before_repack
> > > +		test_cmp prom_before_repack $prom
> > > +	)
> > > +'
> > > +
> > > +test_expect_success 'check multiple .promisor file content after repack' '
> > > +...
> > > +
> > > +		# Repack, and check if correct compared to previous saved .promisor content
> > > +		git -C prom_test repack -a -d &&
> > > +		prom=$(ls $path/*.pack | sed "s/\.pack/.promisor/") &&
> > > +		# $prom should contain "$prom_before_repack1 <date>" & "$prom_before_repack2 <date>"
> > > +		test_grep "$prom_before_repack1 " $prom &&
> > > +		test_grep "$prom_before_repack2 " $prom &&
> > 
> > This test seems to be flakey.
> > 
> >   https://github.com/git/git/actions/runs/24095497271/job/70292906676#step:10:5274
> > 
> > shows that $prom gets two file names, and because test_grep is
> > expecting a single source to grep inside, the first test_grep
> > fails.
> 
> Uff yeah, I see.
> 
> I also saw your other mail regarding the "SQUASH???" commit you made
> (inside the `seen` branch). I'm not so sure if it is useful to solve
> this issue tho.
> 
> It looks like, for some reason, `repack -a` fails to repack everything
> into a single pack, but I believe that `repack -a -f` should force it to
> repack everything no matter what (I think??).

Ok, I think that I have fixed it. Forked the repo and tested multiple
times with the GitHub Actions-based CI. Issue doesn't appear anymore.

Thank you super much for the help Junio.

May I ask you for a tiny bit more of your time to answer the following
question that I originally asked in the cover letter:

>The "CodingGuidelines" explicitly state that:
>  "A C file must directly include the header files that declare the
>   functions and the types it uses, except for the functions and types
>   that are made available to it by including one of the header files
>   it must include by the previous rule"
> where "the previous rule" is (if I understand correctly), the one related
> to "<git-compat-util.h>". From what I understand then, I should have
> added an include for "strmap.h" (which is needed for `strset`), correct?
> And if I am correct, shouldn't "strbuf.h", "hash.h", "odb.h",
> "string-list.h" and "strvec.h" also be included?

Thank you so much in advance. Pretty sure the next patch version will be
the last one.

Lorenzo