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

[Nicolas Schier]

On Thu 24 Nov 2022 06:52:06 GMT, Masahiro Yamada wrote:
> On Thu, Nov 24, 2022 at 6:02 AM Nicolas Schier <nicolas@xxxxxxxxx> 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@xxxxxxxxxx>
> > > ---
> > >
> > > (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@xxxxxxxxx>

> 
> 
> -- 
> Best Regards
> Masahiro Yamada

-- 
epost|xmpp: nicolas@xxxxxxxxx          irc://oftc.net/nsc
↳ gpg: 18ed 52db e34f 860e e9fb  c82b 7d97 0932 55a0 ce7f
     -- frykten for herren er opphav til kunnskap --

Attachment: signature.asc
Description: PGP signature