Git development
 help / color / mirror / Atom feed
* [PATCH] generate-configlist: collapse depfile for older Ninja
@ 2026-04-21 19:17 Toon Claes
  2026-04-22  6:36 ` Patrick Steinhardt
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Toon Claes @ 2026-04-21 19:17 UTC (permalink / raw)
  To: git; +Cc: D. Ben Knoble, Patrick Steinhardt, Toon Claes

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, add a fourth optional
argument to `generate-configlist.sh` that can be empty or "collapse".
When this argument is "collapse", there is a post-processing step on
`config-list.h.d` to put the dependencies for `config-list.h` on a
single line, like:

    config-list.h: Documentation/config.adoc Documentation/config/add.adoc ...

This works around the bug in older versions of Ninja.

In `meson.build`, the Ninja version is detected to determine if the
"collapse" argument needs to be provided to the script. Thus newer
versions of Ninja, and the Makefile build system still get dependencies
on separate lines in `config-list.h.d`.

In this post-processing, also the no-op targets are dropped because they
are simply not needed for Ninja.

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
---
 meson.build                  | 10 ++++++++++
 tools/generate-configlist.sh | 24 +++++++++++++++++++++++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 11488623bf..44e6f679c3 100644
--- a/meson.build
+++ b/meson.build
@@ -725,6 +725,15 @@ if not get_option('breaking_changes')
   builtin_sources += 'builtin/pack-redundant.c'
 endif
 
+configlist_gen_format = ''
+if get_option('backend') == 'ninja'
+  ninja = find_program('ninja', 'ninja-build', native: true)
+  ninja_version = run_command(ninja, '--version', check: true).stdout().strip()
+  if ninja_version.version_compare('<1.10.0')
+    configlist_gen_format = 'collapse'
+  endif
+endif
+
 builtin_sources += custom_target(
   output: 'config-list.h',
   depfile: 'config-list.h.d',
@@ -735,6 +744,7 @@ builtin_sources += custom_target(
     meson.current_source_dir(),
     '@OUTPUT@',
     '@DEPFILE@',
+    configlist_gen_format,
   ],
   env: script_environment,
 )
diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
index e28054f9e0..553fbfeb4b 100755
--- a/tools/generate-configlist.sh
+++ b/tools/generate-configlist.sh
@@ -3,10 +3,14 @@
 SOURCE_DIR="$1"
 OUTPUT="$2"
 DEPFILE="$3"
+FORMAT="${4}"
 
 if test -z "$SOURCE_DIR" || ! test -d "$SOURCE_DIR" || test -z "$OUTPUT"
 then
-	echo >&2 "USAGE: $0 <SOURCE_DIR> <OUTPUT> [<DEPFILE>]"
+	echo >&2 "USAGE: $0 <SOURCE_DIR> <OUTPUT> [<DEPFILE> [collapse]]"
+	echo >&2 ""
+	echo >&2 "  Pass 'collapse' to write all deps on a single line."
+	echo >&2 "  Ninja < 1.10.0 chokes on depfiles with multiple lines."
 	exit 1
 fi
 
@@ -49,4 +53,22 @@ then
 			"$SOURCE_DIR"/Documentation/config/*.adoc |
 			sed -e 's/[# ]/\\&/g'
 	} >"$DEPFILE"
+
+	# Due to a bug in Ninja versions before 1.10.0 the depfile parser state
+	# is not reset on newlines, causing the target to be recorded as a
+	# dependency of itself when there is one dependency per line.
+	# The bug is fixed in ninja-build/ninja@1daa7470ab7e (depfile_parser:
+	# remove restriction on multiple outputs, 2019-11-20).
+	# But to work around the issue, do a "collapse" post-processing step
+	# if wanted.
+	if test "$FORMAT" = "collapse"
+	then
+		{
+			printf '%s: ' "$OUTPUT"
+			sed -n -e "s/^$QUOTED_OUTPUT: //p" "$DEPFILE" |
+				tr '\n' ' '
+			echo
+		} >"$DEPFILE.tmp"
+		mv "$DEPFILE.tmp" "$DEPFILE"
+	fi
 fi

---
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
change-id: 20260421-toon-fix-almalinux8-102de9138294


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] generate-configlist: collapse depfile for older Ninja
  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 18:35 ` [PATCH] " D. Ben Knoble
  2 siblings, 1 reply; 11+ messages in thread
