From: D. Ben Knoble <ben.knoble+github@gmail.com>
Date: Tue, 17 Feb 2026 13:28:10 GMT
Subject: Re: [PATCH v4] meson: regenerate config-list.h when Documentation changes
Message-ID: <CALnO6CAGfd2orSzjvxfAYx=xBnB=QdOwT-f5g0pQrcxO_19BGQ@mail.gmail.com>
In-Reply-To: <aZQSf9yaNa693IFF@pks.im>

On Tue, Feb 17, 2026 at 2:02 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Mon, Feb 16, 2026 at 05:28:57PM -0500, D. Ben Knoble wrote:
> > The Meson-based build doesn't know when to rebuild config-list.h, so the
> > header is sometimes stale.
> >
> > For example, an old build directory might have config-list.h from before
> > 4173df5187 (submodule: introduce extensions.submodulePathConfig,
> > 2026-01-12), which added submodule.<name>.gitdir to the list. Without
> > it, t9902-completion.sh fails. Regenerating the config-list.h artifact
> > from sources fixes the artifact and the test.
> >
> > Teach the meson build to depend on the Documentation files that
> > generate-configlist.sh reads by having it an additional output as a list
> > of dependency files, since Meson does not have (or want) builtin support
> > for globbing like Make. We assume that if a user adds a new file under
> > Documentation/config then they will also edit one of the existing files
> > to include that new file, and that will trigger a rebuild.
> >
> > Also mark the generator script as a dependency.
> >
> > Combining the following commands helps debug dependencies:
> >
> >     ninja -C <builddir> -t deps config-list.h
> >     ninja -C <builddir> -t browse config-list.h
> >
> > The former lists all the dependencies discovered from our output ".d"
> > file (the config documentation) and the latter shows the dependency on
> > the script itself, among other useful edges in the dependency graph.
>
> Good hint, thanks for putting it here.
>
> > diff --git a/generate-configlist.sh b/generate-configlist.sh
> > index 75c39ade209..63a2e8b6938 100755
> > --- a/generate-configlist.sh
> > +++ b/generate-configlist.sh
> > @@ -36,3 +37,12 @@ print_config_list ()
> >       echo
> >       print_config_list
> >  } >"$OUTPUT"
> > +
> > +if test -n "$DEPFILE"
> > +then
> > +     for doc in "$SOURCE_DIR"/Documentation/*config.adoc \
> > +             "$SOURCE_DIR"/Documentation/config/*.adoc
> > +     do
> > +             printf "$OUTPUT: %s\n" "$(printf '%s\n' "$doc" | sed 's/[# ]/\\&/g')"
>
> Tiny nit: can't we simplify this to "$(echo "$doc" | sed ...)"?

Given how unportable echo is [1], I'd prefer to keep printf.

[1]: https://github.com/benknoble/echocho, for one. If the doc started
with "-e" for example the results might be unreliable.


> > diff --git a/meson.build b/meson.build
> > index 3a1d12caa4b..e4b8f1e33d2 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -720,11 +720,14 @@ endif
> >
> >  builtin_sources += custom_target(
> >    output: 'config-list.h',
> > +  depfile: 'config-list.h.d',
> > +  depend_files: [ 'generate-configlist.sh' ],
> >    command: [
> >      shell,
> > -    meson.current_source_dir() + '/generate-configlist.sh',
> > +    meson.current_source_dir() / 'generate-configlist.sh',
> >      meson.current_source_dir(),
> >      '@OUTPUT@',
> > +    '@DEPFILE@',
> >    ],
> >    env: script_environment,
> >  )
>
> And this looks as expected now, too. I also verified locally that this
> correctly rebuilds things when either the script or any of the files
> change.
>
> I wonder whether we also want to have a second patch on top to adjust
> our Makefile? E.g. something like this:
>
> diff --git a/Makefile b/Makefile
> index 47ed9fa7fd..ed82775ec2 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2688,8 +2688,10 @@ $(BUILT_INS): git$X
>
>  config-list.h: generate-configlist.sh
>
> -config-list.h: Documentation/*config.adoc Documentation/config/*.adoc
> -       $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh . $@
> +config-list.h: generate-configlist.sh
> +       $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh . $@ .depend/config-list.h.d
> +
> +-include .depend/config-list.h.d
>
>  command-list.h: generate-cmdlist.sh command-list.txt
>
>
> Please feel free to skip this part though.
>
> Thanks!
>
> Patrick

That seems reasonable. I'll add to the next version.