Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221123151828.509565-1-masahiroy@kernel.org> <20221123151828.509565-3-masahiroy@kernel.org>
 <Y36KLFArg5etHRD8@bergen.fjasle.eu>
In-Reply-To: <Y36KLFArg5etHRD8@bergen.fjasle.eu>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Thu, 24 Nov 2022 06:52:06 +0900
Message-ID: <CAK7LNASWd-8awDOwE3iU9nx8QPj3VnoFtnUiX3NLyT+fx3BfjQ@mail.gmail.com>
Subject: Re: [PATCH v2 3/5] kbuild: add read-file macro
To:     Nicolas Schier <nicolas@fjasle.eu>
Cc:     linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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.


-- 
Best Regards
Masahiro Yamada