[Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing

Yann E. MORIN yann.morin.1998 at free.fr
Sun Mar 3 13:23:29 UTC 2019


Martin, All,

On 2019-03-03 12:49 +0100, Martin Kepplinger spake thusly:
> during "make printvars > compare" the following error occurs -
> reproducible after any "make *_defconfig":
> 
> /bin/bash: support/dependencies/check-host-.sh: no such file or directory
> /bin/bash: -c: line 0: syntax error: ')' unexpected
> /bin/bash: -c: line 0: `set -e; TMP="$(mktemp)"; if () >/dev/null 2>&1; then echo ""; else echo ""; fi;
> 
> which is 2 errors, resulting from $(1) arguments being empty, but
> called anyway. So this simply skips parts when otherwise we would exit
> when wrong scripts are tried be executed.

Actually, there are many more such spurious errors, depending on the
configuration you use. For example, my current .config, which is not even
yet built (i.e. I ran 'make foo_defconfig; make printvars >/dev/null'),
yields:

    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    /bin/sh: /home/ymorin/dev/buildroot/O/host/bin/pkg-config: No such file or directory
    sed: can't read ./.config: No such file or directory
    sed: can't read ./.config: No such file or directory
    sed: can't read ./.config: No such file or directory
    sed: can't read ./.config: No such file or directory
    sed: can't read ./.config: No such file or directory
    make[1]: support/dependencies/check-host-.sh: Command not found
    /bin/sh: -c: line 0: syntax error near unexpected token `)'
    /bin/sh: -c: line 0: `set -e; TMP="$(mktemp)"; if () >/dev/null 2>&1; then echo ""; else echo ""; fi; rm -f "$TMP"'

And I'm pretty sure I'm not even getting everyone of them, as this
.config is neither using an external toolchain, nor building a kernel or
any other kconfig package, where we also call a lot of different macros
as well...

So, before trying to hide those errors under the rug, I'd prefer we
udnerstand actually *why* they are reported in the first place: When
doing a normal build, there is no such error being displayed, so why are
they displayed when doing printvars?

The reason is that we use a lot of internal macros. In Makefiles, a
macro is just a variable, except it is not meant to be evaluate with
$(...) like other variables, but it is expected to be called with
$(call ...) instead, which will set the special variables $(1), $(2)
etc...

And then, macros may call to the shell, either explicitly by way of
$(shell ...) or implicitly because it contains shell evaluation `...`.

However, when we call printvars, all variables get evaluated with
$(...), because we have *no* way of knowing whether a variable is indeed
a variable or a macro.

So, obviously, we end up evaluating variables which are instead supposed
to be called, and we end up with those spurious error messages.

And when that macro would call to the shell, we get those spurious
errors.

In my opinion, whe should not try to hide those messages, because:

  * it makes the code heavier and harder to read and maintain. For
    example, the next developer who looks at try-run would be very
    tempted to wonder why-on-earth that macro tests whether it has
    an argument (i.e. it is called or evaluated), as this is not
    customary to do so, usually, and thus they would be tempted, and
    rightfully so, to send a patch to remove the test.

  * it is easy to add even more such macros, either in the infra or in
    packages, and especially in packages in br2-external, and there will
    always be such spurious error messages, because see above, it is not
    customary to test the macro parameters.

So, I'd be tempted to just live with that situation, and add a small
explanation why there might be errors, and list the usual ones.

Regards,
Yann E. MORIN.

> When comparing the outputs, this removes "host-lzip" from all packages'
> *EXTRACT_DEPENDENCIES.
> 
> Signed-off-by: Martin Kepplinger <martink at posteo.de>
> ---
> 
> hi,                                                                             
>                                                                                 
> ok so this neither breaks my build and "make printvars" succeeds too. When      
> comparing "make printvars" output now, "host-lzip" is _removed_ from all        
> packages' dependiencies (in contrast to the previous patch). 
> 
> thanks
> 
>                             martin
> 
> 
> 
>  package/Makefile.in                  | 8 ++++----
>  support/dependencies/dependencies.mk | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index dc818a2c18..ae1e2e16f6 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -232,18 +232,18 @@ HOST_LDFLAGS  += -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib
>  # Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise)
>  # Exit code chooses option. "$$TMP" is can be used as temporary file and
>  # is automatically cleaned up.
> -try-run = $(shell set -e;               \
> +try-run = $(if $(1), $(shell set -e;	\
>  	TMP="$$(mktemp)";               \
>  	if ($(1)) >/dev/null 2>&1;      \
>  	then echo "$(2)";               \
>  	else echo "$(3)";               \
>  	fi;                             \
> -	rm -f "$$TMP")
> +	rm -f "$$TMP"))
>  
>  # host-cc-option
>  # Usage: HOST_FOO_CFLAGS += $(call host-cc-option,-no-pie,)
> -host-cc-option = $(call try-run,\
> -        $(HOSTCC) $(HOST_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2))
> +host-cc-option = $(if $(1), $(call try-run,\
> +        $(HOSTCC) $(HOST_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2)))
>  
>  
>  # host-intltool should be executed with the system perl, so we save
> diff --git a/support/dependencies/dependencies.mk b/support/dependencies/dependencies.mk
> index 4fac5c731b..c53bbe01ce 100644
> --- a/support/dependencies/dependencies.mk
> +++ b/support/dependencies/dependencies.mk
> @@ -15,7 +15,7 @@ else
>  # script should use 'which' to find a candidate. The script should return
>  # the path to the suitable host tool, or nothing if no suitable tool was found.
>  define suitable-host-package
> -$(shell support/dependencies/check-host-$(1).sh $(2))
> +$(if $(1), $(shell support/dependencies/check-host-$(1).sh $(2)))
>  endef
>  endif
>  # host utilities needs host-tar to extract the source code tarballs, so
> -- 
> 2.20.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'



More information about the buildroot mailing list