All of lore.kernel.org
 help / color / mirror / Atom feed
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.  |
'------------------------------^-------^------------------^--------------------'

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.