| Message ID | 20181114105557.12599-4-thomas.petazzoni@bootlin.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Per-package host/target directory support | expand |
Thomas, All, On 2018-11-14 11:55 +0100, Thomas Petazzoni spake thusly: > In the current code, the creation of the main output folders > (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a > global "dirs" target. While this works fine in the current situation, > it doesn't work well in a context where per-package host and target > directories are used. > > For example, with the current code and per-package host directories, > the output/staging symbolic link ends up being created as a link to > the per-package package sysroot directory of the first package being > built, instead of the global sysroot. > > This commit reworks the creation of those directories by having the > package/pkg-generic.mk code ensure that the build directory, target > directory, host directory, staging directory and binaries directory > exist before they are needed. I don't think we do need anything special to create HOST_DIR, STAGING_DIR and TARGET_DIR: - host-skeleton is the first package to run, and that one does ensure the existence of $(HOST_DIR) by the mere act of creating it (which it cirrently does not do, but should) - skeleton is the first package to install in target/ and staging, and currently the skeleton-custom and skeleton-init-common should both create target/ and staging/ > Two new targets, host-finalize and staging-finalize are added in the > main Makefile to create the compatibility symlinks for host and > staging directories. They will be extended later with additional logic > for per-package directories. > > Thanks to those changes, the global "dirs" target is entirely removed. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > --- > Makefile | 21 +++++++++------------ > fs/common.mk | 1 + > package/pkg-generic.mk | 7 ++++++- > 3 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/Makefile b/Makefile > index 2819d44124..e675ac26aa 100644 > --- a/Makefile > +++ b/Makefile > @@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\ > > endif > > -.PHONY: dirs > -dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \ > - $(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR) > - > $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) > $(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig > > @@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ > -C $(HOST_DIR) "." > > -# Populating the staging with the base directories is handled by the skeleton package > -$(STAGING_DIR): > - @mkdir -p $(STAGING_DIR) > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > - > RSYNC_VCS_EXCLUSIONS = \ > --exclude .svn --exclude .git --exclude .hg --exclude .bzr \ > --exclude CVS > @@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize > # Avoid the rootfs name leaking down the dependency chain > target-finalize: ROOTFS= > > +host-finalize: $(HOST_DIR_SYMLINK) > + > +.PHONY: staging-finalize > +staging-finalize: > + @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > + > .PHONY: target-finalize > -target-finalize: $(PACKAGES) > +target-finalize: $(PACKAGES) host-finalize Shouldn't we ensure that host-finalize be done before staging-finalize too? We would also need to ensure that packages are all installed before running staging-finalize. So,=maybe we need to add: staging-finalize: $(PACKAGES) host-finalize Regards, Yann E. MORIN. > @$(call MESSAGE,"Finalizing target directory") > # Check files that are touched by more than one package > ./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt > @@ -782,7 +779,7 @@ endif > touch $(TARGET_DIR)/usr > > .PHONY: target-post-image > -target-post-image: $(TARGETS_ROOTFS) target-finalize > +target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize > @rm -f $(ROOTFS_COMMON_TAR) > @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \ > $(call MESSAGE,"Executing post-image script $(s)"); \ > @@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR) > @cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config > > .PHONY: legal-info > -legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \ > +legal-info: legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \ > $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) > @cat support/legal-info/README.header >>$(LEGAL_REPORT) > @if [ -r $(LEGAL_WARNINGS) ]; then \ > diff --git a/fs/common.mk b/fs/common.mk > index 2a5a202a89..96658428ba 100644 > --- a/fs/common.mk > +++ b/fs/common.mk > @@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) > $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot > $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) > @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") > + mkdir -p $$(@D) > rm -rf $$(ROOTFS_$(2)_DIR) > mkdir -p $$(ROOTFS_$(2)_DIR) > echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index f34f46afc8..309fd8cd48 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted: > $(BUILD_DIR)/%/.stamp_rsynced: > @$(call step_start,rsync) > @$(call MESSAGE,"Syncing from source dir $(SRCDIR)") > + @mkdir -p $(@D) > $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep)) > @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) > rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D) > @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built:: > $(BUILD_DIR)/%/.stamp_host_installed: > @$(call step_start,install-host) > @$(call MESSAGE,"Installing to host directory") > + @mkdir -p $(HOST_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep)) > @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed: > $(BUILD_DIR)/%/.stamp_staging_installed: > @$(call step_start,install-staging) > @$(call MESSAGE,"Installing to staging directory") > + @mkdir -p $(STAGING_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_STAGING_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) > @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed: > # Install to images dir > $(BUILD_DIR)/%/.stamp_images_installed: > @$(call step_start,install-image) > + @mkdir -p $(BINARIES_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep)) > @$(call MESSAGE,"Installing to images directory") > +$($(PKG)_INSTALL_IMAGES_CMDS) > @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed: > $(BUILD_DIR)/%/.stamp_target_installed: > @$(call step_start,install-target) > @$(call MESSAGE,"Installing to target") > + @mkdir -p $(TARGET_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_TARGET_CMDS) > $(if $(BR2_INIT_SYSTEMD),\ > @@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD): $$($(2)_TARGET_CONFIGURE) > $(1)-configure: $$($(2)_TARGET_CONFIGURE) > $$($(2)_TARGET_CONFIGURE): | $$($(2)_FINAL_DEPENDENCIES) > > -$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare > +$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare > $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies > > ifeq ($$($(2)_OVERRIDE_SRCDIR),) > -- > 2.19.1 >
Thomas, On 11/14/18, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > In the current code, the creation of the main output folders > (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a > global "dirs" target. While this works fine in the current situation, > it doesn't work well in a context where per-package host and target > directories are used. > > For example, with the current code and per-package host directories, > the output/staging symbolic link ends up being created as a link to > the per-package package sysroot directory of the first package being > built, instead of the global sysroot. > > This commit reworks the creation of those directories by having the > package/pkg-generic.mk code ensure that the build directory, target > directory, host directory, staging directory and binaries directory > exist before they are needed. > > Two new targets, host-finalize and staging-finalize are added in the > main Makefile to create the compatibility symlinks for host and > staging directories. They will be extended later with additional logic > for per-package directories. > > Thanks to those changes, the global "dirs" target is entirely removed. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > --- > Makefile | 21 +++++++++------------ > fs/common.mk | 1 + > package/pkg-generic.mk | 7 ++++++- > 3 files changed, 16 insertions(+), 13 deletions(-) > > diff --git a/Makefile b/Makefile > index 2819d44124..e675ac26aa 100644 > --- a/Makefile > +++ b/Makefile > @@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\ > > endif > > -.PHONY: dirs > -dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \ > - $(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR) > - > $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) > $(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" > HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig > > @@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) > --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ > -C $(HOST_DIR) "." > > -# Populating the staging with the base directories is handled by the > skeleton package > -$(STAGING_DIR): > - @mkdir -p $(STAGING_DIR) > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > - With per package disabled, the staging directory never gets created early enough for packages to be able to reference content. Specifically I noticed docker couldn't find -lsystemd. I'm assuming it's because of the new target dependency on target-post-image? > RSYNC_VCS_EXCLUSIONS = \ > --exclude .svn --exclude .git --exclude .hg --exclude .bzr \ > --exclude CVS > @@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize > # Avoid the rootfs name leaking down the dependency chain > target-finalize: ROOTFS= > > +host-finalize: $(HOST_DIR_SYMLINK) > + > +.PHONY: staging-finalize > +staging-finalize: > + @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > + > .PHONY: target-finalize > -target-finalize: $(PACKAGES) > +target-finalize: $(PACKAGES) host-finalize > @$(call MESSAGE,"Finalizing target directory") > # Check files that are touched by more than one package > ./support/scripts/check-uniq-files -t target > $(BUILD_DIR)/packages-file-list.txt > @@ -782,7 +779,7 @@ endif > touch $(TARGET_DIR)/usr > > .PHONY: target-post-image > -target-post-image: $(TARGETS_ROOTFS) target-finalize > +target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize > @rm -f $(ROOTFS_COMMON_TAR) > @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \ > $(call MESSAGE,"Executing post-image script $(s)"); \ > @@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR) > @cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config > > .PHONY: legal-info > -legal-info: dirs legal-info-clean legal-info-prepare $(foreach > p,$(PACKAGES),$(p)-all-legal-info) \ > +legal-info: legal-info-clean legal-info-prepare $(foreach > p,$(PACKAGES),$(p)-all-legal-info) \ > $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) > @cat support/legal-info/README.header >>$(LEGAL_REPORT) > @if [ -r $(LEGAL_WARNINGS) ]; then \ > diff --git a/fs/common.mk b/fs/common.mk > index 2a5a202a89..96658428ba 100644 > --- a/fs/common.mk > +++ b/fs/common.mk > @@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) > $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot > $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) > @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") > + mkdir -p $$(@D) > rm -rf $$(ROOTFS_$(2)_DIR) > mkdir -p $$(ROOTFS_$(2)_DIR) > echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk > index f34f46afc8..309fd8cd48 100644 > --- a/package/pkg-generic.mk > +++ b/package/pkg-generic.mk > @@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted: > $(BUILD_DIR)/%/.stamp_rsynced: > @$(call step_start,rsync) > @$(call MESSAGE,"Syncing from source dir $(SRCDIR)") > + @mkdir -p $(@D) > $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep)) > @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) > rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) > $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D) > @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built:: > $(BUILD_DIR)/%/.stamp_host_installed: > @$(call step_start,install-host) > @$(call MESSAGE,"Installing to host directory") > + @mkdir -p $(HOST_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep)) > @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed: > $(BUILD_DIR)/%/.stamp_staging_installed: > @$(call step_start,install-staging) > @$(call MESSAGE,"Installing to staging directory") > + @mkdir -p $(STAGING_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_STAGING_CMDS) > $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call > $(hook))$(sep)) > @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed: > # Install to images dir > $(BUILD_DIR)/%/.stamp_images_installed: > @$(call step_start,install-image) > + @mkdir -p $(BINARIES_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep)) > @$(call MESSAGE,"Installing to images directory") > +$($(PKG)_INSTALL_IMAGES_CMDS) > @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed: > $(BUILD_DIR)/%/.stamp_target_installed: > @$(call step_start,install-target) > @$(call MESSAGE,"Installing to target") > + @mkdir -p $(TARGET_DIR) > $(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep)) > +$($(PKG)_INSTALL_TARGET_CMDS) > $(if $(BR2_INIT_SYSTEMD),\ > @@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD): $$($(2)_TARGET_CONFIGURE) > $(1)-configure: $$($(2)_TARGET_CONFIGURE) > $$($(2)_TARGET_CONFIGURE): | $$($(2)_FINAL_DEPENDENCIES) > > -$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare > +$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare > $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies > > ifeq ($$($(2)_OVERRIDE_SRCDIR),) > -- > 2.19.1 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot >
Hello, On Thu, 15 Nov 2018 22:09:46 +0100, Yann E. MORIN wrote: > I don't think we do need anything special to create HOST_DIR, > STAGING_DIR and TARGET_DIR: > > - host-skeleton is the first package to run, and that one does ensure > the existence of $(HOST_DIR) by the mere act of creating it (which > it cirrently does not do, but should) > > - skeleton is the first package to install in target/ and staging, and > currently the skeleton-custom and skeleton-init-common should both > create target/ and staging/ So, what you suggest is to remove the following hunks from my patch ? @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built:: $(BUILD_DIR)/%/.stamp_host_installed: @$(call step_start,install-host) @$(call MESSAGE,"Installing to host directory") + @mkdir -p $(HOST_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep)) @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed: $(BUILD_DIR)/%/.stamp_staging_installed: @$(call step_start,install-staging) @$(call MESSAGE,"Installing to staging directory") + @mkdir -p $(STAGING_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed: # Install to images dir $(BUILD_DIR)/%/.stamp_images_installed: @$(call step_start,install-image) + @mkdir -p $(BINARIES_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep)) @$(call MESSAGE,"Installing to images directory") +$($(PKG)_INSTALL_IMAGES_CMDS) @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed: $(BUILD_DIR)/%/.stamp_target_installed: @$(call step_start,install-target) @$(call MESSAGE,"Installing to target") + @mkdir -p $(TARGET_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_TARGET_CMDS) $(if $(BR2_INIT_SYSTEMD),\ Is this what you mean ? > > +host-finalize: $(HOST_DIR_SYMLINK) > > + > > +.PHONY: staging-finalize > > +staging-finalize: > > + @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > > + > > .PHONY: target-finalize > > -target-finalize: $(PACKAGES) > > +target-finalize: $(PACKAGES) host-finalize > > Shouldn't we ensure that host-finalize be done before staging-finalize > too? Why so ? > We would also need to ensure that packages are all installed before > running staging-finalize. > > So,=maybe we need to add: > > staging-finalize: $(PACKAGES) host-finalize Same, we don't really need all packages to be installed just to create that dummy symlink. In fact, maybe I should go one step further: STAGING_DIR is part of HOST_DIR, so maybe I should simply create this compatibility "staging" symlink as part of "host-finalize". I.e: STAGING_DIR_SYMLINK := $(BASE_DIR)/staging $(STAGING_DIR_SYMLINK): $(BASE_DIR) @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging host-finalize: $(HOST_DIR_SYMLINK) $(STAGING_DIR_SYMLINK What do you think about this idea ? Thanks, Thomas
Hello Matt, Thanks for the feedback, and for testing the patch series. On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote: > > -# Populating the staging with the base directories is handled by the > > skeleton package > > -$(STAGING_DIR): > > - @mkdir -p $(STAGING_DIR) > > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > > - > > With per package disabled, the staging directory never gets created > early enough for packages to be able to reference content. > Specifically I noticed docker couldn't find -lsystemd. I'm assuming > it's because of the new target dependency on target-post-image? The STAGING_DIR itself is definitely created at the beginning of the build, at the first package installing something into it. However, with this patch, the $(BASE_DIR)/staging symlink indeed gets created at the end of the build rather than at the beginning of the build. However, this symlink is only a compatibility one. The code should use $(STAGING_DIR) everywhere, which should not depend on this "staging" compatibility symlink. I've started a build here to reproduce the issue, but in general it would be nice when you report issues to include an example defconfig that exhibits the problem, as well as the build log. It would be a lot easier to understand the specifics of the problem, and be able to reproduce it. Thanks, Thomas
Hello, On Fri, 16 Nov 2018 15:15:36 +0100, Thomas Petazzoni wrote: > On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote: > > > > -# Populating the staging with the base directories is handled by the > > > skeleton package > > > -$(STAGING_DIR): > > > - @mkdir -p $(STAGING_DIR) > > > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > > > - > > > > With per package disabled, the staging directory never gets created > > early enough for packages to be able to reference content. > > Specifically I noticed docker couldn't find -lsystemd. I'm assuming > > it's because of the new target dependency on target-post-image? > > The STAGING_DIR itself is definitely created at the beginning of the > build, at the first package installing something into it. > > However, with this patch, the $(BASE_DIR)/staging symlink indeed gets > created at the end of the build rather than at the beginning of the > build. However, this symlink is only a compatibility one. The code > should use $(STAGING_DIR) everywhere, which should not depend on this > "staging" compatibility symlink. > > I've started a build here to reproduce the issue, but in general it > would be nice when you report issues to include an example defconfig > that exhibits the problem, as well as the build log. It would be a lot > easier to understand the specifics of the problem, and be able to > reproduce it. FWIW, the following defconfig: BR2_arm=y BR2_cortex_a8=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y BR2_INIT_SYSTEMD=y BR2_SYSTEM_BIN_SH_NONE=y # BR2_PACKAGE_BUSYBOX is not set BR2_PACKAGE_DOCKER_ENGINE=y # BR2_TARGET_ROOTFS_TAR is not set with ppsh-v4, but BR2_PER_PACKAGE_FOLDERS disabled, builds just fine, and dockerd is linked with libsystemd. Could you give more details about the issue you're seeing ? Thanks, Thomas
Thomas, On Fri, Nov 16, 2018 at 9:14 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Fri, 16 Nov 2018 15:15:36 +0100, Thomas Petazzoni wrote: > > > On Thu, 15 Nov 2018 19:21:27 -0600, Matthew Weber wrote: > > > > > > -# Populating the staging with the base directories is handled by the > > > > skeleton package > > > > -$(STAGING_DIR): > > > > - @mkdir -p $(STAGING_DIR) > > > > - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging > > > > - > > > > > > With per package disabled, the staging directory never gets created > > > early enough for packages to be able to reference content. > > > Specifically I noticed docker couldn't find -lsystemd. I'm assuming > > > it's because of the new target dependency on target-post-image? > > > > The STAGING_DIR itself is definitely created at the beginning of the > > build, at the first package installing something into it. > > > > However, with this patch, the $(BASE_DIR)/staging symlink indeed gets > > created at the end of the build rather than at the beginning of the > > build. However, this symlink is only a compatibility one. The code > > should use $(STAGING_DIR) everywhere, which should not depend on this > > "staging" compatibility symlink. > > > > I've started a build here to reproduce the issue, but in general it > > would be nice when you report issues to include an example defconfig > > that exhibits the problem, as well as the build log. It would be a lot > > easier to understand the specifics of the problem, and be able to > > reproduce it. > > FWIW, the following defconfig: > > BR2_arm=y > BR2_cortex_a8=y > BR2_TOOLCHAIN_EXTERNAL=y > BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM=y > BR2_INIT_SYSTEMD=y > BR2_SYSTEM_BIN_SH_NONE=y > # BR2_PACKAGE_BUSYBOX is not set > BR2_PACKAGE_DOCKER_ENGINE= > # BR2_TARGET_ROOTFS_TAR is not set My testing was against next with ppsh-v4 applied. BR2_aarch64=y BR2_TOOLCHAIN_EXTERNAL=y BR2_INIT_SYSTEMD=y BR2_PACKAGE_DOCKER_ENGINE=y BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT=y # BR2_TARGET_ROOTFS_TAR is not set Looking at it a bit further, the "go build" linking fails when the BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT is set. Since there is a pending patch for the docker package, I built with the current "next" that doesn't have [1] and also with it applied. I observed that [1] fixed the static client build. When they broke out the client from the daemon the linking approach must have changed. I guess what I observed doesn't rule out a "go" linker issue. I'll see if I can get it to dump the link search path. [1] https://patchwork.ozlabs.org/patch/994635/ Matt
Hi Thomas, Matthew, Matthew Weber <matthew.weber@rockwellcollins.com> writes: > BR2_aarch64=y > BR2_TOOLCHAIN_EXTERNAL=y > BR2_INIT_SYSTEMD=y > BR2_PACKAGE_DOCKER_ENGINE=y > BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT=y > # BR2_TARGET_ROOTFS_TAR is not set > > Looking at it a bit further, the "go build" linking fails when the > BR2_PACKAGE_DOCKER_ENGINE_STATIC_CLIENT is set. Since there is a > pending patch for the docker package, I built with the current "next" > that doesn't have [1] and also with it applied. I observed that [1] > fixed the static client build. When they broke out the client from > the daemon the linking approach must have changed. The CLI is linked statically correctly with the new patch applied, which also splits the CLI and daemon into separate patches and upgrades Docker to the latest stable. I'll submit the most recent revision of the patch tonight, after finishing testing the new release with an odroid xu4, bananapi m1+, and a pi 0. Best, Christian
diff --git a/Makefile b/Makefile index 2819d44124..e675ac26aa 100644 --- a/Makefile +++ b/Makefile @@ -572,10 +572,6 @@ $(foreach pkg,$(call UPPERCASE,$(PACKAGES)),\ endif -.PHONY: dirs -dirs: $(BUILD_DIR) $(STAGING_DIR) $(BASE_TARGET_DIR) \ - $(HOST_DIR) $(HOST_DIR_SYMLINK) $(BINARIES_DIR) - $(BUILD_DIR)/buildroot-config/auto.conf: $(BR2_CONFIG) $(MAKE1) $(EXTRAMAKEARGS) HOSTCC="$(HOSTCC_NOCCACHE)" HOSTCXX="$(HOSTCXX_NOCCACHE)" syncconfig @@ -605,11 +601,6 @@ sdk: prepare-sdk $(BR2_TAR_HOST_DEPENDENCY) --transform='s#^\.#$(BR2_SDK_PREFIX)#' \ -C $(HOST_DIR) "." -# Populating the staging with the base directories is handled by the skeleton package -$(STAGING_DIR): - @mkdir -p $(STAGING_DIR) - @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging - RSYNC_VCS_EXCLUSIONS = \ --exclude .svn --exclude .git --exclude .hg --exclude .bzr \ --exclude CVS @@ -710,8 +701,14 @@ $(TARGETS_ROOTFS): target-finalize # Avoid the rootfs name leaking down the dependency chain target-finalize: ROOTFS= +host-finalize: $(HOST_DIR_SYMLINK) + +.PHONY: staging-finalize +staging-finalize: + @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging + .PHONY: target-finalize -target-finalize: $(PACKAGES) +target-finalize: $(PACKAGES) host-finalize @$(call MESSAGE,"Finalizing target directory") # Check files that are touched by more than one package ./support/scripts/check-uniq-files -t target $(BUILD_DIR)/packages-file-list.txt @@ -782,7 +779,7 @@ endif touch $(TARGET_DIR)/usr .PHONY: target-post-image -target-post-image: $(TARGETS_ROOTFS) target-finalize +target-post-image: $(TARGETS_ROOTFS) target-finalize staging-finalize @rm -f $(ROOTFS_COMMON_TAR) @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \ $(call MESSAGE,"Executing post-image script $(s)"); \ @@ -811,7 +808,7 @@ legal-info-prepare: $(LEGAL_INFO_DIR) @cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config .PHONY: legal-info -legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \ +legal-info: legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p)-all-legal-info) \ $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) @cat support/legal-info/README.header >>$(LEGAL_REPORT) @if [ -r $(LEGAL_WARNINGS) ]; then \ diff --git a/fs/common.mk b/fs/common.mk index 2a5a202a89..96658428ba 100644 --- a/fs/common.mk +++ b/fs/common.mk @@ -145,6 +145,7 @@ $$(BINARIES_DIR)/rootfs.$(1): ROOTFS=$(2) $$(BINARIES_DIR)/rootfs.$(1): FAKEROOT_SCRIPT=$$(ROOTFS_$(2)_DIR)/fakeroot $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") + mkdir -p $$(@D) rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk index f34f46afc8..309fd8cd48 100644 --- a/package/pkg-generic.mk +++ b/package/pkg-generic.mk @@ -173,6 +173,7 @@ $(BUILD_DIR)/%/.stamp_extracted: $(BUILD_DIR)/%/.stamp_rsynced: @$(call step_start,rsync) @$(call MESSAGE,"Syncing from source dir $(SRCDIR)") + @mkdir -p $(@D) $(foreach hook,$($(PKG)_PRE_RSYNC_HOOKS),$(call $(hook))$(sep)) @test -d $(SRCDIR) || (echo "ERROR: $(SRCDIR) does not exist" ; exit 1) rsync -au --chmod=u=rwX,go=rX $(RSYNC_VCS_EXCLUSIONS) $($(PKG)_OVERRIDE_SRCDIR_RSYNC_EXCLUSIONS) $(call qstrip,$(SRCDIR))/ $(@D) @@ -238,6 +239,7 @@ $(BUILD_DIR)/%/.stamp_built:: $(BUILD_DIR)/%/.stamp_host_installed: @$(call step_start,install-host) @$(call MESSAGE,"Installing to host directory") + @mkdir -p $(HOST_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep)) @@ -267,6 +269,7 @@ $(BUILD_DIR)/%/.stamp_host_installed: $(BUILD_DIR)/%/.stamp_staging_installed: @$(call step_start,install-staging) @$(call MESSAGE,"Installing to staging directory") + @mkdir -p $(STAGING_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_STAGING_CMDS) $(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep)) @@ -298,6 +301,7 @@ $(BUILD_DIR)/%/.stamp_staging_installed: # Install to images dir $(BUILD_DIR)/%/.stamp_images_installed: @$(call step_start,install-image) + @mkdir -p $(BINARIES_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_IMAGES_HOOKS),$(call $(hook))$(sep)) @$(call MESSAGE,"Installing to images directory") +$($(PKG)_INSTALL_IMAGES_CMDS) @@ -309,6 +313,7 @@ $(BUILD_DIR)/%/.stamp_images_installed: $(BUILD_DIR)/%/.stamp_target_installed: @$(call step_start,install-target) @$(call MESSAGE,"Installing to target") + @mkdir -p $(TARGET_DIR) $(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep)) +$($(PKG)_INSTALL_TARGET_CMDS) $(if $(BR2_INIT_SYSTEMD),\ @@ -735,7 +740,7 @@ $$($(2)_TARGET_BUILD): $$($(2)_TARGET_CONFIGURE) $(1)-configure: $$($(2)_TARGET_CONFIGURE) $$($(2)_TARGET_CONFIGURE): | $$($(2)_FINAL_DEPENDENCIES) -$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare +$$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | prepare $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies ifeq ($$($(2)_OVERRIDE_SRCDIR),)
In the current code, the creation of the main output folders (BUILD_DIR, STAGING_DIR, HOST_DIR, TARGET_DIR, etc.) is done by a global "dirs" target. While this works fine in the current situation, it doesn't work well in a context where per-package host and target directories are used. For example, with the current code and per-package host directories, the output/staging symbolic link ends up being created as a link to the per-package package sysroot directory of the first package being built, instead of the global sysroot. This commit reworks the creation of those directories by having the package/pkg-generic.mk code ensure that the build directory, target directory, host directory, staging directory and binaries directory exist before they are needed. Two new targets, host-finalize and staging-finalize are added in the main Makefile to create the compatibility symlinks for host and staging directories. They will be extended later with additional logic for per-package directories. Thanks to those changes, the global "dirs" target is entirely removed. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> --- Makefile | 21 +++++++++------------ fs/common.mk | 1 + package/pkg-generic.mk | 7 ++++++- 3 files changed, 16 insertions(+), 13 deletions(-)