Re: [PATCH v2] kbuild: use printf instead of echo -e extension

[Sascha Hauer <sha@pengutronix.de>]

Hi Ahmad,

The subject seems outdated, you are not using printf at all here.

On Thu, Apr 13, 2023 at 10:45:47AM +0200, Ahmad Fatoum wrote:
> We are using echo -e, so the \n in the string being echo'd are
> interpreted. As -e is not POSIX and dash doesn't provide it,
> we use a strange /usr/bin/env echo -e construct hoping that
> whatever non-builtin echo is first in the search path supports -e.
> 
> As the new lines are just used to separate CPP directives, we can
> just pass the directives as $(CPP) flags. This has the same result,
> but is potable and avoids NixOS complaining when building barebox.

s/potable/portable/

> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - drop echo -e eltogether after Sascha pointed out why it needs
>     /usr/bin/env.
> 
> Tested with two fragments:
>   - dts variable is defined for the DT it references
>   - fragments are included (checked with #warning)
>   - fragment change triggers rebuild
>   - original DT change triggers rebuild
>   - fragmentless case works ok
>     
>  scripts/Makefile.lib | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 4deaa5dfa73c..befd47ed6600 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -372,8 +372,8 @@ $(obj)/%.dtb.z: $(obj)/%.dtb FORCE
>  dts-frags = $(subst $(quote),,$(CONFIG_EXTERNAL_DTS_FRAGMENTS))
>  quiet_cmd_dtc = DTC     $@
>  # For compatibility between make 4.2 and 4.3
> -cmd_dtc = /usr/bin/env echo -e '$(pound)define $(subst -,_,$(*F))_dts 1\n'$(foreach f,$< $(2),'$(pound)include "$(f)"\n') | \
> -	$(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) - ; \
> +cmd_dtc = echo | $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) \
> +	-D'$(subst -,_,$(*F))_dts=1' $(foreach f,$< $(2),-include '$(f)') - ; \

Good idea to get rid of the echo -e dependency entirely, I like it :)

You could replace the '-' for stdin with /dev/null and then drop the
echo |. Maybe this makes it a bit easier to read.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |