Re: [PATCH 4/8] isar-bootstrap/buildchroot/sdkchroot: refactor PF and WORKDIR

[Claudius Heine <claudius.heine.ext@siemens.com>]

Hi Andreas,

On 16/04/2019 15.43, Andreas Reichel wrote:
> On Tue, Apr 16, 2019 at 03:26:42PM +0200, [ext] claudius.heine.ext@siemens.com wrote:
>> From: Claudius Heine <ch@denx.de>
>>
>> Each of those are dependent on both HOST_ARCH and DISTRO_ARCH, so the
>> stamps etc. should not be shared.
>>
>> Signed-off-by: Claudius Heine <ch@denx.de>
>> ---
>>   meta/classes/image-sdk-extension.bbclass              |  2 +-
>>   meta/conf/bitbake.conf                                |  4 ++--
>>   .../isar-bootstrap/isar-bootstrap-host.bb             | 11 +++--------
>>   .../isar-bootstrap/isar-bootstrap-target.bb           |  6 +-----
>>   meta/recipes-devtools/buildchroot/buildchroot-host.bb |  1 +
>>   meta/recipes-devtools/buildchroot/buildchroot.inc     |  3 +--
>>   meta/recipes-devtools/sdkchroot/sdkchroot.bb          |  3 +--
>>   7 files changed, 10 insertions(+), 20 deletions(-)
>>
>> diff --git a/meta/classes/image-sdk-extension.bbclass b/meta/classes/image-sdk-extension.bbclass
>> index 1838080..8df3ed1 100644
>> --- a/meta/classes/image-sdk-extension.bbclass
>> +++ b/meta/classes/image-sdk-extension.bbclass
>> @@ -5,7 +5,7 @@
>>   #
>>   # This class extends the image.bbclass to supply the creation of a sdk
>>   
>> -SDKCHROOT_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/sdkchroot-${HOST_DISTRO}-${HOST_ARCH}"
>> +SDKCHROOT_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/sdkchroot-${HOST_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}"
>>   
>>   do_populate_sdk[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>>   do_populate_sdk[depends] = "sdkchroot:do_build"
>> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>> index 0e521bb..7c1c89f 100644
>> --- a/meta/conf/bitbake.conf
>> +++ b/meta/conf/bitbake.conf
>> @@ -50,8 +50,8 @@ DEPLOY_DIR_BOOTSTRAP = "${DEPLOY_DIR}/bootstrap"
>>   DEPLOY_DIR_IMAGE = "${DEPLOY_DIR}/images/${MACHINE}"
>>   DL_DIR ?= "${TOPDIR}/downloads"
>>   SSTATE_DIR ?= "${TMPDIR}/sstate-cache"
>> -BUILDCHROOT_HOST_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot-host/rootfs"
>> -BUILDCHROOT_TARGET_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot-target/rootfs"
>> +BUILDCHROOT_HOST_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot-host-${DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}/rootfs"
>> +BUILDCHROOT_TARGET_DIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot-target-${DISTRO}-${DISTRO_ARCH}/rootfs"
> 
> maybe first define a variable for ${DISTRO}-${DISTRO_ARCH} and then
> prepend this with ${HOST_ARCH}- where needed makes the code more
> readable.

I understand your point, but this is the 'bitbake.conf' so any variable 
will be available everywhere. So I would like to avoid creating any 
helper variables that are of limited use anywhere else and create mostly 
noise.

Also changing the order would lead to more inconsistencies IMO.
Everywhere the 'DISTRO' code word is named first, then followed by the 
native architecture of that root file system. This change just adds the 
architecture of the target machine to the last place for the 
cross-tool-chain.

Thanks,
Claudius

> 
>>   CACHE = "${TMPDIR}/cache"
>>   
>>   OVERRIDES = "${DISTRO_ARCH}:${MACHINE}:${DISTRO}:forcevariable"
>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>> index 08b068f..6d0f5f5 100644
>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-host.bb
>> @@ -6,17 +6,13 @@
>>   # SPDX-License-Identifier: MIT
>>   
>>   Description = "Minimal host Debian root file system"
>> +PF = "${PN}-${HOST_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}"
>>   
>> -WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${HOST_DISTRO}-${HOST_ARCH}"
>> -DEPLOY_ISAR_BOOTSTRAP = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}"
>> -ISAR_BOOTSTRAP_LOCK = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}.lock"
>> +DEPLOY_ISAR_BOOTSTRAP = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}"
>> +ISAR_BOOTSTRAP_LOCK = "${DEPLOY_DIR_BOOTSTRAP}/${HOST_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}.lock"
>>   
> 
> Same here
> 
>>   require isar-bootstrap.inc
>> -inherit isar-bootstrap-helper
>>   
>> -do_generate_keyring[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> -
>> -do_apt_config_prepare[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>   do_apt_config_prepare[dirs] = "${WORKDIR}"
>>   do_apt_config_prepare[vardeps] += "\
>>                                      APTPREFS \
>> @@ -47,7 +43,6 @@ addtask apt_config_prepare before do_bootstrap after do_unpack
>>   
>>   OVERRIDES_append = ":${@get_distro_needs_https_support(d, True)}"
>>   
>> -do_bootstrap[stamp-extra-info] = "${HOST_DISTRO}-${HOST_ARCH}"
>>   do_bootstrap[vardeps] += "HOST_DISTRO_APT_SOURCES"
>>   do_bootstrap() {
>>       isar_bootstrap --host
>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb b/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
>> index 79f3e34..552a317 100644
>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap-target.bb
>> @@ -6,16 +6,13 @@
>>   # SPDX-License-Identifier: MIT
>>   
>>   Description = "Minimal target Debian root file system"
>> +PF = "${PN}-${DISTRO}-${DISTRO_ARCH}"
>>   
>> -WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>   DEPLOY_ISAR_BOOTSTRAP = "${DEPLOY_DIR_BOOTSTRAP}/${DISTRO}-${DISTRO_ARCH}"
>>   ISAR_BOOTSTRAP_LOCK = "${DEPLOY_DIR_BOOTSTRAP}/${DISTRO}-${DISTRO_ARCH}.lock"
>>   
>>   require isar-bootstrap.inc
>>   
>> -do_generate_keyring[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>> -
>> -do_apt_config_prepare[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>   do_apt_config_prepare[dirs] = "${WORKDIR}"
>>   do_apt_config_prepare[vardeps] += "\
>>                                      APTPREFS \
>> @@ -44,7 +41,6 @@ addtask apt_config_prepare before do_bootstrap after do_unpack
>>   
>>   OVERRIDES_append = ":${@get_distro_needs_https_support(d, False)}"
>>   
>> -do_bootstrap[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>   do_bootstrap[vardeps] += "DISTRO_APT_SOURCES"
>>   do_bootstrap() {
>>       isar_bootstrap
>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot-host.bb b/meta/recipes-devtools/buildchroot/buildchroot-host.bb
>> index c8c10c0..e8f7f69 100644
>> --- a/meta/recipes-devtools/buildchroot/buildchroot-host.bb
>> +++ b/meta/recipes-devtools/buildchroot/buildchroot-host.bb
>> @@ -6,6 +6,7 @@
>>   DESCRIPTION = "Isar development filesystem for host"
>>   
>>   require buildchroot.inc
>> +PF = "${PN}-${HOST_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}"
>>   
>>   BUILDCHROOT_PREINSTALL ?= "make \
>>                              debhelper \
>> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
>> index 764bc5e..cdb4c07 100644
>> --- a/meta/recipes-devtools/buildchroot/buildchroot.inc
>> +++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
>> @@ -12,13 +12,12 @@ SRC_URI = "file://configscript.sh \
>>              file://common.sh \
>>              file://deps.sh"
>>   PV = "1.0"
>> +PF = "${PN}-${DISTRO}-${DISTRO_ARCH}"
>>   
>>   inherit isar-bootstrap-helper
>>   
>> -WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}"
>>   BUILDCHROOT_DIR = "${WORKDIR}/rootfs"
>>   
>> -do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>   do_build[root_cleandirs] = "${BUILDCHROOT_DIR} \
>>                               ${BUILDCHROOT_DIR}/isar-apt \
>>                               ${BUILDCHROOT_DIR}/downloads \
>> diff --git a/meta/recipes-devtools/sdkchroot/sdkchroot.bb b/meta/recipes-devtools/sdkchroot/sdkchroot.bb
>> index 895a454..249fa94 100644
>> --- a/meta/recipes-devtools/sdkchroot/sdkchroot.bb
>> +++ b/meta/recipes-devtools/sdkchroot/sdkchroot.bb
>> @@ -14,6 +14,7 @@ SRC_URI = " \
>>   PV = "0.1"
>>   
>>   inherit isar-bootstrap-helper
>> +PF = "${PN}-${ROOTFS_DISTRO}-${HOST_ARCH}-${DISTRO_ARCH}"
>>   
>>   SDKCHROOT_PREINSTALL := "debhelper \
>>                              autotools-dev \
>> @@ -25,11 +26,9 @@ SDKCHROOT_PREINSTALL := "debhelper \
>>                              devscripts \
>>                              equivs"
>>   
>> -WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PN}-${HOST_DISTRO}-${HOST_ARCH}"
>>   S = "${WORKDIR}/rootfs"
>>   
>>   do_build[dirs] = "${DEPLOY_DIR_IMAGE}"
>> -do_build[stamp-extra-info] = "${HOST_DISTRO}-${HOST_ARCH}"
>>   do_build[root_cleandirs] = "${S} \
>>                               ${S}/isar-apt"
>>   
>> -- 
>> 2.20.1
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups "isar-users" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
>> To post to this group, send email to isar-users@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20190416132647.3074-5-claudius.heine.ext%40siemens.com.
>> For more options, visit https://groups.google.com/d/optout.
> 

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de