diff mbox series

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

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

Commit Message

Masahiro Yamada Nov. 23, 2022, 3:18 p.m. UTC 
Since GMU 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.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

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

Comments

Nicolas Schier Nov. 23, 2022, 9:01 p.m. UTC | #1 
On Thu 24 Nov 2022 00:18:26 GMT, Masahiro Yamada wrote:
> Since GMU 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.
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
> (no changes since v1)
> 
>  Makefile                  |  2 +-
>  scripts/Kbuild.include    | 12 ++++++++++++
>  scripts/Makefile.modfinal |  2 +-
>  scripts/Makefile.modinst  |  2 +-
>  4 files changed, 15 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 9996f34327cb..722846c23264 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,14 @@ 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
> +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
> +

I like the implementation of read-file, but I am afraid that the 
MAKE_VERSION comparison breaks all make versions w/ a minor version 
number: 3.99.9x, 4.2.x, 4.3.x.

Not beautiful, but this might possibly work:
  ifneq ($(filter 3.% 4.0 4.0.% 4.1 4.1.%,$(MAKE_VERSION)),)
  read-file = ...


Kind regards,
Nicolas


>  ###
>  # 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
Masahiro Yamada Nov. 23, 2022, 9:52 p.m. UTC | #2 
On Thu, Nov 24, 2022 at 6:02 AM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Thu 24 Nov 2022 00:18:26 GMT, Masahiro Yamada wrote:
> > Since GMU 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.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > (no changes since v1)
> >
> >  Makefile                  |  2 +-
> >  scripts/Kbuild.include    | 12 ++++++++++++
> >  scripts/Makefile.modfinal |  2 +-
> >  scripts/Makefile.modinst  |  2 +-
> >  4 files changed, 15 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 9996f34327cb..722846c23264 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,14 @@ 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
> > +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
> > +
>
> I like the implementation of read-file, but I am afraid that the
> MAKE_VERSION comparison breaks all make versions w/ a minor version
> number: 3.99.9x, 4.2.x, 4.3.x.


I think these should work correctly
unless I missed something terribly.


In the ASCII-sorting, they are sorted like this:

3.99.9x  4.2   4.2.x  4.3.x






The bad scenarios I came up with is GNU Make 4.10
and GNU Make 10.0 because $(sort ) will sort

10  4.10  4.2




GNU Make 4.3 was released in Jan 2020
GNU Make 4.4 was released in Oct 2022



If the current release pace continues,
we will have about 10 years until GNU Make hits 4.10

Until then, we can remove this ifeq.
Nicolas Schier Nov. 24, 2022, 4:45 a.m. UTC | #3 
On Thu 24 Nov 2022 06:52:06 GMT, Masahiro Yamada wrote:
> On Thu, Nov 24, 2022 at 6:02 AM Nicolas Schier <nicolas@fjasle.eu> wrote:
> >
> > On Thu 24 Nov 2022 00:18:26 GMT, Masahiro Yamada wrote:
> > > Since GMU 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.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >  Makefile                  |  2 +-
> > >  scripts/Kbuild.include    | 12 ++++++++++++
> > >  scripts/Makefile.modfinal |  2 +-
> > >  scripts/Makefile.modinst  |  2 +-
> > >  4 files changed, 15 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 9996f34327cb..722846c23264 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,14 @@ 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
> > > +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
> > > +
> >
> > I like the implementation of read-file, but I am afraid that the
> > MAKE_VERSION comparison breaks all make versions w/ a minor version
> > number: 3.99.9x, 4.2.x, 4.3.x.
> 
> 
> I think these should work correctly
> unless I missed something terribly.
> 
> 
> In the ASCII-sorting, they are sorted like this:
> 
> 3.99.9x  4.2   4.2.x  4.3.x
> 
> 
> 
> 
> 
> 
> The bad scenarios I came up with is GNU Make 4.10
> and GNU Make 10.0 because $(sort ) will sort
> 
> 10  4.10  4.2
> 
> 
> 
> 
> GNU Make 4.3 was released in Jan 2020
> GNU Make 4.4 was released in Oct 2022
> 
> 
> 
> If the current release pace continues,
> we will have about 10 years until GNU Make hits 4.10
> 
> Until then, we can remove this ifeq.
> 

yes, you're right.  I should have checked my assumption before 
bothering you with it.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

> 
> 
> -- 
> Best Regards
> Masahiro Yamada
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 9996f34327cb..722846c23264 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,14 @@  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
+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