{"thread":{"id":"63050","subject":"[PATCH] docs: fix repository-layout when building with breaking changes","startedAt":"2025-03-03T16:11:53Z","lastAt":"2025-03-12T13:43:15Z","messageCount":17,"participants":["Phillip Wood via GitGitGadget","Junio C Hamano","Patrick Steinhardt","Phillip Wood","Karthik Nayak","phillip.wood123@gmail.com"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"513407","messageId":"pull.1871.git.1741018310447.gitgitgadget@gmail.com","threadId":"63050","inReplyTo":null,"subject":"[PATCH] docs: fix repository-layout when building with breaking changes","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-03T16:11:50Z","receivedAt":"2025-03-03T16:11:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nSince commit 8ccc75c2452 (remote: announce removal of \"branches/\" and\n\"remotes/\", 2025-01-22) enabling WITH_BREAKING_CHANGES when building git\nremoves support for reading branches from \".git/branches\" and remotes\nfrom \".git/remotes\". However those locations are still documented in\ngitrepository-layout.adoc even though the build does not support them.\n\nRectify this by adding a new document attribute\n\"without-breaking-changes\" and use it to make the inclusion of those\nsections of the documentation conditional. The name of the attribute is\nbased on the similar test prerequisite added in c5bc9a7f94a (Makefile:\nwire up build option for deprecated features, 2025-01-22).\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n    docs: fix repository-layout when building with breaking changes\n    \n    I copied the name from the test prerequisite as I didn't want to have\n    different names for condition used in the tests and documentation. I do\n    have some reservations about the naming though as it means we end up\n    having to use ifdef::!without-breaking-changes[] or test_expect_success\n    !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is\n    a double negative.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1871%2Fphillipwood%2Fbreaking-changes-documentation-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1871/phillipwood/breaking-changes-documentation-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1871\n\n Documentation/Makefile                  | 4 ++++\n Documentation/gitrepository-layout.adoc | 4 ++++\n Documentation/meson.build               | 4 ++++\n 3 files changed, 12 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex a734c6d6243..53a05eb8030 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -221,6 +221,10 @@ asciidoc.conf: asciidoc.conf.in FORCE\n \t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)/..\",$<,$@)\n endif\n \n+ifndef WITH_BREAKING_CHANGES\n+ASCIIDOC_EXTRA += -awithout-breaking-changes\n+endif\n+\n ASCIIDOC_DEPS += docinfo.html\n \n SHELL_PATH ?= $(SHELL)\ndiff --git a/Documentation/gitrepository-layout.adoc b/Documentation/gitrepository-layout.adoc\nindex 6348ef1dcdf..62ef1c98c26 100644\n--- a/Documentation/gitrepository-layout.adoc\n+++ b/Documentation/gitrepository-layout.adoc\n@@ -152,6 +152,7 @@ config.worktree::\n \tworking directory in multiple working directory setup (see\n \tlinkgit:git-worktree[1]).\n \n+ifdef::without-breaking-changes[]\n branches::\n \tA deprecated way to store shorthands to be used\n \tto specify a URL to 'git fetch', 'git pull' and 'git push'.\n@@ -164,6 +165,7 @@ branches::\n \t\"$GIT_COMMON_DIR/branches\" will be used instead.\n +\n Git will stop reading remotes from this directory in Git 3.0.\n+endif::without-breaking-changes[]\n \n hooks::\n \tHooks are customization scripts used by various Git\n@@ -231,6 +233,7 @@ info/sparse-checkout::\n \tThis file stores sparse checkout patterns.\n \tSee also: linkgit:git-read-tree[1].\n \n+ifdef::without-breaking-changes[]\n remotes::\n \tStores shorthands for URL and default refnames for use\n \twhen interacting with remote repositories via 'git fetch',\n@@ -241,6 +244,7 @@ remotes::\n \t\"$GIT_COMMON_DIR/remotes\" will be used instead.\n +\n Git will stop reading remotes from this directory in Git 3.0.\n+endif::without-breaking-changes[]\n \n logs::\n \tRecords of changes made to refs are stored in this directory.\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex ead8e482131..4e4fca283c1 100644\n--- a/Documentation/meson.build\n+++ b/Documentation/meson.build\n@@ -283,6 +283,10 @@ elif docs_backend == 'asciidoctor'\n   ]\n endif\n \n+if not get_option('breaking_changes')\n+   asciidoc_common_options += ['--attribute', 'without-breaking-changes']\n+endif\n+\n git = find_program('git', required: false)\n xmlto = find_program('xmlto')\n \n\nbase-commit: 03944513488db4a81fdb4c21c3b515e4cb260b05\n-- \ngitgitgadget\n"},{"id":"513419","messageId":"xmqqbjuivuzm.fsf@gitster.g","threadId":"63050","inReplyTo":"pull.1871.git.1741018310447.gitgitgadget@gmail.com","subject":"Re: [PATCH] docs: fix repository-layout when building with breaking changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-03T18:18:05Z","receivedAt":"2025-03-03T18:18:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Phillip Wood via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n>     I copied the name from the test prerequisite as I didn't want to have\n>     different names for condition used in the tests and documentation. I do\n>     have some reservations about the naming though as it means we end up\n>     having to use ifdef::!without-breaking-changes[] or test_expect_success\n>     !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is\n>     a double negative.\n\nIt was exactly the first thing that came to my mind when I saw the\nchange to the Makefile in the patch.  Unless our breaking changes\nare all removals, which is not likely to be the case in the longer\nterm, \"without-breaking-changes\" would be an invitation for\nconfusing double negatives.\n\n> +ifdef::without-breaking-changes[]\n>  branches::\n>  \tA deprecated way to store shorthands to be used\n>  \tto specify a URL to 'git fetch', 'git pull' and 'git push'.\n> @@ -164,6 +165,7 @@ branches::\n>  \t\"$GIT_COMMON_DIR/branches\" will be used instead.\n>  +\n>  Git will stop reading remotes from this directory in Git 3.0.\n> +endif::without-breaking-changes[]\n>  \n>  hooks::\n>  \tHooks are customization scripts used by various Git\n> @@ -231,6 +233,7 @@ info/sparse-checkout::\n>  \tThis file stores sparse checkout patterns.\n>  \tSee also: linkgit:git-read-tree[1].\n>  \n> +ifdef::without-breaking-changes[]\n>  remotes::\n>  \tStores shorthands for URL and default refnames for use\n>  \twhen interacting with remote repositories via 'git fetch',\n> @@ -241,6 +244,7 @@ remotes::\n>  \t\"$GIT_COMMON_DIR/remotes\" will be used instead.\n>  +\n>  Git will stop reading remotes from this directory in Git 3.0.\n> +endif::without-breaking-changes[]\n>  \n>  logs::\n>  \tRecords of changes made to refs are stored in this directory.\n\nThe above parts of the documentation getting commented out all look\nsensible to exclude in a build that omits these older mechanisms.\nBut can we do it with !with-breaking-changes instead?\n"},{"id":"513492","messageId":"Z8afGexTdlqDnPV8@pks.im","threadId":"63050","inReplyTo":"xmqqbjuivuzm.fsf@gitster.g","subject":"Re: [PATCH] docs: fix repository-layout when building with breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-04T06:35:05Z","receivedAt":"2025-03-04T06:35:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 03, 2025 at 10:18:05AM -0800, Junio C Hamano wrote:\n> \"Phillip Wood via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n> >     I copied the name from the test prerequisite as I didn't want to have\n> >     different names for condition used in the tests and documentation. I do\n> >     have some reservations about the naming though as it means we end up\n> >     having to use ifdef::!without-breaking-changes[] or test_expect_success\n> >     !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is\n> >     a double negative.\n> \n> It was exactly the first thing that came to my mind when I saw the\n> change to the Makefile in the patch.  Unless our breaking changes\n> are all removals, which is not likely to be the case in the longer\n> term, \"without-breaking-changes\" would be an invitation for\n> confusing double negatives.\n\nI remember not quite being happy with the double-negation myself. I\ndon't mind renaming the prerequisite we have in our test suite for\nconsistency, as well, if you want to do that.\n\nPatrick\n"},{"id":"513517","messageId":"2583368f-c7ad-4088-b78d-f6d972c4ee47@gmail.com","threadId":"63050","inReplyTo":"Z8afGexTdlqDnPV8@pks.im","subject":"Re: [PATCH] docs: fix repository-layout when building with breaking changes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-04T10:23:25Z","receivedAt":"2025-03-04T10:23:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 04/03/2025 06:35, Patrick Steinhardt wrote:\n> On Mon, Mar 03, 2025 at 10:18:05AM -0800, Junio C Hamano wrote:\n>> \"Phillip Wood via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>>\n>>>      I copied the name from the test prerequisite as I didn't want to have\n>>>      different names for condition used in the tests and documentation. I do\n>>>      have some reservations about the naming though as it means we end up\n>>>      having to use ifdef::!without-breaking-changes[] or test_expect_success\n>>>      !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is\n>>>      a double negative.\n>>\n>> It was exactly the first thing that came to my mind when I saw the\n>> change to the Makefile in the patch.  Unless our breaking changes\n>> are all removals, which is not likely to be the case in the longer\n>> term, \"without-breaking-changes\" would be an invitation for\n>> confusing double negatives.\n> \n> I remember not quite being happy with the double-negation myself. I\n> don't mind renaming the prerequisite we have in our test suite for\n> consistency, as well, if you want to do that.\n\nYes, I can do that when I re-roll the patches at \nhttps://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/ \nto use WITH_BREAKING_CHANGES\n\nBest Wishes\n\nPhillip\n\n> Patrick\n> \n\n"},{"id":"513538","messageId":"xmqqr03csry6.fsf@gitster.g","threadId":"63050","inReplyTo":"2583368f-c7ad-4088-b78d-f6d972c4ee47@gmail.com","subject":"Re: [PATCH] docs: fix repository-layout when building with breaking changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-04T16:04:17Z","receivedAt":"2025-03-04T16:04:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> I remember not quite being happy with the double-negation myself. I\n>> don't mind renaming the prerequisite we have in our test suite for\n>> consistency, as well, if you want to do that.\n>\n> Yes, I can do that when I re-roll the patches at\n> https://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/\n> to use WITH_BREAKING_CHANGES\n\nThanks, both.\n"},{"id":"513598","messageId":"pull.1871.v2.git.1741171357627.gitgitgadget@gmail.com","threadId":"63050","inReplyTo":"pull.1871.git.1741018310447.gitgitgadget@gmail.com","subject":"[PATCH v2] docs: fix repository-layout when building with breaking changes","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-05T10:42:37Z","receivedAt":"2025-03-05T10:42:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nSince commit 8ccc75c2452 (remote: announce removal of \"branches/\" and\n\"remotes/\", 2025-01-22) enabling WITH_BREAKING_CHANGES when building git\nremoves support for reading branches from \".git/branches\" and remotes\nfrom \".git/remotes\". However those locations are still documented in\ngitrepository-layout.adoc even though the build does not support them.\n\nRectify this by adding a new document attribute \"with-breaking-changes\"\nand use it to make the inclusion of those sections of the documentation\nconditional. Note that the name of the attribute does not match the test\nprerequisite WITHOUT_BREAKING_CHANGES added in c5bc9a7f94a (Makefile:\nwire up build option for deprecated features, 2025-01-22). This is to\navoid the awkward double negative ifndef::without_breaking_changes for\ndocumentation that should be included when WITH_BREAKING_CHANGES is\nenabled. The test prerequisite will be renamed to match the\ndocumentation attribute in a future patch series.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n    docs: fix repository-layout when building with breaking changes\n    \n    Thanks to Junio and Patrick for their comments on V1. I've renamed the\n    attribute to with-breaking-changes as suggested. I'll add a patch to\n    rename the test prerequisite to match this when I re-roll\n    https://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/\n    to use WITH_BREAKING_CHANGES.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1871%2Fphillipwood%2Fbreaking-changes-documentation-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1871/phillipwood/breaking-changes-documentation-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1871\n\nRange-diff vs v1:\n\n 1:  42877011c6d ! 1:  d8dcdca4c2b docs: fix repository-layout when building with breaking changes\n     @@ Commit message\n          from \".git/remotes\". However those locations are still documented in\n          gitrepository-layout.adoc even though the build does not support them.\n      \n     -    Rectify this by adding a new document attribute\n     -    \"without-breaking-changes\" and use it to make the inclusion of those\n     -    sections of the documentation conditional. The name of the attribute is\n     -    based on the similar test prerequisite added in c5bc9a7f94a (Makefile:\n     -    wire up build option for deprecated features, 2025-01-22).\n     +    Rectify this by adding a new document attribute \"with-breaking-changes\"\n     +    and use it to make the inclusion of those sections of the documentation\n     +    conditional. Note that the name of the attribute does not match the test\n     +    prerequisite WITHOUT_BREAKING_CHANGES added in c5bc9a7f94a (Makefile:\n     +    wire up build option for deprecated features, 2025-01-22). This is to\n     +    avoid the awkward double negative ifndef::without_breaking_changes for\n     +    documentation that should be included when WITH_BREAKING_CHANGES is\n     +    enabled. The test prerequisite will be renamed to match the\n     +    documentation attribute in a future patch series.\n      \n          Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n      \n     @@ Documentation/Makefile: asciidoc.conf: asciidoc.conf.in FORCE\n       \t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)/..\",$<,$@)\n       endif\n       \n     -+ifndef WITH_BREAKING_CHANGES\n     -+ASCIIDOC_EXTRA += -awithout-breaking-changes\n     ++ifdef WITH_BREAKING_CHANGES\n     ++ASCIIDOC_EXTRA += -awith-breaking-changes\n      +endif\n      +\n       ASCIIDOC_DEPS += docinfo.html\n     @@ Documentation/gitrepository-layout.adoc: config.worktree::\n       \tworking directory in multiple working directory setup (see\n       \tlinkgit:git-worktree[1]).\n       \n     -+ifdef::without-breaking-changes[]\n     ++ifndef::with-breaking-changes[]\n       branches::\n       \tA deprecated way to store shorthands to be used\n       \tto specify a URL to 'git fetch', 'git pull' and 'git push'.\n     @@ Documentation/gitrepository-layout.adoc: branches::\n       \t\"$GIT_COMMON_DIR/branches\" will be used instead.\n       +\n       Git will stop reading remotes from this directory in Git 3.0.\n     -+endif::without-breaking-changes[]\n     ++endif::with-breaking-changes[]\n       \n       hooks::\n       \tHooks are customization scripts used by various Git\n     @@ Documentation/gitrepository-layout.adoc: info/sparse-checkout::\n       \tThis file stores sparse checkout patterns.\n       \tSee also: linkgit:git-read-tree[1].\n       \n     -+ifdef::without-breaking-changes[]\n     ++ifndef::with-breaking-changes[]\n       remotes::\n       \tStores shorthands for URL and default refnames for use\n       \twhen interacting with remote repositories via 'git fetch',\n     @@ Documentation/gitrepository-layout.adoc: remotes::\n       \t\"$GIT_COMMON_DIR/remotes\" will be used instead.\n       +\n       Git will stop reading remotes from this directory in Git 3.0.\n     -+endif::without-breaking-changes[]\n     ++endif::with-breaking-changes[]\n       \n       logs::\n       \tRecords of changes made to refs are stored in this directory.\n     @@ Documentation/meson.build: elif docs_backend == 'asciidoctor'\n         ]\n       endif\n       \n     -+if not get_option('breaking_changes')\n     -+   asciidoc_common_options += ['--attribute', 'without-breaking-changes']\n     ++if get_option('breaking_changes')\n     ++   asciidoc_common_options += ['--attribute', 'with-breaking-changes']\n      +endif\n      +\n     - git = find_program('git', required: false)\n     - xmlto = find_program('xmlto')\n     + xmlto = find_program('xmlto', dirs: program_path)\n       \n     + cmd_lists = [\n\n\n Documentation/Makefile                  | 4 ++++\n Documentation/gitrepository-layout.adoc | 4 ++++\n Documentation/meson.build               | 4 ++++\n 3 files changed, 12 insertions(+)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex c9a7cf662f0..671267a8ac7 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -224,6 +224,10 @@ asciidoc.conf: asciidoc.conf.in FORCE\n \t$(QUIET_GEN)$(call version_gen,\"$(shell pwd)/..\",$<,$@)\n endif\n \n+ifdef WITH_BREAKING_CHANGES\n+ASCIIDOC_EXTRA += -awith-breaking-changes\n+endif\n+\n ASCIIDOC_DEPS += docinfo.html\n \n SHELL_PATH ?= $(SHELL)\ndiff --git a/Documentation/gitrepository-layout.adoc b/Documentation/gitrepository-layout.adoc\nindex 6348ef1dcdf..7421ef956d3 100644\n--- a/Documentation/gitrepository-layout.adoc\n+++ b/Documentation/gitrepository-layout.adoc\n@@ -152,6 +152,7 @@ config.worktree::\n \tworking directory in multiple working directory setup (see\n \tlinkgit:git-worktree[1]).\n \n+ifndef::with-breaking-changes[]\n branches::\n \tA deprecated way to store shorthands to be used\n \tto specify a URL to 'git fetch', 'git pull' and 'git push'.\n@@ -164,6 +165,7 @@ branches::\n \t\"$GIT_COMMON_DIR/branches\" will be used instead.\n +\n Git will stop reading remotes from this directory in Git 3.0.\n+endif::with-breaking-changes[]\n \n hooks::\n \tHooks are customization scripts used by various Git\n@@ -231,6 +233,7 @@ info/sparse-checkout::\n \tThis file stores sparse checkout patterns.\n \tSee also: linkgit:git-read-tree[1].\n \n+ifndef::with-breaking-changes[]\n remotes::\n \tStores shorthands for URL and default refnames for use\n \twhen interacting with remote repositories via 'git fetch',\n@@ -241,6 +244,7 @@ remotes::\n \t\"$GIT_COMMON_DIR/remotes\" will be used instead.\n +\n Git will stop reading remotes from this directory in Git 3.0.\n+endif::with-breaking-changes[]\n \n logs::\n \tRecords of changes made to refs are stored in this directory.\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex 0a0f2bfa14a..594546d68b1 100644\n--- a/Documentation/meson.build\n+++ b/Documentation/meson.build\n@@ -284,6 +284,10 @@ elif docs_backend == 'asciidoctor'\n   ]\n endif\n \n+if get_option('breaking_changes')\n+   asciidoc_common_options += ['--attribute', 'with-breaking-changes']\n+endif\n+\n xmlto = find_program('xmlto', dirs: program_path)\n \n cmd_lists = [\n\nbase-commit: db91954e18654eeebc54c900f44c704002e1866d\n-- \ngitgitgadget\n"},{"id":"513604","messageId":"xmqqzfhzlbie.fsf_-_@gitster.g","threadId":"63050","inReplyTo":"pull.1871.v2.git.1741171357627.gitgitgadget@gmail.com","subject":"[PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-05T15:53:29Z","receivedAt":"2025-03-05T15:53:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but\nforgot to add \"git pack-redundant\" on the EXCLUDED_PROGRAMS list,\nwhich made \"make check-docs\" target notice that the command has been\nremoved but still is documented.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n * The command is still listed in the resulting \"git help git\"\n   output, as cmd-list.perl does not yet know which commands on the\n   list are to be ignored under WITH_BREAKING_CHANGES.\n\n Makefile | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git c/Makefile w/Makefile\nindex a9b2de0692..95ac0820e9 100644\n--- c/Makefile\n+++ w/Makefile\n@@ -1283,7 +1283,9 @@ BUILTIN_OBJS += builtin/mv.o\n BUILTIN_OBJS += builtin/name-rev.o\n BUILTIN_OBJS += builtin/notes.o\n BUILTIN_OBJS += builtin/pack-objects.o\n-ifndef WITH_BREAKING_CHANGES\n+ifdef WITH_BREAKING_CHANGES\n+EXCLUDED_PROGRAMS += git-pack-redundant\n+else\n BUILTIN_OBJS += builtin/pack-redundant.o\n endif\n BUILTIN_OBJS += builtin/pack-refs.o\n"},{"id":"513742","messageId":"082af6a3-a7ba-440d-af84-6c59827a2929@gmail.com","threadId":"63050","inReplyTo":"xmqqzfhzlbie.fsf_-_@gitster.g","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-07T10:32:23Z","receivedAt":"2025-03-07T10:32:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 05/03/2025 15:53, Junio C Hamano wrote:\n> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but\n> forgot to add \"git pack-redundant\" on the EXCLUDED_PROGRAMS list,\n> which made \"make check-docs\" target notice that the command has been\n> removed but still is documented.\n> \n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>   * The command is still listed in the resulting \"git help git\"\n>     output, as cmd-list.perl does not yet know which commands on the\n>     list are to be ignored under WITH_BREAKING_CHANGES.\n\nGood catch. It seems the meson build was also forgotten in 68f51871df8 \n(builtin/pack-redundant: remove subcommand with breaking changes, \n2025-01-22) as we still compile builtin/pack-redundant.c and build the \ndocumentation. We should probably wrap the function declaration for \ncmd_pack_redundant() in builtin.h with \"#ifndef WITH_BREAKING_CHANGES\" \nas well though I don't think that is urgent.\n\nBest Wishes\n\nPhillip\n\n\n>   Makefile | 4 +++-\n>   1 file changed, 3 insertions(+), 1 deletion(-)\n> \n> diff --git c/Makefile w/Makefile\n> index a9b2de0692..95ac0820e9 100644\n> --- c/Makefile\n> +++ w/Makefile\n> @@ -1283,7 +1283,9 @@ BUILTIN_OBJS += builtin/mv.o\n>   BUILTIN_OBJS += builtin/name-rev.o\n>   BUILTIN_OBJS += builtin/notes.o\n>   BUILTIN_OBJS += builtin/pack-objects.o\n> -ifndef WITH_BREAKING_CHANGES\n> +ifdef WITH_BREAKING_CHANGES\n> +EXCLUDED_PROGRAMS += git-pack-redundant\n> +else\n>   BUILTIN_OBJS += builtin/pack-redundant.o\n>   endif\n>   BUILTIN_OBJS += builtin/pack-refs.o\n\n"},{"id":"513775","messageId":"56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com","threadId":"63050","inReplyTo":"082af6a3-a7ba-440d-af84-6c59827a2929@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-07T15:07:25Z","receivedAt":"2025-03-07T15:07:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/03/2025 10:32, Phillip Wood wrote:\n> On 05/03/2025 15:53, Junio C Hamano wrote:\n>> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but\n>> forgot to add \"git pack-redundant\" on the EXCLUDED_PROGRAMS list,\n>> which made \"make check-docs\" target notice that the command has been\n>> removed but still is documented.\n>>\n>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>> ---\n>>   * The command is still listed in the resulting \"git help git\"\n>>     output, as cmd-list.perl does not yet know which commands on the\n>>     list are to be ignored under WITH_BREAKING_CHANGES.\n> \n> Good catch. It seems the meson build was also forgotten in 68f51871df8 \n> (builtin/pack-redundant: remove subcommand with breaking changes, \n> 2025-01-22) as we still compile builtin/pack-redundant.c and build the \n> documentation. We should probably wrap the function declaration for \n> cmd_pack_redundant() in builtin.h with \"#ifndef WITH_BREAKING_CHANGES\" \n> as well though I don't think that is urgent.\n\nI just had a look at fixing the meson build but it seems to be tricky as\nthe manpage sources are stored in a meson dictionary and meson\ndictionaries are immutable so I don't know how one is supposed to\nconditionally add items.\n\nI also noticed that while we store the correct value for\nWITH_BREAKING_CHANGES in GIT-BUILD-OPTIONS it is not defined when\nbuilding the C sources and so we still build the pack-redundant builtin.\n\nThe diff below stops us from building pack-redundant with\n-Dbreaking_changes=true but still builds the documentation. I don't intend\nspending any more time one this\n\nBest Wishes\n\nPhillip\n\ndiff --git a/builtin.h b/builtin.h\nindex 89928ccf92f..8483975c191 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -197,7 +197,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix, struct repository *r\n  int cmd_name_rev(int argc, const char **argv, const char *prefix, struct repository *repo);\n  int cmd_notes(int argc, const char **argv, const char *prefix, struct repository *repo);\n  int cmd_pack_objects(int argc, const char **argv, const char *prefix, struct repository *repo);\n+#ifndef WITH_BREAKING_CHANGES\n  int cmd_pack_redundant(int argc, const char **argv, const char *prefix, struct repository *repo);\n+#endif\n  int cmd_patch_id(int argc, const char **argv, const char *prefix, struct repository *repo);\n  int cmd_prune(int argc, const char **argv, const char *prefix, struct repository *repo);\n  int cmd_prune_packed(int argc, const char **argv, const char *prefix, struct repository *repo);\ndiff --git a/meson.build b/meson.build\nindex e86085b0a47..5c039fe525a 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -581,7 +581,6 @@ builtin_sources = [\n    'builtin/name-rev.c',\n    'builtin/notes.c',\n    'builtin/pack-objects.c',\n-  'builtin/pack-redundant.c',\n    'builtin/pack-refs.c',\n    'builtin/patch-id.c',\n    'builtin/prune-packed.c',\n@@ -632,6 +631,10 @@ builtin_sources = [\n    'builtin/write-tree.c',\n  ]\n  \n+if not get_option('breaking_changes')\n+  builtin_sources += 'builtin/pack-redundant.c'\n+endif\n+\n  builtin_sources += custom_target(\n    output: 'config-list.h',\n    command: [\n@@ -674,6 +677,7 @@ build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_opt\n  \n  if get_option('breaking_changes')\n    build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n+  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : 'c')\n  else\n    build_options_config.set('WITH_BREAKING_CHANGES', '')\n  endif\n\n"},{"id":"513787","messageId":"xmqqfrjoehur.fsf@gitster.g","threadId":"63050","inReplyTo":"56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-07T19:55:08Z","receivedAt":"2025-03-07T19:55:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> I also noticed that while we store the correct value for\n> WITH_BREAKING_CHANGES in GIT-BUILD-OPTIONS it is not defined when\n> building the C sources and so we still build the pack-redundant builtin.\n>\n> The diff below stops us from building pack-redundant with\n> -Dbreaking_changes=true but still builds the documentation. I don't intend\n> spending any more time one this\n\nThanks.\n\nI am afraid that Patrick's plate may already be full, but I am\nhoping that there are others who are interested in getting the meson\nbuild support into a usable shape.  Any takers?\n\n"},{"id":"513805","messageId":"CAOLa=ZRdqk-VXX4LWptqx3C3ABxNcxU+5cDTPxWt3k9UW1m3OA@mail.gmail.com","threadId":"63050","inReplyTo":"56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-07T22:42:39Z","receivedAt":"2025-03-07T22:42:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"FPhillip Wood <phillip.wood123@gmail.com> writes:\n\n> On 07/03/2025 10:32, Phillip Wood wrote:\n>> On 05/03/2025 15:53, Junio C Hamano wrote:\n>>> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but\n>>> forgot to add \"git pack-redundant\" on the EXCLUDED_PROGRAMS list,\n>>> which made \"make check-docs\" target notice that the command has been\n>>> removed but still is documented.\n>>>\n>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>>> ---\n>>>   * The command is still listed in the resulting \"git help git\"\n>>>     output, as cmd-list.perl does not yet know which commands on the\n>>>     list are to be ignored under WITH_BREAKING_CHANGES.\n>>\n>> Good catch. It seems the meson build was also forgotten in 68f51871df8\n>> (builtin/pack-redundant: remove subcommand with breaking changes,\n>> 2025-01-22) as we still compile builtin/pack-redundant.c and build the\n>> documentation. We should probably wrap the function declaration for\n>> cmd_pack_redundant() in builtin.h with \"#ifndef WITH_BREAKING_CHANGES\"\n>> as well though I don't think that is urgent.\n>\n> I just had a look at fixing the meson build but it seems to be tricky as\n> the manpage sources are stored in a meson dictionary and meson\n> dictionaries are immutable so I don't know how one is supposed to\n> conditionally add items.\n>\n\nBut dictonaries can be combined [1]. So we could probably do something\nlike I've added below.\n\n[1]: https://mesonbuild.com/Reference-manual_elementary_dict.html\n\n-- 8< --\n\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex 0a0f2bfa14..fcfec63e9b 100644\n--- a/Documentation/meson.build\n+++ b/Documentation/meson.build\n@@ -96,7 +96,6 @@ manpages = {\n   'git-notes.adoc' : 1,\n   'git-p4.adoc' : 1,\n   'git-pack-objects.adoc' : 1,\n-  'git-pack-redundant.adoc' : 1,\n   'git-pack-refs.adoc' : 1,\n   'git-patch-id.adoc' : 1,\n   'git-prune-packed.adoc' : 1,\n@@ -205,6 +204,14 @@ manpages = {\n   'gitworkflows.adoc' : 7,\n }\n\n+manpages_breaking_changes = {\n+    'git-pack-redundant.adoc' : 1,\n+}\n+\n+if not get_option('breaking_changes')\n+  manpages += manpages_breaking_changes\n+endif\n+\n docs_backend = get_option('docs_backend')\n if docs_backend == 'auto'\n   if find_program('asciidoc', dirs: program_path, required: false).found()\n@@ -475,7 +482,7 @@ endif\n # Sanity check that we are not missing any tests present in 't/'. This check\n # only runs once at configure time and is thus best-effort, only. Furthermore,\n # it only verifies man pages for the sake of simplicity.\n-configured_manpages = manpages.keys() + [ 'git-bisect-lk2009.adoc',\n'git-tools.adoc' ]\n+configured_manpages = manpages.keys() +\nmanpages_breaking_changes.keys() + [ 'git-bisect-lk2009.adoc',\n'git-tools.adoc' ]\n actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n   check: true,\n   env: script_environment,\n"},{"id":"513848","messageId":"b5fb3292-216a-4456-b456-e9ed38affc22@gmail.com","threadId":"63050","inReplyTo":"56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-09T10:52:44Z","receivedAt":"2025-03-09T10:52:48Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 07/03/2025 15:07, Phillip Wood wrote:\n> On 07/03/2025 10:32, Phillip Wood wrote:\n> \n> The diff below stops us from building pack-redundant with\n> -Dbreaking_changes=true but still builds the documentation. I don't intend\n> spending any more time one this\n> \n> [...]\n >\n>   if get_option('breaking_changes')\n>     build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n> +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : \n> 'c')\n\nLooking again at this I think it should probably be\n\n     libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease'\n\nto match the rest of our meson.build. As a newcomer to meson I find it \nconfusing that the CFLAGS for the build targets are set implicitly by \ntheir libgit dependency.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"513849","messageId":"1d9ab1ed-54bf-4359-85c2-28060cbf7198@gmail.com","threadId":"63050","inReplyTo":"CAOLa=ZRdqk-VXX4LWptqx3C3ABxNcxU+5cDTPxWt3k9UW1m3OA@mail.gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-09T10:52:51Z","receivedAt":"2025-03-09T10:52:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Karthik\n\nOn 07/03/2025 22:42, Karthik Nayak wrote:\n> FPhillip Wood <phillip.wood123@gmail.com> writes:\n> \n>> On 07/03/2025 10:32, Phillip Wood wrote:\n>>> On 05/03/2025 15:53, Junio C Hamano wrote:\n>>>> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but\n>>>> forgot to add \"git pack-redundant\" on the EXCLUDED_PROGRAMS list,\n>>>> which made \"make check-docs\" target notice that the command has been\n>>>> removed but still is documented.\n>>>>\n>>>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>>>> ---\n>>>>    * The command is still listed in the resulting \"git help git\"\n>>>>      output, as cmd-list.perl does not yet know which commands on the\n>>>>      list are to be ignored under WITH_BREAKING_CHANGES.\n>>>\n>>> Good catch. It seems the meson build was also forgotten in 68f51871df8\n>>> (builtin/pack-redundant: remove subcommand with breaking changes,\n>>> 2025-01-22) as we still compile builtin/pack-redundant.c and build the\n>>> documentation. We should probably wrap the function declaration for\n>>> cmd_pack_redundant() in builtin.h with \"#ifndef WITH_BREAKING_CHANGES\"\n>>> as well though I don't think that is urgent.\n>>\n>> I just had a look at fixing the meson build but it seems to be tricky as\n>> the manpage sources are stored in a meson dictionary and meson\n>> dictionaries are immutable so I don't know how one is supposed to\n>> conditionally add items.\n>>\n> \n> But dictonaries can be combined [1]. So we could probably do something\n> like I've added below.\n\nThanks, my web search took me to a different page in the documentation \n[1]. Looking carefully there is an example of adding an element to a \ndictionary right at the end of that section but it is not mentioned \nanywhere in the text. I do find the meson documentation hard to use.\n\nI think it would be best if someone with more knowledge of meson than me \ntook this forward\n\nThanks\n\nPhillip\n\n[1] https://mesonbuild.com/Syntax.html#dictionaries\n\n> [1]: https://mesonbuild.com/Reference-manual_elementary_dict.html\n> \n> -- 8< --\n> \n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index 0a0f2bfa14..fcfec63e9b 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -96,7 +96,6 @@ manpages = {\n>     'git-notes.adoc' : 1,\n>     'git-p4.adoc' : 1,\n>     'git-pack-objects.adoc' : 1,\n> -  'git-pack-redundant.adoc' : 1,\n>     'git-pack-refs.adoc' : 1,\n>     'git-patch-id.adoc' : 1,\n>     'git-prune-packed.adoc' : 1,\n> @@ -205,6 +204,14 @@ manpages = {\n>     'gitworkflows.adoc' : 7,\n>   }\n> \n> +manpages_breaking_changes = {\n> +    'git-pack-redundant.adoc' : 1,\n> +}\n> +\n> +if not get_option('breaking_changes')\n> +  manpages += manpages_breaking_changes\n> +endif\n> +\n>   docs_backend = get_option('docs_backend')\n>   if docs_backend == 'auto'\n>     if find_program('asciidoc', dirs: program_path, required: false).found()\n> @@ -475,7 +482,7 @@ endif\n>   # Sanity check that we are not missing any tests present in 't/'. This check\n>   # only runs once at configure time and is thus best-effort, only. Furthermore,\n>   # it only verifies man pages for the sake of simplicity.\n> -configured_manpages = manpages.keys() + [ 'git-bisect-lk2009.adoc',\n> 'git-tools.adoc' ]\n> +configured_manpages = manpages.keys() +\n> manpages_breaking_changes.keys() + [ 'git-bisect-lk2009.adoc',\n> 'git-tools.adoc' ]\n>   actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n>     check: true,\n>     env: script_environment,\n\n"},{"id":"513868","messageId":"Z86Jze2qZ5s5OyOB@pks.im","threadId":"63050","inReplyTo":"b5fb3292-216a-4456-b456-e9ed38affc22@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-10T06:42:25Z","receivedAt":"2025-03-10T06:42:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 09, 2025 at 10:52:44AM +0000, Phillip Wood wrote:\n> On 07/03/2025 15:07, Phillip Wood wrote:\n> > On 07/03/2025 10:32, Phillip Wood wrote:\n> > \n> > The diff below stops us from building pack-redundant with\n> > -Dbreaking_changes=true but still builds the documentation. I don't intend\n> > spending any more time one this\n> > \n> > [...]\n> >\n> >   if get_option('breaking_changes')\n> >     build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n> > +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language :\n> > 'c')\n> \n> Looking again at this I think it should probably be\n> \n>     libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease'\n> \n> to match the rest of our meson.build. As a newcomer to meson I find it\n> confusing that the CFLAGS for the build targets are set implicitly by their\n> libgit dependency.\n\nYup, that would be preferable indeed, thanks!\n\nPatrick\n"},{"id":"513980","messageId":"Z9BLZWUarN0kC4CQ@pks.im","threadId":"63050","inReplyTo":"Z86Jze2qZ5s5OyOB@pks.im","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-11T14:40:37Z","receivedAt":"2025-03-11T14:40:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote:\n> On Sun, Mar 09, 2025 at 10:52:44AM +0000, Phillip Wood wrote:\n> > On 07/03/2025 15:07, Phillip Wood wrote:\n> > > On 07/03/2025 10:32, Phillip Wood wrote:\n> > > \n> > > The diff below stops us from building pack-redundant with\n> > > -Dbreaking_changes=true but still builds the documentation. I don't intend\n> > > spending any more time one this\n> > > \n> > > [...]\n> > >\n> > >   if get_option('breaking_changes')\n> > >     build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n> > > +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language :\n> > > 'c')\n> > \n> > Looking again at this I think it should probably be\n> > \n> >     libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease'\n> > \n> > to match the rest of our meson.build. As a newcomer to meson I find it\n> > confusing that the CFLAGS for the build targets are set implicitly by their\n> > libgit dependency.\n> \n> Yup, that would be preferable indeed, thanks!\n\nTo set expectations: do you have the time/intent to work on this and\npolish it up into a patch? Otherwise I'm happy to pick it up.\n\nThanks!\n\nPatrick\n"},{"id":"514035","messageId":"75a8b321-4cf0-4d75-93a7-b616ae818d81@gmail.com","threadId":"63050","inReplyTo":"Z9BLZWUarN0kC4CQ@pks.im","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-12T10:39:49Z","receivedAt":"2025-03-12T10:39:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 11/03/2025 14:40, Patrick Steinhardt wrote:\n> On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote:\n> \n> To set expectations: do you have the time/intent to work on this and\n> polish it up into a patch? Otherwise I'm happy to pick it up.\n\nIf you're happy to pick this up that would be great. I'm unlikely to \nhave time for git related things for the next week or so and you're also \nmuch more familiar with meson than me.\n\nThanks\n\nPhillip\n\n"},{"id":"514056","messageId":"Z9GPbggliX7asG2a@pks.im","threadId":"63050","inReplyTo":"75a8b321-4cf0-4d75-93a7-b616ae818d81@gmail.com","subject":"Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T13:43:10Z","receivedAt":"2025-03-12T13:43:15Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Mar 12, 2025 at 10:39:49AM +0000, phillip.wood123@gmail.com wrote:\n> Hi Patrick\n> \n> On 11/03/2025 14:40, Patrick Steinhardt wrote:\n> > On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote:\n> > \n> > To set expectations: do you have the time/intent to work on this and\n> > polish it up into a patch? Otherwise I'm happy to pick it up.\n> \n> If you're happy to pick this up that would be great. I'm unlikely to have\n> time for git related things for the next week or so and you're also much\n> more familiar with meson than me.\n\nOkay, I've sent the patch series in [1]. Thanks!\n\n[1]: <20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im>\n\nPatrick\n"}]}