[PATCH v2 1/2] Revert "kbuild: remove unused dtc-version.sh script"
We need this to make building dtc optional. It makes no sense to build our own dtc if the system one works correctly.
This reverts commit ddb87a0b40262ff99d675e946f57427642303938.
Signed-off-by: Simon Glass sjg@chromium.org ---
scripts/Kbuild.include | 1 + scripts/dtc-version.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100755 scripts/dtc-version.sh
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index dfb67226b0..b34dedade7 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -152,6 +152,7 @@ cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
# added for U-Boot binutils-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/binutils-version.sh $(AS)) +dtc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/dtc-version.sh $(DTC))
# cc-ldoption # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both) diff --git a/scripts/dtc-version.sh b/scripts/dtc-version.sh new file mode 100755 index 0000000000..0744c39eb0 --- /dev/null +++ b/scripts/dtc-version.sh @@ -0,0 +1,21 @@ +#!/bin/sh +# +# dtc-version dtc-command +# +# Prints the dtc version of `dtc-command' in a canonical 6-digit form +# such as `010404' for dtc 1.4.4 +# + +dtc="$*" + +if [ ${#dtc} -eq 0 ]; then + echo "Error: No dtc command specified." + printf "Usage:\n\t$0 <dtc-command>\n" + exit 1 +fi + +MAJOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 1) +MINOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 2) +PATCH=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 3 | cut -d - -f 1) + +printf "%02d%02d%02d\n" $MAJOR $MINOR $PATCH
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script - Update the check to make sure libfdt is available if needed
Signed-off-by: Simon Glass sjg@chromium.org ---
Makefile | 21 ++++++++++++++++++--- dts/Kconfig | 4 ---- scripts/Kbuild.include | 5 ++++- scripts/Makefile | 1 - scripts/dtc-version.sh | 36 +++++++++++++++++++++++++++++++----- 5 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile index b8a4b5058a..90cb83ed32 100644 --- a/Makefile +++ b/Makefile @@ -410,7 +410,12 @@ PERL = perl PYTHON ?= python PYTHON2 = python2 PYTHON3 = python3 -DTC ?= $(objtree)/scripts/dtc/dtc + +# DTC is automatically built if the version of $(DTC) is older that needed. +# Use the system dtc if it is new enough. +DTC ?= dtc +DTC_MIN_VERSION := 010406 + CHECK = sparse
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ @@ -1797,12 +1802,12 @@ include/config/uboot.release: include/config/auto.conf FORCE # version.h and scripts_basic is processed / created.
# Listed in dependency order -PHONY += prepare archprepare prepare0 prepare1 prepare2 prepare3 +PHONY += prepare archprepare prepare0 prepare1 prepare2 prepare3 prepare4
# prepare3 is used to check if we are building in a separate output directory, # and if so do: # 1) Check that make has not been executed in the kernel src $(srctree) -prepare3: include/config/uboot.release +prepare4: include/config/uboot.release ifneq ($(KBUILD_SRC),) @$(kecho) ' Using $(srctree) as source for U-Boot' $(Q)if [ -f $(srctree)/.config -o -d $(srctree)/include/config ]; then \ @@ -1812,6 +1817,14 @@ ifneq ($(KBUILD_SRC),) fi; endif
+# Checks for dtc and builds it if needed +prepare3: prepare4 + $(eval DTC := $(call dtc-version,010406,$(build_dtc),$(CONFIG_PYLIBFDT))) + echo here $(DTC) $(build_dtc) + if test "$(DTC)" = "$(build_dtc)"; then \ + $(MAKE) $(build)=scripts/dtc; \ + fi + # prepare2 creates a makefile if using a separate output directory prepare2: prepare3 outputmakefile cfg
@@ -1963,6 +1976,8 @@ SYSTEM_MAP = \ System.map: u-boot @$(call SYSTEM_MAP,$<) > $@
+build_dtc := $(objtree)/scripts/dtc/dtc + #########################################################################
# ARM relocations should all be R_ARM_RELATIVE (32-bit) or diff --git a/dts/Kconfig b/dts/Kconfig index 046a54a173..f8b808606c 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -5,9 +5,6 @@ config SUPPORT_OF_CONTROL bool
-config DTC - bool - config PYLIBFDT bool
@@ -24,7 +21,6 @@ menu "Device Tree Control"
config OF_CONTROL bool "Run-time configuration via Device Tree" - select DTC select OF_LIBFDT if !OF_PLATDATA help This feature provides for run-time configuration of U-Boot diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index b34dedade7..8c167cef2d 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -151,8 +151,11 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \ cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
# added for U-Boot +# $1: min_version +# 32: build_dtc +# $3: need_pylibfdt binutils-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/binutils-version.sh $(AS)) -dtc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/dtc-version.sh $(DTC)) +dtc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/dtc-version.sh $(DTC) $1 $2 $3)
# cc-ldoption # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both) diff --git a/scripts/Makefile b/scripts/Makefile index e7b353f77f..cfe9fef804 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -10,4 +10,3 @@ always := $(hostprogs-y)
# Let clean descend into subdirs subdir- += basic kconfig -subdir-$(CONFIG_DTC) += dtc diff --git a/scripts/dtc-version.sh b/scripts/dtc-version.sh index 0744c39eb0..75ba82830d 100755 --- a/scripts/dtc-version.sh +++ b/scripts/dtc-version.sh @@ -1,12 +1,26 @@ #!/bin/sh # -# dtc-version dtc-command +# dtc-version dtc_command min_version build_dtc need_pylibfdt # -# Prints the dtc version of `dtc-command' in a canonical 6-digit form -# such as `010404' for dtc 1.4.4 +# Selects which version of dtc to use +# +# If need_pylibfdt is non-empty then the script first checks that the Python +# libfdt library is available. If not it outputs $build_dtc and exits +# +# Otherwise, if the version of dtc_command is < min_version, prints build_dtc +# else prints dtc_command. The min_version is in the format MMmmpp where: +# +# MM is the major version +# mm is the minor version +# pp is the patch level +# +# For example 010406 means 1.4.6 #
-dtc="$*" +dtc="$1" +min_version="$2" +build_dtc="$3" +need_pylibfdt="$4"
if [ ${#dtc} -eq 0 ]; then echo "Error: No dtc command specified." @@ -14,8 +28,20 @@ if [ ${#dtc} -eq 0 ]; then exit 1 fi
+if [ -n "${need_pylibfdt}" ]; then + if ! echo "import libfdt" | python3 2>/dev/null; then + echo $build_dtc + exit 0 + fi +fi + MAJOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 1) MINOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 2) PATCH=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 3 | cut -d - -f 1)
-printf "%02d%02d%02d\n" $MAJOR $MINOR $PATCH +version="$(printf "%02d%02d%02d" $MAJOR $MINOR $PATCH)" +if test $version -lt $min_version; then \ + echo $build_dtc +else + echo $dtc +fi
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
Cc: Vagrant
Best regards
Heinrich
Signed-off-by: Simon Glass sjg@chromium.org
Makefile | 21 ++++++++++++++++++--- dts/Kconfig | 4 ---- scripts/Kbuild.include | 5 ++++- scripts/Makefile | 1 - scripts/dtc-version.sh | 36 +++++++++++++++++++++++++++++++----- 5 files changed, 53 insertions(+), 14 deletions(-)
diff --git a/Makefile b/Makefile index b8a4b5058a..90cb83ed32 100644 --- a/Makefile +++ b/Makefile @@ -410,7 +410,12 @@ PERL = perl PYTHON ?= python PYTHON2 = python2 PYTHON3 = python3 -DTC ?= $(objtree)/scripts/dtc/dtc
+# DTC is automatically built if the version of $(DTC) is older that needed. +# Use the system dtc if it is new enough. +DTC ?= dtc +DTC_MIN_VERSION := 010406
CHECK = sparse
CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ @@ -1797,12 +1802,12 @@ include/config/uboot.release: include/config/auto.conf FORCE # version.h and scripts_basic is processed / created.
# Listed in dependency order -PHONY += prepare archprepare prepare0 prepare1 prepare2 prepare3 +PHONY += prepare archprepare prepare0 prepare1 prepare2 prepare3 prepare4
# prepare3 is used to check if we are building in a separate output directory, # and if so do: # 1) Check that make has not been executed in the kernel src $(srctree) -prepare3: include/config/uboot.release +prepare4: include/config/uboot.release ifneq ($(KBUILD_SRC),) @$(kecho) ' Using $(srctree) as source for U-Boot' $(Q)if [ -f $(srctree)/.config -o -d $(srctree)/include/config ]; then \ @@ -1812,6 +1817,14 @@ ifneq ($(KBUILD_SRC),) fi; endif
+# Checks for dtc and builds it if needed +prepare3: prepare4
- $(eval DTC := $(call
dtc-version,010406,$(build_dtc),$(CONFIG_PYLIBFDT)))
- echo here $(DTC) $(build_dtc)
- if test "$(DTC)" = "$(build_dtc)"; then \
$(MAKE) $(build)=scripts/dtc; \- fi
# prepare2 creates a makefile if using a separate output directory prepare2: prepare3 outputmakefile cfg
@@ -1963,6 +1976,8 @@ SYSTEM_MAP = \ System.map: u-boot @$(call SYSTEM_MAP,$<) > $@
+build_dtc := $(objtree)/scripts/dtc/dtc
#########################################################################
# ARM relocations should all be R_ARM_RELATIVE (32-bit) or diff --git a/dts/Kconfig b/dts/Kconfig index 046a54a173..f8b808606c 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -5,9 +5,6 @@ config SUPPORT_OF_CONTROL bool
-config DTC
- bool
config PYLIBFDT bool
@@ -24,7 +21,6 @@ menu "Device Tree Control"
config OF_CONTROL bool "Run-time configuration via Device Tree"
- select DTC select OF_LIBFDT if !OF_PLATDATA help This feature provides for run-time configuration of U-Boot
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index b34dedade7..8c167cef2d 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -151,8 +151,11 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \ cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
# added for U-Boot +# $1: min_version +# 32: build_dtc +# $3: need_pylibfdt binutils-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/binutils-version.sh $(AS)) -dtc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/dtc-version.sh $(DTC)) +dtc-version = $(shell $(CONFIG_SHELL) $(srctree)/scripts/dtc-version.sh $(DTC) $1 $2 $3)
# cc-ldoption # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both) diff --git a/scripts/Makefile b/scripts/Makefile index e7b353f77f..cfe9fef804 100644 --- a/scripts/Makefile +++ b/scripts/Makefile @@ -10,4 +10,3 @@ always := $(hostprogs-y)
# Let clean descend into subdirs subdir- += basic kconfig -subdir-$(CONFIG_DTC) += dtc diff --git a/scripts/dtc-version.sh b/scripts/dtc-version.sh index 0744c39eb0..75ba82830d 100755 --- a/scripts/dtc-version.sh +++ b/scripts/dtc-version.sh @@ -1,12 +1,26 @@ #!/bin/sh # -# dtc-version dtc-command +# dtc-version dtc_command min_version build_dtc need_pylibfdt # -# Prints the dtc version of `dtc-command' in a canonical 6-digit form -# such as `010404' for dtc 1.4.4 +# Selects which version of dtc to use +# +# If need_pylibfdt is non-empty then the script first checks that the Python +# libfdt library is available. If not it outputs $build_dtc and exits +# +# Otherwise, if the version of dtc_command is < min_version, prints build_dtc +# else prints dtc_command. The min_version is in the format MMmmpp where: +# +# MM is the major version +# mm is the minor version +# pp is the patch level +# +# For example 010406 means 1.4.6 #
-dtc="$*" +dtc="$1" +min_version="$2" +build_dtc="$3" +need_pylibfdt="$4"
if [ ${#dtc} -eq 0 ]; then echo "Error: No dtc command specified." @@ -14,8 +28,20 @@ if [ ${#dtc} -eq 0 ]; then exit 1 fi
+if [ -n "${need_pylibfdt}" ]; then
- if ! echo "import libfdt" | python3 2>/dev/null; then
echo $build_dtcexit 0- fi
+fi
MAJOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 1) MINOR=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 2) PATCH=$($dtc -v | head -1 | awk '{print $NF}' | cut -d . -f 3 | cut -d
- -f 1)
-printf "%02d%02d%02d\n" $MAJOR $MINOR $PATCH +version="$(printf "%02d%02d%02d" $MAJOR $MINOR $PATCH)" +if test $version -lt $min_version; then \
- echo $build_dtc
+else
- echo $dtc
+fi
Hi Heinrich,
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
Cc: Vagrant
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
Regards, SImon
Regards, Simon
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related information at all!
live well, vagrant
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related
This is probably one of those cases where we should just continue to act like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their ways, we can adopt that.
Hi Tom.
On Tue, 28 Apr 2020 at 08:19, Tom Rini trini@konsulko.com wrote:
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related
This is probably one of those cases where we should just continue to act like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their ways, we can adopt that.
It seems that Vagrant wants to use the system dtc by default and require an explicit option to use the in-built dtc. I don't think that would work well for most users though.
It is in my view somewhat mad to build dtc for every one of 1400 boards as I do today when running buildman.
Having said that it doesn't seem like we can come up with a default behaviour that makes everyone happy, so the status quo is best.
But what about adding a build flag / envvar to select between:
- Use system default - Build internal version - Use system default if new enough, else build
Then we can satisfy distros as well as speed up the build for those that care.
I haven't heard from Marek on this thread, actually.
Regards, Simon
On Tue, Apr 28, 2020 at 09:41:14AM -0600, Simon Glass wrote:
Hi Tom.
On Tue, 28 Apr 2020 at 08:19, Tom Rini trini@konsulko.com wrote:
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org:
At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. This is wasteful when the system already has a suitable version available.
Update the Makefile logic to build dtc only if the version available is too old.
This saves about 2.5 seconds of elapsed time on a clean build for me.
- Add a patch to bring back the dtc-version.sh script
- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related
This is probably one of those cases where we should just continue to act like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their ways, we can adopt that.
It seems that Vagrant wants to use the system dtc by default and require an explicit option to use the in-built dtc. I don't think that would work well for most users though.
Right, and this is where I disagree and point to the kernel. Get that changed first.
It is in my view somewhat mad to build dtc for every one of 1400 boards as I do today when running buildman.
This is a different funny case. Perhaps ccache could be helpful here? I think the way it's used in OpenEmbedded, such that you have a cache that's more local to what's building vs global cache, could be helpful here too. A ccache instance per CI job / world build could help. A flag to buildman to support that could help, yes? Thanks!
Hi Tom,
On Tue, 28 Apr 2020 at 09:52, Tom Rini trini@konsulko.com wrote:
On Tue, Apr 28, 2020 at 09:41:14AM -0600, Simon Glass wrote:
Hi Tom.
On Tue, 28 Apr 2020 at 08:19, Tom Rini trini@konsulko.com wrote:
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org: >At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. >This >is wasteful when the system already has a suitable version available. > >Update the Makefile logic to build dtc only if the version available is >too old. > >This saves about 2.5 seconds of elapsed time on a clean build for me. > >- Add a patch to bring back the dtc-version.sh script >- Update the check to make sure libfdt is available if needed
U -Boot has been set up to create reproducible builds. With this patch dtc will have to be made a build dependency to provide reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
This may require changes in the packaging of U-Boot in Linux distributions. Nothing to stop this patch, just something to keep in mind.
You presume that future versions of dtc will always be backward compatible with U-Boot. Ok, we do the same for other tools like gcc too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related
This is probably one of those cases where we should just continue to act like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their ways, we can adopt that.
It seems that Vagrant wants to use the system dtc by default and require an explicit option to use the in-built dtc. I don't think that would work well for most users though.
Right, and this is where I disagree and point to the kernel. Get that changed first.
It is in my view somewhat mad to build dtc for every one of 1400 boards as I do today when running buildman.
This is a different funny case. Perhaps ccache could be helpful here? I think the way it's used in OpenEmbedded, such that you have a cache that's more local to what's building vs global cache, could be helpful here too. A ccache instance per CI job / world build could help. A flag to buildman to support that could help, yes? Thanks!
So not allow using the system dtc? Or are you OK with a build option?
The thing is I would prefer to avoid another level of cache for a problem that only exists because of our kernel design decision.
As to changing the kernel, I cannot imagine that happening as they change dtc all the time.
Regards, Simon
On Tue, Apr 28, 2020 at 04:40:47PM -0600, Simon Glass wrote:
Hi Tom,
On Tue, 28 Apr 2020 at 09:52, Tom Rini trini@konsulko.com wrote:
On Tue, Apr 28, 2020 at 09:41:14AM -0600, Simon Glass wrote:
Hi Tom.
On Tue, 28 Apr 2020 at 08:19, Tom Rini trini@konsulko.com wrote:
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote:
On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass sjg@chromium.org: > >At present U-Boot always builds dtc if CONFIG_OF_CONTROL is defined. > >This > >is wasteful when the system already has a suitable version available. > > > >Update the Makefile logic to build dtc only if the version available is > >too old. > > > >This saves about 2.5 seconds of elapsed time on a clean build for me. > > > >- Add a patch to bring back the dtc-version.sh script > >- Update the check to make sure libfdt is available if needed > > U -Boot has been set up to create reproducible builds. With this > patch dtc will have to be made a build dependency to provide > reproducibility. Cf. https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility > > This may require changes in the packaging of U-Boot in Linux > distributions. Nothing to stop this patch, just something to keep in > mind. > > You presume that future versions of dtc will always be backward > compatible with U-Boot. Ok, we do the same for other tools like gcc > too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not use embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to fix a problem in a single source package, rather than an arbitrary number of source packages that happen to share some embedded copy of the code from who knows when...
So at least from my perspective, I'd be happy to use the Debian packaged dtc (a.k.a. device-tree-compiler), rather than the one embedded in u-boot sources.
Silently switching to the embedded copy sounds a little scary; I would prefer for that to be explicit... a build flag to specify one way or the other and failing is better that being too clever about autodetecting.
Should we disable this check (and always build dtc) when doing a repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally *always* be reproducible, given the same sources and toolchain... several distributions and software projects provide information sufficient to reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder is explicitly attempting to be reproducible; It's a bit of a kludge to try and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of date or time related things; even better would be to not embded time related
This is probably one of those cases where we should just continue to act like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their ways, we can adopt that.
It seems that Vagrant wants to use the system dtc by default and require an explicit option to use the in-built dtc. I don't think that would work well for most users though.
Right, and this is where I disagree and point to the kernel. Get that changed first.
It is in my view somewhat mad to build dtc for every one of 1400 boards as I do today when running buildman.
This is a different funny case. Perhaps ccache could be helpful here? I think the way it's used in OpenEmbedded, such that you have a cache that's more local to what's building vs global cache, could be helpful here too. A ccache instance per CI job / world build could help. A flag to buildman to support that could help, yes? Thanks!
So not allow using the system dtc? Or are you OK with a build option?
I'd rather not have a build option as that's going to encourage people to use it, and then that'll lead to problems down the line.
The thing is I would prefer to avoid another level of cache for a problem that only exists because of our kernel design decision.
As to changing the kernel, I cannot imagine that happening as they change dtc all the time.
We really need to stay better in sync with the kernel here too. Trying to get more syncs of kbuild/kconfig/dtc/etc with kernel releases is on my list.
Hi Tom,
On Thu, 30 Apr 2020 at 09:05, Tom Rini trini@konsulko.com wrote:
On Tue, Apr 28, 2020 at 04:40:47PM -0600, Simon Glass wrote:
Hi Tom,
On Tue, 28 Apr 2020 at 09:52, Tom Rini trini@konsulko.com wrote:
On Tue, Apr 28, 2020 at 09:41:14AM -0600, Simon Glass wrote:
Hi Tom.
On Tue, 28 Apr 2020 at 08:19, Tom Rini trini@konsulko.com wrote:
On Mon, Apr 27, 2020 at 04:10:06PM -0700, Vagrant Cascadian wrote:
On 2020-04-27, Simon Glass wrote: > On Sun, 26 Apr 2020 at 18:58, Heinrich Schuchardt <
xypron.glpk@gmx.de> wrote:
>> >> Am April 27, 2020 12:29:29 AM UTC schrieb Simon Glass <
sjg@chromium.org>:
>> >At present U-Boot always builds dtc if CONFIG_OF_CONTROL is
defined.
>> >This >> >is wasteful when the system already has a suitable version
available.
>> > >> >Update the Makefile logic to build dtc only if the version
available is
>> >too old. >> > >> >This saves about 2.5 seconds of elapsed time on a clean
build for me.
>> > >> >- Add a patch to bring back the dtc-version.sh script >> >- Update the check to make sure libfdt is available if needed >> >> U -Boot has been set up to create reproducible builds. With
this
>> patch dtc will have to be made a build dependency to provide >> reproducibility. Cf.
https://www.debian.org/doc/debian-policy/ch-source.html#reproducibility
>> >> This may require changes in the packaging of U-Boot in Linux >> distributions. Nothing to stop this patch, just something to
keep in
>> mind. >> >> You presume that future versions of dtc will always be
backward
>> compatible with U-Boot. Ok, we do the same for other tools
like gcc
>> too (with surprises at each new major release).
In general when packaging for Debian, the preference is to not
use
embedded code copies if at all possible. This does require paying attention to backwards and forwards compatibility issues a bit.
A simple example: The security team in Debian generally likes to
fix a
problem in a single source package, rather than an arbitrary
number of
source packages that happen to share some embedded copy of the
code from
who knows when...
So at least from my perspective, I'd be happy to use the Debian
packaged
dtc (a.k.a. device-tree-compiler), rather than the one embedded
in
u-boot sources.
Silently switching to the embedded copy sounds a little scary; I
would
prefer for that to be explicit... a build flag to specify one
way or the
other and failing is better that being too clever about
autodetecting.
> Should we disable this check (and always build dtc) when doing
a
> repeatable build? Is that SOURCE_DATE_EPOCH?
And with my Reproducible Builds hat on, builds would ideally
*always* be
reproducible, given the same sources and toolchain... several distributions and software projects provide information
sufficient to
reproduce the build environment:
https://reproducible-builds.org/docs/recording/
While SOURCE_DATE_EPOCH is definitely one sign that the builder
is
explicitly attempting to be reproducible; It's a bit of a kludge
to try
and be more reproducible just because SOURCE_DATE_EPOCH is set. SOURCE_DATE_EPOCH should really only affect the behavior of
date or
time related things; even better would be to not embded time
related
This is probably one of those cases where we should just continue
to act
like the linux kernel and always use and build our own copy of dtc. Then, when someone convinces the kernel folks to change their
ways, we
can adopt that.
It seems that Vagrant wants to use the system dtc by default and require an explicit option to use the in-built dtc. I don't think
that
would work well for most users though.
Right, and this is where I disagree and point to the kernel. Get that changed first.
It is in my view somewhat mad to build dtc for every one of 1400 boards as I do today when running buildman.
This is a different funny case. Perhaps ccache could be helpful here? I think the way it's used in OpenEmbedded, such that you have a cache that's more local to what's building vs global cache, could be helpful here too. A ccache instance per CI job / world build could help. A flag to buildman to support that could help, yes? Thanks!
So not allow using the system dtc? Or are you OK with a build option?
I'd rather not have a build option as that's going to encourage people to use it, and then that'll lead to problems down the line.
OK, let's drop this patch then. If I can think of some other way, I'll let you know.
The thing is I would prefer to avoid another level of cache for a problem that only exists because of our kernel design decision.
As to changing the kernel, I cannot imagine that happening as they change dtc all the time.
We really need to stay better in sync with the kernel here too. Trying to get more syncs of kbuild/kconfig/dtc/etc with kernel releases is on my list.
-- Tom
participants (4)
-
Heinrich Schuchardt -
Simon Glass -
Tom Rini -
Vagrant Cascadian