2022-11-26 23:18:09

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 3/5] kbuild: add read-file macro

Since GNU Make 4.2, $(file ...) supports the read operater '<', which
is useful to read a file without forking any process. No warning is
shown even if the input file is missing.

For older Make versions, it falls back to the cat command.

The added ifeq will break when GNU Make 4.10 or 10.0 is released.
It will take a long time if the current release pace continues.

Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Nicolas Schier <[email protected]>
---

(no changes since v1)

Makefile | 2 +-
scripts/Kbuild.include | 15 +++++++++++++++
scripts/Makefile.modfinal | 2 +-
scripts/Makefile.modinst | 2 +-
4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index eb80332f7b51..60ce9dcafc72 100644
--- a/Makefile
+++ b/Makefile
@@ -369,7 +369,7 @@ else # !mixed-build
include $(srctree)/scripts/Kbuild.include

# Read KERNELRELEASE from include/config/kernel.release (if it exists)
-KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
+KERNELRELEASE = $(call read-file, include/config/kernel.release)
KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 4b8cf464b53b..55c2243f91c8 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -10,6 +10,10 @@ empty :=
space := $(empty) $(empty)
space_escape := _-_SPACE_-_
pound := \#
+define newline
+
+
+endef

###
# Comparison macros.
@@ -55,6 +59,17 @@ stringify = $(squote)$(quote)$1$(quote)$(squote)
kbuild-dir = $(if $(filter /%,$(src)),$(src),$(srctree)/$(src))
kbuild-file = $(or $(wildcard $(kbuild-dir)/Kbuild),$(kbuild-dir)/Makefile)

+###
+# Read a file, replacing newlines with spaces
+#
+# This ifeq will break when GNU Make 4.10 is released.
+# Remove this conditional until then.
+ifeq ($(call test-ge, $(MAKE_VERSION), 4.2),y)
+read-file = $(subst $(newline),$(space),$(file < $1))
+else
+read-file = $(shell cat $1 2>/dev/null)
+endif
+
###
# Easy method for doing a status message
kecho := :
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 25bedd83644b..7252f6cf7837 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -13,7 +13,7 @@ include $(srctree)/scripts/Kbuild.include
include $(srctree)/scripts/Makefile.lib

# find all modules listed in modules.order
-modules := $(sort $(shell cat $(MODORDER)))
+modules := $(sort $(call read-file, $(MODORDER)))

__modfinal: $(modules)
@:
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index a4c987c23750..509d424dbbd2 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -9,7 +9,7 @@ __modinst:
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

-modules := $(sort $(shell cat $(MODORDER)))
+modules := $(sort $(call read-file, $(MODORDER)))

ifeq ($(KBUILD_EXTMOD),)
dst := $(MODLIB)/kernel
--
2.34.1


2022-12-07 16:05:44

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro

From: Masahiro Yamada <[email protected]>
Date: Sun, 27 Nov 2022 07:56:22 +0900

> Since GNU Make 4.2, $(file ...) supports the read operater '<', which
> is useful to read a file without forking any process. No warning is
> shown even if the input file is missing.
>
> For older Make versions, it falls back to the cat command.
>
> The added ifeq will break when GNU Make 4.10 or 10.0 is released.
> It will take a long time if the current release pace continues.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Nicolas Schier <[email protected]>
> ---
>
> (no changes since v1)
>
> Makefile | 2 +-
> scripts/Kbuild.include | 15 +++++++++++++++
> scripts/Makefile.modfinal | 2 +-
> scripts/Makefile.modinst | 2 +-
> 4 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index eb80332f7b51..60ce9dcafc72 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -369,7 +369,7 @@ else # !mixed-build
> include $(srctree)/scripts/Kbuild.include
>
> # Read KERNELRELEASE from include/config/kernel.release (if it exists)
> -KERNELRELEASE = $(shell cat include/config/kernel.release 2> /dev/null)
> +KERNELRELEASE = $(call read-file, include/config/kernel.release)
> KERNELVERSION = $(VERSION)$(if $(PATCHLEVEL),.$(PATCHLEVEL)$(if $(SUBLEVEL),.$(SUBLEVEL)))$(EXTRAVERSION)
> export VERSION PATCHLEVEL SUBLEVEL KERNELRELEASE KERNELVERSION
>
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 4b8cf464b53b..55c2243f91c8 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -10,6 +10,10 @@ empty :=
> space := $(empty) $(empty)
> space_escape := _-_SPACE_-_
> pound := \#
> +define newline
> +
> +
> +endef
>
> ###
> # Comparison macros.
> @@ -55,6 +59,17 @@ stringify = $(squote)$(quote)$1$(quote)$(squote)
> kbuild-dir = $(if $(filter /%,$(src)),$(src),$(srctree)/$(src))
> kbuild-file = $(or $(wildcard $(kbuild-dir)/Kbuild),$(kbuild-dir)/Makefile)
>
> +###
> +# Read a file, replacing newlines with spaces
> +#
> +# This ifeq will break when GNU Make 4.10 is released.
> +# Remove this conditional until then.
> +ifeq ($(call test-ge, $(MAKE_VERSION), 4.2),y)
> +read-file = $(subst $(newline),$(space),$(file < $1))
> +else
> +read-file = $(shell cat $1 2>/dev/null)
> +endif
> +

