Re: [PATCH v7 08/10] docs: move pack format docs to man section 5

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 3, 2022, 16:25 UTC
Message-ID: <xmqqv8r9b8gm.fsf@gitster.g>
In-Reply-To: <patch-v7-08.10-3505fa86039-20220802T125258Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 6 quoted lines

> @@ -125,7 +139,7 @@ Git index format
>      entry is encoded as if the path name for the previous entry is an
>      empty string).  At the beginning of an entry, an integer N in the
>      variable width encoding (the same encoding as the offset is encoded
> -    for OFS_DELTA pack entries; see pack-format.txt) is stored, followed
> +    for OFS_DELTA pack entries; see linkgit:gitformat-pack[5]) is stored, followed

Possible #leftoverbits for cleaning up around here are:

 * Update the above description to use the term "offset encoding"
   from gitformat-patck[5] to give readers the term to look for;

 * Update <varint.h> and mention that the encode/decode pair
   declared there are to handle variable-length integers in the
   "offset encoding".

 * We might want to add an entry to the glossary, but the varint
   representation is not end-user facing, so it may not be necessary
   and the mention in <varint.h> would be enough.

Show 20 quoted lines

> diff --git a/Documentation/technical/cruft-packs.txt b/Documentation/gitformat-pack-cruft.txt
> similarity index 96%
> rename from Documentation/technical/cruft-packs.txt
> rename to Documentation/gitformat-pack-cruft.txt
> index d81f3a8982f..908f752bd84 100644
> --- a/Documentation/technical/cruft-packs.txt
> +++ b/Documentation/gitformat-pack-cruft.txt
> @@ -1,4 +1,17 @@
> -= Cruft packs
> +gitformat-pack-cruft(5)
> +=======================
> +
> +NAME
> +----
> +gitformat-pack-cruft - The cruft pack file format
> +
> +SYNOPSIS
> +--------
> +[verse]
> +$GIT_DIR/objects/pack/pack-*.mtimes

I do not think this is quite right.

A reader who wants to learn about cruft packs, they need to know:

 * what a packfile is and what is in .pack/.idx pair, as a
   prerequisite information, that is described elsewhere

 * what it is, what it is used for, and how it is meant to be used,
   why the mtimes from the original loose objects need to be kept,
   all of which is not file-format per-se, but is part of the
   design, described in the original technical/cruft-packs.txt

 * how .mtimes file records the _additional_ pieces of per-object
   information only maintained for the "cruft pack" objects.

The original technical/cruft-packs.txt document was the design document for the cruft packs feature as a whole, and it made sense to write about both design and the implementation detail of the mtimes format.

But that is not "cruft pack file format". I think this can be fixed either by having at least two documents (one for "cruft packs" overall design, plus another one for "mtimes file format"), or by having one document that is clearly more than "file format". The posted patch gives us a mixture of the two, that results in neither of the two.

Stepping back a bit, unless we have separate .pack(5) and .idx(5) manual pages, I suspect that having the format of .pack, .idx, .midx, and .mtimes described in the same "file formats for packfiles" document might make more sense. After all, those who come to the packfile format document from "pack protocol" document do not mind that the former talks about .idx file, which they would find no use for.

I hope Taylor can help us with his input when he comes back from his honeymoon.

Show 16 quoted lines

> diff --git a/Documentation/technical/pack-format.txt b/Documentation/gitformat-pack.txt
> similarity index 95%
> rename from Documentation/technical/pack-format.txt
> rename to Documentation/gitformat-pack.txt
> index b520aa9c45b..546c99f8871 100644
> --- a/Documentation/technical/pack-format.txt
> +++ b/Documentation/gitformat-pack.txt
> @@ -1,5 +1,29 @@
> -Git pack format
> -===============
> +gitformat-pack(5)
> +=================
> +
> +NAME
> +----
> +gitformat-pack - Git pack format

OK.

Show 17 quoted lines

> diff --git a/command-list.txt b/command-list.txt
> index ed859fdd798..4f30a6c30c8 100644
> --- a/command-list.txt
> +++ b/command-list.txt
> @@ -210,7 +210,12 @@ gitdiffcore                             guide
>  giteveryday                             guide
>  gitfaq                                  guide
>  gitformat-bundle                        developerinterfaces
> +gitformat-chunk                         developerinterfaces
>  gitformat-commit-graph                  developerinterfaces
> +gitformat-index                         developerinterfaces
> +gitformat-pack                          developerinterfaces
> +gitformat-pack-cruft                    developerinterfaces
> +gitformat-signature                     developerinterfaces
>  gitglossary                             guide
>  githooks                                userinterfaces
>  gitignore                               userinterfaces

OK.