[PATCH] kbuild: use which $(DTC) as a dependency
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb files
and we extend it so it calls which automatically (similar to scripts/dtc-version.sh)
Signed-off-by: Richard Marko srk@48.io --- scripts/Makefile.lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 8dc6ec82cd..04fc9b0752 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -351,7 +351,7 @@ endif
dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list)))
-$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE +$(obj)/%.dtb: $(src)/%.dts $(shell which $(DTC)) $(dtsi_include_list_deps) FORCE $(call if_changed_dep,dtc)
pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp)
On 17/10/2023 12.44, Richard Marko wrote:
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb files
and we extend it so it calls which automatically (similar to scripts/dtc-version.sh)
Let's please not deviate from linux Kbuild unless we absolutely have to. In this case, passing an absolute path works just fine, and is AFAICT the only documented way to pass DTC. E.g. doc/build/gcc.rst has
DTC=/usr/bin/dtc make
as example.
[And if we do this, then at the very least this should also be done for the .dtbo rule and wherever else $(DTC) is used. But I'm really not a fan of calling out to $(shell which ...) in the rule itself. Perhaps if it was done just once, near DTC/DTC_IN_TREE, something like (not real make syntax I think)
ifneq ($(DTC), $(DTC_IN_TREE)) DTC := $(shell which $(DTC)) endif
But I don't think we should be doing this at all.]
Rasmus
On 10/17/23 13:27, Rasmus Villemoes wrote:
On 17/10/2023 12.44, Richard Marko wrote:
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb files
and we extend it so it calls which automatically (similar to scripts/dtc-version.sh)
Let's please not deviate from linux Kbuild unless we absolutely have to. In this case, passing an absolute path works just fine, and is AFAICT the only documented way to pass DTC. E.g. doc/build/gcc.rst has
DTC=/usr/bin/dtc makeas example.
[And if we do this, then at the very least this should also be done for the .dtbo rule and wherever else $(DTC) is used. But I'm really not a fan of calling out to $(shell which ...) in the rule itself. Perhaps if it was done just once, near DTC/DTC_IN_TREE, something like (not real make syntax I think)
ifneq ($(DTC), $(DTC_IN_TREE)) DTC := $(shell which $(DTC)) endif
But I don't think we should be doing this at all.]
Rasmus
I'm fine with passing a full path and not adding this.
The DTC=dtc thingie was done in NixOS and it stopped working for recent versions. It took me a while to figure out what is going on because the failure message seems very unrelated and no dtc is called except for dtc-version.sh check which passes just fine. Alternatively we could fail earlier if $(DTC) is not an absolute path and get rid of `which` call in dtc-version.sh but I'm not sure how to formulate that into make either.
Richard
On 17. 10. 2023. 12:44, Richard Marko wrote:
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
Can you please use command -v dtc instead as which is not part of POSIX?
Regards, Robert
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb files
and we extend it so it calls which automatically (similar to scripts/dtc-version.sh)
Signed-off-by: Richard Marko srk@48.io
scripts/Makefile.lib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 8dc6ec82cd..04fc9b0752 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -351,7 +351,7 @@ endif
dtsi_include_list_deps = $(addprefix $(obj)/,$(subst $(quote),,$(dtsi_include_list)))
-$(obj)/%.dtb: $(src)/%.dts $(DTC) $(dtsi_include_list_deps) FORCE +$(obj)/%.dtb: $(src)/%.dts $(shell which $(DTC)) $(dtsi_include_list_deps) FORCE $(call if_changed_dep,dtc)
pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp)
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb file
This patch checks that DTC is an absolute path and fails early if not.
We also replace `which` in `scripts/dtc-version.sh` with POSIXy `command -v`.
Signed-off-by: Richard Marko srk@48.io --- Makefile | 21 +++++++++++++-------- scripts/dtc-version.sh | 2 +- 2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile index b204a50043..9649f88722 100644 --- a/Makefile +++ b/Makefile @@ -2014,18 +2014,23 @@ scripts_dtc: scripts_basic $(Q)if test "$(DTC)" = "$(DTC_INTREE)"; then \ $(MAKE) $(build)=scripts/dtc; \ else \ - if ! $(DTC) -v >/dev/null; then \ - echo '*** Failed to check dtc version: $(DTC)'; \ + if [ ! "$(DTC)" = "$(wildcard $(DTC))" ]; then \ + echo "*** DTC is not an absolute path"; \ false; \ else \ - if test "$(call dtc-version)" -lt $(DTC_MIN_VERSION); then \ - echo '*** Your dtc is too old, please upgrade to dtc $(DTC_MIN_VERSION) or newer'; \ + if ! $(DTC) -v >/dev/null; then \ + echo '*** Failed to check dtc version: $(DTC)'; \ false; \ else \ - if [ -n "$(CONFIG_PYLIBFDT)" ]; then \ - if ! echo "import libfdt" | $(PYTHON3) 2>/dev/null; then \ - echo '*** pylibfdt does not seem to be available with $(PYTHON3)'; \ - false; \ + if test "$(call dtc-version)" -lt $(DTC_MIN_VERSION); then \ + echo '*** Your dtc is too old, please upgrade to dtc $(DTC_MIN_VERSION) or newer'; \ + false; \ + else \ + if [ -n "$(CONFIG_PYLIBFDT)" ]; then \ + if ! echo "import libfdt" | $(PYTHON3) 2>/dev/null; then \ + echo '*** pylibfdt does not seem to be available with $(PYTHON3)'; \ + false; \ + fi; \ fi; \ fi; \ fi; \ diff --git a/scripts/dtc-version.sh b/scripts/dtc-version.sh index 53ff868bcd..1e2f0c8a8b 100755 --- a/scripts/dtc-version.sh +++ b/scripts/dtc-version.sh @@ -15,7 +15,7 @@ if [ ${#dtc} -eq 0 ]; then exit 1 fi
-if ! which $dtc >/dev/null ; then +if ! command -v $dtc >/dev/null ; then echo "Error: Cannot find dtc: $dtc" exit 1 fi
On 19/10/2023 15.04, Richard Marko wrote:
If we try to build using external dtc using
make DTC=dtc
we get a confusing error like
make[2]: *** No rule to make target 'arch/x86/dts/bayleybay.dtb', needed by 'dtbs'. Stop.
Workaround is to use
make DTC=$( which dtc )
which gives make a full path, so the dependency is satisfied.
This was introduced by commit d50af66 kbuild: add dtc as dependency on .dtb file
This patch checks that DTC is an absolute path and fails early if not.
We also replace `which` in `scripts/dtc-version.sh` with POSIXy `command -v`.
I think this patch is very hard to read because of the initial check that moves all the other checks a level down and increases the indent.
I was more thinking you could let make do the work instead of doing it in embedded shell script. Something like
diff --git a/Makefile b/Makefile index aeaa6987360..f313e9dba66 100644 --- a/Makefile +++ b/Makefile @@ -419,6 +419,11 @@ PYTHON3 ?= python3 DTC_INTREE := $(objtree)/scripts/dtc/dtc DTC ?= $(DTC_INTREE) DTC_MIN_VERSION := 010406 +ifneq ($(DTC),$(DTC_INTREE)) +ifneq ($(patsubst /%,,$(DTC)),) +$(error "$$(DTC) must be an absolute path") +endif +endif
CHECK = sparse
seems to work (though I have only tested it very lightly). One could also add
ifeq ($(wildcard $(DTC)),) $(error "$$(DTC) = $(DTC) does not exist") endif
inside the outer ifneq to make the failure very explicit.
Rasmus
participants (4)
-
Rasmus Villemoes -
Richard Marko -
Robert Marko -
sorki