{"thread":{"id":"63100","subject":"[PATCH 0/3] meson: improve handling of `-Dbreaking_changes=true`","startedAt":"2025-03-12T13:17:41Z","lastAt":"2025-03-19T09:37:08Z","messageCount":11,"participants":["Patrick Steinhardt","Karthik Nayak","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"514051","messageId":"20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im","threadId":"63100","inReplyTo":null,"subject":"[PATCH 0/3] meson: improve handling of `-Dbreaking_changes=true`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T13:17:31Z","receivedAt":"2025-03-12T13:17:41Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series improves handling of the breaking changes option\nwith the Meson build system as discussed in the thread starting at [1].\n\nThanks!\n\nPatrick\n\n[1]: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com>\n\n---\nPatrick Steinhardt (3):\n      meson: define WITH_BREAKING_CHANGES when enabling breaking changes\n      meson: don't compile git-pack-redundant(1) with breaking changes\n      meson: don't install git-pack-redundant(1) docs with breaking changes\n\n Documentation/Makefile    |  2 +-\n Documentation/meson.build | 13 +++++++++++--\n meson.build               | 18 +++++++++++-------\n 3 files changed, 23 insertions(+), 10 deletions(-)\n\n\n---\nbase-commit: 87a0bdbf0f72b7561f3cd50636eee33dcb7dbcc3\nchange-id: 20250312-b4-pks-meson-breaking-changes-819afcca2e07\n\n"},{"id":"514052","messageId":"20250312-b4-pks-meson-breaking-changes-v1-1-b89e9a59d228@pks.im","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im","subject":"[PATCH 1/3] meson: define WITH_BREAKING_CHANGES when enabling breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T13:17:32Z","receivedAt":"2025-03-12T13:17:42Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"While Meson already supports the `-Dbreaking_changes=true` option, it\nonly wires up the build option that propagates into the tests. The build\noption is only used for our tests to enable the `WITH_BREAKING_CHANGES`\nprerequisite though, and does not influence the code that is actually\nbeing built.\n\nThe omission went unnoticed because we only have tests right now that\nget disabled when breaking changes are enabled, but not the other way\nround. In other words, we don't have any tests that verify that breaking\nchanges behave as expected.\n\nFix the build issue by setting the `WITH_BREAKING_CHANGES` preprocessor\nmacro when breaking changes are enabled. Note that the `libgit_c_args`\narray is defined after the current spot where we handle the option, so\nto not have multiple sites where we handle it we instead move it after\nthe array has been defined.\n\nBased-on-patch-by: Phillip Wood <phillip.wood123@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n meson.build | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/meson.build b/meson.build\nindex efe2871c9db..4ddc44f510d 100644\n--- a/meson.build\n+++ b/meson.build\n@@ -672,12 +672,6 @@ build_options_config.set_quoted('GIT_TEST_UTF8_LOCALE', get_option('test_utf8_lo\n build_options_config.set_quoted('LOCALEDIR', fs.as_posix(get_option('prefix') / get_option('localedir')))\n build_options_config.set('GITWEBDIR', fs.as_posix(get_option('prefix') / get_option('datadir') / 'gitweb'))\n \n-if get_option('breaking_changes')\n-  build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n-else\n-  build_options_config.set('WITH_BREAKING_CHANGES', '')\n-endif\n-\n if get_option('sane_tool_path').length() != 0\n   sane_tool_path = (host_machine.system() == 'windows' ? ';' : ':').join(get_option('sane_tool_path'))\n   build_options_config.set_quoted('BROKEN_PATH_FIX', 's|^\\# @BROKEN_PATH_FIX@$|git_broken_path_fix \"' + sane_tool_path + '\"|')\n@@ -739,6 +733,13 @@ if get_option('warning_level') in ['2','3', 'everything'] and compiler.get_argum\n   endforeach\n endif\n \n+if get_option('breaking_changes')\n+  build_options_config.set('WITH_BREAKING_CHANGES', 'YesPlease')\n+  libgit_c_args += '-DWITH_BREAKING_CHANGES'\n+else\n+  build_options_config.set('WITH_BREAKING_CHANGES', '')\n+endif\n+\n if get_option('b_sanitize').contains('address')\n   build_options_config.set('SANITIZE_ADDRESS', 'YesCompiledWithIt')\n else\n\n-- \n2.49.0.rc2.394.gf6994c5077.dirty\n\n"},{"id":"514053","messageId":"20250312-b4-pks-meson-breaking-changes-v1-2-b89e9a59d228@pks.im","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im","subject":"[PATCH 2/3] meson: don't compile git-pack-redundant(1) with breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T13:17:33Z","receivedAt":"2025-03-12T13:17:43Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We continue to compile the git-pack-redundant(1) builtin with Meson when\nbreaking changes are enabled even though we ultimately don't expose this\ncommand at all. This is mostly harmless, but given that the intent of\nthe build option is to be as close as possible to the state where the\nbreaking change has been fully implemented this isn't optimal either.\n\nImprove the situation by not compiling the builtin when breaking changes\nare enabled.\n\nBased-on-patch-by: Phillip Wood <phillip.wood123@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n meson.build | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/meson.build b/meson.build\nindex 4ddc44f510d..6b0eb6430af 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\n-- \n2.49.0.rc2.394.gf6994c5077.dirty\n\n"},{"id":"514054","messageId":"20250312-b4-pks-meson-breaking-changes-v1-3-b89e9a59d228@pks.im","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im","subject":"[PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T13:17:34Z","receivedAt":"2025-03-12T13:17:44Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"When breaking changes are enabled we continue to install documentation\nof the git-pack-redundant(1) command even though it is completely\ndisabled and thus inaccessible. Improve this by only installing the\ndocumentation in case breaking changes aren't enabled.\n\nBased-on-patch-by: Karthik Nayak <karthik.188@gmail.com>\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n Documentation/Makefile    |  2 +-\n Documentation/meson.build | 13 +++++++++++--\n 2 files changed, 12 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/Makefile b/Documentation/Makefile\nindex 671267a8ac7..e6b20c021fd 100644\n--- a/Documentation/Makefile\n+++ b/Documentation/Makefile\n@@ -509,7 +509,7 @@ lint-docs-meson:\n \tawk \"/^manpages = {$$/ {flag=1 ; next } /^}$$/ { flag=0 } flag { gsub(/^  \\047/, \\\"\\\"); gsub(/\\047 : [157],\\$$/, \\\"\\\"); print }\" meson.build | \\\n \t\tgrep -v -e '#' -e '^$$' | \\\n \t\tsort >tmp-meson-diff/meson.adoc && \\\n-\tls git*.adoc scalar.adoc | grep -v -e git-bisect-lk2009.adoc -e git-tools.adoc >tmp-meson-diff/actual.adoc && \\\n+\tls git*.adoc scalar.adoc | grep -v -e git-bisect-lk2009.adoc -e git-pack-redundant.adoc -e git-tools.adoc >tmp-meson-diff/actual.adoc && \\\n \tif ! cmp tmp-meson-diff/meson.adoc tmp-meson-diff/actual.adoc; then \\\n \t\techo \"Meson man pages differ from actual man pages:\"; \\\n \t\tdiff -u tmp-meson-diff/meson.adoc tmp-meson-diff/actual.adoc; \\\ndiff --git a/Documentation/meson.build b/Documentation/meson.build\nindex 594546d68b1..a2de85f5aad 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@@ -479,7 +486,9 @@ 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', 'git-tools.adoc' ]\n+configured_manpages = manpages.keys()\n+configured_manpages += manpages_breaking_changes.keys()\n+configured_manpages += [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ]\n actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n   check: true,\n   env: script_environment,\n\n-- \n2.49.0.rc2.394.gf6994c5077.dirty\n\n"},{"id":"514170","messageId":"CAOLa=ZS9FOdqrcg1eYLvKf_vwR4Ag2T+O-RCoGiKdC1i-DTFuw@mail.gmail.com","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-3-b89e9a59d228@pks.im","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-13T11:07:08Z","receivedAt":"2025-03-13T11:07:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> When breaking changes are enabled we continue to install documentation\n> of the git-pack-redundant(1) command even though it is completely\n> disabled and thus inaccessible. Improve this by only installing the\n> documentation in case breaking changes aren't enabled.\n>\n> Based-on-patch-by: Karthik Nayak <karthik.188@gmail.com>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  Documentation/Makefile    |  2 +-\n>  Documentation/meson.build | 13 +++++++++++--\n>  2 files changed, 12 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/Makefile b/Documentation/Makefile\n> index 671267a8ac7..e6b20c021fd 100644\n> --- a/Documentation/Makefile\n> +++ b/Documentation/Makefile\n> @@ -509,7 +509,7 @@ lint-docs-meson:\n>  \tawk \"/^manpages = {$$/ {flag=1 ; next } /^}$$/ { flag=0 } flag { gsub(/^  \\047/, \\\"\\\"); gsub(/\\047 : [157],\\$$/, \\\"\\\"); print }\" meson.build | \\\n>  \t\tgrep -v -e '#' -e '^$$' | \\\n>  \t\tsort >tmp-meson-diff/meson.adoc && \\\n> -\tls git*.adoc scalar.adoc | grep -v -e git-bisect-lk2009.adoc -e git-tools.adoc >tmp-meson-diff/actual.adoc && \\\n> +\tls git*.adoc scalar.adoc | grep -v -e git-bisect-lk2009.adoc -e git-pack-redundant.adoc -e git-tools.adoc >tmp-meson-diff/actual.adoc && \\\n>  \tif ! cmp tmp-meson-diff/meson.adoc tmp-meson-diff/actual.adoc; then \\\n>  \t\techo \"Meson man pages differ from actual man pages:\"; \\\n>  \t\tdiff -u tmp-meson-diff/meson.adoc tmp-meson-diff/actual.adoc; \\\n\nNice, I totally missed then when I was tinkering with this issue.\n\n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index 594546d68b1..a2de85f5aad 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> @@ -479,7 +486,9 @@ 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', 'git-tools.adoc' ]\n> +configured_manpages = manpages.keys()\n> +configured_manpages += manpages_breaking_changes.keys()\n> +configured_manpages += [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ]\n>  actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n>    check: true,\n>    env: script_environment,\n>\n> --\n> 2.49.0.rc2.394.gf6994c5077.dirty\n\nThe rest looks good to me. Thanks!\n"},{"id":"514171","messageId":"CAOLa=ZR6Y1rE7BmJ-gUov_b9DNPs1q7ZDz0v0uUShkN6vVtDjw@mail.gmail.com","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-0-b89e9a59d228@pks.im","subject":"Re: [PATCH 0/3] meson: improve handling of `-Dbreaking_changes=true`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-03-13T11:08:25Z","receivedAt":"2025-03-13T11:08:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> this small patch series improves handling of the breaking changes option\n> with the Meson build system as discussed in the thread starting at [1].\n>\n> Thanks!\n>\n> Patrick\n>\n> [1]: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com>\n>\n\nThanks for picking this up. I think the series looks good, and is inline\nwith the discussion we had earlier.\n\n> ---\n> Patrick Steinhardt (3):\n>       meson: define WITH_BREAKING_CHANGES when enabling breaking changes\n>       meson: don't compile git-pack-redundant(1) with breaking changes\n>       meson: don't install git-pack-redundant(1) docs with breaking changes\n>\n>  Documentation/Makefile    |  2 +-\n>  Documentation/meson.build | 13 +++++++++++--\n>  meson.build               | 18 +++++++++++-------\n>  3 files changed, 23 insertions(+), 10 deletions(-)\n>\n>\n> ---\n> base-commit: 87a0bdbf0f72b7561f3cd50636eee33dcb7dbcc3\n> change-id: 20250312-b4-pks-meson-breaking-changes-819afcca2e07\n"},{"id":"514395","messageId":"44a3ad4e-fb50-447f-bb66-f43b5c5ae012@gmail.com","threadId":"63100","inReplyTo":"20250312-b4-pks-meson-breaking-changes-v1-3-b89e9a59d228@pks.im","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-16T15:19:48Z","receivedAt":"2025-03-16T15:19:51Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 12/03/2025 13:17, Patrick Steinhardt wrote:\n\nThanks for taking this forward\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> @@ -479,7 +486,9 @@ 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', 'git-tools.adoc' ]\n> +configured_manpages = manpages.keys()\n> +configured_manpages += manpages_breaking_changes.keys()\n> +configured_manpages += [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ]\n\nI don't think we need this hunk as we add manpages_breaking_changes into \nmanpages in the hunk above.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"514406","messageId":"Z9gqP3ng0a9Zfpqc@pks.im","threadId":"63100","inReplyTo":"44a3ad4e-fb50-447f-bb66-f43b5c5ae012@gmail.com","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-17T13:57:19Z","receivedAt":"2025-03-17T13:57:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Mar 16, 2025 at 03:19:48PM +0000, Phillip Wood wrote:\n> On 12/03/2025 13:17, Patrick Steinhardt wrote:\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> > @@ -479,7 +486,9 @@ 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', 'git-tools.adoc' ]\n> > +configured_manpages = manpages.keys()\n> > +configured_manpages += manpages_breaking_changes.keys()\n> > +configured_manpages += [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ]\n> \n> I don't think we need this hunk as we add manpages_breaking_changes into\n> manpages in the hunk above.\n\nWe indeed need it: it's required in case the 'breaking_changes' option\nis enabled. In that case we still need to have the man pages here in\nthis variable because we use it to check that the manpage is handled at\nall. Otherwise we would error out because Meson thinks that we forgot to\nwire up this manpage that we found in the source directory.\n\nPatrick\n"},{"id":"514408","messageId":"d341777a-a6e5-46fe-8004-9fe885321905@gmail.com","threadId":"63100","inReplyTo":"Z9gqP3ng0a9Zfpqc@pks.im","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-17T14:50:56Z","receivedAt":"2025-03-17T14:51:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Patrick\n\nOn 17/03/2025 13:57, Patrick Steinhardt wrote:\n> On Sun, Mar 16, 2025 at 03:19:48PM +0000, Phillip Wood wrote:\n>> On 12/03/2025 13:17, Patrick Steinhardt wrote:\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>>> @@ -479,7 +486,9 @@ endif\n>>>    # Sanity check that we are not missing any tests present in 't/'. This check\n\nI think this part of the comment must have been copied from somewhere else\n\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', 'git-tools.adoc' ]\n>>> +configured_manpages = manpages.keys()\n>>> +configured_manpages += manpages_breaking_changes.keys()\n>>> +configured_manpages += [ 'git-bisect-lk2009.adoc', 'git-tools.adoc' ]\n>>\n>> I don't think we need this hunk as we add manpages_breaking_changes into\n>> manpages in the hunk above.\n> \n> We indeed need it: it's required in case the 'breaking_changes' option\n> is enabled. In that case we still need to have the man pages here in\n> this variable because we use it to check that the manpage is handled at\n> all. Otherwise we would error out because Meson thinks that we forgot to\n> wire up this manpage that we found in the source directory.\n\nOh so if we have selected breaking_changes then manpages.keys() does not \ninclude \"git-pack-redundant.adoc\" but that file exists and so we need to \nadd it to the list of configured man pages. If breaking_changes is \nselected then don't we end up adding \"git-pack-redundant.adoc\" to \nconfigured_manpages twice? Does that matter when we come to do\n\nactual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n   check: true,\n   env: script_environment,\n).stdout().strip().split('\\n')\n\nif configured_manpages != actual_manpages\n   ...\n\n? Also I'm confused as to how that comparison works without sorting \nconfigured_manpages. Even if manpages.keys() sorts the keys (the \ndocumentation at [1] is silent on that) we add some out-of-order entries \nto the end of the list.\n\nBest Wishes\n\nPhillip\n\n[1] https://mesonbuild.com/Reference-manual_elementary_dict.html#dictkeys\n"},{"id":"514503","messageId":"7fee83f7-995b-4c84-9216-caa6803a69d9@gmail.com","threadId":"63100","inReplyTo":"d341777a-a6e5-46fe-8004-9fe885321905@gmail.com","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-03-18T10:06:51Z","receivedAt":"2025-03-18T10:06:57Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 17/03/2025 14:50, Phillip Wood wrote:\n> \n> Oh so if we have selected breaking_changes then manpages.keys() does not \n> include \"git-pack-redundant.adoc\" but that file exists and so we need to \n> add it to the list of configured man pages. If breaking_changes is \n> selected then don't we end up adding \"git-pack-redundant.adoc\" to \n> configured_manpages twice? Does that matter when we come to do\n> \n> actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n>    check: true,\n>    env: script_environment,\n> ).stdout().strip().split('\\n')\n> \n> if configured_manpages != actual_manpages\n>    ...\n> \n> ? Also I'm confused as to how that comparison works without sorting \n> configured_manpages. Even if manpages.keys() sorts the keys (the \n> documentation at [1] is silent on that) we add some out-of-order entries \n> to the end of the list.\n\nI think the answer is that the comparison always fails but as there are \nno missing or superfluous man pages the body of the if does not error out.\n\nBest Wishes\n\nPhillip\n"},{"id":"514628","messageId":"Z9qQQAhFk-MNwiom@pks.im","threadId":"63100","inReplyTo":"7fee83f7-995b-4c84-9216-caa6803a69d9@gmail.com","subject":"Re: [PATCH 3/3] meson: don't install git-pack-redundant(1) docs with breaking changes","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-19T09:37:04Z","receivedAt":"2025-03-19T09:37:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 18, 2025 at 10:06:51AM +0000, Phillip Wood wrote:\n> On 17/03/2025 14:50, Phillip Wood wrote:\n> > \n> > Oh so if we have selected breaking_changes then manpages.keys() does not\n> > include \"git-pack-redundant.adoc\" but that file exists and so we need to\n> > add it to the list of configured man pages. If breaking_changes is\n> > selected then don't we end up adding \"git-pack-redundant.adoc\" to\n> > configured_manpages twice? Does that matter when we come to do\n> > \n> > actual_manpages = run_command(shell, '-c', 'ls git*.adoc scalar.adoc',\n> >    check: true,\n> >    env: script_environment,\n> > ).stdout().strip().split('\\n')\n> > \n> > if configured_manpages != actual_manpages\n> >    ...\n> > \n> > ? Also I'm confused as to how that comparison works without sorting\n> > configured_manpages. Even if manpages.keys() sorts the keys (the\n> > documentation at [1] is silent on that) we add some out-of-order entries\n> > to the end of the list.\n> \n> I think the answer is that the comparison always fails but as there are no\n> missing or superfluous man pages the body of the if does not error out.\n\nYeah. We indeed may have it multiple times now, but as you noticed it\nultimately still works. I didn't care too deeply to avoid the\nduplication because in the end this step is only used to verify that we\nhave all manpages wired up in Meson.\n\nPatrick\n"}]}