Great stuff. Used it in my upcoming series to simplify things, works
as expected.

sed-syms = $(subst $(space),\|,$(foreach file,$(sym-files-y),$(call read-file,$(file))))

The only thing that came to my mind while I was implementing the
oneliner above: maybe add ability to read multiple files? For now,
I used a foreach, could it be somehow incorporated into read-file
already?

Besides that:

Reviewed-and-tested-by: Alexander Lobakin <[email protected]>

> ###
> # Easy method for doing a status message
> kecho := :
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 25bedd83644b..7252f6cf7837 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -13,7 +13,7 @@ include $(srctree)/scripts/Kbuild.include
> include $(srctree)/scripts/Makefile.lib
>
> # find all modules listed in modules.order
> -modules := $(sort $(shell cat $(MODORDER)))
> +modules := $(sort $(call read-file, $(MODORDER)))
>
> __modfinal: $(modules)
> @:
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index a4c987c23750..509d424dbbd2 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -9,7 +9,7 @@ __modinst:
> include include/config/auto.conf
> include $(srctree)/scripts/Kbuild.include
>
> -modules := $(sort $(shell cat $(MODORDER)))
> +modules := $(sort $(call read-file, $(MODORDER)))
>
> ifeq ($(KBUILD_EXTMOD),)
> dst := $(MODLIB)/kernel
> --
> 2.34.1

Thanks,
Olek

2022-12-07 16:30:43

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro

From: Alexander Lobakin <[email protected]>
Date: Wed, 7 Dec 2022 16:40:44 +0100

> From: Masahiro Yamada <[email protected]>
> Date: Sun, 27 Nov 2022 07:56:22 +0900
>
> > Since GNU Make 4.2, $(file ...) supports the read operater '<', which
> > is useful to read a file without forking any process. No warning is
> > shown even if the input file is missing.

[...]

> Great stuff. Used it in my upcoming series to simplify things, works
> as expected.
>
> sed-syms = $(subst $(space),\|,$(foreach file,$(sym-files-y),$(call read-file,$(file))))
>
> The only thing that came to my mind while I was implementing the
> oneliner above: maybe add ability to read multiple files? For now,
> I used a foreach, could it be somehow incorporated into read-file
> already?

Oh, nevermind. This one also works:

sed-syms = $(subst $(space),\|,$(call read-file,$(sym-files-y)))

So I believe read-file works for an arbitrary number of files.

>
> Besides that:
>
> Reviewed-and-tested-by: Alexander Lobakin <[email protected]>

[...]

> > --
> > 2.34.1
>
> Thanks,
> Olek

Thanks!
Olek

2022-12-10 15:27:16

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro

On Thu, Dec 8, 2022 at 1:25 AM Alexander Lobakin
<[email protected]> wrote:
>
> From: Alexander Lobakin <[email protected]>
> Date: Wed, 7 Dec 2022 16:40:44 +0100
>
> > From: Masahiro Yamada <[email protected]>
> > Date: Sun, 27 Nov 2022 07:56:22 +0900
> >
> > > Since GNU Make 4.2, $(file ...) supports the read operater '<', which
> > > is useful to read a file without forking any process. No warning is
> > > shown even if the input file is missing.
>
> [...]
>
> > Great stuff. Used it in my upcoming series to simplify things, works
> > as expected.
> >
> > sed-syms = $(subst $(space),\|,$(foreach file,$(sym-files-y),$(call read-file,$(file))))
> >
> > The only thing that came to my mind while I was implementing the
> > oneliner above: maybe add ability to read multiple files? For now,
> > I used a foreach, could it be somehow incorporated into read-file
> > already?
>
> Oh, nevermind. This one also works:
>
> sed-syms = $(subst $(space),\|,$(call read-file,$(sym-files-y)))
>
> So I believe read-file works for an arbitrary number of files.



