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;
From:   Alexander Lobakin <alexandr.lobakin@intel.com>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     Alexander Lobakin <alexandr.lobakin@intel.com>,
        linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
        Nicolas Schier <nicolas@fjasle.eu>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH v3 3/5] kbuild: add read-file macro
Date:   Wed,  7 Dec 2022 16:40:44 +0100
Message-Id: <20221207154044.2181347-1-alexandr.lobakin@intel.com>
In-Reply-To: <20221126225624.751661-3-masahiroy@kernel.org>
References: <20221126225624.751661-1-masahiroy@kernel.org> <20221126225624.751661-3-masahiroy@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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