Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Toon Claes <toon@iotcl.com>, git@vger.kernel.org
Cc: "D. Ben Knoble" <ben.knoble+github@gmail.com>,
	Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v3] generate-configlist: collapse depfile for older Ninja
Date: Fri, 15 May 2026 10:35:32 +0100	[thread overview]
Message-ID: <e32f559e-0b28-40be-875b-956de2bb2fca@gmail.com> (raw)
In-Reply-To: <20260515-toon-fix-almalinux8-v3-1-b545a0647f0f@iotcl.com>

Hi Toon

Thanks for re-rolling, this version looks good to me

Phillip

On 15/05/2026 09:42, Toon Claes wrote:
> The tools/generate-configlist.sh script generates two files:
>    * config-list.h
>    * config-list.h.d
> 
> The former is included by the source code and the latter defines on
> which files the former depends.
> 
> The contents of `config-list.h.d` consists of two sections:
> 
>      config-list.h: Documentation/config.adoc
>      config-list.h: Documentation/git-config.adoc
>      config-list.h: Documentation/config/add.adoc
>      config-list.h: Documentation/config/advice.adoc
>      config-list.h: Documentation/config/alias.adoc
>      config-list.h: Documentation/config/am.adoc
>      config-list.h: Documentation/config/apply.adoc
>      ...
> 
> This first section actually defines on which individual files
> `config-list.h` depends and thus needs to be rebuild if one of those
> changes.
> 
> And the second section contains content like:
> 
>      Documentation/config.adoc:
>      Documentation/git-config.adoc:
>      Documentation/config/add.adoc:
>      Documentation/config/advice.adoc:
>      Documentation/config/alias.adoc:
>      Documentation/config/am.adoc:
>      Documentation/config/apply.adoc:
>      ...
> 
> These rules exist to ensure Make won't fail with the following error if
> one of the .adoc files is renamed or removed:
> 
>     make: *** No rule to make target 'Documentation/config.adoc', needed by 'config-list.h'.
> 
> With the no-op targets defined in `config-list.h.d`, Make knows there's
> no work to be done to generate these files, so it doesn't error out if
> it doesn't exist.
> 
> For the Makefile build system this works great. And since
> ebeea3c471 (build: regenerate config-list.h when Documentation changes,
> 2026-02-24) this script is also called from the Meson build system.
> Nevertheless, on AlmaLinux 8 the following build failure is seen:
> 
>      ninja: error: dependency cycle: config-list.h -> config-list.h
> 
> This version of this distro uses Ninja 1.8.2 and it seems to have some
> issues with the format of the `config-list.h.d` file.
> 
> Ninja versions before 1.10.0 do not reset the depfile parser state on
> newlines. This causes issues when the depfile has one dependency per
> line, like we have in `config-list.h.d`:
> 
>      config-list.h: Documentation/config.adoc
>      config-list.h: Documentation/config/add.adoc
> 
> The parser only recognizes the first "config-list.h:" as a target. On
> subsequent lines it is still in dependency-parsing mode, so the repeated
> output name is recorded as an input. This causes the error mentioned
> above.
> 
> The bug in Ninja is fixed in 1.10, with commit
> ninja-build/ninja@1daa7470ab7e (depfile_parser: remove restriction on
> multiple outputs, 2019-11-20).
> 
> To be compatible with older versions of Ninja, collapse the dependencies
> for `config-list.h` into a single line like:
> 
>      config-list.h: Documentation/config.adoc Documentation/config/add.adoc ...
> 
> This works around the bug in older versions of Ninja, and is fully
> compatible Make and with more recent versions of Ninja. And while the
> no-op targets are not needed for Ninja, they also don't do any harm.
> 
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Toon Claes <toon@iotcl.com>
> ---
> At GitLab we build images for various distros, including AlmaLinux 8.
> On this distro we got this error while compiling Git.
> 
>      ninja: error: dependency cycle: config-list.h -> config-list.h
> 
> It seems this is caused by a bug in older versions of Ninja. There are
> more details in the commit message, but here are a few simple steps to
> reproduce:
> 
> docker run --rm -it -v $(pwd):/git -w /git almalinux:8 bash
>      dnf -yq install epel-release
>      dnf -yq install shadow-utils sudo make pkg-config gcc findutils \
>          diffutils perl python3 gawk gettext zlib-devel expat-devel \
>          openssl-devel curl-devel pcre2-devel cargo
>      pip3 install --prefix=/usr meson ninja==1.8.2
>      meson setup build --warnlevel 2 --werror
>      ninja -C build config-list.h
>      ninja -C build config-list.h   # fails with dependency cycle
> ---
> Changes in v3:
> - Stop using \n in sed(1) replacement strings because it is not
>    portable.
> - Link to v2: https://patch.msgid.link/20260422-toon-fix-almalinux8-v2-1-45d8471ed0e9@iotcl.com
> 
> Changes in v2:
> - Simplify the changes *a lot* by doing the collapsing unconditionally.
> - Link to v1: https://patch.msgid.link/20260421-toon-fix-almalinux8-v1-1-aec1d54addde@iotcl.com
> ---
>   tools/generate-configlist.sh | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
> index e28054f9e0..d1d2ba4bb7 100755
> --- a/tools/generate-configlist.sh
> +++ b/tools/generate-configlist.sh
> @@ -42,9 +42,12 @@ if test -n "$DEPFILE"
>   then
>   	QUOTED_OUTPUT="$(printf '%s\n' "$OUTPUT" | sed 's,[&/\],\\&,g')"
>   	{
> +		printf '%s' "$QUOTED_OUTPUT: "
>   		printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>   			"$SOURCE_DIR"/Documentation/config/*.adoc |
> -			sed -e 's/[# ]/\\&/g' -e "s/^/$QUOTED_OUTPUT: /"
> +			sed -e 's/[# ]/\\&/g' |
> +			tr '\n' ' '
> +		printf '\n'
>   		printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>   			"$SOURCE_DIR"/Documentation/config/*.adoc |
>   			sed -e 's/[# ]/\\&/g'
> 
> ---
> base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
> change-id: 20260421-toon-fix-almalinux8-102de9138294
> 
> 


  reply	other threads:[~2026-05-15  9:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21 19:17 [PATCH] generate-configlist: collapse depfile for older Ninja Toon Claes
2026-04-22  6:36 ` Patrick Steinhardt
2026-04-22  7:09   ` Toon Claes
2026-04-22  7:21 ` [PATCH v2] " Toon Claes
2026-04-22 10:30   ` Patrick Steinhardt
2026-04-22 13:45   ` Phillip Wood
2026-04-22 14:12     ` Phillip Wood
2026-05-15  8:44     ` Toon Claes
2026-05-15  8:42   ` [PATCH v3] " Toon Claes
2026-05-15  9:35     ` Phillip Wood [this message]
2026-04-22 18:35 ` [PATCH] " D. Ben Knoble

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e32f559e-0b28-40be-875b-956de2bb2fca@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ben.knoble+github@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --cc=toon@iotcl.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox