[PATCH] docs: fix repository-layout when building with breaking changes

[PATCH] docs: fix repository-layout when building with breaking changes

[Phillip Wood via GitGitGadget]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

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 <phillip.wood@dunelm.org.uk>
---
    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

Re: [PATCH] docs: fix repository-layout when building with breaking changes

[Junio C Hamano]

"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> 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?

Re: [PATCH] docs: fix repository-layout when building with breaking changes

[Patrick Steinhardt]

On Mon, Mar 03, 2025 at 10:18:05AM -0800, Junio C Hamano wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> 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

Re: [PATCH] docs: fix repository-layout when building with breaking changes

[Phillip Wood]

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" <gitgitgadget@gmail.com> 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
> 

Re: [PATCH] docs: fix repository-layout when building with breaking changes

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> 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.

[PATCH v2] docs: fix repository-layout when building with breaking changes

[Phillip Wood via GitGitGadget]

From: Phillip Wood <phillip.wood@dunelm.org.uk>

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 <phillip.wood@dunelm.org.uk>
---
    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 <phillip.wood@dunelm.org.uk>
      
     @@ 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

[PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Junio C Hamano]

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 <gitster@pobox.com>
---
 * 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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Phillip Wood]

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 <gitster@pobox.com>
> ---
>   * 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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Phillip Wood]

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 <gitster@pobox.com>
>> ---
>>   * 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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> 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?

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2903 bytes --]

FPhillip Wood <phillip.wood123@gmail.com> 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 <gitster@pobox.com>
>>> ---
>>>   * 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,

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Phillip Wood]

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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Phillip Wood]

Hi Karthik

On 07/03/2025 22:42, Karthik Nayak wrote:
> FPhillip Wood <phillip.wood123@gmail.com> 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 <gitster@pobox.com>
>>>> ---
>>>>    * 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,

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Patrick Steinhardt]

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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Patrick Steinhardt]

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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[phillip.wood123]

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

Re: [PATCH] docs: fix check-docs with WITH_BREAKING_CHANGES

[Patrick Steinhardt]

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