Re: [PATCH v2] doc: remove misleading documentation on pack names

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 22, 2020, 21:09 UTC
Message-ID: <xmqq5zaf5ka6.fsf@gitster.c.googlers.com>
In-Reply-To: <20200722202629.109277-1-johannes@sipsolutions.net>

Johannes Berg <johannes@sipsolutions.net> writes:

Show 10 quoted lines

> The index-pack documentation explicitly states that the pack
> name is derived from the sorted list of object names, but
> that clearly isn't true. I can't seem to be able to figure
> out if this was ever changed though.
>
> Be less explicit in the docs as to what the exact output is,
> and just say that it's whatever goes into the pack name.
>
> Really it seems to be the sha1 of the entire file, without
> the checksum footer.

Please avoid "seems to be" and spend a bit of effort digging the history especially when we are not in a hurry to get to the definite answer. We can go "less explicit", or be a bit more informative by saying that it is the trailer hash that is standard practice shared across our binary files like the index and the packfile.

I think this is 1190a1ac (pack-objects: name pack files after trailer hash, 2013-12-05). It forgot to update the comment before write_idx_file() function when it did this change:

 /*
  * On entry *sha1 contains the pack content SHA1 hash, on exit it is
  * the SHA1 hash of sorted object names. The objects array passed in
  * will be sorted by SHA1 on exit.
  */
 const char *write_idx_file(const char *index_name, struct pack_idx_entry **objects,
 			   int nr_objects, const struct pack_idx_option *opts,
-			   unsigned char *sha1)
+			   const unsigned char *sha1)
 {

Obviously, after it turned *sha1 into 'const', it no longer is possible for it to have anything different from what was passed in upon exit.

Show 5 quoted lines

> +Once the index has been created, the hash that goes into the name of
> +the pack/idx file is printed to stdout. If --stdin was also used then
> +this is prefixed by either "pack\t", or "keep\t" if a new .keep file
> +was successfully created. This is useful to remove a .keep file used
> +as a lock to prevent the race with 'git repack' mentioned above.

The change is good---I made sure that among these filve lines, what changed was only the first one and half lines. I however would have preferred not to see the line rewrapping.

Thanks.