# [PATCH] docs: fix repository-layout when building with breaking changes 17 messages from 2025-03-03 to 2025-03-12. Participants: Phillip Wood via GitGitGadget, Junio C Hamano, Patrick Steinhardt, Phillip Wood, Karthik Nayak, phillip.wood123@gmail.com. Thread: https://gitlist.dev/t/63050 ## Phillip Wood via GitGitGadget, 2025-03-03 16:11 Subject: [PATCH] docs: fix repository-layout when building with breaking changes Message-ID: URL: https://gitlist.dev/e/pull.1871.git.1741018310447.gitgitgadget%40gmail.com ``` From: Phillip Wood Since commit 8ccc75c2452 (remote: announce removal of "branches/" and "remotes/", 2025-01-22) enabling WITH_BREAKING_CHANGES when building git removes support for reading branches from ".git/branches" and remotes from ".git/remotes". However those locations are still documented in gitrepository-layout.adoc even though the build does not support them. Rectify this by adding a new document attribute "without-breaking-changes" and use it to make the inclusion of those sections of the documentation conditional. The name of the attribute is based on the similar test prerequisite added in c5bc9a7f94a (Makefile: wire up build option for deprecated features, 2025-01-22). Signed-off-by: Phillip Wood --- docs: fix repository-layout when building with breaking changes I copied the name from the test prerequisite as I didn't want to have different names for condition used in the tests and documentation. I do have some reservations about the naming though as it means we end up having to use ifdef::!without-breaking-changes[] or test_expect_success !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is a double negative. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1871%2Fphillipwood%2Fbreaking-changes-documentation-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1871/phillipwood/breaking-changes-documentation-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1871 Documentation/Makefile | 4 ++++ Documentation/gitrepository-layout.adoc | 4 ++++ Documentation/meson.build | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/Documentation/Makefile b/Documentation/Makefile index a734c6d6243..53a05eb8030 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -221,6 +221,10 @@ asciidoc.conf: asciidoc.conf.in FORCE $(QUIET_GEN)$(call version_gen,"$(shell pwd)/..",$<,$@) endif +ifndef WITH_BREAKING_CHANGES +ASCIIDOC_EXTRA += -awithout-breaking-changes +endif + ASCIIDOC_DEPS += docinfo.html SHELL_PATH ?= $(SHELL) diff --git a/Documentation/gitrepository-layout.adoc b/Documentation/gitrepository-layout.adoc index 6348ef1dcdf..62ef1c98c26 100644 --- a/Documentation/gitrepository-layout.adoc +++ b/Documentation/gitrepository-layout.adoc @@ -152,6 +152,7 @@ config.worktree:: working directory in multiple working directory setup (see linkgit:git-worktree[1]). +ifdef::without-breaking-changes[] branches:: A deprecated way to store shorthands to be used to specify a URL to 'git fetch', 'git pull' and 'git push'. @@ -164,6 +165,7 @@ branches:: "$GIT_COMMON_DIR/branches" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. +endif::without-breaking-changes[] hooks:: Hooks are customization scripts used by various Git @@ -231,6 +233,7 @@ info/sparse-checkout:: This file stores sparse checkout patterns. See also: linkgit:git-read-tree[1]. +ifdef::without-breaking-changes[] remotes:: Stores shorthands for URL and default refnames for use when interacting with remote repositories via 'git fetch', @@ -241,6 +244,7 @@ remotes:: "$GIT_COMMON_DIR/remotes" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. +endif::without-breaking-changes[] logs:: Records of changes made to refs are stored in this directory. diff --git a/Documentation/meson.build b/Documentation/meson.build index ead8e482131..4e4fca283c1 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -283,6 +283,10 @@ elif docs_backend == 'asciidoctor' ] endif +if not get_option('breaking_changes') + asciidoc_common_options += ['--attribute', 'without-breaking-changes'] +endif + git = find_program('git', required: false) xmlto = find_program('xmlto') base-commit: 03944513488db4a81fdb4c21c3b515e4cb260b05 -- gitgitgadget ``` ## Junio C Hamano, 2025-03-03 18:18 Subject: Re: [PATCH] docs: fix repository-layout when building with breaking changes Message-ID: URL: https://gitlist.dev/e/xmqqbjuivuzm.fsf%40gitster.g In-Reply-To: ``` "Phillip Wood via GitGitGadget" writes: > I copied the name from the test prerequisite as I didn't want to have > different names for condition used in the tests and documentation. I do > have some reservations about the naming though as it means we end up > having to use ifdef::!without-breaking-changes[] or test_expect_success > !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is > a double negative. It was exactly the first thing that came to my mind when I saw the change to the Makefile in the patch. Unless our breaking changes are all removals, which is not likely to be the case in the longer term, "without-breaking-changes" would be an invitation for confusing double negatives. > +ifdef::without-breaking-changes[] > branches:: > A deprecated way to store shorthands to be used > to specify a URL to 'git fetch', 'git pull' and 'git push'. > @@ -164,6 +165,7 @@ branches:: > "$GIT_COMMON_DIR/branches" will be used instead. > + > Git will stop reading remotes from this directory in Git 3.0. > +endif::without-breaking-changes[] > > hooks:: > Hooks are customization scripts used by various Git > @@ -231,6 +233,7 @@ info/sparse-checkout:: > This file stores sparse checkout patterns. > See also: linkgit:git-read-tree[1]. > > +ifdef::without-breaking-changes[] > remotes:: > Stores shorthands for URL and default refnames for use > when interacting with remote repositories via 'git fetch', > @@ -241,6 +244,7 @@ remotes:: > "$GIT_COMMON_DIR/remotes" will be used instead. > + > Git will stop reading remotes from this directory in Git 3.0. > +endif::without-breaking-changes[] > > logs:: > Records of changes made to refs are stored in this directory. The above parts of the documentation getting commented out all look sensible to exclude in a build that omits these older mechanisms. But can we do it with !with-breaking-changes instead? ``` ## Patrick Steinhardt, 2025-03-04 06:35 Subject: Re: [PATCH] docs: fix repository-layout when building with breaking changes Message-ID: URL: https://gitlist.dev/e/Z8afGexTdlqDnPV8%40pks.im In-Reply-To: ``` On Mon, Mar 03, 2025 at 10:18:05AM -0800, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" writes: > > > I copied the name from the test prerequisite as I didn't want to have > > different names for condition used in the tests and documentation. I do > > have some reservations about the naming though as it means we end up > > having to use ifdef::!without-breaking-changes[] or test_expect_success > > !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is > > a double negative. > > It was exactly the first thing that came to my mind when I saw the > change to the Makefile in the patch. Unless our breaking changes > are all removals, which is not likely to be the case in the longer > term, "without-breaking-changes" would be an invitation for > confusing double negatives. I remember not quite being happy with the double-negation myself. I don't mind renaming the prerequisite we have in our test suite for consistency, as well, if you want to do that. Patrick ``` ## Phillip Wood, 2025-03-04 10:23 Subject: Re: [PATCH] docs: fix repository-layout when building with breaking changes Message-ID: <2583368f-c7ad-4088-b78d-f6d972c4ee47@gmail.com> URL: https://gitlist.dev/e/2583368f-c7ad-4088-b78d-f6d972c4ee47%40gmail.com In-Reply-To: ``` Hi Patrick On 04/03/2025 06:35, Patrick Steinhardt wrote: > On Mon, Mar 03, 2025 at 10:18:05AM -0800, Junio C Hamano wrote: >> "Phillip Wood via GitGitGadget" writes: >> >>> I copied the name from the test prerequisite as I didn't want to have >>> different names for condition used in the tests and documentation. I do >>> have some reservations about the naming though as it means we end up >>> having to use ifdef::!without-breaking-changes[] or test_expect_success >>> !WITHOUT_BREAKING_CHANGES to document and test breaking changes which is >>> a double negative. >> >> It was exactly the first thing that came to my mind when I saw the >> change to the Makefile in the patch. Unless our breaking changes >> are all removals, which is not likely to be the case in the longer >> term, "without-breaking-changes" would be an invitation for >> confusing double negatives. > > I remember not quite being happy with the double-negation myself. I > don't mind renaming the prerequisite we have in our test suite for > consistency, as well, if you want to do that. Yes, I can do that when I re-roll the patches at https://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/ to use WITH_BREAKING_CHANGES Best Wishes Phillip > Patrick > ``` ## Junio C Hamano, 2025-03-04 16:04 Subject: Re: [PATCH] docs: fix repository-layout when building with breaking changes Message-ID: URL: https://gitlist.dev/e/xmqqr03csry6.fsf%40gitster.g In-Reply-To: <2583368f-c7ad-4088-b78d-f6d972c4ee47@gmail.com> ``` Phillip Wood writes: >> I remember not quite being happy with the double-negation myself. I >> don't mind renaming the prerequisite we have in our test suite for >> consistency, as well, if you want to do that. > > Yes, I can do that when I re-roll the patches at > https://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/ > to use WITH_BREAKING_CHANGES Thanks, both. ``` ## Phillip Wood via GitGitGadget, 2025-03-05 10:42 Subject: [PATCH v2] docs: fix repository-layout when building with breaking changes Message-ID: URL: https://gitlist.dev/e/pull.1871.v2.git.1741171357627.gitgitgadget%40gmail.com In-Reply-To: ``` From: Phillip Wood Since commit 8ccc75c2452 (remote: announce removal of "branches/" and "remotes/", 2025-01-22) enabling WITH_BREAKING_CHANGES when building git removes support for reading branches from ".git/branches" and remotes from ".git/remotes". However those locations are still documented in gitrepository-layout.adoc even though the build does not support them. Rectify this by adding a new document attribute "with-breaking-changes" and use it to make the inclusion of those sections of the documentation conditional. Note that the name of the attribute does not match the test prerequisite WITHOUT_BREAKING_CHANGES added in c5bc9a7f94a (Makefile: wire up build option for deprecated features, 2025-01-22). This is to avoid the awkward double negative ifndef::without_breaking_changes for documentation that should be included when WITH_BREAKING_CHANGES is enabled. The test prerequisite will be renamed to match the documentation attribute in a future patch series. Signed-off-by: Phillip Wood --- docs: fix repository-layout when building with breaking changes Thanks to Junio and Patrick for their comments on V1. I've renamed the attribute to with-breaking-changes as suggested. I'll add a patch to rename the test prerequisite to match this when I re-roll https://lore.kernel.org/git/pull.1863.git.1740149837.gitgitgadget@gmail.com/ to use WITH_BREAKING_CHANGES. Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1871%2Fphillipwood%2Fbreaking-changes-documentation-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1871/phillipwood/breaking-changes-documentation-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1871 Range-diff vs v1: 1: 42877011c6d ! 1: d8dcdca4c2b docs: fix repository-layout when building with breaking changes @@ Commit message from ".git/remotes". However those locations are still documented in gitrepository-layout.adoc even though the build does not support them. - Rectify this by adding a new document attribute - "without-breaking-changes" and use it to make the inclusion of those - sections of the documentation conditional. The name of the attribute is - based on the similar test prerequisite added in c5bc9a7f94a (Makefile: - wire up build option for deprecated features, 2025-01-22). + Rectify this by adding a new document attribute "with-breaking-changes" + and use it to make the inclusion of those sections of the documentation + conditional. Note that the name of the attribute does not match the test + prerequisite WITHOUT_BREAKING_CHANGES added in c5bc9a7f94a (Makefile: + wire up build option for deprecated features, 2025-01-22). This is to + avoid the awkward double negative ifndef::without_breaking_changes for + documentation that should be included when WITH_BREAKING_CHANGES is + enabled. The test prerequisite will be renamed to match the + documentation attribute in a future patch series. Signed-off-by: Phillip Wood @@ Documentation/Makefile: asciidoc.conf: asciidoc.conf.in FORCE $(QUIET_GEN)$(call version_gen,"$(shell pwd)/..",$<,$@) endif -+ifndef WITH_BREAKING_CHANGES -+ASCIIDOC_EXTRA += -awithout-breaking-changes ++ifdef WITH_BREAKING_CHANGES ++ASCIIDOC_EXTRA += -awith-breaking-changes +endif + ASCIIDOC_DEPS += docinfo.html @@ Documentation/gitrepository-layout.adoc: config.worktree:: working directory in multiple working directory setup (see linkgit:git-worktree[1]). -+ifdef::without-breaking-changes[] ++ifndef::with-breaking-changes[] branches:: A deprecated way to store shorthands to be used to specify a URL to 'git fetch', 'git pull' and 'git push'. @@ Documentation/gitrepository-layout.adoc: branches:: "$GIT_COMMON_DIR/branches" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. -+endif::without-breaking-changes[] ++endif::with-breaking-changes[] hooks:: Hooks are customization scripts used by various Git @@ Documentation/gitrepository-layout.adoc: info/sparse-checkout:: This file stores sparse checkout patterns. See also: linkgit:git-read-tree[1]. -+ifdef::without-breaking-changes[] ++ifndef::with-breaking-changes[] remotes:: Stores shorthands for URL and default refnames for use when interacting with remote repositories via 'git fetch', @@ Documentation/gitrepository-layout.adoc: remotes:: "$GIT_COMMON_DIR/remotes" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. -+endif::without-breaking-changes[] ++endif::with-breaking-changes[] logs:: Records of changes made to refs are stored in this directory. @@ Documentation/meson.build: elif docs_backend == 'asciidoctor' ] endif -+if not get_option('breaking_changes') -+ asciidoc_common_options += ['--attribute', 'without-breaking-changes'] ++if get_option('breaking_changes') ++ asciidoc_common_options += ['--attribute', 'with-breaking-changes'] +endif + - git = find_program('git', required: false) - xmlto = find_program('xmlto') + xmlto = find_program('xmlto', dirs: program_path) + cmd_lists = [ Documentation/Makefile | 4 ++++ Documentation/gitrepository-layout.adoc | 4 ++++ Documentation/meson.build | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/Documentation/Makefile b/Documentation/Makefile index c9a7cf662f0..671267a8ac7 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -224,6 +224,10 @@ asciidoc.conf: asciidoc.conf.in FORCE $(QUIET_GEN)$(call version_gen,"$(shell pwd)/..",$<,$@) endif +ifdef WITH_BREAKING_CHANGES +ASCIIDOC_EXTRA += -awith-breaking-changes +endif + ASCIIDOC_DEPS += docinfo.html SHELL_PATH ?= $(SHELL) diff --git a/Documentation/gitrepository-layout.adoc b/Documentation/gitrepository-layout.adoc index 6348ef1dcdf..7421ef956d3 100644 --- a/Documentation/gitrepository-layout.adoc +++ b/Documentation/gitrepository-layout.adoc @@ -152,6 +152,7 @@ config.worktree:: working directory in multiple working directory setup (see linkgit:git-worktree[1]). +ifndef::with-breaking-changes[] branches:: A deprecated way to store shorthands to be used to specify a URL to 'git fetch', 'git pull' and 'git push'. @@ -164,6 +165,7 @@ branches:: "$GIT_COMMON_DIR/branches" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. +endif::with-breaking-changes[] hooks:: Hooks are customization scripts used by various Git @@ -231,6 +233,7 @@ info/sparse-checkout:: This file stores sparse checkout patterns. See also: linkgit:git-read-tree[1]. +ifndef::with-breaking-changes[] remotes:: Stores shorthands for URL and default refnames for use when interacting with remote repositories via 'git fetch', @@ -241,6 +244,7 @@ remotes:: "$GIT_COMMON_DIR/remotes" will be used instead. + Git will stop reading remotes from this directory in Git 3.0. +endif::with-breaking-changes[] logs:: Records of changes made to refs are stored in this directory. diff --git a/Documentation/meson.build b/Documentation/meson.build index 0a0f2bfa14a..594546d68b1 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -284,6 +284,10 @@ elif docs_backend == 'asciidoctor' ] endif +if get_option('breaking_changes') + asciidoc_common_options += ['--attribute', 'with-breaking-changes'] +endif + xmlto = find_program('xmlto', dirs: program_path) cmd_lists = [ base-commit: db91954e18654eeebc54c900f44c704002e1866d -- gitgitgadget ``` ## Junio C Hamano, 2025-03-05 15:53 Subject: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/xmqqzfhzlbie.fsf_-_%40gitster.g In-Reply-To: ``` We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but forgot to add "git pack-redundant" on the EXCLUDED_PROGRAMS list, which made "make check-docs" target notice that the command has been removed but still is documented. Signed-off-by: Junio C Hamano --- * The command is still listed in the resulting "git help git" output, as cmd-list.perl does not yet know which commands on the list are to be ignored under WITH_BREAKING_CHANGES. Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git c/Makefile w/Makefile index a9b2de0692..95ac0820e9 100644 --- c/Makefile +++ w/Makefile @@ -1283,7 +1283,9 @@ BUILTIN_OBJS += builtin/mv.o BUILTIN_OBJS += builtin/name-rev.o BUILTIN_OBJS += builtin/notes.o BUILTIN_OBJS += builtin/pack-objects.o -ifndef WITH_BREAKING_CHANGES +ifdef WITH_BREAKING_CHANGES +EXCLUDED_PROGRAMS += git-pack-redundant +else BUILTIN_OBJS += builtin/pack-redundant.o endif BUILTIN_OBJS += builtin/pack-refs.o ``` ## Phillip Wood, 2025-03-07 10:32 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: <082af6a3-a7ba-440d-af84-6c59827a2929@gmail.com> URL: https://gitlist.dev/e/082af6a3-a7ba-440d-af84-6c59827a2929%40gmail.com In-Reply-To: ``` On 05/03/2025 15:53, Junio C Hamano wrote: > We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but > forgot to add "git pack-redundant" on the EXCLUDED_PROGRAMS list, > which made "make check-docs" target notice that the command has been > removed but still is documented. > > Signed-off-by: Junio C Hamano > --- > * The command is still listed in the resulting "git help git" > output, as cmd-list.perl does not yet know which commands on the > list are to be ignored under WITH_BREAKING_CHANGES. Good catch. It seems the meson build was also forgotten in 68f51871df8 (builtin/pack-redundant: remove subcommand with breaking changes, 2025-01-22) as we still compile builtin/pack-redundant.c and build the documentation. We should probably wrap the function declaration for cmd_pack_redundant() in builtin.h with "#ifndef WITH_BREAKING_CHANGES" as well though I don't think that is urgent. Best Wishes Phillip > Makefile | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git c/Makefile w/Makefile > index a9b2de0692..95ac0820e9 100644 > --- c/Makefile > +++ w/Makefile > @@ -1283,7 +1283,9 @@ BUILTIN_OBJS += builtin/mv.o > BUILTIN_OBJS += builtin/name-rev.o > BUILTIN_OBJS += builtin/notes.o > BUILTIN_OBJS += builtin/pack-objects.o > -ifndef WITH_BREAKING_CHANGES > +ifdef WITH_BREAKING_CHANGES > +EXCLUDED_PROGRAMS += git-pack-redundant > +else > BUILTIN_OBJS += builtin/pack-redundant.o > endif > BUILTIN_OBJS += builtin/pack-refs.o ``` ## Phillip Wood, 2025-03-07 15:07 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com> URL: https://gitlist.dev/e/56cf842a-7c1f-4354-b191-35bcc1e139bd%40gmail.com In-Reply-To: <082af6a3-a7ba-440d-af84-6c59827a2929@gmail.com> ``` On 07/03/2025 10:32, Phillip Wood wrote: > On 05/03/2025 15:53, Junio C Hamano wrote: >> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but >> forgot to add "git pack-redundant" on the EXCLUDED_PROGRAMS list, >> which made "make check-docs" target notice that the command has been >> removed but still is documented. >> >> Signed-off-by: Junio C Hamano >> --- >>   * The command is still listed in the resulting "git help git" >>     output, as cmd-list.perl does not yet know which commands on the >>     list are to be ignored under WITH_BREAKING_CHANGES. > > Good catch. It seems the meson build was also forgotten in 68f51871df8 > (builtin/pack-redundant: remove subcommand with breaking changes, > 2025-01-22) as we still compile builtin/pack-redundant.c and build the > documentation. We should probably wrap the function declaration for > cmd_pack_redundant() in builtin.h with "#ifndef WITH_BREAKING_CHANGES" > as well though I don't think that is urgent. I just had a look at fixing the meson build but it seems to be tricky as the manpage sources are stored in a meson dictionary and meson dictionaries are immutable so I don't know how one is supposed to conditionally add items. I also noticed that while we store the correct value for WITH_BREAKING_CHANGES in GIT-BUILD-OPTIONS it is not defined when building the C sources and so we still build the pack-redundant builtin. The diff below stops us from building pack-redundant with -Dbreaking_changes=true but still builds the documentation. I don't intend spending any more time one this Best Wishes Phillip diff --git a/builtin.h b/builtin.h index 89928ccf92f..8483975c191 100644 --- a/builtin.h +++ b/builtin.h @@ -197,7 +197,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix, struct repository *r int cmd_name_rev(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_notes(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_pack_objects(int argc, const char **argv, const char *prefix, struct repository *repo); +#ifndef WITH_BREAKING_CHANGES int cmd_pack_redundant(int argc, const char **argv, const char *prefix, struct repository *repo); +#endif int cmd_patch_id(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_prune(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_prune_packed(int argc, const char **argv, const char *prefix, struct repository *repo); diff --git a/meson.build b/meson.build index e86085b0a47..5c039fe525a 100644 --- a/meson.build +++ b/meson.build @@ -581,7 +581,6 @@ builtin_sources = [ 'builtin/name-rev.c', 'builtin/notes.c', 'builtin/pack-objects.c', - 'builtin/pack-redundant.c', 'builtin/pack-refs.c', 'builtin/patch-id.c', 'builtin/prune-packed.c', @@ -632,6 +631,10 @@ builtin_sources = [ 'builtin/write-tree.c', ] +if not get_option('breaking_changes') + builtin_sources += 'builtin/pack-redundant.c' +endif + builtin_sources += custom_target( output: 'config-list.h', command: [ @@ -674,6 +677,7 @@ build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_opt if get_option('breaking_changes') build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease') + add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : 'c') else build_options_config.set('WITH_BREAKING_CHANGES', '') endif ``` ## Junio C Hamano, 2025-03-07 19:55 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/xmqqfrjoehur.fsf%40gitster.g In-Reply-To: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com> ``` Phillip Wood writes: > I also noticed that while we store the correct value for > WITH_BREAKING_CHANGES in GIT-BUILD-OPTIONS it is not defined when > building the C sources and so we still build the pack-redundant builtin. > > The diff below stops us from building pack-redundant with > -Dbreaking_changes=true but still builds the documentation. I don't intend > spending any more time one this Thanks. I am afraid that Patrick's plate may already be full, but I am hoping that there are others who are interested in getting the meson build support into a usable shape. Any takers? ``` ## Karthik Nayak, 2025-03-07 22:42 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/CAOLa%3DZRdqk-VXX4LWptqx3C3ABxNcxU%2B5cDTPxWt3k9UW1m3OA%40mail.gmail.com In-Reply-To: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com> ``` FPhillip Wood writes: > On 07/03/2025 10:32, Phillip Wood wrote: >> On 05/03/2025 15:53, Junio C Hamano wrote: >>> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but >>> forgot to add "git pack-redundant" on the EXCLUDED_PROGRAMS list, >>> which made "make check-docs" target notice that the command has been >>> removed but still is documented. >>> >>> Signed-off-by: Junio C Hamano >>> --- >>>   * The command is still listed in the resulting "git help git" >>>     output, as cmd-list.perl does not yet know which commands on the >>>     list are to be ignored under WITH_BREAKING_CHANGES. >> >> Good catch. It seems the meson build was also forgotten in 68f51871df8 >> (builtin/pack-redundant: remove subcommand with breaking changes, >> 2025-01-22) as we still compile builtin/pack-redundant.c and build the >> documentation. We should probably wrap the function declaration for >> cmd_pack_redundant() in builtin.h with "#ifndef WITH_BREAKING_CHANGES" >> as well though I don't think that is urgent. > > I just had a look at fixing the meson build but it seems to be tricky as > the manpage sources are stored in a meson dictionary and meson > dictionaries are immutable so I don't know how one is supposed to > conditionally add items. > But dictonaries can be combined [1]. So we could probably do something like I've added below. [1]: https://mesonbuild.com/Reference-manual_elementary_dict.html -- 8< -- diff --git a/Documentation/meson.build b/Documentation/meson.build index 0a0f2bfa14..fcfec63e9b 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -96,7 +96,6 @@ manpages = { 'git-notes.adoc' : 1, 'git-p4.adoc' : 1, 'git-pack-objects.adoc' : 1, - 'git-pack-redundant.adoc' : 1, 'git-pack-refs.adoc' : 1, 'git-patch-id.adoc' : 1, 'git-prune-packed.adoc' : 1, @@ -205,6 +204,14 @@ manpages = { 'gitworkflows.adoc' : 7, } +manpages_breaking_changes = { + 'git-pack-redundant.adoc' : 1, +} + +if not get_option('breaking_changes') + manpages += manpages_breaking_changes +endif + docs_backend = get_option('docs_backend') if docs_backend == 'auto' if find_program('asciidoc', dirs: program_path, required: false).found() @@ -475,7 +482,7 @@ endif # Sanity check that we are not missing any tests present in 't/'. This check # only runs once at configure time and is thus best-effort, only. Furthermore, # it only verifies man pages for the sake of simplicity. -configured_manpages = manpages.keys() + [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ] +configured_manpages = manpages.keys() + manpages_breaking_changes.keys() + [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ] actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc', check: true, env: script_environment, ``` ## Phillip Wood, 2025-03-09 10:52 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/b5fb3292-216a-4456-b456-e9ed38affc22%40gmail.com In-Reply-To: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com> ``` On 07/03/2025 15:07, Phillip Wood wrote: > On 07/03/2025 10:32, Phillip Wood wrote: > > The diff below stops us from building pack-redundant with > -Dbreaking_changes=true but still builds the documentation. I don't intend > spending any more time one this > > [...] > >  if get_option('breaking_changes') >    build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease') > +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : > 'c') Looking again at this I think it should probably be libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease' to match the rest of our meson.build. As a newcomer to meson I find it confusing that the CFLAGS for the build targets are set implicitly by their libgit dependency. Best Wishes Phillip ``` ## Phillip Wood, 2025-03-09 10:52 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: <1d9ab1ed-54bf-4359-85c2-28060cbf7198@gmail.com> URL: https://gitlist.dev/e/1d9ab1ed-54bf-4359-85c2-28060cbf7198%40gmail.com In-Reply-To: ``` Hi Karthik On 07/03/2025 22:42, Karthik Nayak wrote: > FPhillip Wood writes: > >> On 07/03/2025 10:32, Phillip Wood wrote: >>> On 05/03/2025 15:53, Junio C Hamano wrote: >>>> We correctly omit builtin/pack-objects.o from BUILTIN_OBJS, but >>>> forgot to add "git pack-redundant" on the EXCLUDED_PROGRAMS list, >>>> which made "make check-docs" target notice that the command has been >>>> removed but still is documented. >>>> >>>> Signed-off-by: Junio C Hamano >>>> --- >>>>   * The command is still listed in the resulting "git help git" >>>>     output, as cmd-list.perl does not yet know which commands on the >>>>     list are to be ignored under WITH_BREAKING_CHANGES. >>> >>> Good catch. It seems the meson build was also forgotten in 68f51871df8 >>> (builtin/pack-redundant: remove subcommand with breaking changes, >>> 2025-01-22) as we still compile builtin/pack-redundant.c and build the >>> documentation. We should probably wrap the function declaration for >>> cmd_pack_redundant() in builtin.h with "#ifndef WITH_BREAKING_CHANGES" >>> as well though I don't think that is urgent. >> >> I just had a look at fixing the meson build but it seems to be tricky as >> the manpage sources are stored in a meson dictionary and meson >> dictionaries are immutable so I don't know how one is supposed to >> conditionally add items. >> > > But dictonaries can be combined [1]. So we could probably do something > like I've added below. Thanks, my web search took me to a different page in the documentation [1]. Looking carefully there is an example of adding an element to a dictionary right at the end of that section but it is not mentioned anywhere in the text. I do find the meson documentation hard to use. I think it would be best if someone with more knowledge of meson than me took this forward Thanks Phillip [1] https://mesonbuild.com/Syntax.html#dictionaries > [1]: https://mesonbuild.com/Reference-manual_elementary_dict.html > > -- 8< -- > > diff --git a/Documentation/meson.build b/Documentation/meson.build > index 0a0f2bfa14..fcfec63e9b 100644 > --- a/Documentation/meson.build > +++ b/Documentation/meson.build > @@ -96,7 +96,6 @@ manpages = { > 'git-notes.adoc' : 1, > 'git-p4.adoc' : 1, > 'git-pack-objects.adoc' : 1, > - 'git-pack-redundant.adoc' : 1, > 'git-pack-refs.adoc' : 1, > 'git-patch-id.adoc' : 1, > 'git-prune-packed.adoc' : 1, > @@ -205,6 +204,14 @@ manpages = { > 'gitworkflows.adoc' : 7, > } > > +manpages_breaking_changes = { > + 'git-pack-redundant.adoc' : 1, > +} > + > +if not get_option('breaking_changes') > + manpages += manpages_breaking_changes > +endif > + > docs_backend = get_option('docs_backend') > if docs_backend == 'auto' > if find_program('asciidoc', dirs: program_path, required: false).found() > @@ -475,7 +482,7 @@ endif > # Sanity check that we are not missing any tests present in 't/'. This check > # only runs once at configure time and is thus best-effort, only. Furthermore, > # it only verifies man pages for the sake of simplicity. > -configured_manpages = manpages.keys() + [ 'git-bisect-lk2009.adoc', > 'git-tools.adoc' ] > +configured_manpages = manpages.keys() + > manpages_breaking_changes.keys() + [ 'git-bisect-lk2009.adoc', > 'git-tools.adoc' ] > actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc', > check: true, > env: script_environment, ``` ## Patrick Steinhardt, 2025-03-10 06:42 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/Z86Jze2qZ5s5OyOB%40pks.im In-Reply-To: ``` On Sun, Mar 09, 2025 at 10:52:44AM +0000, Phillip Wood wrote: > On 07/03/2025 15:07, Phillip Wood wrote: > > On 07/03/2025 10:32, Phillip Wood wrote: > > > > The diff below stops us from building pack-redundant with > > -Dbreaking_changes=true but still builds the documentation. I don't intend > > spending any more time one this > > > > [...] > > > >  if get_option('breaking_changes') > >    build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease') > > +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : > > 'c') > > Looking again at this I think it should probably be > > libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease' > > to match the rest of our meson.build. As a newcomer to meson I find it > confusing that the CFLAGS for the build targets are set implicitly by their > libgit dependency. Yup, that would be preferable indeed, thanks! Patrick ``` ## Patrick Steinhardt, 2025-03-11 14:40 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/Z9BLZWUarN0kC4CQ%40pks.im In-Reply-To: ``` On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote: > On Sun, Mar 09, 2025 at 10:52:44AM +0000, Phillip Wood wrote: > > On 07/03/2025 15:07, Phillip Wood wrote: > > > On 07/03/2025 10:32, Phillip Wood wrote: > > > > > > The diff below stops us from building pack-redundant with > > > -Dbreaking_changes=true but still builds the documentation. I don't intend > > > spending any more time one this > > > > > > [...] > > > > > >  if get_option('breaking_changes') > > >    build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease') > > > +  add_project_arguments('-DWITH_BREAKING_CHANGES=YesPlease', language : > > > 'c') > > > > Looking again at this I think it should probably be > > > > libgit_c_args += '-DWITH_BREAKING_CHANGES=YesPlease' > > > > to match the rest of our meson.build. As a newcomer to meson I find it > > confusing that the CFLAGS for the build targets are set implicitly by their > > libgit dependency. > > Yup, that would be preferable indeed, thanks! To set expectations: do you have the time/intent to work on this and polish it up into a patch? Otherwise I'm happy to pick it up. Thanks! Patrick ``` ## phillip.wood123@gmail.com, 2025-03-12 10:39 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: <75a8b321-4cf0-4d75-93a7-b616ae818d81@gmail.com> URL: https://gitlist.dev/e/75a8b321-4cf0-4d75-93a7-b616ae818d81%40gmail.com In-Reply-To: ``` Hi Patrick On 11/03/2025 14:40, Patrick Steinhardt wrote: > On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote: > > To set expectations: do you have the time/intent to work on this and > polish it up into a patch? Otherwise I'm happy to pick it up. If you're happy to pick this up that would be great. I'm unlikely to have time for git related things for the next week or so and you're also much more familiar with meson than me. Thanks Phillip ``` ## Patrick Steinhardt, 2025-03-12 13:43 Subject: Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES Message-ID: URL: https://gitlist.dev/e/Z9GPbggliX7asG2a%40pks.im In-Reply-To: <75a8b321-4cf0-4d75-93a7-b616ae818d81@gmail.com> ``` On Wed, Mar 12, 2025 at 10:39:49AM +0000, phillip.wood123@gmail.com wrote: > Hi Patrick > > On 11/03/2025 14:40, Patrick Steinhardt wrote: > > On Mon, Mar 10, 2025 at 07:42:25AM +0100, Patrick Steinhardt wrote: > > > > To set expectations: do you have the time/intent to work on this and > > polish it up into a patch? Otherwise I'm happy to pick it up. > > If you're happy to pick this up that would be great. I'm unlikely to have > time for git related things for the next week or so and you're also much > more familiar with meson than me. Okay, I've sent the patch series in [1]. Thanks! [1]: <20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im> Patrick ```