Re: [PATCH v4 1/4] pack-write: make index-pack output comment hash agnostic

[Justin Tobler <jltobler@gmail.com>]

On 26/10/26 08:01AM, Karthik Nayak wrote:
> Justin Tobler <jltobler@gmail.com> writes:
> 
> >  	/*
> > -	 * The first thing we expect from index-pack's output
> > -	 * is "pack\t%40s\n" or "keep\t%40s\n" (46 bytes) where
> > -	 * %40s is the newly created pack SHA1 name.  In the "keep"
> > -	 * case, we need it to remove the corresponding .keep file
> > -	 * later on.  If we don't get that then tough luck with it.
> > +	 * The first thing we expect from index-pack's output is
> > +	 * "pack\t<name>\n" or "keep\t<name>\n", where <name> is the hexadecimal
> > +	 * name of the packfile it has just written. The line is therefore five
> > +	 * bytes of prefix, `hexsz` bytes of name, and a trailing newline. A
> > +	 * "keep" prefix tells us that index-pack has also created the
> > +	 * corresponding ".keep" file.
> > +	 *
> > +	 * In the "keep" case, we need it to remove the corresponding ".keep"
> > +	 * file later on. If we don't get that then tough luck with it.
> 
> We start off saying we expect either '"pack\t<name>\n" or
> "keep\t<name>\n"'. But the last line makes it seem like the latter is
> always expected? Is that the case, or should this comment be moved to
> the caller in 'fetch-pack.c'?

Even though all callers of this function specify the `--keep` option to
the git-index-pack(1) generating the output, it is expected that the
output may be either "pack" or "keep". When git-index-pack(1) tries to
create a lockfile, but one already exists, it simply ignores the error
and returns a "pack" output line. The last line is from the original
comment and indicates that when we do identify that a ".keep" file was
created, it is something the caller must handle later on.

The intent of this patch was just to update the comment to be hash
agnostic. The next patch splits out the `read_index_pack_report()`
function from `index_pack_lockfile()` and the last comment line is
removed accordingly. So I'm inclined to leave this patch as-is.

-Justin