Preparation for per-package host/target
cover Preparation for per-package host/target
Hi Thomas, all,
maybe my post from last week was too involved so here's a more focused version.
I have played with your per-package patches and found them very interesting because the possible parallelization speeds up the build considerably.
For a start, here are two patches I needed in order to solve some build failures. I'd be happy for feedback as well as some idea how further development on this feature is planned.
best regards,
Andreas
Andreas Naumann (2):
fs: Add common rootfs dependencies to PACKAGES
linux: Make dtc install step more reliable
fs/common.mk | 1 + linux/linux.mk | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
Signature · 1 line
2.19.1
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Up to now only the filesystem-specific dependencies are explicitely added to PACKAGES. The common dependencies, e.g. host-fakeroot, were built indirectly via the rootfs-common/rootfs-tar dependency.
However, when switching to per-package host/target, we need to assemble the global host from the per-package folders. In order to do this by looping through PACKAGES, we need to directly add the common rootfs dependencies.
That way then the filesystem recipes may continue to use these tools from the global host directory.
--- fs/common.mk | 1 + 1 file changed, 1 insertion(+)
fs/common.mk +1 -0
@@ -65,6 +65,7 @@ ROOTFS_COMMON_DEPENDENCIES = \ host-fakeroot host-makedevs \ $(BR2_TAR_HOST_DEPENDENCY) \ $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd) +PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES) $(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs
Signature · 1 line
2.19.1
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Quoted 21 lines · Andreas Naumann
Up to now only the filesystem-specific dependencies are explicitely added to PACKAGES. The common dependencies, e.g. host-fakeroot, were built indirectly via the rootfs-common/rootfs-tar dependency.
However, when switching to per-package host/target, we need to assemble the global host from the per-package folders. In order to do this by looping through PACKAGES, we need to directly add the common rootfs dependencies.
That way then the filesystem recipes may continue to use these tools from the global host directory.Signed-off-by Andreas Naumann <[email protected]>--- fs/common.mk | 1 + 1 file changed, 1 insertion(+)fs/common.mk +0
-0
fs/common.mk ROOTFS_COMMON_DEPENDENCIES = \ · line 68
Quoted 4 lines
fs/common.mk +1
-0@@ -65,6 +65,7 @@ ROOTFS_COMMON_DEPENDENCIES = \ host-fakeroot host-makedevs \ $(BR2_TAR_HOST_DEPENDENCY) \ $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd) +PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
I don't really like this, for the simple reason that it will enable the
dependencies even if they are not needed because we're not building any rootfs.
It's a small thing though, and working around it is a lot more complicated (you
could e.g. select a new Config.in symbol that is selected by all the rootfses). And when we ever get the Config.in.host for all host packages, it's solved already. So
Regards,
Arnout
Quoted 4 lines
fs/common.mk +0
-0$(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Hi Arnout,
Am 09.11.18 um 23:11 schrieb Arnout Vandecappelle:
Quoted 3 lines · Andreas Naumann
...
Quoted 4 lines
Quoted 1 line
+PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
I don't really like this, for the simple reason that it will enable the
dependencies even if they are not needed because we're not building any rootfs.
good point, I dont like that either. I'd propose to change/move the patch into something like
PACKAGES += $$(filter-out $$(PACKAGES),$$(ROOTFS_COMMON_DEPENDENCIES))
inside the rootfs-conditional clause at the bottom of fs/common.mk. Actually I'm not sure the filter-out stuff is really needed since duplicate targets may accumulate in PACKAGES anyway.
best regards,
Andreas
Quoted 20 lines
It's a small thing though, and working around it is a lot more complicated (you
could e.g. select a new Config.in symbol that is selected by all the rootfses). And when we ever get the Config.in.host for all host packages, it's solved already. SoRegards,
ArnoutQuoted 4 lines
$(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Andreas, All,
On 2018-11-12 13:53 +0100, Andreas Naumann spake thusly:
Quoted 8 lines
Am 09.11.18 um 23:11 schrieb Arnout Vandecappelle:
Quoted 1 line · Andreas Naumann
...
Quoted 4 lines
Quoted 1 line
+PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
I don't really like this, for the simple reason that it will enable the
dependencies even if they are not needed because we're not building any rootfs.
Quoted 6 lines
good point, I dont like that either. I'd propose to change/move the patch into something like
PACKAGES += $$(filter-out $$(PACKAGES),$$(ROOTFS_COMMON_DEPENDENCIES))
inside the rootfs-conditional clause at the bottom offs/common.mk. Actually I'm not sure the filter-out stuff is really needed since duplicate targets may accumulate in PACKAGES anyway.
Yes, that would be OK to moce them to the per-filesystem macro, but we already have:
ROOTFS_$(2)_DEPENDENCIES += rootfs-common
So maybe just do:
ROOTFS_$(2)_DEPENDENCIES += \
rootfs-common \
$$(ROOTFS_COMMON_DEPENDENCIES)Regards,
Yann E. MORIN.
Quoted 23 lines
Quoted 11 lines
It's a small thing though, and working around it is a lot more complicated (you
could e.g. select a new Config.in symbol that is selected by all the rootfses). And when we ever get the Config.in.host for all host packages, it's solved already. SoRegards,
ArnoutQuoted 8 lines
Quoted 3 lines
$(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Quoted 21 lines · Yann E. MORIN
Andreas, All,
On 2018-11-12 13:53 +0100, Andreas Naumann spake thusly:
Quoted 8 lines
Am 09.11.18 um 23:11 schrieb Arnout Vandecappelle:
Quoted 1 line · Andreas Naumann
...
Quoted 4 lines
Quoted 1 line
+PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
I don't really like this, for the simple reason that it will enable the
dependencies even if they are not needed because we're not building any rootfs.Quoted 6 lines
good point, I dont like that either. I'd propose to change/move the patch into something like
PACKAGES += $$(filter-out $$(PACKAGES),$$(ROOTFS_COMMON_DEPENDENCIES))
inside the rootfs-conditional clause at the bottom offs/common.mk. Actually I'm not sure the filter-out stuff is really needed since duplicate targets may accumulate in PACKAGES anyway.Yes, that would be OK to moce them to the per-filesystem macro, but we already have:
Quoted 7 lines
ROOTFS_$(2)_DEPENDENCIES += rootfs-commonSo maybe just do:
ROOTFS_$(2)_DEPENDENCIES += \ rootfs-common \ $$(ROOTFS_COMMON_DEPENDENCIES)
It's about adding to PACKAGES, not _DEPENDENCIES.
Regards,
Arnout
Quoted 28 lines
Regards,
Yann E. MORIN.Quoted 23 lines
Quoted 11 lines
It's a small thing though, and working around it is a lot more complicated (you
could e.g. select a new Config.in symbol that is selected by all the rootfses). And when we ever get the Config.in.host for all host packages, it's solved already. SoRegards,
ArnoutQuoted 8 lines
Quoted 3 lines
$(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Arnout, All,
On 2018-11-12 18:32 +0100, Arnout Vandecappelle spake thusly:
Quoted 24 lines · Yann E. MORIN
Quoted 23 lines
Andreas, All,
On 2018-11-12 13:53 +0100, Andreas Naumann spake thusly:
Quoted 8 lines
Am 09.11.18 um 23:11 schrieb Arnout Vandecappelle:
Quoted 1 line · Andreas Naumann
...
Quoted 4 lines
Quoted 1 line
+PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
I don't really like this, for the simple reason that it will enable the
dependencies even if they are not needed because we're not building any rootfs.Quoted 6 lines
good point, I dont like that either. I'd propose to change/move the patch into something like
PACKAGES += $$(filter-out $$(PACKAGES),$$(ROOTFS_COMMON_DEPENDENCIES))
inside the rootfs-conditional clause at the bottom offs/common.mk. Actually I'm not sure the filter-out stuff is really needed since duplicate targets may accumulate in PACKAGES anyway.Yes, that would be OK to moce them to the per-filesystem macro, but we already have:
ROOTFS_$(2)_DEPENDENCIES += rootfs-common
Quoted 7 lines
Quoted 5 lines
So maybe just do:
ROOTFS_$(2)_DEPENDENCIES += \ rootfs-common \ $$(ROOTFS_COMMON_DEPENDENCIES)It's about adding to PACKAGES, not
_DEPENDENCIES.
Yeah, but _DEPENDENCIES are added to PACKAGES.
And I think with the series I just posted [0]. there is no reason to have dependencies in the common step anymore, since there are now only calls to 'cat' and 'printf' in there. So it makes sense to make those dependencies, dependencies of each individual filesystem now.
Regards,
Yann E. MORIN.
Quoted 28 lines
Quoted 28 lines
Regards,
Yann E. MORIN.Quoted 23 lines
Quoted 11 lines
It's a small thing though, and working around it is a lot more complicated (you
could e.g. select a new Config.in symbol that is selected by all the rootfses). And when we ever get the Config.in.host for all host packages, it's solved already. SoRegards,
ArnoutQuoted 8 lines
Quoted 3 lines
$(ROOTFS_COMMON_TAR): ROOTFS=COMMON $(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
In reply to Andreas NaumannHello Andres,
fs/common.mk ROOTFS_COMMON_DEPENDENCIES = \ · line 68
Quoted 9 lines · Andreas Naumann
fs/common.mk +1
-0@@ -65,6 +65,7 @@ ROOTFS_COMMON_DEPENDENCIES = \ host-fakeroot host-makedevs \ $(BR2_TAR_HOST_DEPENDENCY) \ $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd) +PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)
Thanks a lot for testing the PPSH stuff and providing some bug fixes!
I've integrated into my patch series the following change:
@@ -171,7 +172,7 @@ rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1)
ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y)
TARGETS_ROOTFS += rootfs-$(1)
-PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES))
+PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES)) $(ROOTFS_COMMON_DEPENDENCIES)
endif
# Check for legacy POST_TARGETS rules
which addresses this problem, while adding the dependencies only when a filesystem image is selected. Having duplicates in PACKAGES is not a problem.
I hope to send a v4 of my series soon.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/2 fs: Add common rootfs dependencies to PACKAGES
Hi,
Am 13.11.18 um 15:51 schrieb Thomas Petazzoni:
Quoted 28 lines
Hello Andres,
fs/common.mk ROOTFS_COMMON_DEPENDENCIES = \ · line 68
Quoted 9 lines · Andreas Naumann
fs/common.mk +1
-0@@ -65,6 +65,7 @@ ROOTFS_COMMON_DEPENDENCIES = \ host-fakeroot host-makedevs \ $(BR2_TAR_HOST_DEPENDENCY) \ $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd) +PACKAGES += $(ROOTFS_COMMON_DEPENDENCIES)Thanks a lot for testing the PPSH stuff and providing some bug fixes!
I've integrated into my patch series the following change:
(quoted diff) +1
-1@@ -171,7 +172,7 @@ rootfs-$(1): $$(BINARIES_DIR)/rootfs.$(1) ifeq ($$(BR2_TARGET_ROOTFS_$(2)),y) TARGETS_ROOTFS += rootfs-$(1) -PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES)) +PACKAGES += $$(filter-out rootfs-%,$$(ROOTFS_$(2)_DEPENDENCIES)) $(ROOTFS_COMMON_DEPENDENCIES) endif # Check for legacy POST_TARGETS rules
Quoted 3 lines
which addresses this problem, while adding the dependencies only when a filesystem image is selected. Having duplicates in PACKAGES is not a problem.
ok, I was going to follow Yann's suggestion but the confusion from seemingly adding to DEPENDENCIES instead of PACKAGES makes me favour your solution. Thus I wont post another v2 for this one.
Quoted 2 lines
I hope to send a v4 of my series soon.
Am curious.
regards,
Andreas
Quoted 5 lines
Thanks!
Thomas
PATCH 2/2 linux: Make dtc install step more reliable
In reply to Andreas NaumannChecking for the existence of the dtc binary built by the non-dependent dtc package may cause instable behaviour when giving more freedom on the order of how the packages are built (parallelization).
In addidion, when moving to per-package host/target method, the check would always trigger in the isolated host, leading to linux-dtc always being installed as dtc.
This in turn may lead to undesired overwriting of the real host-dtc binary when finally assembling the global host dir.
--- linux/linux.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
linux/linux.mk +1 -1
@@ -465,7 +465,7 @@ define LINUX_INSTALL_HOST_TOOLS # Installing dtc (device tree compiler) as host tool, if selected if grep -q "CONFIG_DTC=y" $(@D)/.config; then \ $(INSTALL) -D -m 0755 $(@D)/scripts/dtc/dtc $(HOST_DIR)/bin/linux-dtc ; \ - if [ ! -e $(HOST_DIR)/bin/dtc ]; then \ + if [ "$(BR2_PACKAGE_HOST_DTC)" != "y" ]; then \ ln -sf linux-dtc $(HOST_DIR)/bin/dtc ; \ fi \ fi
Signature · 1 line
2.19.1
PATCH 2/2 linux: Make dtc install step more reliable
Quoted 20 lines · Andreas Naumann
Checking for the existence of the dtc binary built by the non-dependent dtc package may cause instable behaviour when giving more freedom on the order of how the packages are built (parallelization).
In addidion, when moving to per-package host/target method, the check would always trigger in the isolated host, leading to linux-dtc always being installed as dtc.
This in turn may lead to undesired overwriting of the real host-dtc binary when finally assembling the global host dir.Signed-off-by Andreas Naumann <[email protected]>--- linux/linux.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)linux/linux.mk +0
-0
linux/linux.mk define LINUX_INSTALL_HOST_TOOLS · line 470
Quoted 7 lines
linux/linux.mk +1
-1@@ -465,7 +465,7 @@ define LINUX_INSTALL_HOST_TOOLS # Installing dtc (device tree compiler) as host tool, if selected if grep -q "CONFIG_DTC=y" $(@D)/.config; then \ $(INSTALL) -D -m 0755 $(@D)/scripts/dtc/dtc $(HOST_DIR)/bin/linux-dtc ; \ - if [ ! -e $(HOST_DIR)/bin/dtc ]; then \ + if [ "$(BR2_PACKAGE_HOST_DTC)" != "y" ]; then \ ln -sf linux-dtc $(HOST_DIR)/bin/dtc ; \ fi \
Since now it's purely steered by a make variable, you can change it into
$(if $(BR2_PACKAGE_HOST_DTC),ln -sf linux-dtc $(HOST_DIR)/bin/dtc;) \
(note that the semicolon is still needed because it's in the CONFIG_DTC condition).
Regards,
Arnout
Quoted 2 lines
linux/linux.mk +0
-0fi
PATCH 2/2 linux: Make dtc install step more reliable
Hi,
Am 09.11.18 um 23:14 schrieb Arnout Vandecappelle:
Quoted 3 lines · Andreas Naumann
...
Quoted 8 lines
linux/linux.mk define LINUX_INSTALL_HOST_TOOLS · line 470
Quoted 4 lines
linux/linux.mk +1
-1- if [ ! -e $(HOST_DIR)/bin/dtc ]; then \ + if [ "$(BR2_PACKAGE_HOST_DTC)" != "y" ]; then \ ln -sf linux-dtc $(HOST_DIR)/bin/dtc ; \ fi \Since now it's purely steered by a make variable, you can change it into
$(if $(BR2_PACKAGE_HOST_DTC),ln -sf linux-dtc $(HOST_DIR)/bin/dtc;) \
Thanks. I'll resend v2.
best regards,
Andi
Quoted 15 lines
(note that the semicolon is still needed because it's in the
CONFIG_DTCcondition).Regards,
ArnoutQuoted 2 lines
fi
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot
PATCH 2/2 linux: Make dtc install step more reliable
Quoted 12 lines · Andreas Naumann
Hi,
Am 09.11.18 um 23:14 schrieb Arnout Vandecappelle:
Quoted 3 lines · Andreas Naumann
...
Quoted 5 lines
Quoted 4 lines
-??????? if [ ! -e $(HOST_DIR)/bin/dtc ]; then \
+??????? if [ "$(BR2_PACKAGE_HOST_DTC)" != "y" ]; then \
????????????? ln -sf linux-dtc $(HOST_DIR)/bin/dtc ; \
????????? fi \
Quoted 5 lines
Quoted 3 lines
? Since now it's purely steered by a make variable, you can change it into
????????? $(if $(BR2_PACKAGE_HOST_DTC),ln -sf linux-dtc $(HOST_DIR)/bin/dtc;) \
Thanks. I'll resend v2.
Please test as well. I have a good track record of proposing broken code :-)
Regards,
Arnout
[snip]
cover Preparation for per-package host/target
In reply to Andreas NaumannHello,
Quoted 1 line · Andreas Naumann
maybe my post from last week was too involved so here's a more focused version.
I did see your patches, and I'm happy to see that people are interested in the per-package host/target. I have resumed work on this topic recently, and I hope to submit a new patch series in the next few weeks.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com