* [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing
@ 2019-03-03 11:49 Martin Kepplinger
2019-03-03 13:23 ` Yann E. MORIN
0 siblings, 1 reply; 4+ messages in thread
From: Martin Kepplinger @ 2019-03-03 11:49 UTC (permalink / raw)
To: buildroot
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.
When comparing the outputs, this removes "host-lzip" from all packages'
*EXTRACT_DEPENDENCIES.
Signed-off-by: Martin Kepplinger <martink@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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing 2019-03-03 11:49 [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing Martin Kepplinger @ 2019-03-03 13:23 ` Yann E. MORIN 2019-03-03 13:57 ` Martin Kepplinger 0 siblings, 1 reply; 4+ messages in thread From: Yann E. MORIN @ 2019-03-03 13:23 UTC (permalink / raw) To: buildroot 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@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. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing 2019-03-03 13:23 ` Yann E. MORIN @ 2019-03-03 13:57 ` Martin Kepplinger 2019-03-03 16:11 ` Yann E. MORIN 0 siblings, 1 reply; 4+ messages in thread From: Martin Kepplinger @ 2019-03-03 13:57 UTC (permalink / raw) To: buildroot Am 3. M?rz 2019 14:23:29 MEZ schrieb "Yann E. MORIN" <yann.morin.1998@free.fr>: >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. > I totally think so too. Could you add a note about this to the docs? thanks >Regards, >Yann E. MORIN. > >> When comparing the outputs, this removes "host-lzip" from all >packages' >> *EXTRACT_DEPENDENCIES. >> >> Signed-off-by: Martin Kepplinger <martink@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 >> -- Martin Kepplinger sent from mobile ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing 2019-03-03 13:57 ` Martin Kepplinger @ 2019-03-03 16:11 ` Yann E. MORIN 0 siblings, 0 replies; 4+ messages in thread From: Yann E. MORIN @ 2019-03-03 16:11 UTC (permalink / raw) To: buildroot Martin, All, On 2019-03-03 14:57 +0100, Martin Kepplinger spake thusly: > Am 3. M?rz 2019 14:23:29 MEZ schrieb "Yann E. MORIN" <yann.morin.1998@free.fr>: > >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; [--SNIP--] > >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. > > I totally think so too. Could you add a note about this to the docs? Since you raised the topic, you can as well do that change in the docs. The manual is in the Buildroot tree, in docs/manual/. You will want to have a look at docs/manual/make-tips.txt which documents and explains printvars, extend that with a note about the spurious errors, and send a patch. Thank you! :-) Regards, Yann E. MORIN. -- .-----------------.--------------------.------------------.--------------------. | 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. | '------------------------------^-------^------------------^--------------------' ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-03 16:11 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-03 11:49 [Buildroot] [PATCH v2] make printvars: avoid invalid calls when arguments are missing Martin Kepplinger 2019-03-03 13:23 ` Yann E. MORIN 2019-03-03 13:57 ` Martin Kepplinger 2019-03-03 16:11 ` Yann E. MORIN
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox