diff mbox series

[v3,3/5] kbuild: add read-file macro

Message ID 20221126225624.751661-3-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v3,1/5] kbuild: add test-{le,ge,lt,gt} macros | expand

Commit Message

Masahiro Yamada Nov. 26, 2022, 10:56 p.m. UTC 
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 <masahiroy@kernel.org>
Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
---

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

Comments

Alexander Lobakin Dec. 7, 2022, 3:40 p.m. UTC | #1 
From: Masahiro Yamada <masahiroy@kernel.org>
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 <masahiroy@kernel.org>
> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
> ---
> 
> (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 <alexandr.lobakin@intel.com>

>  ###
>  # 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
Alexander Lobakin Dec. 7, 2022, 4:22 p.m. UTC | #2 
From: Alexander Lobakin <alexandr.lobakin@intel.com>
Date: Wed, 7 Dec 2022 16:40:44 +0100

> From: Masahiro Yamada <masahiroy@kernel.org>
> 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 <alexandr.lobakin@intel.com>

[...]

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

Thanks!
Olek
Masahiro Yamada Dec. 10, 2022, 2:10 p.m. UTC | #3 
On Thu, Dec 8, 2022 at 1:25 AM Alexander Lobakin
<alexandr.lobakin@intel.com> wrote:
>
> From: Alexander Lobakin <alexandr.lobakin@intel.com>
> Date: Wed, 7 Dec 2022 16:40:44 +0100
>
> > From: Masahiro Yamada <masahiroy@kernel.org>
> > 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 <alexandr.lobakin@intel.com>
>
> [...]
>
> > > --
> > > 2.34.1
> >
> > Thanks,
> > Olek
>
> Thanks!
> Olek
Nicolas Schier Dec. 10, 2022, 9:02 p.m. UTC | #4 
On Sat, Dec 10, 2022 at 11:10:12PM +0900 Masahiro Yamada wrote:
> On Thu, Dec 8, 2022 at 1:25 AM Alexander Lobakin
> <alexandr.lobakin@intel.com> wrote:
> >
> > From: Alexander Lobakin <alexandr.lobakin@intel.com>
> > Date: Wed, 7 Dec 2022 16:40:44 +0100
> >
> > > From: Masahiro Yamada <masahiroy@kernel.org>
> > > 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)
Alexander Lobakin Dec. 12, 2022, 2:37 p.m. UTC | #5 
From: Nicolas Schier <nicolas@fjasle.eu>
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
> > <alexandr.lobakin@intel.com> wrote:
> > >
> > > From: Alexander Lobakin <alexandr.lobakin@intel.com>
> > > Date: Wed, 7 Dec 2022 16:40:44 +0100
> > >
> > > > From: Masahiro Yamada <masahiroy@kernel.org>
> > > > 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
diff mbox series

Patch

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