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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mar 7, 2025, 15:07 UTC
Message-ID: <56cf842a-7c1f-4354-b191-35bcc1e139bd@gmail.com>
In-Reply-To: <082af6a3-a7ba-440d-af84-6c59827a2929@gmail.com>

On 07/03/2025 10:32, Phillip Wood wrote:

Show 18 quoted lines

> 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