Really?


In my understanding, $(call read-file, foo bar) reads a single file "foo bar".
(a space in the file name).






>
> >
> > Besides that:
> >
> > Reviewed-and-tested-by: Alexander Lobakin <[email protected]>
>
> [...]
>
> > > --
> > > 2.34.1
> >
> > Thanks,
> > Olek
>
> Thanks!
> Olek



--
Best Regards
Masahiro Yamada

2022-12-10 21:58:34

by Nicolas Schier

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro

On Sat, Dec 10, 2022 at 11:10:12PM +0900 Masahiro Yamada wrote:
> On Thu, Dec 8, 2022 at 1:25 AM Alexander Lobakin
> <[email protected]> wrote:
> >
> > From: Alexander Lobakin <[email protected]>
> > Date: Wed, 7 Dec 2022 16:40:44 +0100
> >
> > > From: Masahiro Yamada <[email protected]>
> > > Date: Sun, 27 Nov 2022 07:56:22 +0900
> > >
> > > > Since GNU Make 4.2, $(file ...) supports the read operater '<', which
> > > > is useful to read a file without forking any process. No warning is
> > > > shown even if the input file is missing.
> >
> > [...]
> >
> > > Great stuff. Used it in my upcoming series to simplify things, works
> > > as expected.
> > >
> > > sed-syms = $(subst $(space),\|,$(foreach file,$(sym-files-y),$(call read-file,$(file))))
> > >
> > > The only thing that came to my mind while I was implementing the
> > > oneliner above: maybe add ability to read multiple files? For now,
> > > I used a foreach, could it be somehow incorporated into read-file
> > > already?
> >
> > Oh, nevermind. This one also works:
> >
> > sed-syms = $(subst $(space),\|,$(call read-file,$(sym-files-y)))
> >
> > So I believe read-file works for an arbitrary number of files.
>
>
>
> Really?
>
>
> In my understanding, $(call read-file, foo bar) reads a single file "foo bar".
> (a space in the file name).

yes, except for make < 4.2, due to:

read-file = $(shell cat $1 2>/dev/null)

2022-12-12 15:24:53

by Alexander Lobakin

[permalink] [raw]
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro

From: Nicolas Schier <[email protected]>
Date: Sat, 10 Dec 2022 22:02:48 +0100

> On Sat, Dec 10, 2022 at 11:10:12PM +0900 Masahiro Yamada wrote:
> > On Thu, Dec 8, 2022 at 1:25 AM Alexander Lobakin
> > <[email protected]> wrote:
> > >
> > > From: Alexander Lobakin <[email protected]>
> > > Date: Wed, 7 Dec 2022 16:40:44 +0100
> > >
> > > > From: Masahiro Yamada <[email protected]>
> > > > Date: Sun, 27 Nov 2022 07:56:22 +0900
> > > >
> > > > > Since GNU Make 4.2, $(file ...) supports the read operater '<', which
> > > > > is useful to read a file without forking any process. No warning is
> > > > > shown even if the input file is missing.
> > >
> > > [...]
> > >
> > > > Great stuff. Used it in my upcoming series to simplify things, works
> > > > as expected.
> > > >
> > > > sed-syms = $(subst $(space),\|,$(foreach file,$(sym-files-y),$(call read-file,$(file))))
> > > >
> > > > The only thing that came to my mind while I was implementing the
> > > > oneliner above: maybe add ability to read multiple files? For now,
> > > > I used a foreach, could it be somehow incorporated into read-file
> > > > already?
> > >
> > > Oh, nevermind. This one also works:
> > >
> > > sed-syms = $(subst $(space),\|,$(call read-file,$(sym-files-y)))
> > >
> > > So I believe read-file works for an arbitrary number of files.
> >
> >
> >
> > Really?
> >
> >
> > In my understanding, $(call read-file, foo bar) reads a single file "foo bar".
> > (a space in the file name).
>
> yes, except for make < 4.2, due to:
>
> read-file = $(shell cat $1 2>/dev/null)

$ make --version
GNU Make 4.3
Built for x86_64-redhat-linux-gnu

But breh, I forgot to port test-ge, so Kbuild was always calling
cat :D :clownface:
Correct, current implementation (as of v3) reads only single file.
Not sure whether read-file should handle multiple at a time.

Thanks,
Olek