From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH next v5 7/9] core: implement per-package SDK and target
Date: Tue, 20 Nov 2018 22:36:12 +0100 [thread overview]
Message-ID: <20181120213612.GL2601@scaer> (raw)
In-Reply-To: <20181120163522.4281-8-thomas.petazzoni@bootlin.com>
Thomas, All,
On 2018-11-20 17:35 +0100, Thomas Petazzoni spake thusly:
> This commit implemnts the core of the move to per-package SDK and
> target directories. The main idea is that instead of having a global
> output/host and output/target in which all packages install files, we
> switch to per-package host and target folders, that only contain their
> explicit dependencies.
[--SNIP--]
> output/per-package/busybox/target
> output/per-package/busybox/host
> output/per-package/host-fakeroot/target
> output/per-package/host-fakeroot/host
I'm not fond of the naming here...
In the end, can we expect to have a layout that would look like:
output/build/busybox/host/
output/build/busybox/target/
output/build/busybox/busybox-1.2.3/ # Source tree, and
# currently, build dir
And then we can add for OOT:
output/build/busybox/build-dir/
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[--SNIP--]
> diff --git a/Makefile b/Makefile
> index 23032988a5..f3c0e2326e 100644
> --- a/Makefile
> +++ b/Makefile
>@@ -207,7 +207,8 @@ BINARIES_DIR := $(BASE_DIR)/images
> # The target directory is common to all packages,
> # but there is one that is specific to each filesystem.
> BASE_TARGET_DIR := $(BASE_DIR)/target
>-TARGET_DIR = $(if
>$(ROOTFS),$(ROOTFS_$(ROOTFS)_TARGET_DIR),$(BASE_TARGET_DIR))
>+PER_PACKAGE_DIR := $(BASE_DIR)/per-package
As I said above, I'm not too fond of that naming.... I have nothing
better to suggest for now, except that we can change that later (enough
bike-shedding).
> # initial definition so that 'make clean' works for most users, even
> # without
> # .config. HOST_DIR will be overwritten later when .config is included.
> HOST_DIR := $(BASE_DIR)/host
> @@ -230,6 +231,7 @@ ifeq ($(filter $(noconfig_targets),$(MAKECMDGOALS)),)
> -include $(BR2_CONFIG)
> endif
>
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),)
> # Parallel execution of this Makefile is disabled because it changes
> # the packages building order, that can be a problem for two reasons:
> # - If a package has an unspecified optional dependency and that
> @@ -245,6 +247,13 @@ endif
> # use the -j<jobs> option when building, e.g:
> # make -j$((`getconf _NPROCESSORS_ONLN`+1))
> .NOTPARALLEL:
> +endif
I would have postponed parallel build activation to a separate commit.
> @@ -455,7 +464,11 @@ LZCAT := $(call qstrip,$(BR2_LZCAT))
> TAR_OPTIONS = $(call qstrip,$(BR2_TAR_OPTIONS)) -xf
>
> # packages compiled for the host go here
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
> +HOST_DIR = $(if $(PKG),$(PER_PACKAGE_DIR)/$($(PKG)_NAME)/host,$(call qstrip,$(BR2_HOST_DIR)))
> +else
> HOST_DIR := $(call qstrip,$(BR2_HOST_DIR))
Why do we still have HOST_DIR as immediately-defined?
Since HOST_DIR is simply defined in the per-package case, I see no
reason to have immediately-defined in the standard case either.
> +endif
>
> ifneq ($(HOST_DIR),$(BASE_DIR)/host)
> HOST_DIR_SYMLINK = $(BASE_DIR)/host
> @@ -701,14 +714,28 @@ $(TARGETS_ROOTFS): target-finalize
> # Avoid the rootfs name leaking down the dependency chain
> target-finalize: ROOTFS=
>
> -host-finalize: $(HOST_DIR_SYMLINK)
> +host-finalize: $(PACKAGES) $(HOST_DIR) $(HOST_DIR_SYMLINK)
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
> + @$(call MESSAGE,"Creating global host directory")
> + $(foreach pkg,$(PACKAGES),\
Please, sort PACKAGES, so that:
- the copy is always done in the same order (sort always sort in the
ascii order);
- duplicates get removed.
> + rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> + $(PER_PACKAGE_DIR)/$(pkg)/host/ \
> + $(HOST_DIR)$(sep))
> +endif
>
> .PHONY: staging-finalize
> staging-finalize:
> @ln -snf $(STAGING_DIR) $(BASE_DIR)/staging
>
> .PHONY: target-finalize
> -target-finalize: $(PACKAGES) host-finalize
> +target-finalize: $(PACKAGES) $(TARGET_DIR) host-finalize
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
> + @$(call MESSAGE,"Creating global target directory")
> + $(foreach pkg,$(PACKAGES),\
Ditto, sort PACKAGES.
> + rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
> + $(PER_PACKAGE_DIR)/$(pkg)/target/ \
> + $(TARGET_DIR)$(sep))
> +endif
> @$(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
> @@ -972,7 +999,7 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf prepare-kconfig
>
> # staging and target directories do NOT list these as
> # dependencies anywhere else
> -$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST):
> +$(BUILD_DIR) $(BASE_TARGET_DIR) $(HOST_DIR) $(BINARIES_DIR) $(LEGAL_INFO_DIR) $(REDIST_SOURCES_DIR_TARGET) $(REDIST_SOURCES_DIR_HOST) $(PER_PACKAGE_DIR):
Can we now split this long line, please? ;-)
> diff --git a/fs/common.mk b/fs/common.mk
> index 96658428ba..fc4be3cc05 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -172,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)
If I am not mistaken, this is now in master:
https://git.buildroot.org/buildroot/commit/?id=305e4487e5c18ed89bf2aa106b2068f9dce686fb
But that's missing from next, indeed.
> endif
>
> # Check for legacy POST_TARGETS rules
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9b4db845b6..884b868a9a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -98,7 +98,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += check_bin_arch
> # have a proper DT_RPATH or DT_RUNPATH tag
> define check_host_rpath
> $(if $(filter install-host,$(2)),\
> - $(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR)))
> + $(if $(filter end,$(1)),support/scripts/check-host-rpath $(3) $(HOST_DIR) $(PER_PACKAGE_DIR)))
> endef
> GLOBAL_INSTRUMENTATION_HOOKS += check_host_rpath
>
> @@ -126,6 +126,21 @@ endef
> GLOBAL_INSTRUMENTATION_HOOKS += step_user
> endif
>
> +# $1: deps
> +ifeq ($(BR2_PER_PACKAGE_FOLDERS),y)
> +define prepare-per-package-folder
The location where you define this macro, in the instrumentation hooks,
is not optimum I think. You even did not provide a help-comment for it.
> + mkdir -p $(HOST_DIR) $(TARGET_DIR)
> + $(foreach pkg,$(1),\
> + rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
> + $(PER_PACKAGE_DIR)/$(pkg)/host/ \
> + $(HOST_DIR)$(sep))
> + $(foreach pkg,$(1),\
> + rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
> + $(PER_PACKAGE_DIR)/$(pkg)/target/ \
> + $(TARGET_DIR)$(sep))
Why do you need two 'foreach' calls? Can't the following work?
$(foreach pkg,$(1),\
rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/host/ \
$(PER_PACKAGE_DIR)/$(pkg)/host/ \
$(HOST_DIR)$(sep) \
rsync -a --link-dest=$(PER_PACKAGE_DIR)/$(pkg)/target/ \
$(PER_PACKAGE_DIR)/$(pkg)/target/ \
$(TARGET_DIR)$(sep))
Regards,
Yann E. MORIN.
> +endef
> +endif
> +
> ################################################################################
> # Implicit targets -- produce a stamp file for each step of a package build
> ################################################################################
> @@ -133,6 +148,7 @@ endif
> # Retrieve the archive
> $(BUILD_DIR)/%/.stamp_downloaded:
> @$(call step_start,download)
> + $(call prepare-per-package-folder,$($(PKG)_FINAL_DOWNLOAD_DEPENDENCIES))
> $(foreach hook,$($(PKG)_PRE_DOWNLOAD_HOOKS),$(call $(hook))$(sep))
> # Only show the download message if it isn't already downloaded
> $(Q)for p in $($(PKG)_ALL_DOWNLOADS); do \
> @@ -159,6 +175,7 @@ $(BUILD_DIR)/%/.stamp_actual_downloaded:
> $(BUILD_DIR)/%/.stamp_extracted:
> @$(call step_start,extract)
> @$(call MESSAGE,"Extracting")
> + $(call prepare-per-package-folder,$($(PKG)_FINAL_EXTRACT_DEPENDENCIES))
> $(foreach hook,$($(PKG)_PRE_EXTRACT_HOOKS),$(call $(hook))$(sep))
> $(Q)mkdir -p $(@D)
> $($(PKG)_EXTRACT_CMDS)
> @@ -219,6 +236,7 @@ $(foreach dir,$(call qstrip,$(BR2_GLOBAL_PATCH_DIR)),\
> $(BUILD_DIR)/%/.stamp_configured:
> @$(call step_start,configure)
> @$(call MESSAGE,"Configuring")
> + $(call prepare-per-package-folder,$($(PKG)_FINAL_DEPENDENCIES))
> $(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
> $($(PKG)_CONFIGURE_CMDS)
> $(foreach hook,$($(PKG)_POST_CONFIGURE_HOOKS),$(call $(hook))$(sep))
> diff --git a/support/scripts/check-host-rpath b/support/scripts/check-host-rpath
> index 6c5767da05..787a1763b0 100755
> --- a/support/scripts/check-host-rpath
> +++ b/support/scripts/check-host-rpath
> @@ -11,6 +11,7 @@ export LC_ALL=C
> main() {
> local pkg="${1}"
> local hostdir="${2}"
> + local perpackagedir="${3}"
> local file ret
>
> # Remove duplicate and trailing '/' for proper match
> @@ -20,7 +21,7 @@ main() {
> while read file; do
> is_elf "${file}" || continue
> elf_needs_rpath "${file}" "${hostdir}" || continue
> - check_elf_has_rpath "${file}" "${hostdir}" && continue
> + check_elf_has_rpath "${file}" "${hostdir}" "${perpackagedir}" && continue
> if [ ${ret} -eq 0 ]; then
> ret=1
> printf "***\n"
> @@ -57,6 +58,7 @@ elf_needs_rpath() {
> check_elf_has_rpath() {
> local file="${1}"
> local hostdir="${2}"
> + local perpackagedir="${3}"
> local rpath dir
>
> while read rpath; do
> @@ -65,6 +67,7 @@ check_elf_has_rpath() {
> dir="$( sed -r -e 's:/+:/:g; s:/$::;' <<<"${dir}" )"
> [ "${dir}" = "${hostdir}/lib" ] && return 0
> [ "${dir}" = "\$ORIGIN/../lib" ] && return 0
> + [[ ${dir} =~ ${perpackagedir}/[^/]*/host/lib ]] && return 0
> done
> done < <( readelf -d "${file}" \
> |sed -r -e '/.* \(R(UN)?PATH\) +Library r(un)?path: \[(.+)\]$/!d' \
> --
> 2.19.1
>
--
.-----------------.--------------------.------------------.--------------------.
| 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. |
'------------------------------^-------^------------------^--------------------'
next prev parent reply other threads:[~2018-11-20 21:36 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-20 16:35 [Buildroot] [PATCH next v5 0/9] Per-package host/target directory support Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 1/9] Makefile: evaluate CCACHE and HOST{CC, CXX} at time of use Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 2/9] support/scripts/check-host-rpath: split condition on two statements Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 3/9] Makefile: rework main directory creation logic Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 4/9] Makefile: move .NOTPARALLEL statement after including .config file Thomas Petazzoni
2018-11-20 20:18 ` Yann E. MORIN
2018-11-21 13:44 ` Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 5/9] Makefile: define TARGET_DIR_WARNING_FILE relative to TARGET_DIR Thomas Petazzoni
2018-11-20 20:32 ` Yann E. MORIN
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 6/9] package/pkg-generic: adjust config scripts tweaks for per-package folders Thomas Petazzoni
2018-11-20 20:53 ` Yann E. MORIN
2018-11-23 12:46 ` Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 7/9] core: implement per-package SDK and target Thomas Petazzoni
2018-11-20 21:36 ` Yann E. MORIN [this message]
2018-11-20 23:32 ` Arnout Vandecappelle
2018-11-23 13:25 ` Thomas Petazzoni
2018-11-23 15:44 ` Arnout Vandecappelle
2018-11-23 13:14 ` Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 8/9] package/pkg-generic: make libtool .la files compatible with per-package folders Thomas Petazzoni
2018-11-20 16:35 ` [Buildroot] [PATCH next v5 9/9] package/pkg-kconfig: handle KCONFIG_DEPENDENCIES " Thomas Petazzoni
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181120213612.GL2601@scaer \
--to=yann.morin.1998@free.fr \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox