{"thread":{"id":"58682","subject":"[PATCH 2/2] fsck: document msg-id","startedAt":"2022-10-24T16:16:06Z","lastAt":"2022-10-28T11:15:00Z","messageCount":35,"participants":["John Cai via GitGitGadget","Junio C Hamano","John Cai","Jeff King","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"465619","messageId":"3aec3d2c9ca65a37a367c3a7c9081bbd4cd44ae0.1666623639.git.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":"pull.1369.git.git.1666623639.gitgitgadget@gmail.com","subject":"[PATCH 2/2] fsck: document msg-id","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-24T15:00:38Z","receivedAt":"2022-10-24T16:16:06Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai \n\nThe git-config documentation lacks mention of specific that\nare supported. While git-help --config will display a list of these options,\noften developers' first instinct is to consult the git docs to find valid\nconfig values.\n\nAdd a section under the docs for fsck. with the msg-ids that\ngit-fsck recognizes.\n\nSigned-off-by: John Cai \n---\n Documentation/config/fsck.txt | 5 ++\n Documentation/fsck-msgids.txt | 133 ++++++++++++++++++++++++++++++++++\n 2 files changed, 138 insertions(+)\n create mode 100644 Documentation/fsck-msgids.txt\n\ndiff --git a/Documentation/config/fsck.txt b/Documentation/config/fsck.txt\nindex 450e8c38e34..b0632075f22 100644\n--- a/Documentation/config/fsck.txt\n+++ b/Documentation/config/fsck.txt\n@@ -35,6 +35,11 @@ allow new instances of the same breakages go unnoticed.\n Setting an unknown `fsck.` value will cause fsck to die, but\n doing the same for `receive.fsck.` and `fetch.fsck.`\n will only cause git to warn.\n++\n+The following `` are supported:\n++\n+\n+include::../fsck-msgids.txt[]\n \n fsck.skipList::\n \tThe path to a list of object names (i.e. one unabbreviated SHA-1 per\ndiff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt\nnew file mode 100644\nindex 00000000000..888fa3308b7\n--- /dev/null\n+++ b/Documentation/fsck-msgids.txt\n@@ -0,0 +1,133 @@\n+--\n+`badDate`;;\n+\tInvalid date format in an author/committer line.\n+\n+`badEmail`;;\n+\tInvalid email format in an author/committer line.\n+\n+`badFilemode`;;\n+\tA tree contains a bad filemode entry.\n+\n+`badName`;;\n+\tAn author/committer name is empty.\n+\n+`badObjectSha1`;;\n+\tAn object has a bad sha1.\n+\n+`badParentSha1`;;\n+\tA commit object has a bad parent sha1.\n+\n+`badTagName`;;\n+\tA tag has an invalid format.\n+\n+`badTimezone`;;\n+\tFound an invalid time zone in an author/committer line.\n+\n+`badTree`;;\n+\tA tree cannot be parsed.\n+\n+`badTreeSha1`;;\n+\tA tree has an invalid format.\n+\n+`badType`;;\n+\tFound an invalid object type.\n+\n+`duplicateEntries`;;\n+\tA tree contains duplicate file entries.\n+\n+`emptyName`;;\n+\tA path contains an empty name.\n+\n+`fullPathName`;;\n+\tA path contains the full path starting with \"/\".\n+\n+`gitAttributesSymlink`;;\n+\t`.gitattributes` is a symlink.\n+\n+`gitignoreSymlink`;;\n+\t`.gitignore` is a symlink.\n+\n+`gitmodulesBlob`;;\n+\tA non-blob found at `.gitmodules`.\n+\n+`gitmodulesMissing`;;\n+\tUnable to read `.gitmodules` blob.\n+\n+`gitmodulesName`;;\n+\tA submodule name is invalid.\n+\n+`gitmodulesParse`;;\n+\tCould not parse `.gitmodules` blob.\n+\n+`gitmodulesLarge`;\n+\t`.gitmodules` blob is too large to parse.\n+\n+`gitmodulesPath`;;\n+\t`.gitmodules` path is invalid.\n+\n+`gitmodulesSymlink`;;\n+\t`.gitmodules` is a symlink.\n+\n+`gitmodulesUpdate`;;\n+\tFound an invalid submodule update setting.\n+\n+`gitmodulesUrl`;;\n+\tFound an invalid submodule url.\n+\n+`hasDot`;;\n+\tA tree contains an entry named `.`.\n+\n+`hasDotdot`;;\n+\tA tree contains an entry named `..`.\n+\n+`hasDotgit`;;\n+\tA tree contains an entry named `.git`.\n+\n+`mailmapSymlink`;;\n+\t`.mailmap` is a symlink.\n+\n+`missingAuthor`;;\n+\tAuthor is missing.\n+\n+`missingCommitter`;;\n+\tCommitter is missing.\n+\n+`missingEmail`;;\n+\tEmail is missing in an author/committer line.\n+\n+`missingNameBeforeEmail`;;\n+\tMissing space before an email in an author/committer line.\n+\n+`missingObject`;;\n+\tMissing `object` line in tag object.\n+\n+`missingSpaceBeforeDate`;;\n+\tMissing space before date in an author/committer line.\n+\n+`missingSpaceBeforeEmail`;;\n+\tMissing space before the email in author/committer line.\n+\n+`missingTag`;;\n+\tUnexpected end after `type` line in a tag object.\n+\n+`missingTypeEntry`;;\n+\tMissing `type` line in a tag object.\n+\n+`multipleAuthors`;;`\n+\tMultiple author lines found in a commit.\n+\n+`nulInCommit`;;\n+\tFound a NUL byte in the commit object body.\n+\n+`treeNotSorted`;;\n+\tA tree is not properly sorted.\n+\n+`unknownType`;;\n+\tFound an unknown object type.\n+\n+`zeroPaddingDate`;;\n+\tFound a zero padded date in an author/commiter line.\n+\n+`zeroPaddedFilemode`;;\n+\tFound a zero padded filemode in a tree.\n+--\n-- \ngitgitgadget\n"},{"id":"465621","messageId":"f32ff5dc4eed5bffe8bb0759c408c3d0861ddb50.1666623639.git.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":"pull.1369.git.git.1666623639.gitgitgadget@gmail.com","subject":"[PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-24T15:00:37Z","receivedAt":"2022-10-24T16:30:08Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai \n\nThe BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\nit.\n\nSigned-off-by: John Cai \n---\n fsck.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/fsck.h b/fsck.h\nindex 6f801e53b1d..7d99f6ea337 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -24,7 +24,6 @@ enum fsck_msg_type {\n \tFUNC(BAD_NAME, ERROR) \\\n \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n-\tFUNC(BAD_TAG_OBJECT, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\n-- \ngitgitgadget\n\n"},{"id":"465622","messageId":"pull.1369.git.git.1666623639.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":null,"subject":"[PATCH 0/2] Document fsck msg ids","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-24T15:00:36Z","receivedAt":"2022-10-24T17:02:41Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"fsck does a number of checks, and prints warning or error messages based on\nthe type of check. The only place the config values that control whether to\nignore these checks are documented is through git-help(1). However, often\ntimes peoples' first instinct to look for a list of valid config values is\nin the documentation page for git-config(1). These fsck. configuration\nvalues can be hard to find.\n\nDocument these so that both git-config and git-fsck documentation has a list\nof valid s.\n\nPatch [1/2] removes an unused msg-id BAD_TAG_OBJECT Patch [2/2] adds a\nfsck-msgids.txt that lists msg-ids that fsck checks for\n\nJohn Cai (2):\n fsck: remove the unused BAD_TAG_OBJECT\n fsck: document msg-id\n\n Documentation/config/fsck.txt | 5 ++\n Documentation/fsck-msgids.txt | 133 ++++++++++++++++++++++++++++++++++\n fsck.h | 1 -\n 3 files changed, 138 insertions(+), 1 deletion(-)\n create mode 100644 Documentation/fsck-msgids.txt\n\n\nbase-commit: 45c9f05c44b1cb6bd2d6cb95a22cf5e3d21d5b63\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1369%2Fjohn-cai%2Fjc%2Fdocument-fsck-msg-id-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1369/john-cai/jc/document-fsck-msg-id-v1\nPull-Request: https://github.com/git/git/pull/1369\n-- \ngitgitgadget\n"},{"id":"465625","messageId":"xmqq8rl5t9ca.fsf@gitster.g","threadId":"58682","inReplyTo":"3aec3d2c9ca65a37a367c3a7c9081bbd4cd44ae0.1666623639.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] fsck: document msg-id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-24T17:33:41Z","receivedAt":"2022-10-24T19:03:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"John Cai via GitGitGadget\" writes:\n\n> From: John Cai \n>\n> The git-config documentation lacks mention of specific that\n> are supported. While git-help --config will display a list of these options,\n> often developers' first instinct is to consult the git docs to find valid\n> config values.\n\nGood observation. \"help --config\" only gives names and no\ndescription, so maintaining the description like this new file does\nmake sense.\n\nCan you also add a referencing comment to fsck.h to tell that folks\nwho add a new error type that they need to update the -msgids.txt\nfile as well?\n\nDoes this list in the new file cover all existing messages, by the\nway?\n\nI also wonder if the order of the entries in this file should be\nalphabetical, unlike the entries in fsck.h where more severe ones\ncome earlier than the less severe ones. In a sense, matching the\norder of two lists makes it easier to maintain, and grouping by\nseverity in the doc might or might not find it easier to scan and\nfind what they are, but one of the biggest reason why users will\ncome to this list is when they see a particular error message and\nwant to understand what that means, no? At that point it would be\neasier to look things up if \n\n (1) the list contains EVERYTHING, even the ones that you are not\n supposed to configure away.\n\n (2) the list is sorted alphabetically, regardless of the severity.\n\nThe first point suggests that it may be a mistake to have the main\nlist appear in the \"what configuration knobs are available for\nsquelching fsck error messages\". It is OK to refer from there to\nthe main list, but the main list should list everything, with\ncomments like \"(this error cannot be configured away)\", no?\n\nIn other words, I think it is better to have a master list of\neverything, with their message ID and textual description, which\nessentially is your fsck-msgids.txt but additionally mention which\nones are by default FATAL and cannot be configured away (even\nbetter, we can mention what severity level each of them is by\ndefault, by mirroring that is in fsck.h).\n\nAnd that master list should NOT be made a part of fsck.\nconfiguration item, like this patch did. It should be a separate\nsection in \"git fsck --help\" output whose section title is \"Fsck\nerrors\" or something. \n\nThen the existing description of fsck. configuration can\nrefer to that \"Fsck errors\" section for what msg types exist.\n\nAnother thing that I noticed while thinking about the patch, but is\nbetter left outside the scope of this patch, is that an attempt to\nconfigure fatal ones away is prevented by fsck.c::fsck_set_msg_type() \nbut \"git help config | grep '^fsck\\.'\" lists them. I think the\n\"help config\" should be fixed.\n\nI almost suggested to extend the FOREACH_FSCK_MSG_ID() definition in\nfsck.h so that fsck-msgids.txt gets auto-generated (what is missing\nin fsck.h that prevents us from doing so is the textual explanation\nyou added in the new file in your patch---they could come from\ncomments on the same line, for example, and can be extracted via a\nPerl or sed script at build time). I do not know if it is a good\nidea, especially if we forsee a future in which we may be\ntranslating the documentation, so decided against making such a\nsuggestion.\n\nBut at least, we could _lint_ the manually curated fsck-msgids.txt\nusing what is in fsck.h to see if a new MSG_ID added to fsck.h is\nmissing from the doc, or a MSG_ID whose default severity is updated\nin fsck.h is stale in the doc, etc. That can be left for the future\nupdates, but we should at least instruct developers to keep them in\nsync in fsck.h by adding a comment.\n\nThanks.\n"},{"id":"465627","messageId":"xmqq4jvtt7cd.fsf@gitster.g","threadId":"58682","inReplyTo":"xmqqwn8ptb0p.fsf@gitster.g","subject":"Re: [PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-24T18:16:50Z","receivedAt":"2022-10-24T19:54:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> \"John Cai via GitGitGadget\" writes:\n>\n>> From: John Cai \n>>\n>> The BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\n>> it.\n>\n> Do you have a ready reference to the change that made it no longer\n> needed (or stopped detecting the error the message was meant to be\n> issued against)?\n\n2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\nchecking the tagged object referred to by a tag object, which the\nerror message BAD_TAG_OBJECT was about. Since then the\nBAD_TAG_OBJECT message is no longer used.\n\n\n\n"},{"id":"465628","messageId":"4FFF6EB5-B9A3-4E32-840C-3FD6E893F5BF@gmail.com","threadId":"58682","inReplyTo":"xmqq4jvtt7cd.fsf@gitster.g","subject":"Re: [PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"John Cai","fromEmail":"johncai86@gmail.com","sentAt":"2022-10-24T18:33:39Z","receivedAt":"2022-10-24T20:17:56Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"Hi Junio,\n\nOn 24 Oct 2022, at 14:16, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n>> \"John Cai via GitGitGadget\" writes:\n>>\n>>> From: John Cai \n>>>\n>>> The BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\n>>> it.\n>>\n>> Do you have a ready reference to the change that made it no longer\n>> needed (or stopped detecting the error the message was meant to be\n>> issued against)?\n>\n> 2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\n> checking the tagged object referred to by a tag object, which the\n> error message BAD_TAG_OBJECT was about. Since then the\n> BAD_TAG_OBJECT message is no longer used.\n\nThanks for doing my work for me :)\n"},{"id":"465629","messageId":"xmqqwn8ptb0p.fsf@gitster.g","threadId":"58682","inReplyTo":"f32ff5dc4eed5bffe8bb0759c408c3d0861ddb50.1666623639.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-24T16:57:26Z","receivedAt":"2022-10-24T20:29:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"John Cai via GitGitGadget\" writes:\n\n> From: John Cai \n>\n> The BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\n> it.\n\nDo you have a ready reference to the change that made it no longer\nneeded (or stopped detecting the error the message was meant to be\nissued against)?\n\n\n> Signed-off-by: John Cai \n> ---\n> fsck.h | 1 -\n> 1 file changed, 1 deletion(-)\n>\n> diff --git a/fsck.h b/fsck.h\n> index 6f801e53b1d..7d99f6ea337 100644\n> --- a/fsck.h\n> +++ b/fsck.h\n> @@ -24,7 +24,6 @@ enum fsck_msg_type {\n> \tFUNC(BAD_NAME, ERROR) \\\n> \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n> \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n> -\tFUNC(BAD_TAG_OBJECT, ERROR) \\\n> \tFUNC(BAD_TIMEZONE, ERROR) \\\n> \tFUNC(BAD_TREE, ERROR) \\\n> \tFUNC(BAD_TREE_SHA1, ERROR) \\\n"},{"id":"465645","messageId":"xmqqpmehrr6w.fsf@gitster.g","threadId":"58682","inReplyTo":"pull.1369.git.git.1666623639.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Document fsck msg ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-24T18:51:03Z","receivedAt":"2022-10-24T21:03:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here is to illustrate what I meant in my review comments on [2/2],\non top of these two patches.\n\nTwo structural changes worth noting are:\n\n- I removed the inclusin from config/fsck.txt because otherwise the\n entry for fsck. becomes way too long and the next entry,\n fsck.skipList, may become harder to find. But I am OK if you\n really want to make the list of msgids a sublist here, as long as\n it makes it clear that those with default severity FATAL cannot be\n configured away. That has to be done outside of fsck-msgids.txt\n file (in other words, in config/fsck.txt where it includes the\n file) as it would be a good idea to keep the file a \"pure\" list,\n to keep it easy to auto-generate in the future.\n\n- The description of each item begins with (SEVERITY) marking, as\n the ones that are (FATAL) cannot be configured.\n\nI think I added all error types that your patch missed. Also I\nadded (FATAL)/(ERROR)/(WARN)/(INFO) markings to show what their\ndefault severity levels are, but I may have made mistakes.\n\nIf you want to take any part of these changes, feel free to do so,\nbut please do so after double checking. This was primarily done to\nillustrate how the end result should look like if I were doing this\ntopic, and not to show how each item should be described exactly.\n\nThanks.\n\n Documentation/config/fsck.txt | 5 +-\n Documentation/fsck-msgids.txt | 188 ++++++++++++++++++++++--------------------\n Documentation/git-fsck.txt | 12 +++\n fsck.h | 6 ++\n 4 files changed, 119 insertions(+), 92 deletions(-)\n\ndiff --git c/Documentation/config/fsck.txt w/Documentation/config/fsck.txt\nindex b0632075f2..a3c865df56 100644\n--- c/Documentation/config/fsck.txt\n+++ w/Documentation/config/fsck.txt\n@@ -36,10 +36,9 @@ Setting an unknown `fsck.` value will cause fsck to die, but\n doing the same for `receive.fsck.` and `fetch.fsck.`\n will only cause git to warn.\n +\n-The following `` are supported:\n-+\n+See `Fsck Messages` section of linkgit:git-fsck[1] for supported\n+values of ``.\n \n-include::../fsck-msgids.txt[]\n \n fsck.skipList::\n \tThe path to a list of object names (i.e. one unabbreviated SHA-1 per\ndiff --git c/Documentation/fsck-msgids.txt w/Documentation/fsck-msgids.txt\nindex 888fa3308b..6831982bdb 100644\n--- c/Documentation/fsck-msgids.txt\n+++ w/Documentation/fsck-msgids.txt\n@@ -1,133 +1,143 @@\n---\n-`badDate`;;\n-\tInvalid date format in an author/committer line.\n+`badDate`::\n+\t(ERROR) Invalid date format in an author/committer line.\n \n-`badEmail`;;\n-\tInvalid email format in an author/committer line.\n+`badDateOverflow`::\n+\t(ERROR) Invalid date value in an author/committer line.\n \n-`badFilemode`;;\n-\tA tree contains a bad filemode entry.\n+`badEmail`::\n+\t(ERROR) Invalid email format in an author/committer line.\n \n-`badName`;;\n-\tAn author/committer name is empty.\n+`badFilemode`::\n+\t(INFO) A tree contains a bad filemode entry.\n \n-`badObjectSha1`;;\n-\tAn object has a bad sha1.\n+`badName`::\n+\t(ERROR) An author/committer name is empty.\n \n-`badParentSha1`;;\n-\tA commit object has a bad parent sha1.\n+`badObjectSha1`::\n+\t(ERROR) An object has a bad sha1.\n \n-`badTagName`;;\n-\tA tag has an invalid format.\n+`badParentSha1`::\n+\t(ERROR) A commit object has a bad parent sha1.\n \n-`badTimezone`;;\n-\tFound an invalid time zone in an author/committer line.\n+`badTagName`::\n+\t(INFO) A tag has an invalid format.\n \n-`badTree`;;\n-\tA tree cannot be parsed.\n+`badTimezone`::\n+\t(ERROR) Found an invalid time zone in an author/committer line.\n \n-`badTreeSha1`;;\n-\tA tree has an invalid format.\n+`badTree`::\n+\t(ERROR) A tree cannot be parsed.\n \n-`badType`;;\n-\tFound an invalid object type.\n+`badTreeSha1`::\n+\t(ERROR) A tree has an invalid format.\n \n-`duplicateEntries`;;\n-\tA tree contains duplicate file entries.\n+`badType`::\n+\t(ERROR) Found an invalid object type.\n \n-`emptyName`;;\n-\tA path contains an empty name.\n+`duplicateEntries`::\n+\t(ERROR) A tree contains duplicate file entries.\n \n-`fullPathName`;;\n-\tA path contains the full path starting with \"/\".\n+`emptyName`::\n+\t(WARN) A path contains an empty name.\n \n-`gitAttributesSymlink`;;\n-\t`.gitattributes` is a symlink.\n+`fullPathName`::\n+\t(WARN) A path contains the full path starting with \"/\".\n \n-`gitignoreSymlink`;;\n-\t`.gitignore` is a symlink.\n+`gitAttributesSymlink`::\n+\t(INFO) `.gitattributes` is a symlink.\n \n-`gitmodulesBlob`;;\n-\tA non-blob found at `.gitmodules`.\n+`gitignoreSymlink`::\n+\t(INFO) `.gitignore` is a symlink.\n \n-`gitmodulesMissing`;;\n-\tUnable to read `.gitmodules` blob.\n+`gitmodulesBlob`::\n+\t(ERROR) A non-blob found at `.gitmodules`.\n \n-`gitmodulesName`;;\n-\tA submodule name is invalid.\n+`gitmodulesMissing`::\n+\t(ERROR) Unable to read `.gitmodules` blob.\n \n-`gitmodulesParse`;;\n-\tCould not parse `.gitmodules` blob.\n+`gitmodulesName`::\n+\t(ERROR) A submodule name is invalid.\n+\n+`gitmodulesParse`::\n+\t(INFO) Could not parse `.gitmodules` blob.\n \n `gitmodulesLarge`;\n-\t`.gitmodules` blob is too large to parse.\n+\t(ERROR) `.gitmodules` blob is too large to parse.\n+\n+`gitmodulesPath`::\n+\t(ERROR) `.gitmodules` path is invalid.\n+\n+`gitmodulesSymlink`::\n+\t(ERROR) `.gitmodules` is a symlink.\n+\n+`gitmodulesUpdate`::\n+\t(ERROR) Found an invalid submodule update setting.\n \n-`gitmodulesPath`;;\n-\t`.gitmodules` path is invalid.\n+`gitmodulesUrl`::\n+\t(ERROR) Found an invalid submodule url.\n \n-`gitmodulesSymlink`;;\n-\t`.gitmodules` is a symlink.\n+`hasDot`::\n+\t(WARN) A tree contains an entry named `.`.\n \n-`gitmodulesUpdate`;;\n-\tFound an invalid submodule update setting.\n+`hasDotdot`::\n+\t(WARN) A tree contains an entry named `..`.\n \n-`gitmodulesUrl`;;\n-\tFound an invalid submodule url.\n+`hasDotgit`::\n+\t(WARN) A tree contains an entry named `.git`.\n \n-`hasDot`;;\n-\tA tree contains an entry named `.`.\n+`mailmapSymlink`::\n+\t(WARN) `.mailmap` is a symlink.\n \n-`hasDotdot`;;\n-\tA tree contains an entry named `..`.\n+`missingAuthor`::\n+\t(ERROR) Author is missing.\n \n-`hasDotgit`;;\n-\tA tree contains an entry named `.git`.\n+`missingCommitter`::\n+\t(ERROR) Committer is missing.\n \n-`mailmapSymlink`;;\n-\t`.mailmap` is a symlink.\n+`missingEmail`::\n+\t(ERROR) Email is missing in an author/committer line.\n \n-`missingAuthor`;;\n-\tAuthor is missing.\n+`missingNameBeforeEmail`::\n+\t(ERROR) Missing name before an email in an author/committer line.\n \n-`missingCommitter`;;\n-\tCommitter is missing.\n+`missingObject`::\n+\t(ERROR) Missing `object` line in tag object.\n \n-`missingEmail`;;\n-\tEmail is missing in an author/committer line.\n+`missingSpaceBeforeDate`::\n+\t(ERROR) Missing space before date in an author/committer line.\n \n-`missingNameBeforeEmail`;;\n-\tMissing space before an email in an author/committer line.\n+`missingSpaceBeforeEmail`::\n+\t(ERROR) Missing space before the email in author/committer line.\n \n-`missingObject`;;\n-\tMissing `object` line in tag object.\n+`missingTag`::\n+\t(ERROR) Unexpected end after `type` line in a tag object.\n \n-`missingSpaceBeforeDate`;;\n-\tMissing space before date in an author/committer line.\n+`missingTagEntry`::\n+\t(ERROR) Missing `tag` line in a tag object.\n \n-`missingSpaceBeforeEmail`;;\n-\tMissing space before the email in author/committer line.\n+`missingTypeEntry`::\n+\t(ERROR) Missing `type` line in a tag object.\n \n-`missingTag`;;\n-\tUnexpected end after `type` line in a tag object.\n+`multipleAuthors`::`\n+\t(ERROR) Multiple author lines found in a commit.\n \n-`missingTypeEntry`;;\n-\tMissing `type` line in a tag object.\n+`nulInCommit`::\n+\t(WARN) Found a NUL byte in the commit object body.\n \n-`multipleAuthors`;;`\n-\tMultiple author lines found in a commit.\n+`nulInHeader`::\n+\t(FATAL) NUL byte exists in the object header.\n \n-`nulInCommit`;;\n-\tFound a NUL byte in the commit object body.\n+`treeNotSorted`::\n+\t(ERROR) A tree is not properly sorted.\n \n-`treeNotSorted`;;\n-\tA tree is not properly sorted.\n+`unknownType`::\n+\t(ERROR) Found an unknown object type.\n \n-`unknownType`;;\n-\tFound an unknown object type.\n+`unterminatedHeader`::\n+\t(FATAL) Missing end-of-line in the object header.\n \n-`zeroPaddingDate`;;\n-\tFound a zero padded date in an author/commiter line.\n+`zeroPaddingDate`::\n+\t(ERROR) Found a zero padded date in an author/commiter line.\n \n-`zeroPaddedFilemode`;;\n-\tFound a zero padded filemode in a tree.\n---\n+`zeroPaddedFilemode`::\n+\t(WARN) Found a zero padded filemode in a tree.\ndiff --git c/Documentation/git-fsck.txt w/Documentation/git-fsck.txt\nindex 29318ea957..8124ae2e67 100644\n--- c/Documentation/git-fsck.txt\n+++ w/Documentation/git-fsck.txt\n@@ -152,6 +152,18 @@ hash mismatch ::\n \tobject database value.\n \tThis indicates a serious data integrity problem.\n \n+\n+FSCK MESSAGES\n+-------------\n+\n+The following lists the types of errors `git fsck` detects and what\n+each error means, with their default severity. The severity of the\n+errors, other than those that are marked as \"(FATAL)\", can be tweaked\n+by setting corresponding `fsck.` configuration variable.\n+\n+include::fsck-msgids.txt[]\n+\n+\n Environment Variables\n ---------------------\n \ndiff --git c/fsck.h w/fsck.h\nindex 7d99f6ea33..1fb26c4868 100644\n--- c/fsck.h\n+++ w/fsck.h\n@@ -13,6 +13,12 @@ enum fsck_msg_type {\n \tFSCK_WARN,\n };\n \n+/*\n+ * Documentation/fsck-msgids.txt is meant to document these; when\n+ * modifying this list in any way, make sure to make corresponding\n+ * changes there, too, to keep them in sync.\n+ */\n+\n #define FOREACH_FSCK_MSG_ID(FUNC) \\\n \t/* fatal errors */ \\\n \tFUNC(NUL_IN_HEADER, FATAL) \\\n"},{"id":"465672","messageId":"Y1ciT5zuFiZgavIN@coredump.intra.peff.net","threadId":"58682","inReplyTo":"4FFF6EB5-B9A3-4E32-840C-3FD6E893F5BF@gmail.com","subject":"Re: [PATCH 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-24T23:39:59Z","receivedAt":"2022-10-25T00:55:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 24, 2022 at 02:33:39PM -0400, John Cai wrote:\n\n> >>> From: John Cai \n> >>>\n> >>> The BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\n> >>> it.\n> >>\n> >> Do you have a ready reference to the change that made it no longer\n> >> needed (or stopped detecting the error the message was meant to be\n> >> issued against)?\n> >\n> > 2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\n> > checking the tagged object referred to by a tag object, which the\n> > error message BAD_TAG_OBJECT was about. Since then the\n> > BAD_TAG_OBJECT message is no longer used.\n> \n> Thanks for doing my work for me :)\n\nThanks for finding this. That commit is mine, and I should have removed\nit then. :)\n\nFor future reference, I did the same search via:\n\n git log -SBAD_TAG_OBJECT fsck.c\n\nI find \"-S\" is great for these kind of \"when did this stop getting used\"\nqueries.\n\n-Peff\n"},{"id":"465679","messageId":"797c625167120ddb676db07fdab99ce4986a4dd9.1666667864.git.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":"pull.1369.v2.git.git.1666667864.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] fsck: remove the unused BAD_TAG_OBJECT","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-25T03:17:43Z","receivedAt":"2022-10-25T03:17:52Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai \n\n2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\nchecking the tagged object referred to by a tag object, which is what the\nerror message BAD_TAG_OBJECT was for. Since then the BAD_TAG_OBJECT\nmessage is no longer used anywhere.\n\nRemove the BAD_TAG_OBJECT msg-id.\n\nSigned-off-by: John Cai \nReviewed-by: Junio C Hamano \n---\n fsck.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/fsck.h b/fsck.h\nindex 6f801e53b1d..7d99f6ea337 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -24,7 +24,6 @@ enum fsck_msg_type {\n \tFUNC(BAD_NAME, ERROR) \\\n \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n-\tFUNC(BAD_TAG_OBJECT, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\n-- \ngitgitgadget\n\n"},{"id":"465680","messageId":"pull.1369.v2.git.git.1666667864.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":"pull.1369.git.git.1666623639.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] Document fsck msg ids","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-25T03:17:42Z","receivedAt":"2022-10-25T03:17:55Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"fsck does a number of checks, and prints warning or error messages based on\nthe type of check. The only place the config values that control whether to\nignore these checks are documented is through git-help(1). However, often\ntimes peoples' first instinct to look for a list of valid config values is\nin the documentation page for git-config(1). These fsck. configuration\nvalues can be hard to find.\n\nDocument these so that both git-config and git-fsck documentation has a list\nof valid s.\n\nPatch [1/2] removes an unused msg-id BAD_TAG_OBJECT Patch [2/2] adds a\nfsck-msgids.txt that lists msg-ids that fsck checks for\n\nSince v1:\n\n * provided a full list of error messages along with corresponding default\n severity\n * no longer including the whole list in git-config\n * add a comment in fsck.h to remind developers to keep the documentation\n accurate when making changes to the list of error messages\n\nJohn Cai (2):\n fsck: remove the unused BAD_TAG_OBJECT\n fsck: document msg-id\n\n Documentation/config/fsck.txt | 4 +\n Documentation/fsck-msgids.txt | 149 ++++++++++++++++++++++++++++++++++\n Documentation/git-fsck.txt | 12 +++\n fsck.h | 7 +-\n 4 files changed, 171 insertions(+), 1 deletion(-)\n create mode 100644 Documentation/fsck-msgids.txt\n\n\nbase-commit: 45c9f05c44b1cb6bd2d6cb95a22cf5e3d21d5b63\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1369%2Fjohn-cai%2Fjc%2Fdocument-fsck-msg-id-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1369/john-cai/jc/document-fsck-msg-id-v2\nPull-Request: https://github.com/git/git/pull/1369\n\nRange-diff vs v1:\n\n 1: f32ff5dc4ee ! 1: 797c6251671 fsck: remove the unused BAD_TAG_OBJECT\n @@ Metadata\n ## Commit message ##\n fsck: remove the unused BAD_TAG_OBJECT\n \n - The BAD_TAG_OBJECT msg-id is not being used anymore, so we can remove\n - it.\n + 2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\n + checking the tagged object referred to by a tag object, which is what the\n + error message BAD_TAG_OBJECT was for. Since then the BAD_TAG_OBJECT\n + message is no longer used anywhere.\n +\n + Remove the BAD_TAG_OBJECT msg-id.\n \n Signed-off-by: John Cai \n + Reviewed-by: Junio C Hamano \n \n ## fsck.h ##\n @@ fsck.h: enum fsck_msg_type {\n 2: 3aec3d2c9ca ! 2: ad43adab435 fsck: document msg-id\n @@ Metadata\n ## Commit message ##\n fsck: document msg-id\n \n - The git-config documentation lacks mention of specific that\n - are supported. While git-help --config will display a list of these options,\n - often developers' first instinct is to consult the git docs to find valid\n + The documentation lacks mention of specific that are supported.\n + While git-help --config will display a list of these options, often\n + developers' first instinct is to consult the git docs to find valid\n config values.\n \n - Add a section under the docs for fsck. with the msg-ids that\n - git-fsck recognizes.\n + Add a list of fsck error messages, and link to it from the git-fsck\n + documentation.\n \n Signed-off-by: John Cai \n + Helped-by: Junio C Hamano \n \n ## Documentation/config/fsck.txt ##\n @@ Documentation/config/fsck.txt: allow new instances of the same breakages go unnoticed.\n @@ Documentation/config/fsck.txt: allow new instances of the same breakages go unno\n doing the same for `receive.fsck.` and `fetch.fsck.`\n will only cause git to warn.\n ++\n -+The following `` are supported:\n -++\n ++See `Fsck Messages` section of linkgit:git-fsck[1] for supported\n ++values of ``.\n +\n -+include::../fsck-msgids.txt[]\n \n fsck.skipList::\n \tThe path to a list of object names (i.e. one unabbreviated SHA-1 per\n \n ## Documentation/fsck-msgids.txt (new) ##\n @@\n -+--\n -+`badDate`;;\n -+\tInvalid date format in an author/committer line.\n ++`badDate`::\n ++\t(ERROR) Invalid date format in an author/committer line.\n ++\n ++`badDateOverflow`::\n ++\t(ERROR) Invalid date value in an author/committer line.\n ++\n ++`badEmail`::\n ++\t(ERROR) Invalid email format in an author/committer line.\n +\n -+`badEmail`;;\n -+\tInvalid email format in an author/committer line.\n ++`badFilemode`::\n ++\t(INFO) A tree contains a bad filemode entry.\n +\n -+`badFilemode`;;\n -+\tA tree contains a bad filemode entry.\n ++`badName`::\n ++\t(ERROR) An author/committer name is empty.\n +\n -+`badName`;;\n -+\tAn author/committer name is empty.\n ++`badObjectSha1`::\n ++\t(ERROR) An object has a bad sha1.\n +\n -+`badObjectSha1`;;\n -+\tAn object has a bad sha1.\n ++`badParentSha1`::\n ++\t(ERROR) A commit object has a bad parent sha1.\n +\n -+`badParentSha1`;;\n -+\tA commit object has a bad parent sha1.\n ++`badTagName`::\n ++\t(INFO) A tag has an invalid format.\n +\n -+`badTagName`;;\n -+\tA tag has an invalid format.\n ++`badTimezone`::\n ++\t(ERROR) Found an invalid time zone in an author/committer line.\n +\n -+`badTimezone`;;\n -+\tFound an invalid time zone in an author/committer line.\n ++`badTree`::\n ++\t(ERROR) A tree cannot be parsed.\n +\n -+`badTree`;;\n -+\tA tree cannot be parsed.\n ++`badTreeSha1`::\n ++\t(ERROR) A tree has an invalid format.\n +\n -+`badTreeSha1`;;\n -+\tA tree has an invalid format.\n ++`badType`::\n ++\t(ERROR) Found an invalid object type.\n +\n -+`badType`;;\n -+\tFound an invalid object type.\n ++`duplicateEntries`::\n ++\t(ERROR) A tree contains duplicate file entries.\n +\n -+`duplicateEntries`;;\n -+\tA tree contains duplicate file entries.\n ++`emptyName`::\n ++\t(WARN) A path contains an empty name.\n +\n -+`emptyName`;;\n -+\tA path contains an empty name.\n ++`extraHeaderEntry`::\n ++\t(IGNORE) Extra headers found after `tagger`.\n +\n -+`fullPathName`;;\n -+\tA path contains the full path starting with \"/\".\n ++`fullPathName`::\n ++\t(WARN) A path contains the full path starting with \"/\".\n +\n -+`gitAttributesSymlink`;;\n -+\t`.gitattributes` is a symlink.\n ++`gitAttributesSymlink`::\n ++\t(INFO) `.gitattributes` is a symlink.\n +\n -+`gitignoreSymlink`;;\n -+\t`.gitignore` is a symlink.\n ++`gitignoreSymlink`::\n ++\t(INFO) `.gitignore` is a symlink.\n +\n -+`gitmodulesBlob`;;\n -+\tA non-blob found at `.gitmodules`.\n ++`gitmodulesBlob`::\n ++\t(ERROR) A non-blob found at `.gitmodules`.\n +\n -+`gitmodulesMissing`;;\n -+\tUnable to read `.gitmodules` blob.\n ++`gitmodulesMissing`::\n ++\t(ERROR) Unable to read `.gitmodules` blob.\n +\n -+`gitmodulesName`;;\n -+\tA submodule name is invalid.\n ++`gitmodulesName`::\n ++\t(ERROR) A submodule name is invalid.\n +\n -+`gitmodulesParse`;;\n -+\tCould not parse `.gitmodules` blob.\n ++`gitmodulesParse`::\n ++\t(INFO) Could not parse `.gitmodules` blob.\n +\n +`gitmodulesLarge`;\n -+\t`.gitmodules` blob is too large to parse.\n ++\t(ERROR) `.gitmodules` blob is too large to parse.\n ++\n ++`gitmodulesPath`::\n ++\t(ERROR) `.gitmodules` path is invalid.\n ++\n ++`gitmodulesSymlink`::\n ++\t(ERROR) `.gitmodules` is a symlink.\n +\n -+`gitmodulesPath`;;\n -+\t`.gitmodules` path is invalid.\n ++`gitmodulesUpdate`::\n ++\t(ERROR) Found an invalid submodule update setting.\n +\n -+`gitmodulesSymlink`;;\n -+\t`.gitmodules` is a symlink.\n ++`gitmodulesUrl`::\n ++\t(ERROR) Found an invalid submodule url.\n +\n -+`gitmodulesUpdate`;;\n -+\tFound an invalid submodule update setting.\n ++`hasDot`::\n ++\t(WARN) A tree contains an entry named `.`.\n +\n -+`gitmodulesUrl`;;\n -+\tFound an invalid submodule url.\n ++`hasDotdot`::\n ++\t(WARN) A tree contains an entry named `..`.\n +\n -+`hasDot`;;\n -+\tA tree contains an entry named `.`.\n ++`hasDotgit`::\n ++\t(WARN) A tree contains an entry named `.git`.\n +\n -+`hasDotdot`;;\n -+\tA tree contains an entry named `..`.\n ++`mailmapSymlink`::\n ++\t(INFO) `.mailmap` is a symlink.\n +\n -+`hasDotgit`;;\n -+\tA tree contains an entry named `.git`.\n ++`missingAuthor`::\n ++\t(ERROR) Author is missing.\n +\n -+`mailmapSymlink`;;\n -+\t`.mailmap` is a symlink.\n ++`missingCommitter`::\n ++\t(ERROR) Committer is missing.\n +\n -+`missingAuthor`;;\n -+\tAuthor is missing.\n ++`missingEmail`::\n ++\t(ERROR) Email is missing in an author/committer line.\n +\n -+`missingCommitter`;;\n -+\tCommitter is missing.\n ++`missingNameBeforeEmail`::\n ++\t(ERROR) Missing name before an email in an author/committer line.\n +\n -+`missingEmail`;;\n -+\tEmail is missing in an author/committer line.\n ++`missingObject`::\n ++\t(ERROR) Missing `object` line in tag object.\n +\n -+`missingNameBeforeEmail`;;\n -+\tMissing space before an email in an author/committer line.\n ++`missingSpaceBeforeDate`::\n ++\t(ERROR) Missing space before date in an author/committer line.\n +\n -+`missingObject`;;\n -+\tMissing `object` line in tag object.\n ++`missingSpaceBeforeEmail`::\n ++\t(ERROR) Missing space before the email in author/committer line.\n +\n -+`missingSpaceBeforeDate`;;\n -+\tMissing space before date in an author/committer line.\n ++`missingTag`::\n ++\t(ERROR) Unexpected end after `type` line in a tag object.\n +\n -+`missingSpaceBeforeEmail`;;\n -+\tMissing space before the email in author/committer line.\n ++`missingTagEntry`::\n ++\t(ERROR) Missing `tag` line in a tag object.\n +\n -+`missingTag`;;\n -+\tUnexpected end after `type` line in a tag object.\n ++`missingTypeEntry`::\n ++\t(ERROR) Missing `type` line in a tag object.\n +\n -+`missingTypeEntry`;;\n -+\tMissing `type` line in a tag object.\n ++`multipleAuthors`::\n ++\t(ERROR) Multiple author lines found in a commit.\n +\n -+`multipleAuthors`;;`\n -+\tMultiple author lines found in a commit.\n ++`nulInCommit`::\n ++\t(WARN) Found a NUL byte in the commit object body.\n +\n -+`nulInCommit`;;\n -+\tFound a NUL byte in the commit object body.\n ++`nulInHeader`::\n ++\t(FATAL) NUL byte exists in the object header.\n +\n -+`treeNotSorted`;;\n -+\tA tree is not properly sorted.\n ++`nulInSha1`::\n ++\t(WARN) Tree contains entries pointing to a null sha1.\n +\n -+`unknownType`;;\n -+\tFound an unknown object type.\n ++`treeNotSorted`::\n ++\t(ERROR) A tree is not properly sorted.\n +\n -+`zeroPaddingDate`;;\n -+\tFound a zero padded date in an author/commiter line.\n ++`unknownType`::\n ++\t(ERROR) Found an unknown object type.\n +\n -+`zeroPaddedFilemode`;;\n -+\tFound a zero padded filemode in a tree.\n -+--\n ++`unterminatedHeader`::\n ++\t(FATAL) Missing end-of-line in the object header.\n ++\n ++`zeroPaddingDate`::\n ++\t(ERROR) Found a zero padded date in an author/commiter line.\n ++\n ++`zeroPaddedFilemode`::\n ++\t(WARN) Found a zero padded filemode in a tree.\n +\n + ## Documentation/git-fsck.txt ##\n +@@ Documentation/git-fsck.txt: hash mismatch ::\n + \tobject database value.\n + \tThis indicates a serious data integrity problem.\n + \n ++\n ++FSCK MESSAGES\n ++-------------\n ++\n ++The following lists the types of errors `git fsck` detects and what\n ++each error means, with their default severity. The severity of the\n ++error, other than those that are marked as \"(FATAL)\", can be tweaked\n ++by setting the corresponding `fsck.` configuration variable.\n ++\n ++include::fsck-msgids.txt[]\n ++\n ++\n + Environment Variables\n + ---------------------\n + \n +\n + ## fsck.h ##\n +@@ fsck.h: enum fsck_msg_type {\n + \tFSCK_WARN,\n + };\n + \n ++/*\n ++ * Documentation/fsck-msgids.txt documents these; when\n ++ * modifying this list in any way, make sure to keep the\n ++ * two in sync.\n ++ */\n ++\n + #define FOREACH_FSCK_MSG_ID(FUNC) \\\n + \t/* fatal errors */ \\\n + \tFUNC(NUL_IN_HEADER, FATAL) \\\n\n-- \ngitgitgadget\n"},{"id":"465681","messageId":"ad43adab435e9fadd7b896928e234a38c9bb16a6.1666667864.git.gitgitgadget@gmail.com","threadId":"58682","inReplyTo":"pull.1369.v2.git.git.1666667864.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] fsck: document msg-id","fromName":"John Cai via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-10-25T03:17:44Z","receivedAt":"2022-10-25T03:17:58Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"From: John Cai \n\nThe documentation lacks mention of specific that are supported.\nWhile git-help --config will display a list of these options, often\ndevelopers' first instinct is to consult the git docs to find valid\nconfig values.\n\nAdd a list of fsck error messages, and link to it from the git-fsck\ndocumentation.\n\nSigned-off-by: John Cai \nHelped-by: Junio C Hamano \n---\n Documentation/config/fsck.txt | 4 +\n Documentation/fsck-msgids.txt | 149 ++++++++++++++++++++++++++++++++++\n Documentation/git-fsck.txt | 12 +++\n fsck.h | 6 ++\n 4 files changed, 171 insertions(+)\n create mode 100644 Documentation/fsck-msgids.txt\n\ndiff --git a/Documentation/config/fsck.txt b/Documentation/config/fsck.txt\nindex 450e8c38e34..a3c865df567 100644\n--- a/Documentation/config/fsck.txt\n+++ b/Documentation/config/fsck.txt\n@@ -35,6 +35,10 @@ allow new instances of the same breakages go unnoticed.\n Setting an unknown `fsck.` value will cause fsck to die, but\n doing the same for `receive.fsck.` and `fetch.fsck.`\n will only cause git to warn.\n++\n+See `Fsck Messages` section of linkgit:git-fsck[1] for supported\n+values of ``.\n+\n \n fsck.skipList::\n \tThe path to a list of object names (i.e. one unabbreviated SHA-1 per\ndiff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt\nnew file mode 100644\nindex 00000000000..08e19bac8ad\n--- /dev/null\n+++ b/Documentation/fsck-msgids.txt\n@@ -0,0 +1,149 @@\n+`badDate`::\n+\t(ERROR) Invalid date format in an author/committer line.\n+\n+`badDateOverflow`::\n+\t(ERROR) Invalid date value in an author/committer line.\n+\n+`badEmail`::\n+\t(ERROR) Invalid email format in an author/committer line.\n+\n+`badFilemode`::\n+\t(INFO) A tree contains a bad filemode entry.\n+\n+`badName`::\n+\t(ERROR) An author/committer name is empty.\n+\n+`badObjectSha1`::\n+\t(ERROR) An object has a bad sha1.\n+\n+`badParentSha1`::\n+\t(ERROR) A commit object has a bad parent sha1.\n+\n+`badTagName`::\n+\t(INFO) A tag has an invalid format.\n+\n+`badTimezone`::\n+\t(ERROR) Found an invalid time zone in an author/committer line.\n+\n+`badTree`::\n+\t(ERROR) A tree cannot be parsed.\n+\n+`badTreeSha1`::\n+\t(ERROR) A tree has an invalid format.\n+\n+`badType`::\n+\t(ERROR) Found an invalid object type.\n+\n+`duplicateEntries`::\n+\t(ERROR) A tree contains duplicate file entries.\n+\n+`emptyName`::\n+\t(WARN) A path contains an empty name.\n+\n+`extraHeaderEntry`::\n+\t(IGNORE) Extra headers found after `tagger`.\n+\n+`fullPathName`::\n+\t(WARN) A path contains the full path starting with \"/\".\n+\n+`gitAttributesSymlink`::\n+\t(INFO) `.gitattributes` is a symlink.\n+\n+`gitignoreSymlink`::\n+\t(INFO) `.gitignore` is a symlink.\n+\n+`gitmodulesBlob`::\n+\t(ERROR) A non-blob found at `.gitmodules`.\n+\n+`gitmodulesMissing`::\n+\t(ERROR) Unable to read `.gitmodules` blob.\n+\n+`gitmodulesName`::\n+\t(ERROR) A submodule name is invalid.\n+\n+`gitmodulesParse`::\n+\t(INFO) Could not parse `.gitmodules` blob.\n+\n+`gitmodulesLarge`;\n+\t(ERROR) `.gitmodules` blob is too large to parse.\n+\n+`gitmodulesPath`::\n+\t(ERROR) `.gitmodules` path is invalid.\n+\n+`gitmodulesSymlink`::\n+\t(ERROR) `.gitmodules` is a symlink.\n+\n+`gitmodulesUpdate`::\n+\t(ERROR) Found an invalid submodule update setting.\n+\n+`gitmodulesUrl`::\n+\t(ERROR) Found an invalid submodule url.\n+\n+`hasDot`::\n+\t(WARN) A tree contains an entry named `.`.\n+\n+`hasDotdot`::\n+\t(WARN) A tree contains an entry named `..`.\n+\n+`hasDotgit`::\n+\t(WARN) A tree contains an entry named `.git`.\n+\n+`mailmapSymlink`::\n+\t(INFO) `.mailmap` is a symlink.\n+\n+`missingAuthor`::\n+\t(ERROR) Author is missing.\n+\n+`missingCommitter`::\n+\t(ERROR) Committer is missing.\n+\n+`missingEmail`::\n+\t(ERROR) Email is missing in an author/committer line.\n+\n+`missingNameBeforeEmail`::\n+\t(ERROR) Missing name before an email in an author/committer line.\n+\n+`missingObject`::\n+\t(ERROR) Missing `object` line in tag object.\n+\n+`missingSpaceBeforeDate`::\n+\t(ERROR) Missing space before date in an author/committer line.\n+\n+`missingSpaceBeforeEmail`::\n+\t(ERROR) Missing space before the email in author/committer line.\n+\n+`missingTag`::\n+\t(ERROR) Unexpected end after `type` line in a tag object.\n+\n+`missingTagEntry`::\n+\t(ERROR) Missing `tag` line in a tag object.\n+\n+`missingTypeEntry`::\n+\t(ERROR) Missing `type` line in a tag object.\n+\n+`multipleAuthors`::\n+\t(ERROR) Multiple author lines found in a commit.\n+\n+`nulInCommit`::\n+\t(WARN) Found a NUL byte in the commit object body.\n+\n+`nulInHeader`::\n+\t(FATAL) NUL byte exists in the object header.\n+\n+`nulInSha1`::\n+\t(WARN) Tree contains entries pointing to a null sha1.\n+\n+`treeNotSorted`::\n+\t(ERROR) A tree is not properly sorted.\n+\n+`unknownType`::\n+\t(ERROR) Found an unknown object type.\n+\n+`unterminatedHeader`::\n+\t(FATAL) Missing end-of-line in the object header.\n+\n+`zeroPaddingDate`::\n+\t(ERROR) Found a zero padded date in an author/commiter line.\n+\n+`zeroPaddedFilemode`::\n+\t(WARN) Found a zero padded filemode in a tree.\ndiff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt\nindex 29318ea957e..b6a0f8a085c 100644\n--- a/Documentation/git-fsck.txt\n+++ b/Documentation/git-fsck.txt\n@@ -152,6 +152,18 @@ hash mismatch ::\n \tobject database value.\n \tThis indicates a serious data integrity problem.\n \n+\n+FSCK MESSAGES\n+-------------\n+\n+The following lists the types of errors `git fsck` detects and what\n+each error means, with their default severity. The severity of the\n+error, other than those that are marked as \"(FATAL)\", can be tweaked\n+by setting the corresponding `fsck.` configuration variable.\n+\n+include::fsck-msgids.txt[]\n+\n+\n Environment Variables\n ---------------------\n \ndiff --git a/fsck.h b/fsck.h\nindex 7d99f6ea337..8355fd4c8f5 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -13,6 +13,12 @@ enum fsck_msg_type {\n \tFSCK_WARN,\n };\n \n+/*\n+ * Documentation/fsck-msgids.txt documents these; when\n+ * modifying this list in any way, make sure to keep the\n+ * two in sync.\n+ */\n+\n #define FOREACH_FSCK_MSG_ID(FUNC) \\\n \t/* fatal errors */ \\\n \tFUNC(NUL_IN_HEADER, FATAL) \\\n-- \ngitgitgadget\n"},{"id":"465684","messageId":"xmqqwn8oplsh.fsf@gitster.g","threadId":"58682","inReplyTo":"pull.1369.v2.git.git.1666667864.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/2] Document fsck msg ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T04:30:38Z","receivedAt":"2022-10-25T04:31:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"John Cai via GitGitGadget\" writes:\n\n> Patch [1/2] removes an unused msg-id BAD_TAG_OBJECT Patch [2/2] adds a\n> fsck-msgids.txt that lists msg-ids that fsck checks for\n>\n> Since v1:\n>\n> * provided a full list of error messages along with corresponding default\n> severity\n> * no longer including the whole list in git-config\n> * add a comment in fsck.h to remind developers to keep the documentation\n> accurate when making changes to the list of error messages\n\nThanks. I did a bit of sanity checking and it made my earlier\nsuspicion stronger. We MUST have at least an automated checker to\ncheck the doc against the fsck.h header, if not an automated\ngenerator of the doc from the fsck.h header.\n\n * Just like your [1/2] pointed out an error type that is no longer\n in use, MISSING_TREE_OBJECT is not used. It seems that this was\n never used since it was introduced at 159e7b08 (fsck: detect\n gitmodules files, 2018-05-02)\n\n * A few items were misspelled.\n\n * A handful items were missing.\n\nThe latter two are shown in the attached, which I am tempted to just\nsquash into your 2/2. I think the MISSING_TREE_OBJECT one deserves\na separate commit between 1/2 and 2/2.\n\n Documentation/fsck-msgids.txt | 20 ++++++++++++++++----\n 1 file changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git c/Documentation/fsck-msgids.txt w/Documentation/fsck-msgids.txt\nindex 08e19bac8a..7af76ff99a 100644\n--- c/Documentation/fsck-msgids.txt\n+++ w/Documentation/fsck-msgids.txt\n@@ -43,10 +43,10 @@\n `extraHeaderEntry`::\n \t(IGNORE) Extra headers found after `tagger`.\n \n-`fullPathName`::\n+`fullPathname`::\n \t(WARN) A path contains the full path starting with \"/\".\n \n-`gitAttributesSymlink`::\n+`gitattributesSymlink`::\n \t(INFO) `.gitattributes` is a symlink.\n \n `gitignoreSymlink`::\n@@ -55,6 +55,9 @@\n `gitmodulesBlob`::\n \t(ERROR) A non-blob found at `.gitmodules`.\n \n+`gitmodulesLarge`::\n+\t(ERROR) The `.gitmodules` file is too large to parse.\n+\n `gitmodulesMissing`::\n \t(ERROR) Unable to read `.gitmodules` blob.\n \n@@ -118,6 +121,15 @@\n `missingTagEntry`::\n \t(ERROR) Missing `tag` line in a tag object.\n \n+`missingTaggerEntry`::\n+\t(INFO) Missing `tagger` line in a tag object.\n+\n+`missingTree`::\n+\t(ERROR) Missing `tree` line in a commit object.\n+\n+`missingType`::\n+\t(ERROR) Invalid type value on the `type` line in a tag object.\n+\n `missingTypeEntry`::\n \t(ERROR) Missing `type` line in a tag object.\n \n@@ -130,7 +142,7 @@\n `nulInHeader`::\n \t(FATAL) NUL byte exists in the object header.\n \n-`nulInSha1`::\n+`nullSha1`::\n \t(WARN) Tree contains entries pointing to a null sha1.\n \n `treeNotSorted`::\n@@ -142,7 +154,7 @@\n `unterminatedHeader`::\n \t(FATAL) Missing end-of-line in the object header.\n \n-`zeroPaddingDate`::\n+`zeroPaddedDate`::\n \t(ERROR) Found a zero padded date in an author/commiter line.\n \n `zeroPaddedFilemode`::\n"},{"id":"465685","messageId":"xmqqfsfcplcm.fsf@gitster.g","threadId":"58682","inReplyTo":"xmqqwn8oplsh.fsf@gitster.g","subject":"Re: [PATCH v2 0/2] Document fsck msg ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T04:40:09Z","receivedAt":"2022-10-25T04:40:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Thanks. I did a bit of sanity checking and it made my earlier\n> suspicion stronger. We MUST have at least an automated checker to\n> check the doc against the fsck.h header, if not an automated\n> generator of the doc from the fsck.h header.\n\nFYI, here are a pair of quick-and-dirty Perl scripts that I used for\nthe sanity checking. The first one \"parses\" the fsck-msgs.txt and\nformats lines like\n\n badTagName\tINFO\n\ni.e. camelCased error message name, a TAB, and the severity.\n\nThe second one reads the FOREACH_FSCK_MSG_ID() definition in fsck.h\nthat look like \"FUNC(BAD_TAG_NAME, INFO)\", camelcases the name and\nshows what can be compared with the output of the first one.\n\nThere are two sanity checks that must pass when a developer updates\nthe documentation.\n\n - The output from m.perl on the documentation must already be sorted.\n\n - The output from n.perl on fsck.h, when sorted, must match the\n output from m.perl on the documentation.\n\n$ cat >m.perl <<\\EOF\n#!/usr/bin/perl\n\nmy ($previous, $current);\n\nwhile (<>) {\n\tif (!defined $current) {\n\t\tif (/^\\`([a-zA-Z0-9]*)\\`::/) {\n\t\t\t$current = $1;\n\t\t\tif ((defined $previous) &&\n\t\t\t ($current le $previous)) {\n\t\t\t\tprint STDERR \"$previous >= $current???\\n\";\n\t\t\t}\n\t\t}\n\t} elsif (/^\\s+\\(([A-Z]+)\\) /) {\n\t\tprint \"$current\t$1\\n\";\n\t\t$previous = $current;\n\t\tundef $current;\n\t}\n}\nEOF\n$ cat >n.perl <<\\EOF\n#!/usr/bin/perl\n\nwhile (<>) {\n\tif (/^\\s+FUNC\\(([0-9A-Z_]+), ([A-Z]+)\\)/) {\n\t\tmy ($name, $severity) = ($1, $2);\n\t\tmy ($first) = 1;\n\t\t$name = join('',\n\t\t\t map {\n\t\t\t\t y/A-Z/a-z/;\n\t\t\t\t if (!$first) {\n\t\t\t\t\t s/^(.)/uc($1)/e;\n\t\t\t\t } else {\n\t\t\t\t\t $first = 0;\n\t\t\t\t }\n\t\t\t\t $_;\n\t\t\t }\n\t\t\t split(/_/, $name));\n\t\tprint \"$name\t$severity\\n\";\n\t}\n}\nEOF\n$ perl m.perl Documentation/fsck-msgids.txt >/var/tmp/1\n$ sort /var/tmp/1 >/var/tmp/2\n$ diff -u /var/tmp/1 /var/tmp/2\n#### no output should appear in the above comparison\n$ perl n.perl fsck.h | sort >/var/tmp/2\n$ diff -u /var/tmp/1 /var/tmp/2\n#### no output should appear in the above comparison\n\n"},{"id":"465686","messageId":"xmqqa65kpju4.fsf_-_@gitster.g","threadId":"58682","inReplyTo":"xmqqwn8oplsh.fsf@gitster.g","subject":"[PATCH] Documentation: add lint-fsck-msgids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T05:12:51Z","receivedAt":"2022-10-25T05:13:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"During the initial development of the fsck-msgids.txt feature, it\nhas become apparent that it is very much error prone to make sure\nthe description in the documentation file are sorted and correctly\nmatch what is in the fsck.h header file.\n\nAdd a quick-and-dirty Perl script and doc-lint target to sanity\ncheck that the fsck-msgids.txt is consistent with the error type\nlist in the fsck.h header file.\n\nSigned-off-by: Junio C Hamano \n---\n\n * So the quick-and-dirty one gets repackaged with minimum polish to\n become such a lint-doc checker. With the MISSING_TREE_OBJECT\n removed from fsck.h header, and the fixes to the msgids.txt\n documentation applied, the result passes the check.\n\n Documentation/Makefile | 11 +++++\n Documentation/lint-fsck-msgids.perl | 70 +++++++++++++++++++++++++++++\n 2 files changed, 81 insertions(+)\n create mode 100755 Documentation/lint-fsck-msgids.perl\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex d47acb2e25..5e1a7f655c 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -476,8 +476,19 @@ $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt\n .PHONY: lint-docs-man-section-order\n lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)\n \n+.PHONY: lint-docs-fsck-msgids\n+LINT_DOCS_FSCK_MSGIDS = .build/lint-docs/fsck-msgids.ok\n+$(LINT_DOCS_FSCK_MSGIDS): lint-fsck-msgids.perl\n+$(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt\n+\t$(call mkdir_p_parent_template)\n+\t$(QUIET_GEN)$(PERL_PATH) lint-fsck-msgids.perl \\\n+\t\t../fsck.h fsck-msgids.txt $@\n+\n+lint-docs-fsck-msgids: $(LINT_DOCS_FSCK_MSGIDS)\n+\n ## Lint: list of targets above\n .PHONY: lint-docs\n+lint-docs: lint-docs-fsck-msgids\n lint-docs: lint-docs-gitlink\n lint-docs: lint-docs-man-end-blurb\n lint-docs: lint-docs-man-section-order\ndiff --git a/Documentation/lint-fsck-msgids.perl b/Documentation/lint-fsck-msgids.perl\nnew file mode 100755\nindex 0000000000..1233ffe815\n--- /dev/null\n+++ b/Documentation/lint-fsck-msgids.perl\n@@ -0,0 +1,70 @@\n+#!/usr/bin/perl\n+\n+my ($fsck_h, $fsck_msgids_txt, $okfile) = @ARGV;\n+\n+my (%in_fsck_h, $fh, $bad);\n+\n+open($fh, \"<\", \"$fsck_h\") or die;\n+while (<$fh>) {\n+\tif (/^\\s+FUNC\\(([0-9A-Z_]+), ([A-Z]+)\\)/) {\n+\t\tmy ($name, $severity) = ($1, $2);\n+\t\tmy ($first) = 1;\n+\t\t$name = join('',\n+\t\t\t map {\n+\t\t\t\t y/A-Z/a-z/;\n+\t\t\t\t if (!$first) {\n+\t\t\t\t\t s/^(.)/uc($1)/e;\n+\t\t\t\t } else {\n+\t\t\t\t\t $first = 0;\n+\t\t\t\t }\n+\t\t\t\t $_;\n+\t\t\t }\n+\t\t\t split(/_/, $name));\n+\t\t$in_fsck_h{$name} = $severity;\n+\t}\n+}\n+close($fh);\n+\n+open($fh, \"<\", \"$fsck_msgids_txt\") or die;\n+my ($previous, $current);\n+while (<$fh>) {\n+\tif (!defined $current) {\n+\t\tif (/^\\`([a-zA-Z0-9]*)\\`::/) {\n+\t\t\t$current = $1;\n+\t\t\tif ((defined $previous) &&\n+\t\t\t ($current le $previous)) {\n+\t\t\t\tprint STDERR \"$previous >= $current in doc\\n\";\n+\t\t\t\t$bad = 1;\n+\t\t\t}\n+\t\t}\n+\t} elsif (/^\\s+\\(([A-Z]+)\\) /) {\n+\t\tmy ($level) = $1;\n+\t\tif (!exists $in_fsck_h{$current}) {\n+\t\t\tprint STDERR \"$current does not exist in fsck.h\\n\";\n+\t\t\t$bad = 1;\n+\t\t} elsif ($in_fsck_h{$current} eq \"\") {\n+\t\t\tprint STDERR \"$current defined twice\\n\";\n+\t\t\t$bad = 1;\n+\t\t} elsif ($in_fsck_h{$current} ne $level) {\n+\t\t\tprint STDERR \"$current severity $level != $in_fsck_h{$current}\\n\";\n+\t\t\t$bad = 1;\n+\t\t}\n+\t\t$previous = $current;\n+\t\t$in_fsck_h{$current} = \"\"; # mark as seen.\n+\t\tundef $current;\n+\t}\n+}\n+close($fh);\n+\n+for my $key (keys %in_fsck_h) {\n+\tif ($in_fsck_h{$key} ne \"\") {\n+\t\tprint STDERR \"$key not explained in doc.\\n\";\n+\t\t$bad = 1;\n+\t}\n+}\n+\n+die if ($bad);\n+\n+open($fh, \">\", \"$okfile\");\n+print $fh \"good\\n\";\n+close($fh);\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465689","messageId":"221025.86wn8ock07.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"xmqq8rl5t9ca.fsf@gitster.g","subject":"Re: [PATCH 2/2] fsck: document msg-id","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-25T09:41:50Z","receivedAt":"2022-10-25T09:47:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Oct 24 2022, Junio C Hamano wrote:\n\n> \"John Cai via GitGitGadget\" writes:\n>\n> I almost suggested to extend the FOREACH_FSCK_MSG_ID() definition in\n> fsck.h so that fsck-msgids.txt gets auto-generated (what is missing\n> in fsck.h that prevents us from doing so is the textual explanation\n> you added in the new file in your patch---they could come from\n> comments on the same line, for example, and can be extracted via a\n> Perl or sed script at build time).\n\nI think this is the best eventual approach, whether we want it now is\nanother matter...\n\n> I do not know if it is a good\n> idea, especially if we forsee a future in which we may be\n> translating the documentation, so decided against making such a\n> suggestion.\n\n...I just wanted to point out that difficulty of translating such a\nthing should not be a reason to hold that back, because it's not hard to\ntranslate such an arrangement.\n\nWe'd just point the po-doc extraction at the generated *.txt, we'd need\nto re-arrange the Makefile dependencies a bit, but it shouldn't be a\nproblem.\n\nThe *.pot-file generation is a step that only the translation\ncoordinator *needs* to run, so even if it's a manual procedure, or\nrequires first building the sources...\n\n> But at least, we could _lint_ the manually curated fsck-msgids.txt\n> using what is in fsck.h to see if a new MSG_ID added to fsck.h is\n> missing from the doc, or a MSG_ID whose default severity is updated\n> in fsck.h is stale in the doc, etc. That can be left for the future\n> updates, but we should at least instruct developers to keep them in\n> sync in fsck.h by adding a comment.\n\nYeah, that's a good step.\n"},{"id":"465713","messageId":"xmqqlep3opj9.fsf@gitster.g","threadId":"58682","inReplyTo":"221025.86wn8ock07.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] fsck: document msg-id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T16:07:22Z","receivedAt":"2022-10-25T16:07:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> We'd just point the po-doc extraction at the generated *.txt, we'd need\n> to re-arrange the Makefile dependencies a bit, but it shouldn't be a\n> problem.\n>\n> The *.pot-file generation is a step that only the translation\n> coordinator *needs* to run, so even if it's a manual procedure, or\n> requires first building the sources...\n\nI thought you made it more distributed to remove the coordinator's\ninvolvement in .pot generation recently, which resulted in us not\ntracking the .pot at all, no? Now all the l10n folks need to do the\nextraction?\n"},{"id":"465735","messageId":"20221025224224.2352979-1-gitster@pobox.com","threadId":"58682","inReplyTo":"pull.1369.v2.git.git.1666667864.gitgitgadget@gmail.com","subject":"[PATCH v3 0/4] document fsck error message ids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T22:42:20Z","receivedAt":"2022-10-25T22:42:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"git fsck\" reports various anomalies it finds in the form of message\ntokens, like `badDate`, in its error message, and allows most of\nthem to be tweaked for their severity levels via configuration\nvarialbes, like `fsck.badDate`. We however do not have them\ncentrally documented anywhere, other than the header file itself\n(and the header file does not have explanation on what they mean).\n\nThis is John's work, with a bit of help from me.\n\n * Patches 1 and 2 remove fsck error message IDs from fsck.h that\n are not used in today's code.\n\n * In patch 3, \"git fsck --help\" gains a section that lists all the\n fsck error message ids with explanation. \"git config --help\"\n gains a reference to the section.\n\n * While we worked on patch 3, we made many small mistakes (like\n spelling the tokens with incorrect camelCasing, or failing to\n list a few messages), which revealed the need of automated tool\n to catch them. Patch 4 adds to \"make check-docs\" a mechanism to\n verify the documentation covers what is defined in fsck.h\n correctly.\n\nPossible future directions that are left outside the scope of this\ntopic are:\n\n * \"git config --help\" may want to include the same list inline\n instead of having a reference to \"git fsck --help\".\n\n * Instead of the lint-docs support, we may want to auto-generate\n the documentation from fsck.h.\n\n\nJohn Cai (2):\n fsck: remove the unused BAD_TAG_OBJECT\n fsck: document msg-id\n\nJunio C Hamano (2):\n fsck: remove the unused MISSING_TREE_OBJECT\n Documentation: add lint-fsck-msgids\n\n Documentation/Makefile | 11 ++\n Documentation/config/fsck.txt | 4 +\n Documentation/fsck-msgids.txt | 161 ++++++++++++++++++++++++++++\n Documentation/git-fsck.txt | 12 +++\n Documentation/lint-fsck-msgids.perl | 70 ++++++++++++\n fsck.h | 8 +-\n 6 files changed, 264 insertions(+), 2 deletions(-)\n create mode 100644 Documentation/fsck-msgids.txt\n create mode 100755 Documentation/lint-fsck-msgids.perl\n\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465736","messageId":"20221025224224.2352979-2-gitster@pobox.com","threadId":"58682","inReplyTo":"20221025224224.2352979-1-gitster@pobox.com","subject":"[PATCH v3 1/4] fsck: remove the unused BAD_TAG_OBJECT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T22:42:21Z","receivedAt":"2022-10-25T22:42:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: John Cai \n\n2175a0c6 (fsck: stop checking tag->tagged, 2019-10-18) stopped\nchecking the tagged object referred to by a tag object, which is what the\nerror message BAD_TAG_OBJECT was for. Since then the BAD_TAG_OBJECT\nmessage is no longer used anywhere.\n\nRemove the BAD_TAG_OBJECT msg-id.\n\nSigned-off-by: John Cai \nSigned-off-by: Junio C Hamano \n---\n fsck.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/fsck.h b/fsck.h\nindex 6f801e53b1..7d99f6ea33 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -24,7 +24,6 @@ enum fsck_msg_type {\n \tFUNC(BAD_NAME, ERROR) \\\n \tFUNC(BAD_OBJECT_SHA1, ERROR) \\\n \tFUNC(BAD_PARENT_SHA1, ERROR) \\\n-\tFUNC(BAD_TAG_OBJECT, ERROR) \\\n \tFUNC(BAD_TIMEZONE, ERROR) \\\n \tFUNC(BAD_TREE, ERROR) \\\n \tFUNC(BAD_TREE_SHA1, ERROR) \\\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465737","messageId":"20221025224224.2352979-3-gitster@pobox.com","threadId":"58682","inReplyTo":"20221025224224.2352979-1-gitster@pobox.com","subject":"[PATCH v3 2/4] fsck: remove the unused MISSING_TREE_OBJECT","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T22:42:22Z","receivedAt":"2022-10-25T22:42:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This error type has never been used since it was introduced at\n159e7b08 (fsck: detect gitmodules files, 2018-05-02).\n\nSigned-off-by: Junio C Hamano \n---\n fsck.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/fsck.h b/fsck.h\nindex 7d99f6ea33..1d7c38f268 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -39,7 +39,6 @@ enum fsck_msg_type {\n \tFUNC(MISSING_TAG, ERROR) \\\n \tFUNC(MISSING_TAG_ENTRY, ERROR) \\\n \tFUNC(MISSING_TREE, ERROR) \\\n-\tFUNC(MISSING_TREE_OBJECT, ERROR) \\\n \tFUNC(MISSING_TYPE, ERROR) \\\n \tFUNC(MISSING_TYPE_ENTRY, ERROR) \\\n \tFUNC(MULTIPLE_AUTHORS, ERROR) \\\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465738","messageId":"20221025224224.2352979-4-gitster@pobox.com","threadId":"58682","inReplyTo":"20221025224224.2352979-1-gitster@pobox.com","subject":"[PATCH v3 3/4] fsck: document msg-id","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T22:42:23Z","receivedAt":"2022-10-25T22:42:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"From: John Cai \n\nThe documentation lacks mention of specific that are supported.\nWhile git-help --config will display a list of these options, often\ndevelopers' first instinct is to consult the git docs to find valid\nconfig values.\n\nAdd a list of fsck error messages, and link to it from the git-fsck\ndocumentation.\n\nSigned-off-by: John Cai \nSigned-off-by: Junio C Hamano \n---\n Documentation/config/fsck.txt | 4 +\n Documentation/fsck-msgids.txt | 161 ++++++++++++++++++++++++++++++++++\n Documentation/git-fsck.txt | 12 +++\n fsck.h | 6 ++\n 4 files changed, 183 insertions(+)\n create mode 100644 Documentation/fsck-msgids.txt\n\ndiff --git a/Documentation/config/fsck.txt b/Documentation/config/fsck.txt\nindex 450e8c38e3..a3c865df56 100644\n--- a/Documentation/config/fsck.txt\n+++ b/Documentation/config/fsck.txt\n@@ -35,6 +35,10 @@ allow new instances of the same breakages go unnoticed.\n Setting an unknown `fsck.` value will cause fsck to die, but\n doing the same for `receive.fsck.` and `fetch.fsck.`\n will only cause git to warn.\n++\n+See `Fsck Messages` section of linkgit:git-fsck[1] for supported\n+values of ``.\n+\n \n fsck.skipList::\n \tThe path to a list of object names (i.e. one unabbreviated SHA-1 per\ndiff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt\nnew file mode 100644\nindex 0000000000..7af76ff99a\n--- /dev/null\n+++ b/Documentation/fsck-msgids.txt\n@@ -0,0 +1,161 @@\n+`badDate`::\n+\t(ERROR) Invalid date format in an author/committer line.\n+\n+`badDateOverflow`::\n+\t(ERROR) Invalid date value in an author/committer line.\n+\n+`badEmail`::\n+\t(ERROR) Invalid email format in an author/committer line.\n+\n+`badFilemode`::\n+\t(INFO) A tree contains a bad filemode entry.\n+\n+`badName`::\n+\t(ERROR) An author/committer name is empty.\n+\n+`badObjectSha1`::\n+\t(ERROR) An object has a bad sha1.\n+\n+`badParentSha1`::\n+\t(ERROR) A commit object has a bad parent sha1.\n+\n+`badTagName`::\n+\t(INFO) A tag has an invalid format.\n+\n+`badTimezone`::\n+\t(ERROR) Found an invalid time zone in an author/committer line.\n+\n+`badTree`::\n+\t(ERROR) A tree cannot be parsed.\n+\n+`badTreeSha1`::\n+\t(ERROR) A tree has an invalid format.\n+\n+`badType`::\n+\t(ERROR) Found an invalid object type.\n+\n+`duplicateEntries`::\n+\t(ERROR) A tree contains duplicate file entries.\n+\n+`emptyName`::\n+\t(WARN) A path contains an empty name.\n+\n+`extraHeaderEntry`::\n+\t(IGNORE) Extra headers found after `tagger`.\n+\n+`fullPathname`::\n+\t(WARN) A path contains the full path starting with \"/\".\n+\n+`gitattributesSymlink`::\n+\t(INFO) `.gitattributes` is a symlink.\n+\n+`gitignoreSymlink`::\n+\t(INFO) `.gitignore` is a symlink.\n+\n+`gitmodulesBlob`::\n+\t(ERROR) A non-blob found at `.gitmodules`.\n+\n+`gitmodulesLarge`::\n+\t(ERROR) The `.gitmodules` file is too large to parse.\n+\n+`gitmodulesMissing`::\n+\t(ERROR) Unable to read `.gitmodules` blob.\n+\n+`gitmodulesName`::\n+\t(ERROR) A submodule name is invalid.\n+\n+`gitmodulesParse`::\n+\t(INFO) Could not parse `.gitmodules` blob.\n+\n+`gitmodulesLarge`;\n+\t(ERROR) `.gitmodules` blob is too large to parse.\n+\n+`gitmodulesPath`::\n+\t(ERROR) `.gitmodules` path is invalid.\n+\n+`gitmodulesSymlink`::\n+\t(ERROR) `.gitmodules` is a symlink.\n+\n+`gitmodulesUpdate`::\n+\t(ERROR) Found an invalid submodule update setting.\n+\n+`gitmodulesUrl`::\n+\t(ERROR) Found an invalid submodule url.\n+\n+`hasDot`::\n+\t(WARN) A tree contains an entry named `.`.\n+\n+`hasDotdot`::\n+\t(WARN) A tree contains an entry named `..`.\n+\n+`hasDotgit`::\n+\t(WARN) A tree contains an entry named `.git`.\n+\n+`mailmapSymlink`::\n+\t(INFO) `.mailmap` is a symlink.\n+\n+`missingAuthor`::\n+\t(ERROR) Author is missing.\n+\n+`missingCommitter`::\n+\t(ERROR) Committer is missing.\n+\n+`missingEmail`::\n+\t(ERROR) Email is missing in an author/committer line.\n+\n+`missingNameBeforeEmail`::\n+\t(ERROR) Missing name before an email in an author/committer line.\n+\n+`missingObject`::\n+\t(ERROR) Missing `object` line in tag object.\n+\n+`missingSpaceBeforeDate`::\n+\t(ERROR) Missing space before date in an author/committer line.\n+\n+`missingSpaceBeforeEmail`::\n+\t(ERROR) Missing space before the email in author/committer line.\n+\n+`missingTag`::\n+\t(ERROR) Unexpected end after `type` line in a tag object.\n+\n+`missingTagEntry`::\n+\t(ERROR) Missing `tag` line in a tag object.\n+\n+`missingTaggerEntry`::\n+\t(INFO) Missing `tagger` line in a tag object.\n+\n+`missingTree`::\n+\t(ERROR) Missing `tree` line in a commit object.\n+\n+`missingType`::\n+\t(ERROR) Invalid type value on the `type` line in a tag object.\n+\n+`missingTypeEntry`::\n+\t(ERROR) Missing `type` line in a tag object.\n+\n+`multipleAuthors`::\n+\t(ERROR) Multiple author lines found in a commit.\n+\n+`nulInCommit`::\n+\t(WARN) Found a NUL byte in the commit object body.\n+\n+`nulInHeader`::\n+\t(FATAL) NUL byte exists in the object header.\n+\n+`nullSha1`::\n+\t(WARN) Tree contains entries pointing to a null sha1.\n+\n+`treeNotSorted`::\n+\t(ERROR) A tree is not properly sorted.\n+\n+`unknownType`::\n+\t(ERROR) Found an unknown object type.\n+\n+`unterminatedHeader`::\n+\t(FATAL) Missing end-of-line in the object header.\n+\n+`zeroPaddedDate`::\n+\t(ERROR) Found a zero padded date in an author/commiter line.\n+\n+`zeroPaddedFilemode`::\n+\t(WARN) Found a zero padded filemode in a tree.\ndiff --git a/Documentation/git-fsck.txt b/Documentation/git-fsck.txt\nindex 29318ea957..b6a0f8a085 100644\n--- a/Documentation/git-fsck.txt\n+++ b/Documentation/git-fsck.txt\n@@ -152,6 +152,18 @@ hash mismatch ::\n \tobject database value.\n \tThis indicates a serious data integrity problem.\n \n+\n+FSCK MESSAGES\n+-------------\n+\n+The following lists the types of errors `git fsck` detects and what\n+each error means, with their default severity. The severity of the\n+error, other than those that are marked as \"(FATAL)\", can be tweaked\n+by setting the corresponding `fsck.` configuration variable.\n+\n+include::fsck-msgids.txt[]\n+\n+\n Environment Variables\n ---------------------\n \ndiff --git a/fsck.h b/fsck.h\nindex 1d7c38f268..6fbce68ad6 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -13,6 +13,12 @@ enum fsck_msg_type {\n \tFSCK_WARN,\n };\n \n+/*\n+ * Documentation/fsck-msgids.txt documents these; when\n+ * modifying this list in any way, make sure to keep the\n+ * two in sync.\n+ */\n+\n #define FOREACH_FSCK_MSG_ID(FUNC) \\\n \t/* fatal errors */ \\\n \tFUNC(NUL_IN_HEADER, FATAL) \\\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465739","messageId":"20221025224224.2352979-5-gitster@pobox.com","threadId":"58682","inReplyTo":"20221025224224.2352979-1-gitster@pobox.com","subject":"[PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-25T22:42:24Z","receivedAt":"2022-10-25T22:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"During the initial development of the fsck-msgids.txt feature, it\nhas become apparent that it is very much error prone to make sure\nthe description in the documentation file are sorted and correctly\nmatch what is in the fsck.h header file.\n\nAdd a quick-and-dirty Perl script and doc-lint target to sanity\ncheck that the fsck-msgids.txt is consistent with the error type\nlist in the fsck.h header file.\n\nSigned-off-by: Junio C Hamano \n---\n Documentation/Makefile | 11 +++++\n Documentation/lint-fsck-msgids.perl | 70 +++++++++++++++++++++++++++++\n 2 files changed, 81 insertions(+)\n create mode 100755 Documentation/lint-fsck-msgids.perl\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex d47acb2e25..5e1a7f655c 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -476,8 +476,19 @@ $(LINT_DOCS_MAN_SECTION_ORDER): .build/lint-docs/man-section-order/%.ok: %.txt\n .PHONY: lint-docs-man-section-order\n lint-docs-man-section-order: $(LINT_DOCS_MAN_SECTION_ORDER)\n \n+.PHONY: lint-docs-fsck-msgids\n+LINT_DOCS_FSCK_MSGIDS = .build/lint-docs/fsck-msgids.ok\n+$(LINT_DOCS_FSCK_MSGIDS): lint-fsck-msgids.perl\n+$(LINT_DOCS_FSCK_MSGIDS): ../fsck.h fsck-msgids.txt\n+\t$(call mkdir_p_parent_template)\n+\t$(QUIET_GEN)$(PERL_PATH) lint-fsck-msgids.perl \\\n+\t\t../fsck.h fsck-msgids.txt $@\n+\n+lint-docs-fsck-msgids: $(LINT_DOCS_FSCK_MSGIDS)\n+\n ## Lint: list of targets above\n .PHONY: lint-docs\n+lint-docs: lint-docs-fsck-msgids\n lint-docs: lint-docs-gitlink\n lint-docs: lint-docs-man-end-blurb\n lint-docs: lint-docs-man-section-order\ndiff --git a/Documentation/lint-fsck-msgids.perl b/Documentation/lint-fsck-msgids.perl\nnew file mode 100755\nindex 0000000000..1233ffe815\n--- /dev/null\n+++ b/Documentation/lint-fsck-msgids.perl\n@@ -0,0 +1,70 @@\n+#!/usr/bin/perl\n+\n+my ($fsck_h, $fsck_msgids_txt, $okfile) = @ARGV;\n+\n+my (%in_fsck_h, $fh, $bad);\n+\n+open($fh, \"<\", \"$fsck_h\") or die;\n+while (<$fh>) {\n+\tif (/^\\s+FUNC\\(([0-9A-Z_]+), ([A-Z]+)\\)/) {\n+\t\tmy ($name, $severity) = ($1, $2);\n+\t\tmy ($first) = 1;\n+\t\t$name = join('',\n+\t\t\t map {\n+\t\t\t\t y/A-Z/a-z/;\n+\t\t\t\t if (!$first) {\n+\t\t\t\t\t s/^(.)/uc($1)/e;\n+\t\t\t\t } else {\n+\t\t\t\t\t $first = 0;\n+\t\t\t\t }\n+\t\t\t\t $_;\n+\t\t\t }\n+\t\t\t split(/_/, $name));\n+\t\t$in_fsck_h{$name} = $severity;\n+\t}\n+}\n+close($fh);\n+\n+open($fh, \"<\", \"$fsck_msgids_txt\") or die;\n+my ($previous, $current);\n+while (<$fh>) {\n+\tif (!defined $current) {\n+\t\tif (/^\\`([a-zA-Z0-9]*)\\`::/) {\n+\t\t\t$current = $1;\n+\t\t\tif ((defined $previous) &&\n+\t\t\t ($current le $previous)) {\n+\t\t\t\tprint STDERR \"$previous >= $current in doc\\n\";\n+\t\t\t\t$bad = 1;\n+\t\t\t}\n+\t\t}\n+\t} elsif (/^\\s+\\(([A-Z]+)\\) /) {\n+\t\tmy ($level) = $1;\n+\t\tif (!exists $in_fsck_h{$current}) {\n+\t\t\tprint STDERR \"$current does not exist in fsck.h\\n\";\n+\t\t\t$bad = 1;\n+\t\t} elsif ($in_fsck_h{$current} eq \"\") {\n+\t\t\tprint STDERR \"$current defined twice\\n\";\n+\t\t\t$bad = 1;\n+\t\t} elsif ($in_fsck_h{$current} ne $level) {\n+\t\t\tprint STDERR \"$current severity $level != $in_fsck_h{$current}\\n\";\n+\t\t\t$bad = 1;\n+\t\t}\n+\t\t$previous = $current;\n+\t\t$in_fsck_h{$current} = \"\"; # mark as seen.\n+\t\tundef $current;\n+\t}\n+}\n+close($fh);\n+\n+for my $key (keys %in_fsck_h) {\n+\tif ($in_fsck_h{$key} ne \"\") {\n+\t\tprint STDERR \"$key not explained in doc.\\n\";\n+\t\t$bad = 1;\n+\t}\n+}\n+\n+die if ($bad);\n+\n+open($fh, \">\", \"$okfile\");\n+print $fh \"good\\n\";\n+close($fh);\n-- \n2.38.1-359-g84c4c6d5a5\n\n"},{"id":"465748","messageId":"221026.867d0ncncu.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"20221025224224.2352979-5-gitster@pobox.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-26T02:43:32Z","receivedAt":"2022-10-26T02:47:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Oct 25 2022, Junio C Hamano wrote:\n\n> During the initial development of the fsck-msgids.txt feature, it\n> has become apparent that it is very much error prone to make sure\n> the description in the documentation file are sorted and correctly\n> match what is in the fsck.h header file.\n\nI have local fixes for the same issues in the list of advice in our\ndocs, some of it's missing, wrong, out of date etc.\n\nI tried to quickly adapt the generation script I had for that, which\nworks nicely, and by line count much shorter than the lint :)\n\nHaving to exhaustively list every *.c file that uses fsck.h is a bit of\na bother, but we have the same with the other generated *.h's, so it\nshouldn't be too bad.\n\nAnd this way, if we get it wrong we get a compile error:\n\t\n\t$ git -P diff; make\n\tdiff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt\n\tindex 7af76ff99a5..f0b4308a6e7 100644\n\t--- a/Documentation/fsck-msgids.txt\n\t+++ b/Documentation/fsck-msgids.txt\n\t@@ -1,6 +1,3 @@\n\t-`badDate`::\n\t- (ERROR) Invalid date format in an author/committer line.\n\t-\n\t `badDateOverflow`::\n\t (ERROR) Invalid date value in an author/committer line.\n\t \n\t CC fsck.o\n\tfsck.c:31:9: error: excess elements in array initializer [-Werror]\n\t 31 | { NULL, NULL, NULL, -1 }\n\t | ^\n\tfsck.c:31:9: note: (near initialization for ‘msg_id_info’)\n\tfsck.c: In function ‘fsck_ident’:\n\tfsck.c:810:51: error: ‘FSCK_MSG_BAD_DATE’ undeclared (first use in this function); did you mean ‘FSCK_MSG_BAD_NAME’?\n\t 810 | return report(options, oid, type, FSCK_MSG_BAD_DATE, \"invalid author/committer line - bad date\");\n\t | ^~~~~~~~~~~~~~~~~\n\t | FSCK_MSG_BAD_NAME\n\tfsck.c:810:51: note: each undeclared identifier is reported only once for each function it appears in\n\tcc1: all warnings being treated as errors\n\tmake: *** [Makefile:2617: fsck.o] Error 1\n\nSo, if you're interested:\n\n Makefile | 12 ++++++++++++\n fsck.h | 8 +-------\n generate-fsckmsgids.sh | 42 ++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 55 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 85f03c6aed1..21bbc3dfb9f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -860,6 +860,7 @@ REFTABLE_TEST_LIB = reftable/libreftable_test.a\n GENERATED_H += command-list.h\n GENERATED_H += config-list.h\n GENERATED_H += hook-list.h\n+GENERATED_H += fsck-msgids.h\n \n .PHONY: generated-hdrs\n generated-hdrs: $(GENERATED_H)\n@@ -2289,6 +2290,14 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \\\n \t\t$(filter %.o,$^) $(LIBS)\n \n+# Unfortunately our dependency management of generated headers used\n+# from within other headers suck, so we'll need to list every user of\n+# fsck.h here, but not too bad...\n+FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n+$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n+FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n+$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n+\n help.sp help.s help.o: command-list.h\n builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h\n \n@@ -2333,6 +2342,9 @@ command-list.h: $(wildcard Documentation/git*.txt)\n hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n \n+fsck-msgids.h: generate-fsckmsgids.sh Documentation/fsck-msgids.txt\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-fsckmsgids.sh >$@\n+\n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\\\n \t$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\\\ndiff --git a/fsck.h b/fsck.h\nindex 6fbce68ad67..1a9d68482ea 100644\n--- a/fsck.h\n+++ b/fsck.h\n@@ -2,6 +2,7 @@\n #define GIT_FSCK_H\n \n #include \"oidset.h\"\n+#include \"fsck-msgids.h\"\n \n enum fsck_msg_type {\n \t/* for internal use only */\n@@ -79,13 +80,6 @@ enum fsck_msg_type {\n \t/* ignored (elevated when requested) */ \\\n \tFUNC(EXTRA_HEADER_ENTRY, IGNORE)\n \n-#define MSG_ID(id, msg_type) FSCK_MSG_##id,\n-enum fsck_msg_id {\n-\tFOREACH_FSCK_MSG_ID(MSG_ID)\n-\tFSCK_MSG_MAX\n-};\n-#undef MSG_ID\n-\n struct fsck_options;\n struct object;\n \ndiff --git a/generate-fsckmsgids.sh b/generate-fsckmsgids.sh\nnew file mode 100755\nindex 00000000000..bbf236159aa\n--- /dev/null\n+++ b/generate-fsckmsgids.sh\n@@ -0,0 +1,42 @@\n+#!/bin/sh\n+\n+HT='\t'\n+\n+fsck_list () {\n+\tsed -n \\\n+\t\t-e \"/::$/ {\n+\t\t\ts/::\\$//;\n+\t\t\ts/^\\`//;\n+\t\t\ts/\\`$//;\n+\t\t\tp;\n+\t\t}\" \\\n+\t \n> On Tue, Oct 25 2022, Junio C Hamano wrote:\n> \n> > During the initial development of the fsck-msgids.txt feature, it\n> > has become apparent that it is very much error prone to make sure\n> > the description in the documentation file are sorted and correctly\n> > match what is in the fsck.h header file.\n> \n> I have local fixes for the same issues in the list of advice in our\n> docs, some of it's missing, wrong, out of date etc.\n> \n> I tried to quickly adapt the generation script I had for that, which\n> works nicely, and by line count much shorter than the lint :)\n\nYeah, my instinct here was to generate rather than lint. If you make a\nmistake and the linter hits you over the head, that is better than\nquietly letting your mistake go. But better still is making it\nimpossible to make in the first place.\n\nThe downside is added complexity to the build, but it doesn't seem too\nbad in this case.\n\n(I had a similar thought after getting hit on the head by the recent\nt0450-txt-doc-vs-help.sh).\n\n> Having to exhaustively list every *.c file that uses fsck.h is a bit of\n> a bother, but we have the same with the other generated *.h's, so it\n> shouldn't be too bad.\n\nIt feels like this could be made much shorter by having a separate\nfsck-msgs.h and not including it from fsck.h. Only fsck.c and mktag.c\nneed the actual list. It would probably have to stop being an \"enum\",\nthough.\n\nAnother alternative is to generate the doc from the code, rather than\nthe other way around.\n\n> +# Unfortunately our dependency management of generated headers used\n> +# from within other headers suck, so we'll need to list every user of\n> +# fsck.h here, but not too bad...\n> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n\nI don't understand the \"used within other headers\" part here. Computed\ndependencies will get this right. It's only the initial build (before we\nhave any computed dependencies) that needs to know that the C files in\nquestion depend on the generated file. But that is true whether they do\nso directly or indirectly.\n\n-Peff\n"},{"id":"465756","messageId":"xmqqk04nkrps.fsf@gitster.g","threadId":"58682","inReplyTo":"Y1jG8p1aARb9+CV+@coredump.intra.peff.net","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-26T06:46:07Z","receivedAt":"2022-10-26T06:46:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Another alternative is to generate the doc from the code, rather than\n> the other way around.\n\nYup, that indeed was what I envisioned in the \"Possible future\ndirections\" comment in the cover letter. It might not even be bad\nto do\n\n #define FOREACH_FSCK_MSG_ID(FUNC) \\\n\t/* fatal errors */ \\\n\tFUNC(NUL_IN_HEADER, FATAL, N_(\"NUL byte exists in the object header.\")) \\\n\tFUNC(UNTERMINATED_HEADER, FATAL, N_(\"Missing end-of-line in the object header.\")) \\\n\t/* errors */ \\\n\tFUNC(BAD_DATE, ERROR, N_(\"Invalid date format in an author/committer line.\")) \\\n\t...\n\nas some of the users of the macro may have use of the translatable\nmessages themselves.\n"},{"id":"465762","messageId":"221026.8635badbz5.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"Y1jG8p1aARb9+CV+@coredump.intra.peff.net","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-26T11:35:43Z","receivedAt":"2022-10-26T12:09:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 26 2022, Jeff King wrote:\n\n> On Wed, Oct 26, 2022 at 04:43:32AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> \n>> On Tue, Oct 25 2022, Junio C Hamano wrote:\n>> \n>> > During the initial development of the fsck-msgids.txt feature, it\n>> > has become apparent that it is very much error prone to make sure\n>> > the description in the documentation file are sorted and correctly\n>> > match what is in the fsck.h header file.\n>> \n>> I have local fixes for the same issues in the list of advice in our\n>> docs, some of it's missing, wrong, out of date etc.\n>> \n>> I tried to quickly adapt the generation script I had for that, which\n>> works nicely, and by line count much shorter than the lint :)\n>\n> Yeah, my instinct here was to generate rather than lint. If you make a\n> mistake and the linter hits you over the head, that is better than\n> quietly letting your mistake go. But better still is making it\n> impossible to make in the first place.\n>\n> The downside is added complexity to the build, but it doesn't seem too\n> bad in this case.\n\nYeah, it's not, I have local patches to generate advice-{type,config}.h,\nand builtin.h. This is a quick POC to do it for fsck-msgids.h.\n\nI see I forgot the .gitignore entry, so it's a rough POC :)\n\n> (I had a similar thought after getting hit on the head by the recent\n> t0450-txt-doc-vs-help.sh).\n\nSorry about that. FWIW I've wanted to assert that for a while, and to do\nit by e.g. having the doc *.txt blurbs generated from running \"$buildin\n-h\" during the build.\n\nBut when I suggested that I gathered That Junio wasn't up for that\napproach, it does have its downsides thorugh. E.g. to build the docs\nyou'd now have to compile C code, and e.g. that git-scm.com publisher\nwritten in Ruby would have to compile the code to generate its docs.\n\nOr we could do it the other way around, and scape the *.txt for the *.c\ngeneration, but then we need to run a new script for building\nbuiltin/*.o. Also possible, and I think eventually both are better than\nwhat t0450-txt-doc-vs-help.sh's doing now, but that was a less invasive\nchange than both...\n\n>> Having to exhaustively list every *.c file that uses fsck.h is a bit of\n>> a bother, but we have the same with the other generated *.h's, so it\n>> shouldn't be too bad.\n>\n> It feels like this could be made much shorter by having a separate\n> fsck-msgs.h and not including it from fsck.h. Only fsck.c and mktag.c\n> need the actual list. It would probably have to stop being an \"enum\",\n> though.\n\nYes, that would make it shorter, but C doesn't have forward decls of\nenums, so we'd need to make it \"int\", ....\n\n> Another alternative is to generate the doc from the code, rather than\n> the other way around.\n\n*nod* :)\n\n>> +# Unfortunately our dependency management of generated headers used\n>> +# from within other headers suck, so we'll need to list every user of\n>> +# fsck.h here, but not too bad...\n>> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n>> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n>> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n>> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n>\n> I don't understand the \"used within other headers\" part here. Computed\n> dependencies will get this right. It's only the initial build (before we\n> have any computed dependencies) that needs to know that the C files in\n> question depend on the generated file. But that is true whether they do\n> so directly or indirectly.\n\nI forgot the full story, but luckily I wrote it down in the WIP commits\n:) FWIW if you want to scour that it's mainly:\n\n\thttps://github.com/avar/git/commit/a00f1cb9ea5 # add advice-type.h\n\thttps://github.com/avar/git/commit/9e080923a11 # generate 'struct advice\n\nAlso, before generating builtin.h I've got e.g. this:\n\n\thttps://github.com/avar/git/commit/5a5360d0134 # just check with 'make' that any file is sorted\n\nTo actualy generate it, very WIP:\n\n http://github.com/avar/git/commit/cf1d02fa6b2\n\nAnyway, in partial summary, why (and sorry this got so long):\n\n * Yes, once we build e.g. fsck.o we have the full dependency tree,\n yay....\n * ...but only for gcc & clang, but we support other compilers.\n * ...for other compilers (or gcc & clang without the dep detection\n enabled) what should this do:\n\n\tmake file-that-does-not-use-generated-header.o\n\n It sucks a bit to have e.g. every *.o depend on command-list.h, when\n we only use it in 1-2 places, ditto the other headers.\n\n The approach I took to this was to manually list headers that had\n \"small uses\" (1-10), but say that all of *.h dependend on running the\n *.sh to generate some \"widely used\" headers.\n\n E.g. if we generate builtin.h we'll either need that, or add\n builtin/%.o as a dependency, with a manual listing of the handful of\n uses outside of that subdirectory.\n\n * .... or just not care about any of that, i.e. to have all of our *.sh\n run on the \"first build\" no matter what, which certainly simplifies\n things, but once you have e.g. 5-10 generated headers doing e.g.:\n\n\tmake grep.o\n\n and having it build command-list.h or whatever is a bit noisy, but\n it's a trade-off of manually maintaining deps v.s. not.\n\n * Our \"COMPUTED_HEADER_DEPENDENCIES\" only happens when you build the\n *.o, but we have *.sp and *.s targets too. I have some local changes\n *to generalize that, so we e.g. get proper dependencies for building\n **.s files.\n\n * We also have e.g. \"make hdr-check\", which works just fine now, but\n it's becausue we have no *.h file that uses another generated *.h\n file.\n\n Actually, I suspect the posted WIP change is broken in that regard\n (but doing this for real on my topic branch works), i.e. we need to\n start caring about deps for those auxiliary targets.\n\n * I may be wrong (I'm just looking at some old \"here be dragons\" note\n of mine), but I think there's also an edge case where the dependency\n graph of .depend.d/* is correct *once you've always had it*, but if a\n file now e.g. depends on foo.h, and I make foo.h include a new\n foo-generated.h things go boom.\n\n That issue (it if even exists, sorry, I'm blanking on the details)\n was solvable by just doing a \"make clean\", and building again,\n i.e. once we had re-built once we were OK, it was only a problem with\n an older checkout with an existing build pulling new sources.\n\n Still, I wanted to not make that case annoying either if I added a\n generated header...\n"},{"id":"465924","messageId":"Y1su99Kc0ex1W7bX@coredump.intra.peff.net","threadId":"58682","inReplyTo":"221026.8635badbz5.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-28T01:23:03Z","receivedAt":"2022-10-28T01:23:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 26, 2022 at 01:35:43PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > (I had a similar thought after getting hit on the head by the recent\n> > t0450-txt-doc-vs-help.sh).\n> \n> Sorry about that. FWIW I've wanted to assert that for a while, and to do\n> it by e.g. having the doc *.txt blurbs generated from running \"$buildin\n> -h\" during the build.\n> \n> But when I suggested that I gathered That Junio wasn't up for that\n> approach, it does have its downsides thorugh. E.g. to build the docs\n> you'd now have to compile C code, and e.g. that git-scm.com publisher\n> written in Ruby would have to compile the code to generate its docs.\n\nYes, it would definitely break the git-scm.com importer. It might be\npossible to make it work by actually running \"make man\" (or maybe even\nsome partial targets) in a local repo. The nightly update job pulls all\nof the data using GitHub's API, but it does run in a heroku dyno that\nhas git and gcc. Doing a shallow clone of the tag to build is probably\nmore expensive, but the cost of an actual build isn't that important.\n99% of the scheduled job runs are noops because there's no new version\nof Git to build manpages for; as long as those remain cheap-ish, we're\nOK.\n\nI also think in the long run that the site should move to a system that\nbuilds off a local repo, which we can trigger manually or via GitHub\nActions. But that doesn't exist yet, and I don't think anyone's actively\nworking on it.\n\nI also think it would be nice if the git-scm.com system relied more on\n\"make man USE_ASCIIDOCTOR=1\", possibly by hooking into\nasciidoctor-extensions.rb or similar, rather than munging files in an\nad-hoc manner. But that's also a big change that nobody is actively\nworking on.\n\nAll of which is to say that yes, having docs depend on C code will cause\nwork on the site, but it may be work that takes us in the right\ndirection.\n\n> Or we could do it the other way around, and scape the *.txt for the *.c\n> generation, but then we need to run a new script for building\n> builtin/*.o. Also possible, and I think eventually both are better than\n> what t0450-txt-doc-vs-help.sh's doing now, but that was a less invasive\n> change than both...\n\nI think this particular case is tricky in that direction, because it's a\nbig set of dependencies that aren't necessarily one-to-one. E.g.,\nbuiltin/log.c needs to depend on git-log.txt, but also on git-show.txt\nand git-format-patch.txt.\n\n> >> +# Unfortunately our dependency management of generated headers used\n> >> +# from within other headers suck, so we'll need to list every user of\n> >> +# fsck.h here, but not too bad...\n> >> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n> >> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n> >> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n> >> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n> >\n> > I don't understand the \"used within other headers\" part here. Computed\n> > dependencies will get this right. It's only the initial build (before we\n> > have any computed dependencies) that needs to know that the C files in\n> > question depend on the generated file. But that is true whether they do\n> > so directly or indirectly.\n> \n> I forgot the full story, but luckily I wrote it down in the WIP commits\n> :) FWIW if you want to scour that it's mainly:\n> \n> \thttps://github.com/avar/git/commit/a00f1cb9ea5 # add advice-type.h\n> \thttps://github.com/avar/git/commit/9e080923a11 # generate 'struct advice\n\nThat seems like the same problem to me. It's just that if something is\nin cache.h, then it's needed by basically everything, and so the\ndependency list is big.\n\n> Anyway, in partial summary, why (and sorry this got so long):\n> \n> * Yes, once we build e.g. fsck.o we have the full dependency tree,\n> yay....\n> * ...but only for gcc & clang, but we support other compilers.\n> * ...for other compilers (or gcc & clang without the dep detection\n> enabled) what should this do:\n> \n> \tmake file-that-does-not-use-generated-header.o\n> \n> It sucks a bit to have e.g. every *.o depend on command-list.h, when\n> we only use it in 1-2 places, ditto the other headers.\n\nBut that is already true of non-generated headers. If your system\ndoesn't support computed deps, then all objects depend on all headers.\nYes, that sucks for you. But almost nobody actively develops on such a\nsystem, and people building Git to use rarely notice (because they are\ndoing an initial build, or jumping versions, both of which generally\nrequire recompilation anyway).\n\nThat all comes from b8ba629264 (Makefile: fold MISC_H into LIB_H,\n2012-06-20).\n\n> * We also have e.g. \"make hdr-check\", which works just fine now, but\n> it's becausue we have no *.h file that uses another generated *.h\n> file.\n\nI'm not that surprised. Probably it should depend on $(GENERATED_H), and\npossibly even be checking those files too.\n\n> * I may be wrong (I'm just looking at some old \"here be dragons\" note\n> of mine), but I think there's also an edge case where the dependency\n> graph of .depend.d/* is correct *once you've always had it*, but if a\n> file now e.g. depends on foo.h, and I make foo.h include a new\n> foo-generated.h things go boom.\n\nThat should work because the timestamp on foo.h will have been updated,\ncausing anything that includes it to rebuild (and thus updating its\ncomputed dependencies to include foo-generated.h).\n\n-Peff\n"},{"id":"465925","messageId":"221028.86sfj88xdq.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"Y1su99Kc0ex1W7bX@coredump.intra.peff.net","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-28T02:04:12Z","receivedAt":"2022-10-28T03:01:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 27 2022, Jeff King wrote:\n\n> On Wed, Oct 26, 2022 at 01:35:43PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > (I had a similar thought after getting hit on the head by the recent\n>> > t0450-txt-doc-vs-help.sh).\n>> \n>> Sorry about that. FWIW I've wanted to assert that for a while, and to do\n>> it by e.g. having the doc *.txt blurbs generated from running \"$buildin\n>> -h\" during the build.\n>> \n>> But when I suggested that I gathered That Junio wasn't up for that\n>> approach, it does have its downsides thorugh. E.g. to build the docs\n>> you'd now have to compile C code, and e.g. that git-scm.com publisher\n>> written in Ruby would have to compile the code to generate its docs.\n>\n> Yes, it would definitely break the git-scm.com importer. It might be\n> possible to make it work by actually running \"make man\" (or maybe even\n> some partial targets) in a local repo. The nightly update job pulls all\n> of the data using GitHub's API, but it does run in a heroku dyno that\n> has git and gcc. Doing a shallow clone of the tag to build is probably\n> more expensive, but the cost of an actual build isn't that important.\n> 99% of the scheduled job runs are noops because there's no new version\n> of Git to build manpages for; as long as those remain cheap-ish, we're\n> OK.\n>\n> I also think in the long run that the site should move to a system that\n> builds off a local repo, which we can trigger manually or via GitHub\n> Actions. But that doesn't exist yet, and I don't think anyone's actively\n> working on it.\n>\n> I also think it would be nice if the git-scm.com system relied more on\n> \"make man USE_ASCIIDOCTOR=1\", possibly by hooking into\n> asciidoctor-extensions.rb or similar, rather than munging files in an\n> ad-hoc manner. But that's also a big change that nobody is actively\n> working on.\n>\n> All of which is to say that yes, having docs depend on C code will cause\n> work on the site, but it may be work that takes us in the right\n> direction.\n\nGood to know.\n\n>> Or we could do it the other way around, and scape the *.txt for the *.c\n>> generation, but then we need to run a new script for building\n>> builtin/*.o. Also possible, and I think eventually both are better than\n>> what t0450-txt-doc-vs-help.sh's doing now, but that was a less invasive\n>> change than both...\n>\n> I think this particular case is tricky in that direction, because it's a\n> big set of dependencies that aren't necessarily one-to-one. E.g.,\n> builtin/log.c needs to depend on git-log.txt, but also on git-show.txt\n> and git-format-patch.txt.\n\nI was thinking of just a generated usage-strings.c or whatever. I.e. one\nfile with every usage string in it. Then you don't need to hunt down\nwhich thing goes in which file. We'll just have a variable named\n\"builtin_format_patch_usage\". Include it in \"builtin.h\" and it doesn't\nmatter if that's in builtin/log.c or builtin/format-patch.c.\n\nIt does mean you need to rebuild the C code for other built-ins every\ntime one of the SYNOPSIS sections changes, but doesn't happen too often.\n\n>> >> +# Unfortunately our dependency management of generated headers used\n>> >> +# from within other headers suck, so we'll need to list every user of\n>> >> +# fsck.h here, but not too bad...\n>> >> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n>> >> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n>> >> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n>> >> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n>> >\n>> > I don't understand the \"used within other headers\" part here. Computed\n>> > dependencies will get this right. It's only the initial build (before we\n>> > have any computed dependencies) that needs to know that the C files in\n>> > question depend on the generated file. But that is true whether they do\n>> > so directly or indirectly.\n>> \n>> I forgot the full story, but luckily I wrote it down in the WIP commits\n>> :) FWIW if you want to scour that it's mainly:\n>> \n>> \thttps://github.com/avar/git/commit/a00f1cb9ea5 # add advice-type.h\n>> \thttps://github.com/avar/git/commit/9e080923a11 # generate 'struct advice\n>\n> That seems like the same problem to me. It's just that if something is\n> in cache.h, then it's needed by basically everything, and so the\n> dependency list is big.\n\nI think I either did or was planning to take it out of cache.h as part\nof that, we put way too much in cache.h.\n\nEven advice.c depends on cache.h for its advice.h *sigh*.\n\nTrying it just now putting advice.h in builtin.h instead leaves 10\ntop-level files not-compiling (and they just need the header).\n\nI think it's fine to include common utilties in our \"included by\neverything\" headers, but if we've got ~500 *.c files and something is\nonly needed by ~20 of them (as in this case) we should probably just\ninclude it in those 20.\n\n>> Anyway, in partial summary, why (and sorry this got so long):\n>> \n>> * Yes, once we build e.g. fsck.o we have the full dependency tree,\n>> yay....\n>> * ...but only for gcc & clang, but we support other compilers.\n>> * ...for other compilers (or gcc & clang without the dep detection\n>> enabled) what should this do:\n\nI didn't summarize this well enough, I should have said that whether you\nhave computed deps or not that.....\n\n>> \n>> \tmake file-that-does-not-use-generated-header.o\n>> \n>> It sucks a bit to have e.g. every *.o depend on command-list.h, when\n>> we only use it in 1-2 places, ditto the other headers.\n> But that is already true of non-generated headers. If your system\n> doesn't support computed deps, then all objects depend on all headers.\n\n... this does not build e.g. command-list.h:\n\n\tmake clean\n\tmake grep.o\n\nBut this does:\n\n\tmake clean\n\tmake help.o\n\nBecause we've manually declared that.\n\n[Re-arranging a bit]\n\n> That all comes from b8ba629264 (Makefile: fold MISC_H into LIB_H,\n> 2012-06-20).\n\nYes, but you get that for free whether you have computed deps or\nnot. I.e. your compiler is about to build the code anyway.\n\nBut it wasn't about to run a bunch of shellscripts to generate headers\nit doesn't actually need.\n\n> Yes, that sucks for you. But almost nobody actively develops on such a\n> system, and people building Git to use rarely notice (because they are\n> doing an initial build, or jumping versions, both of which generally\n> require recompilation anyway).\n\nI guess I'm \"almost nobody\" :) Not because I don't use computed deps,\nbut because I tend to e.g. do large interactive rebases with:\n\n\tgit rebase -i 'make grep.o'\n\nOr some other subset of our built assets/objects.\n\nOn that front no one thing is the end of the world, i.e. sure, if we run\nsome shellscript to make a needed-header.h that takes 10ms we can live\nwith it.\n\nBut it adds up, which is one reason I've been slowly trickling in\npatches to optimize various common parts of the Makefile & those\nscripts.\n\nI think before I started that the fixed cost even to have just make\ndecide it had nothing to do was often 500ms-1s.\n\nI think that's down to 1/4 or so of that on \"master\", and I've got some\nlocal patches to get it consistently down to <50ms.\n\nIt adds up, especially when combined with ccache & the like. I.e. if i'm\ntesting 10 commits I happen to have cached a \"rebase -i\" goes from\nnoticeably slow (~10s) to not being so slow as to be distracting.\n\nAnyway, all of which is to say that that's one thing I was aiming for:\nIf I was going to submit patches for new generated headers to find some\ngood trade-off between complexity and over-declaring dependencies.\n\n>> * We also have e.g. \"make hdr-check\", which works just fine now, but\n>> it's becausue we have no *.h file that uses another generated *.h\n>> file.\n>\n> I'm not that surprised. Probably it should depend on $(GENERATED_H), and\n> possibly even be checking those files too.\n>\n>> * I may be wrong (I'm just looking at some old \"here be dragons\" note\n>> of mine), but I think there's also an edge case where the dependency\n>> graph of .depend.d/* is correct *once you've always had it*, but if a\n>> file now e.g. depends on foo.h, and I make foo.h include a new\n>> foo-generated.h things go boom.\n>\n> That should work because the timestamp on foo.h will have been updated,\n> causing anything that includes it to rebuild (and thus updating its\n> computed dependencies to include foo-generated.h).\n\nYes, in general it should work, maybe there's some Heisenbug there. I\ncan't recall or reproduce it, sorry.\n\nBut that's a good thing, maybe there's no issue :)\n"},{"id":"465926","messageId":"08A5BC44-24D9-4C8F-A61A-41983A13553A@gmail.com","threadId":"58682","inReplyTo":"221026.8635badbz5.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"John Cai","fromEmail":"johncai86@gmail.com","sentAt":"2022-10-28T03:02:14Z","receivedAt":"2022-10-28T03:02:23Z","isPatch":true,"sender":{"key":"johncai86@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2354211?v=4"},"body":"Hi Ævar,\n\nOn 26 Oct 2022, at 7:35, Ævar Arnfjörð Bjarmason wrote:\n\n> On Wed, Oct 26 2022, Jeff King wrote:\n>\n>> On Wed, Oct 26, 2022 at 04:43:32AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>\n>>>\n>>> On Tue, Oct 25 2022, Junio C Hamano wrote:\n>>>\n>>>> During the initial development of the fsck-msgids.txt feature, it\n>>>> has become apparent that it is very much error prone to make sure\n>>>> the description in the documentation file are sorted and correctly\n>>>> match what is in the fsck.h header file.\n>>>\n>>> I have local fixes for the same issues in the list of advice in our\n>>> docs, some of it's missing, wrong, out of date etc.\n>>>\n>>> I tried to quickly adapt the generation script I had for that, which\n>>> works nicely, and by line count much shorter than the lint :)\n>>\n>> Yeah, my instinct here was to generate rather than lint. If you make a\n>> mistake and the linter hits you over the head, that is better than\n>> quietly letting your mistake go. But better still is making it\n>> impossible to make in the first place.\n>>\n>> The downside is added complexity to the build, but it doesn't seem too\n>> bad in this case.\n>\n> Yeah, it's not, I have local patches to generate advice-{type,config}.h,\n> and builtin.h. This is a quick POC to do it for fsck-msgids.h.\n>\n> I see I forgot the .gitignore entry, so it's a rough POC :)\n>\n>> (I had a similar thought after getting hit on the head by the recent\n>> t0450-txt-doc-vs-help.sh).\n>\n> Sorry about that. FWIW I've wanted to assert that for a while, and to do\n> it by e.g. having the doc *.txt blurbs generated from running \"$buildin\n> -h\" during the build.\n\nIf we wanted to go this route of generating the docs from the code (which sounds\nlike a better long term solution), how would this work? Would we print out the\nlist of message ids in builtin/fsck.c and write it to\nDocumentation/fsck-msgids.txt ?\n\n>\n> But when I suggested that I gathered That Junio wasn't up for that\n> approach, it does have its downsides thorugh. E.g. to build the docs\n> you'd now have to compile C code, and e.g. that git-scm.com publisher\n> written in Ruby would have to compile the code to generate its docs.\n>\n> Or we could do it the other way around, and scape the *.txt for the *.c\n> generation, but then we need to run a new script for building\n> builtin/*.o. Also possible, and I think eventually both are better than\n> what t0450-txt-doc-vs-help.sh's doing now, but that was a less invasive\n> change than both...\n>\n>>> Having to exhaustively list every *.c file that uses fsck.h is a bit of\n>>> a bother, but we have the same with the other generated *.h's, so it\n>>> shouldn't be too bad.\n>>\n>> It feels like this could be made much shorter by having a separate\n>> fsck-msgs.h and not including it from fsck.h. Only fsck.c and mktag.c\n>> need the actual list. It would probably have to stop being an \"enum\",\n>> though.\n>\n> Yes, that would make it shorter, but C doesn't have forward decls of\n> enums, so we'd need to make it \"int\", ....\n>\n>> Another alternative is to generate the doc from the code, rather than\n>> the other way around.\n>\n> *nod* :)\n>\n>>> +# Unfortunately our dependency management of generated headers used\n>>> +# from within other headers suck, so we'll need to list every user of\n>>> +# fsck.h here, but not too bad...\n>>> +FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects\n>>> +$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h\n>>> +FSCK_MSGIDS_H_LIBS = fetch-pack fsck\n>>> +$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h\n>>\n>> I don't understand the \"used within other headers\" part here. Computed\n>> dependencies will get this right. It's only the initial build (before we\n>> have any computed dependencies) that needs to know that the C files in\n>> question depend on the generated file. But that is true whether they do\n>> so directly or indirectly.\n>\n> I forgot the full story, but luckily I wrote it down in the WIP commits\n> :) FWIW if you want to scour that it's mainly:\n>\n> \thttps://github.com/avar/git/commit/a00f1cb9ea5 # add advice-type.h\n> \thttps://github.com/avar/git/commit/9e080923a11 # generate 'struct advice\n>\n> Also, before generating builtin.h I've got e.g. this:\n>\n> \thttps://github.com/avar/git/commit/5a5360d0134 # just check with 'make' that any file is sorted\n>\n> To actualy generate it, very WIP:\n>\n> http://github.com/avar/git/commit/cf1d02fa6b2\n>\n> Anyway, in partial summary, why (and sorry this got so long):\n>\n> * Yes, once we build e.g. fsck.o we have the full dependency tree,\n> yay....\n> * ...but only for gcc & clang, but we support other compilers.\n> * ...for other compilers (or gcc & clang without the dep detection\n> enabled) what should this do:\n>\n> \tmake file-that-does-not-use-generated-header.o\n>\n> It sucks a bit to have e.g. every *.o depend on command-list.h, when\n> we only use it in 1-2 places, ditto the other headers.\n>\n> The approach I took to this was to manually list headers that had\n> \"small uses\" (1-10), but say that all of *.h dependend on running the\n> *.sh to generate some \"widely used\" headers.\n>\n> E.g. if we generate builtin.h we'll either need that, or add\n> builtin/%.o as a dependency, with a manual listing of the handful of\n> uses outside of that subdirectory.\n>\n> * .... or just not care about any of that, i.e. to have all of our *.sh\n> run on the \"first build\" no matter what, which certainly simplifies\n> things, but once you have e.g. 5-10 generated headers doing e.g.:\n>\n> \tmake grep.o\n>\n> and having it build command-list.h or whatever is a bit noisy, but\n> it's a trade-off of manually maintaining deps v.s. not.\n>\n> * Our \"COMPUTED_HEADER_DEPENDENCIES\" only happens when you build the\n> *.o, but we have *.sp and *.s targets too. I have some local changes\n> *to generalize that, so we e.g. get proper dependencies for building\n> **.s files.\n>\n> * We also have e.g. \"make hdr-check\", which works just fine now, but\n> it's becausue we have no *.h file that uses another generated *.h\n> file.\n>\n> Actually, I suspect the posted WIP change is broken in that regard\n> (but doing this for real on my topic branch works), i.e. we need to\n> start caring about deps for those auxiliary targets.\n>\n> * I may be wrong (I'm just looking at some old \"here be dragons\" note\n> of mine), but I think there's also an edge case where the dependency\n> graph of .depend.d/* is correct *once you've always had it*, but if a\n> file now e.g. depends on foo.h, and I make foo.h include a new\n> foo-generated.h things go boom.\n>\n> That issue (it if even exists, sorry, I'm blanking on the details)\n> was solvable by just doing a \"make clean\", and building again,\n> i.e. once we had re-built once we were OK, it was only a problem with\n> an older checkout with an existing build pulling new sources.\n>\n> Still, I wanted to not make that case annoying either if I added a\n> generated header...\n"},{"id":"465928","messageId":"221028.86k04k8wbk.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"08A5BC44-24D9-4C8F-A61A-41983A13553A@gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-28T03:11:07Z","receivedAt":"2022-10-28T03:24:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Oct 27 2022, John Cai wrote:\n\n> Hi Ævar,\n>\n> On 26 Oct 2022, at 7:35, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Wed, Oct 26 2022, Jeff King wrote:\n>>\n>>> On Wed, Oct 26, 2022 at 04:43:32AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>>>\n>>>>\n>>>> On Tue, Oct 25 2022, Junio C Hamano wrote:\n>>>>\n>>>>> During the initial development of the fsck-msgids.txt feature, it\n>>>>> has become apparent that it is very much error prone to make sure\n>>>>> the description in the documentation file are sorted and correctly\n>>>>> match what is in the fsck.h header file.\n>>>>\n>>>> I have local fixes for the same issues in the list of advice in our\n>>>> docs, some of it's missing, wrong, out of date etc.\n>>>>\n>>>> I tried to quickly adapt the generation script I had for that, which\n>>>> works nicely, and by line count much shorter than the lint :)\n>>>\n>>> Yeah, my instinct here was to generate rather than lint. If you make a\n>>> mistake and the linter hits you over the head, that is better than\n>>> quietly letting your mistake go. But better still is making it\n>>> impossible to make in the first place.\n>>>\n>>> The downside is added complexity to the build, but it doesn't seem too\n>>> bad in this case.\n>>\n>> Yeah, it's not, I have local patches to generate advice-{type,config}.h,\n>> and builtin.h. This is a quick POC to do it for fsck-msgids.h.\n>>\n>> I see I forgot the .gitignore entry, so it's a rough POC :)\n>>\n>>> (I had a similar thought after getting hit on the head by the recent\n>>> t0450-txt-doc-vs-help.sh).\n>>\n>> Sorry about that. FWIW I've wanted to assert that for a while, and to do\n>> it by e.g. having the doc *.txt blurbs generated from running \"$buildin\n>> -h\" during the build.\n>\n> If we wanted to go this route of generating the docs from the code (which sounds\n> like a better long term solution), how would this work? Would we print out the\n> list of message ids in builtin/fsck.c and write it to\n> Documentation/fsck-msgids.txt ?\n\nFirst, for the purposes of this thread I think Jeff and I are far off\ninto the weeds here :)\n\nI think nothing needs to change in how this topic's doing things, we're\njust takling about the longer term.\n\nBut if we go for that: I think in this case & most I can think of\ngenerating the code from the docs is better (as that rough POC I had\nshowed), because:\n\n - You just need a shellscript to scrape the docs to make a *.c or *.h,\n whereas you'd need a C compiler to make the docs if it's the other\n way around. But more importantly:\n\n - The docs are way easier to scrape with some sed/awk/grep/whatever\n few-liner than to scrape C code for generating docs. E.g. see\n config-list.h.\n\n - Scraping the C code sucks so much that we'd probably make some\n dedicated interface for it, e.g. what we have for \"git \n --git-completion-helper\".\n\n In that case it's worth it, but for other things we'n need to make\n the interface & maintain it (even if it's some test helper just for\n the build).\n\nBut mainly it helps to have a use-case, replacing the linter script with\ne.g. the *.sh I demo'd might be a marginal improvement. But e.g. \"git\nhelp -c\" uses one of those generated files (config-list.h), and actually\ndoes something useful ...\n\nIs there a good use-case for the fsck data like that? I'd think that\nwe'd want to make sure the docs are in sync with the code, as in we're\nnot adding new warnings/errors etc. without documenting them. But beyond\nthat maybe not much, and people would just run \"git help fsck\" to get\nthe list of variables..\n"},{"id":"465929","messageId":"Y1tpVqWn5yMYbE21@coredump.intra.peff.net","threadId":"58682","inReplyTo":"221028.86sfj88xdq.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-28T05:32:06Z","receivedAt":"2022-10-28T05:32:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 28, 2022 at 04:04:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > I think this particular case is tricky in that direction, because it's a\n> > big set of dependencies that aren't necessarily one-to-one. E.g.,\n> > builtin/log.c needs to depend on git-log.txt, but also on git-show.txt\n> > and git-format-patch.txt.\n> \n> I was thinking of just a generated usage-strings.c or whatever. I.e. one\n> file with every usage string in it. Then you don't need to hunt down\n> which thing goes in which file. We'll just have a variable named\n> \"builtin_format_patch_usage\". Include it in \"builtin.h\" and it doesn't\n> matter if that's in builtin/log.c or builtin/format-patch.c.\n\nYes, though you have the opposite problem, then: what are the source\nfiles that can produce that usage-strings.c? If your answer is\n\"Documentation/git-*.txt\", then that is a recipe for flaky dependencies.\n\nWe already have one such for config-list.h. Try this:\n\n # introduce a new file\n printf 'foo.bar::\\n\\ta fake config var\\n' \\\n >Documentation/config/foo.txt\n\n # it shows up in the output, as expected\n make\n ./git help --config-for-completion | grep foo\n\n # now drop it\n rm Documentation/config/foo.txt\n\n # oops; make won't rebuild anything, and it's still there\n make\n ./git help --config-for-completion | grep foo\n\nThe bug is that config-list.h depends on a glob. So we notice when a\nfile changes, but not when one goes away. And this isn't just\nhypothetical. Files come and go as you \"git checkout\" around history (or\nbisect). I don't remember the details, but I'm pretty sure I've gotten\nfalse positive test failures out of this before (maybe a topic branch\nwith a bogus entry that broke t0012 or t9902, and then moving back to\nmaster doesn't fix it?).\n\nSo I'd prefer to avoid introducing more flakiness if we can. You might\nbe able to piggy back on command-list.txt in this case (that's what\nmakes command-list.h not flaky).\n\n> It does mean you need to rebuild the C code for other built-ins every\n> time one of the SYNOPSIS sections changes, but doesn't happen too often.\n\nIf it's a c/h combo that only makes the variable names public (not the\ncontents of the strings they point to), then it would only trigger a\nrebuild when a command is added or removed.\n\n> I think I either did or was planning to take it out of cache.h as part\n> of that, we put way too much in cache.h.\n> \n> Even advice.c depends on cache.h for its advice.h *sigh*.\n> \n> Trying it just now putting advice.h in builtin.h instead leaves 10\n> top-level files not-compiling (and they just need the header).\n> \n> I think it's fine to include common utilties in our \"included by\n> everything\" headers, but if we've got ~500 *.c files and something is\n> only needed by ~20 of them (as in this case) we should probably just\n> include it in those 20.\n\nOh, definitely, we should be shrinking cache.h, and not adding more to\nit. Especially not generated stuff.\n\n> >> \tmake file-that-does-not-use-generated-header.o\n> >> \n> >> It sucks a bit to have e.g. every *.o depend on command-list.h, when\n> >> we only use it in 1-2 places, ditto the other headers.\n> > But that is already true of non-generated headers. If your system\n> > doesn't support computed deps, then all objects depend on all headers.\n> \n> ... this does not build e.g. command-list.h:\n> \n> \tmake clean\n> \tmake grep.o\n> \n> But this does:\n> \n> \tmake clean\n> \tmake help.o\n> \n> Because we've manually declared that.\n\nRight, but...does grep.o actually need command-list.h? If it doesn't\n(and that seems to be the case), then all is working as intended. If\ngrep included some other header that included command-list.h, then yeah,\nthat would be a bug. But that is true whether grep.c includes it\ndirectly or not. Any Makefile dependency needs to take into account\nrecursive includes.\n\n> > Yes, that sucks for you. But almost nobody actively develops on such a\n> > system, and people building Git to use rarely notice (because they are\n> > doing an initial build, or jumping versions, both of which generally\n> > require recompilation anyway).\n> \n> I guess I'm \"almost nobody\" :) Not because I don't use computed deps,\n> but because I tend to e.g. do large interactive rebases with:\n> \n> \tgit rebase -i 'make grep.o'\n> \n> Or some other subset of our built assets/objects.\n\nI do that all the time, too. But with computed deps, it works (by which\nI mean it rebuilds only stuff needed by grep.o).\n\nI'm not even sure what we're talking about anymore. If you are saying\nthat no, we don't want to just say \"everything depends on foo.h that is\ngenerated\", I agree. That is wrong to do, and we should specify the\nminimal dependencies where appropriate (and take care to keep that set\nas small as is feasible using small C interfaces).\n\nIf you're saying \"for people without computed dependencies, everything\nwill want to rebuild shell scripts\", then I don't care. We decided long\nago that maintaining a manual list of header dependencies was not worth\ndoing, and people with sub-par compilers will have to suffer.\n\nIn the email you're replying to, I was trying to express the second one.\nBut it sounds like you thought I was trying to argue against the first\none.\n\n-Peff\n"},{"id":"465930","messageId":"xmqqk04kjyxd.fsf@gitster.g","threadId":"58682","inReplyTo":"221028.86k04k8wbk.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-10-28T05:32:30Z","receivedAt":"2022-10-28T05:32:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> First, for the purposes of this thread I think Jeff and I are far off\n> into the weeds here :)\n\nIt is good to clearly separate where we would want to draw the line\nfor this round, to get the already-worked-on-and-immediately-available\nimprovement going, while we envision a future direction for the longer\nterm.\n\n> But if we go for that: I think in this case & most I can think of\n> generating the code from the docs is better (as that rough POC I had\n> showed), because:\n>\n> - You just need a shellscript to scrape the docs to make a *.c or *.h,\n> whereas you'd need a C compiler to make the docs if it's the other\n> way around. But more importantly:\n>\n> - The docs are way easier to scrape with some sed/awk/grep/whatever\n> few-liner than to scrape C code for generating docs. E.g. see\n> config-list.h.\n\nScraping docs is easier because we do not have to choose from\nmultiple choices that are all reasonable ;-). Either way, the\nsource material needs some discipline to keep it scrapable (e.g. to\nkeep the doc scrapable, you'd probably keep each entry a single\nline, or a fixed format like \"::\" followed by \"^I() \"\nfollowed by description. Nothing forbids us from giving developers\na similar rule to keep each entry in FOR_EACH_MSG_ID() macro easier\nto scrape, so it is about the same difficulty going either way.\n\nBut if you choose to make the code the source of truth, you'd have\nto see if it makes more sense to \"compile and run\" instead of\nscraping. That's another thing to consider and choose from, which\nmakes it harder ;-)\n\n> But mainly it helps to have a use-case, replacing the linter script with\n> e.g. the *.sh I demo'd might be a marginal improvement. But e.g. \"git\n> help -c\" uses one of those generated files (config-list.h), and actually\n> does something useful ...\n\nYes, I've shown how N_(\"explanation of the error\") may fit into the\nexisting scheme in a separate message upthread. If we go from code\nto doc, it would be a reasonable starting point.\n\nWhichever way the aout-generation goes, we'd complicate the build\ndependency graph, which is a downside. Another is that third-party\nconsumers of docs now need to generate some docs from the source,\nwhich may be additional burden for them.\n\n> Is there a good use-case for the fsck data like that? I'd think that\n> we'd want to make sure the docs are in sync with the code, as in we're\n> not adding new warnings/errors etc. without documenting them. But beyond\n> that maybe not much, and people would just run \"git help fsck\" to get\n> the list of variables..\n\n\"git help fsck-error-codes\" that does not have a pre-generated\ndocumentation (instead we'd just dump the N_(\"explanation\") to the\noutput) is certainly tempting. I am not sure if it would fly well.\nWhen was the last time you saw a manpage that says \"run this command\nand view its output for the most up-to-date list\" ;-)?\n\n"},{"id":"465931","messageId":"Y1tqOCh+FwoQezYu@coredump.intra.peff.net","threadId":"58682","inReplyTo":"221028.86k04k8wbk.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-28T05:35:52Z","receivedAt":"2022-10-28T05:35:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 28, 2022 at 05:11:07AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> - The docs are way easier to scrape with some sed/awk/grep/whatever\n> few-liner than to scrape C code for generating docs. E.g. see\n> config-list.h.\n> \n> - Scraping the C code sucks so much that we'd probably make some\n> dedicated interface for it, e.g. what we have for \"git \n> --git-completion-helper\".\n\nIn the general case, scraping C code is awful. But if you are looking\nfor one particular thing, it is not unreasonable to say something like\n\"when we write a usage string, we write it like:\n\n const char *git_cmdname_usage[] = \"...\";\n\n> But mainly it helps to have a use-case, replacing the linter script with\n> e.g. the *.sh I demo'd might be a marginal improvement. But e.g. \"git\n> help -c\" uses one of those generated files (config-list.h), and actually\n> does something useful ...\n\nYeah. In cases like config-list.h, it really is helpful for it to be\nsomething we can query at run-time (the other option would be stuffing\nthem into git-completion.bash at build-time, but there are some\ndownsides there).\n\n-Peff\n"},{"id":"465932","messageId":"Y1tqn5qdSzeFwzug@coredump.intra.peff.net","threadId":"58682","inReplyTo":"xmqqk04kjyxd.fsf@gitster.g","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-10-28T05:37:35Z","receivedAt":"2022-10-28T05:37:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 27, 2022 at 10:32:30PM -0700, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n> \n> > First, for the purposes of this thread I think Jeff and I are far off\n> > into the weeds here :)\n> \n> It is good to clearly separate where we would want to draw the line\n> for this round, to get the already-worked-on-and-immediately-available\n> improvement going, while we envision a future direction for the longer\n> term.\n\nYeah, to be clear, I'm OK with your linting script in the near term. I\ndidn't look at it _too_ carefully, but if the linter passes, then I\nthink the proof is in the pudding, and we can consider the topic done\nfor now.\n\n-Peff\n"},{"id":"465936","messageId":"221028.86fsf88aj9.gmgdl@evledraar.gmail.com","threadId":"58682","inReplyTo":"Y1tpVqWn5yMYbE21@coredump.intra.peff.net","subject":"Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-10-28T10:41:58Z","receivedAt":"2022-10-28T11:15:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Oct 28 2022, Jeff King wrote:\n\n> On Fri, Oct 28, 2022 at 04:04:12AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > I think this particular case is tricky in that direction, because it's a\n>> > big set of dependencies that aren't necessarily one-to-one. E.g.,\n>> > builtin/log.c needs to depend on git-log.txt, but also on git-show.txt\n>> > and git-format-patch.txt.\n>> \n>> I was thinking of just a generated usage-strings.c or whatever. I.e. one\n>> file with every usage string in it. Then you don't need to hunt down\n>> which thing goes in which file. We'll just have a variable named\n>> \"builtin_format_patch_usage\". Include it in \"builtin.h\" and it doesn't\n>> matter if that's in builtin/log.c or builtin/format-patch.c.\n>\n> Yes, though you have the opposite problem, then: what are the source\n> files that can produce that usage-strings.c? If your answer is\n> \"Documentation/git-*.txt\", then that is a recipe for flaky dependencies.\n\nFirst, you're not telling me anything new here :) I've looked at\nliterally all of these special-cases recently for t0450*.\n\n> We already have one such for config-list.h. Try this:\n>\n> # introduce a new file\n> printf 'foo.bar::\\n\\ta fake config var\\n' \\\n> >Documentation/config/foo.txt\n>\n> # it shows up in the output, as expected\n> make\n> ./git help --config-for-completion | grep foo\n>\n> # now drop it\n> rm Documentation/config/foo.txt\n>\n> # oops; make won't rebuild anything, and it's still there\n> make\n> ./git help --config-for-completion | grep foo\n\nYes, much of the dependency graph in Documentation/Makefile is simply\nbroken, and that's not the worst offender.\n\nTry adding a new category to command-list.txt, then remove it, and watch\nyour entire build fail until you 'git clean -dxf' (because\nDocumentation/Makefile will only remove the generated cmds-*.txt it\nknows about, but another part includes things based on a wildcard.\n\n> The bug is that config-list.h depends on a glob. So we notice when a\n> file changes, but not when one goes away. And this isn't just\n> hypothetical. Files come and go as you \"git checkout\" around history (or\n> bisect). I don't remember the details, but I'm pretty sure I've gotten\n> false positive test failures out of this before (maybe a topic branch\n> with a bogus entry that broke t0012 or t9902, and then moving back to\n> master doesn't fix it?).\n>\n> So I'd prefer to avoid introducing more flakiness if we can. You might\n> be able to piggy back on command-list.txt in this case (that's what\n> makes command-list.h not flaky).\n\nI'm in violent agreement with you that this problem sucks, but it's also\ntrivial to solve: Just don't create such crappy dependency graphs in our\nMakefiles.\n\nSo, in this case (and keep in mind this is off the cuff, and I'm not\nadvocating for doing this now) is:\n\n 1. We already have an exhaustive list of built-ins in the top-level\n Makefile\n 2. Turn those into corresponding Documentation/git-*.txt dependencies,\n e.g. builtin/rebase.o to Documentation/git-rebase.txt.\n 3. Some won't exist on the other side, we have no\n builtin/cherry-pick.o, but a Documentation/git-cherry-pick.txt etc. I\n know, keep reading... :)\n 4. Slurp up the *.txt, parse it (this code's mostly in t0450 already),\n spew out a generated *.c or *.h.\n\nCritically this is based on an exhaustive known list at this point, if\nwe add/remove built-ins we *will* re-generate it. It's not using a\nwildcard dependency.\n\nAt this point we have e.g. variables like:\n\n - git_builtin_bundle_usage\n - git_builtin_log_usage\n\nBut because we're not smart enough and/or it doesn't correspond to our\n*.txt structure we don't have e.g.:\n\n - git_builtin_bundle_create_usage (a sub-command)\n - git_builtin_fsck_objects_usage\n\nBut that's fine.\n\nWe can just handle remaining special-cases by not removing the cases we\ndidn't handle from the C sources, we'll still need to maintain those on\nboth sides, and have t0450 hopefully catch them drifting.\n\nI.e. a subsequent change to the C code would be something like:\n\t\n\tdiff --git a/builtin/bundle.c b/builtin/bundle.c\n\tindex e80efce3a42..557e00be5a2 100644\n\t--- a/builtin/bundle.c\n\t+++ b/builtin/bundle.c\n\t@@ -11,14 +11,7 @@\n\t * bundle supporting \"fetch\", \"pull\", and \"ls-remote\".\n\t */\n\t \n\t-static const char * const builtin_bundle_usage[] = {\n\t- N_(\"git bundle create [] \"),\n\t- N_(\"git bundle verify [] \"),\n\t- N_(\"git bundle list-heads [...]\"),\n\t- N_(\"git bundle unbundle [...]\"),\n\t- NULL\n\t-};\n\t-\n\t+/* too dumb for these still... */\n\t static const char * const builtin_bundle_create_usage[] = {\n\t N_(\"git bundle create [] \"),\n\t NULL\n\nWe didn't handle everything, but that's OK. Perfect shouldn't be the\nenemy of the good.\n\nWe have on the order of 150 built-ins, by far the majority of them are\nrelativtely easy to correlate to their docs & scrape them out.\n\nThe goal here would be to mostly get rid of the maintenance burden of\nmaintaining these in two places, if we still need to do that in 10-30\ncases instead of 150-200 we've solved most of the problem.\n\n>> It does mean you need to rebuild the C code for other built-ins every\n>> time one of the SYNOPSIS sections changes, but doesn't happen too often.\n>\n> If it's a c/h combo that only makes the variable names public (not the\n> contents of the strings they point to), then it would only trigger a\n> rebuild when a command is added or removed.\n\nYeah, and if we didn't like that we can make it a file per variable\n(which I think sucks), but a full re-build every time we add a built-in\nisn't too bad. And we *way* worse now, every time some utility function\nis added to a few common headers we basically do a re-build.\n\n>> I think I either did or was planning to take it out of cache.h as part\n>> of that, we put way too much in cache.h.\n>> \n>> Even advice.c depends on cache.h for its advice.h *sigh*.\n>> \n>> Trying it just now putting advice.h in builtin.h instead leaves 10\n>> top-level files not-compiling (and they just need the header).\n>> \n>> I think it's fine to include common utilties in our \"included by\n>> everything\" headers, but if we've got ~500 *.c files and something is\n>> only needed by ~20 of them (as in this case) we should probably just\n>> include it in those 20.\n>\n> Oh, definitely, we should be shrinking cache.h, and not adding more to\n> it. Especially not generated stuff.\n\n*more violent agreement noises* :)\n\nFWIW I have some patches to make headway on that stuff, which I've kept\nunsubmitted because I thought nobody was interested...\n\nI.e. a *very large* part of cache.h and friends is stuff that maybe 3-5\nfiles here and there need, not \"nearly everything\", as one might assume.\n\nWe should still have e.g. gettext.h and the like in there,\nprobably.\n\nAlthough for those we should probably split them off into builtin.h and\ne.g. a new libs.h, so that we don't get translations added in\nt/helper/*.c and the like, so maybe ...\n\n>> >> \tmake file-that-does-not-use-generated-header.o\n>> >> \n>> >> It sucks a bit to have e.g. every *.o depend on command-list.h, when\n>> >> we only use it in 1-2 places, ditto the other headers.\n>> > But that is already true of non-generated headers. If your system\n>> > doesn't support computed deps, then all objects depend on all headers.\n>> \n>> ... this does not build e.g. command-list.h:\n>> \n>> \tmake clean\n>> \tmake grep.o\n>> \n>> But this does:\n>> \n>> \tmake clean\n>> \tmake help.o\n>> \n>> Because we've manually declared that.\n>\n> Right, but...does grep.o actually need command-list.h? If it doesn't\n> (and that seems to be the case), then all is working as intended. If\n> grep included some other header that included command-list.h, then yeah,\n> that would be a bug. But that is true whether grep.c includes it\n> directly or not. Any Makefile dependency needs to take into account\n> recursive includes.\n\nYes, it doesn't need it, and it shouldn't build it. I'm saying that this\nis the reason we've needed manually maintained dependencies of these\ngenerated headers.\n\nI.e. if you take the easy way out and say that we have no generated deps\nyet, let's build all of it you would build that stuff with grep.o for no\nreason.\n\nAnd yes, I'm aware that that's what we do right now. I'm saying that\nit's probably OK because we've had 3-4 of these headers, but people\nmight mind if it's more, or a lot more...\n\n>> > Yes, that sucks for you. But almost nobody actively develops on such a\n>> > system, and people building Git to use rarely notice (because they are\n>> > doing an initial build, or jumping versions, both of which generally\n>> > require recompilation anyway).\n>> \n>> I guess I'm \"almost nobody\" :) Not because I don't use computed deps,\n>> but because I tend to e.g. do large interactive rebases with:\n>> \n>> \tgit rebase -i 'make grep.o'\n>> \n>> Or some other subset of our built assets/objects.\n>\n> I do that all the time, too. But with computed deps, it works (by which\n> I mean it rebuilds only stuff needed by grep.o).\n>\n> I'm not even sure what we're talking about anymore. If you are saying\n> that no, we don't want to just say \"everything depends on foo.h that is\n> generated\", I agree. That is wrong to do, and we should specify the\n> minimal dependencies where appropriate (and take care to keep that set\n> as small as is feasible using small C interfaces).\n>\n> If you're saying \"for people without computed dependencies, everything\n> will want to rebuild shell scripts\", then I don't care. We decided long\n> ago that maintaining a manual list of header dependencies was not worth\n> doing, and people with sub-par compilers will have to suffer.\n>\n> In the email you're replying to, I was trying to express the second one.\n> But it sounds like you thought I was trying to argue against the first\n> one.\n\nI'm saying there's a large chasm between using generated dependencies,\nand using them all the time.\n\nMaybe my dev setup is just different, but I do most of my development in\na primary development tree where I've generally built things already, so\nall the incremental compilation we support works out nicely.\n\nBut I also do things like spin up different workrtees for different\nrevisions, build some subset, and destroy them afterwards. Those don't\nbenefit from computed dependencies, so it's nice that we're not\nover-building things.\n\nAnyway:\n\nAll I was trying to get across with the upthread was that I had further\npatches in this area that might be useful, but that part of the reason I\nhadn't submitted them was because I didn't want to introduce suckage\nwith them.\n\nWe don't make a lot of use of generated code, and the uses we have now\nare really isolated (one header used by one *.c file, mostly).\n\nIf we make that a one<->many we should be careful, and there's some\ndragons lurking e.g. for generated *.h's used by other *.h's (all our\nuse is by *.c's at this point).\n"}]}