Re: [GSoC PATCH v2 3/4] repack-promisor: preserve content of promisor files after repack

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

On Mon, Mar 23, 2026 at 02:48:21PM -0700, Junio C Hamano wrote:
> LorenzoPegorari <lorenzo.pegorari2002@gmail.com> writes:
> 
> > @@ -40,6 +40,7 @@ static void finish_repacking_promisor_objects(struct repository *repo,
> >  					      const char *packtmp)
> >  {
> >  	struct strbuf line = STRBUF_INIT;
> > +	int is_first_promisor = 1;
> >  	FILE *out;
> > ... 
> > +		/*
> > +		 * Fetch-pack sometimes generates non-empty .promisor files
> > +		 * containing the ref names and associated hashes at the point of
> > +		 * generation of the corresponding packfile. These pieces of info
> > +		 * are only used for debugging reasons. In order to preserve
> > +		 * these, let's copy the contents of all .promisor files in the
> > +		 * first promisor file created.
> > +		 */
> > +		if (is_first_promisor) {
> > +			copy_all_promisor_files(repo, promisor_name);
> > +			is_first_promisor = 0;
> > +		}
> > +
> 
> Here the underlying assumption seems to be that whichever one of the
> two potential callers of this function, repack_promisor_objects()
> and pack_geometry_repack_promisors(), would handle all the existing
> packs with corresponding .promisor file so it is safe to coalesce
> all the debugging comments from all the existing .promisor files
> into one?
> 
> Is it really true, though?  Especially with geometry repacking
> enabled, wouldn't a regular repack coalesce only the smallish ones
> into a single pack while leaving an already largeish ones intact, or
> something?
> 
> Thanks.

I will look into this. I'm going to drastically rework this patch
series, so that the next version will be much better and better
explained.

Thank you so much for the time,

Lorenzo