From: Patrick Steinhardt @ 2026-04-22  6:36 UTC (permalink / raw)
  To: Toon Claes; +Cc: git, D. Ben Knoble

On Tue, Apr 21, 2026 at 09:17:25PM +0200, Toon Claes wrote:
> 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.

Right, I can reproduce the issue with that version indeed:

    $ meson setup -Ddocs=man build
    $ ninja -C build
    ... successful build ...
    $ ninja -C build
    ninja: error: dependency cycle: config-list.h -> config-list.h

> 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).

Good find.

> To be compatible with older versions of Ninja, add a fourth optional
> argument to `generate-configlist.sh` that can be empty or "collapse".
> When this argument is "collapse", there is a post-processing step on
> `config-list.h.d` to put the dependencies for `config-list.h` on a
> single line, like:
> 
>     config-list.h: Documentation/config.adoc Documentation/config/add.adoc ...
> 
> This works around the bug in older versions of Ninja.

Do we really have to make the logic conditional? I would expect that
this works just fine for newer versions of Ninja and for Make, so it
feels rather pointless to me to have two modes to worry about.

So wouldn't the below simplified version be sufficient?

Thanks!

Patrick

diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
index e28054f9e0..f5f42492c6 100755
--- a/tools/generate-configlist.sh
+++ b/tools/generate-configlist.sh
@@ -44,7 +44,9 @@ then
 	{
 		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' ' ' |
+			sed -e "s/^/$QUOTED_OUTPUT: /" -e 's/ $/\n/'
 		printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
 			"$SOURCE_DIR"/Documentation/config/*.adoc |
 			sed -e 's/[# ]/\\&/g'

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] generate-configlist: collapse depfile for older Ninja
  2026-04-22  6:36 ` Patrick Steinhardt
@ 2026-04-22  7:09   ` Toon Claes
  0 siblings, 0 replies; 11+ messages in thread
From: Toon Claes @ 2026-04-22  7:09 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, D. Ben Knoble

Patrick Steinhardt <ps@pks.im> writes:

> Do we really have to make the logic conditional? I would expect that
> this works just fine for newer versions of Ninja and for Make, so it
> feels rather pointless to me to have two modes to worry about.
>
> So wouldn't the below simplified version be sufficient?

I considered that, but I wasn't sure. I went all-in to show what it
could be, but I'm happy to see simplifications.

Now your version doesn't remove the no-op targets from config-list.h.d.
I did some testing, and while Ninja doesn't need those no-op target,
they are also harmless to keep them. Thus I agree your version is
simpler and better. Let me roll another version.

Thanks!

> diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
> index e28054f9e0..f5f42492c6 100755
> --- a/tools/generate-configlist.sh
> +++ b/tools/generate-configlist.sh
> @@ -44,7 +44,9 @@ then
>  	{
>  		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' ' ' |
> +			sed -e "s/^/$QUOTED_OUTPUT: /" -e 's/ $/\n/'
>  		printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>  			"$SOURCE_DIR"/Documentation/config/*.adoc |
>  			sed -e 's/[# ]/\\&/g'

-- 
Cheers,
Toon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] generate-configlist: collapse depfile for older Ninja
  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:21 ` Toon Claes
  2026-04-22 10:30   ` Patrick Steinhardt
                     ` (2 more replies)
  2026-04-22 18:35 ` [PATCH] " D. Ben Knoble
  2 siblings, 3 replies; 11+ messages in thread
From: Toon Claes @ 2026-04-22  7:21 UTC (permalink / raw)
  To: git; +Cc: D. Ben Knoble, Patrick Steinhardt, Toon Claes

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 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
index e28054f9e0..f5f42492c6 100755
--- a/tools/generate-configlist.sh
+++ b/tools/generate-configlist.sh
@@ -44,7 +44,9 @@ then
 	{
 		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' ' ' |
+			sed -e "s/^/$QUOTED_OUTPUT: /" -e 's/ $/\n/'
 		printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
 			"$SOURCE_DIR"/Documentation/config/*.adoc |
 			sed -e 's/[# ]/\\&/g'

---
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
change-id: 20260421-toon-fix-almalinux8-102de9138294


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generate-configlist: collapse depfile for older Ninja
  2026-04-22  7:21 ` [PATCH v2] " Toon Claes
@ 2026-04-22 10:30   ` Patrick Steinhardt
  2026-04-22 13:45   ` Phillip Wood
  2026-05-15  8:42   ` [PATCH v3] " Toon Claes
  2 siblings, 0 replies; 11+ messages in thread
From: Patrick Steinhardt @ 2026-04-22 10:30 UTC (permalink / raw)
  To: Toon Claes; +Cc: git, D. Ben Knoble

On Wed, Apr 22, 2026 at 09:21:20AM +0200, Toon Claes wrote:
> 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

Thanks. I've tested those changes with Ninja 1.8.2 and can confirm that
it does fix the issue.

Patrick

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generate-configlist: collapse depfile for older Ninja
  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
  2 siblings, 2 replies; 11+ messages in thread
From: Phillip Wood @ 2026-04-22 13:45 UTC (permalink / raw)
  To: Toon Claes, git; +Cc: D. Ben Knoble, Patrick Steinhardt

Hi Toon

On 22/04/2026 08:21, Toon Claes wrote:

Thanks for the excellent commit message which I've trimmed.

> diff --git a/tools/generate-configlist.sh b/tools/generate-configlist.sh
> index e28054f9e0..f5f42492c6 100755
> --- a/tools/generate-configlist.sh
> +++ b/tools/generate-configlist.sh
> @@ -44,7 +44,9 @@ then
>   	{
>   		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' ' ' |
> +			sed -e "s/^/$QUOTED_OUTPUT: /" -e 's/ $/\n/'

I don't think this use of '\n' portable. The sed man page [1] says that 
'\n' matches a newline in the pattern space, but does not mention it 
being supported in the replacement string. We do have an existing use in 
t4150-am.sh:"am newline in subject" which does

	sed -e "s/second/second \\\n foo/" patch1 >patchnl &&

However if I add "cat patchnl" it shows the subject line is

	Subject: [PATCH] second \n foo

so sed has inserted "\n" rather than a newline. Indeed looking at the 
commit message for that test it is testing a fix that c escapes are 
printed verbatim introduced by 4b7cc26a74 (git-am: use printf instead of 
echo on user-supplied strings, 2007-05-25).

I've not tested it but I think

	sed 's/ $/\
/'

will insert a newline. Alternatively we could do

	printf '%s' "$QUOTED_OUTPUT: "
	printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
  			"$SOURCE_DIR"/Documentation/config/*.adoc |
		sed -e 's/[# ]/\\&/g' |
		tr '\n' ' '
	printf '\n'

That leaves a trailing space at the end of the line but I don't think 
that should matter.

As I recall, the depfiles created by gcc have all the dependencies on a 
single line so this should be widely supported and I agree with Patrick 
that we should do this unconditionally.

Thanks

Phillip

[1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

>   		printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>   			"$SOURCE_DIR"/Documentation/config/*.adoc |
>   			sed -e 's/[# ]/\\&/g'
> 
> ---
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> change-id: 20260421-toon-fix-almalinux8-102de9138294
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generate-configlist: collapse depfile for older Ninja
  2026-04-22 13:45   ` Phillip Wood
@ 2026-04-22 14:12     ` Phillip Wood
  2026-05-15  8:44     ` Toon Claes
  1 sibling, 0 replies; 11+ messages in thread
From: Phillip Wood @ 2026-04-22 14:12 UTC (permalink / raw)
  To: Toon Claes, git; +Cc: D. Ben Knoble, Patrick Steinhardt

On 22/04/2026 14:45, Phillip Wood wrote:
> 
>      sed -e "s/second/second \\\n foo/" patch1 >patchnl &&

Of course the extra backslashes would supress any special meaning of 
'\n' so that's not a good example. However the freebsd man page [1] says

    The	escape sequence	\n matches a newline character embedded	in the
    pattern space. You cannot, however, use a literal newline character
    in an address or in the substitute command.

Thanks

Phillip

[1] 
https://man.freebsd.org/cgi/man.cgi?query=sed&apropos=0&sektion=0&manpath=FreeBSD+16.0-CURRENT&format=html

> 
> However if I add "cat patchnl" it shows the subject line is
> 
>      Subject: [PATCH] second \n foo
> 
> so sed has inserted "\n" rather than a newline. Indeed looking at the 
> commit message for that test it is testing a fix that c escapes are 
> printed verbatim introduced by 4b7cc26a74 (git-am: use printf instead of 
> echo on user-supplied strings, 2007-05-25).
> 
> I've not tested it but I think
> 
>      sed 's/ $/\
> /'
> 
> will insert a newline. Alternatively we could do
> 
>      printf '%s' "$QUOTED_OUTPUT: "
>      printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>               "$SOURCE_DIR"/Documentation/config/*.adoc |
>          sed -e 's/[# ]/\\&/g' |
>          tr '\n' ' '
>      printf '\n'
> 
> That leaves a trailing space at the end of the line but I don't think 
> that should matter.
> 
> As I recall, the depfiles created by gcc have all the dependencies on a 
> single line so this should be widely supported and I agree with Patrick 
> that we should do this unconditionally.
> 
> Thanks
> 
> Phillip
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
> 
>>           printf '%s:\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>>               "$SOURCE_DIR"/Documentation/config/*.adoc |
>>               sed -e 's/[# ]/\\&/g'
>>
>> ---
>> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
>> change-id: 20260421-toon-fix-almalinux8-102de9138294
>>
>>
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] generate-configlist: collapse depfile for older Ninja
  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:21 ` [PATCH v2] " Toon Claes
@ 2026-04-22 18:35 ` D. Ben Knoble
  2 siblings, 0 replies; 11+ messages in thread
From: D. Ben Knoble @ 2026-04-22 18:35 UTC (permalink / raw)
  To: Toon Claes; +Cc: git, Patrick Steinhardt, Phillip Wood

On Tue, Apr 21, 2026 at 3:17 PM Toon Claes <toon@iotcl.com> 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).

Fascinating. Thanks for finding and fixing. I did wish while embarking
on this endeavor to find more documentation of what these depfiles
should look like, so I'm not surprised to find some bugs in how they
are parsed.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3] generate-configlist: collapse depfile for older Ninja
  2026-04-22  7:21 ` [PATCH v2] " Toon Claes
  2026-04-22 10:30   ` Patrick Steinhardt
  2026-04-22 13:45   ` Phillip Wood
@ 2026-05-15  8:42   ` Toon Claes
  2026-05-15  9:35     ` Phillip Wood
  2 siblings, 1 reply; 11+ messages in thread
From: Toon Claes @ 2026-05-15  8:42 UTC (permalink / raw)
  To: git; +Cc: D. Ben Knoble, Patrick Steinhardt, Toon Claes

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] generate-configlist: collapse depfile for older Ninja
  2026-04-22 13:45   ` Phillip Wood
  2026-04-22 14:12     ` Phillip Wood
@ 2026-05-15  8:44     ` Toon Claes
  1 sibling, 0 replies; 11+ messages in thread
From: Toon Claes @ 2026-05-15  8:44 UTC (permalink / raw)
  To: phillip.wood, git; +Cc: D. Ben Knoble, Patrick Steinhardt

Phillip Wood <phillip.wood123@gmail.com> writes:

> I don't think this use of '\n' portable. The sed man page [1] says that 
> '\n' matches a newline in the pattern space, but does not mention it 
> being supported in the replacement string. We do have an existing use in 
> t4150-am.sh:"am newline in subject" which does
>
> 	sed -e "s/second/second \\\n foo/" patch1 >patchnl &&
>
> However if I add "cat patchnl" it shows the subject line is
>
> 	Subject: [PATCH] second \n foo
>
> so sed has inserted "\n" rather than a newline. Indeed looking at the 
> commit message for that test it is testing a fix that c escapes are 
> printed verbatim introduced by 4b7cc26a74 (git-am: use printf instead of 
> echo on user-supplied strings, 2007-05-25).
>
> I've not tested it but I think
>
> 	sed 's/ $/\
> /'
>
> will insert a newline. Alternatively we could do
>
> 	printf '%s' "$QUOTED_OUTPUT: "
> 	printf '%s\n' "$SOURCE_DIR"/Documentation/*config.adoc \
>   			"$SOURCE_DIR"/Documentation/config/*.adoc |
> 		sed -e 's/[# ]/\\&/g' |
> 		tr '\n' ' '
> 	printf '\n'

Whoops, I somehow archived your reply without addressing it. Thanks for
finding this. I've sent out v3 with this suggestion.

--
Toon


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3] generate-configlist: collapse depfile for older Ninja
  2026-05-15  8:42   ` [PATCH v3] " Toon Claes
@ 2026-05-15  9:35     ` Phillip Wood
  0 siblings, 0 replies; 11+ messages in thread
From: Phillip Wood @ 2026-05-15  9:35 UTC (permalink / raw)
  To: Toon Claes, git; +Cc: D. Ben Knoble, Patrick Steinhardt

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
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-05-15  9:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-04-22 18:35 ` [PATCH] " D. Ben Knoble

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox