Re: [PATCH 1/2] isar: Change build folders tree

[Henning Schild <henning.schild@siemens.com>]

This one seems pretty big and is doing multiple things at a time. I
suggest to split it into smaller units.

As far as i can tell it is doing at least 3 things at a time.
- rename S to IMAGE_ROOTFS
- change multiconf stamps from "${MACHINE}-${DISTRO}" to
  "${DISTRO}-${MACHINE}"
- change folder structure "work/${PN}/${MACHINE}/${DISTRO}" to
  "work/${DISTRO}-${DISTRO_ARCH}/${PF}"

More inline.

Am Fri, 8 Sep 2017 12:48:33 +0300
schrieb Alexander Smirnov <asmirnov@ilbers.de>:

> Differentiate folders tree, that are generated during build on
> the highest possible level. So the overal tree now looks like the
> following:
> 
>  tmp/${DISTRO}-${DISTRO_ARCH}/${PF}
> 
> This approach eliminates lots of subfolders to diffirentiate
> buildchroot and image for multiconfig builds. Now each configuration
> has private tree.
> 
> Signed-off-by: Alexander Smirnov <asmirnov@ilbers.de>
> ---
>  meta-isar/classes/rpi-sdimg.bbclass              |  2 +-
>  meta-isar/recipes-core/images/isar-image-base.bb | 19
> ++++++++----------- meta/classes/ext4-img.bbclass
> | 10 +++++----- meta/classes/image.bbclass                       | 12
> +++++++----- meta/conf/isar-bitbake.conf                      |  3 ++-
>  meta/recipes-devtools/buildchroot/buildchroot.bb |  9 ++++-----
>  6 files changed, 27 insertions(+), 28 deletions(-)
> 
> diff --git a/meta-isar/classes/rpi-sdimg.bbclass
> b/meta-isar/classes/rpi-sdimg.bbclass index 2614c41..91b09cd 100644
> --- a/meta-isar/classes/rpi-sdimg.bbclass
> +++ b/meta-isar/classes/rpi-sdimg.bbclass
> @@ -47,7 +47,7 @@ do_rpi_sdimg () {
>      BOOT_BLOCKS=$(LC_ALL=C parted -s ${SDIMG} unit b print | awk '/
> 1 / { print substr($4, 1, length($4 -1)) / 512 /2 }') rm -f
> ${WORKDIR}/boot.img mkfs.vfat -n "${BOOTDD_VOLUME_ID}" -S 512 -C
> ${WORKDIR}/boot.img $BOOT_BLOCKS
> -    mcopy -i ${WORKDIR}/boot.img -s ${S}/boot/* ::/
> +    mcopy -i ${WORKDIR}/boot.img -s ${IMAGE_ROOTFS}/boot/* ::/

This could be one patch that replaces S with IMAGE_ROOTFS.

>  
>      # Burn Partitions
>      dd if=${WORKDIR}/boot.img of=${SDIMG} conv=notrunc seek=1
> bs=$(expr ${IMAGE_ROOTFS_ALIGNMENT} \* 1024) && sync && sync diff
> --git a/meta-isar/recipes-core/images/isar-image-base.bb
> b/meta-isar/recipes-core/images/isar-image-base.bb index
> b679d97..178ac05 100644 ---
> a/meta-isar/recipes-core/images/isar-image-base.bb +++
> b/meta-isar/recipes-core/images/isar-image-base.bb @@ -9,6 +9,7 @@
> LICENSE = "gpl-2.0" LIC_FILES_CHKSUM =
> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260aa954499f7abaabaa882bbe"
> PV = "1.0" +PF = "${PN}"
>  
>  inherit image
>  
> @@ -17,11 +18,7 @@ DEPENDS += "${IMAGE_INSTALL}"
>  IMAGE_PREINSTALL += "apt \
>                       dbus"
>  
> -WORKDIR = "${TMPDIR}/work/${PN}/${MACHINE}/${DISTRO}"

> -S = "${WORKDIR}/rootfs"
> -IMAGE_ROOTFS = "${S}"

Part of the S -> IMAGE_ROOTFS

> -
> -do_rootfs[stamp-extra-info] = "${MACHINE}-${DISTRO}"
> +do_rootfs[stamp-extra-info] = "${DISTRO}-${MACHINE}"

This is also a change that should be in a seperate smaller patch.
  
>  do_rootfs() {
>      install -d -m 755 ${WORKDIR}/hooks_multistrap
> @@ -38,20 +35,20 @@ do_rootfs() {
>      sed -i 's|##DISTRO_APT_SOURCE##|${DISTRO_APT_SOURCE}|'
> ${WORKDIR}/multistrap.conf sed -i
> 's|##DISTRO_SUITE##|${DISTRO_SUITE}|' ${WORKDIR}/multistrap.conf sed
> -i 's|##DISTRO_COMPONENTS##|${DISTRO_COMPONENTS}|'
> ${WORKDIR}/multistrap.conf
> -    sed -i
> 's|##CONFIG_SCRIPT##|./tmp/work/${PN}/${MACHINE}/${DISTRO}/configscript.sh|'
> ${WORKDIR}/multistrap.conf
> -    sed -i
> 's|##SETUP_SCRIPT##|./tmp/work/${PN}/${MACHINE}/${DISTRO}/setup.sh|'
> ${WORKDIR}/multistrap.conf
> -    sed -i
> 's|##DIR_HOOKS##|./tmp/work/${PN}/${MACHINE}/${DISTRO}/hooks_multistrap|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##CONFIG_SCRIPT##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/configscript.sh|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##SETUP_SCRIPT##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/setup.sh|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##DIR_HOOKS##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/hooks_multistrap|'
> ${WORKDIR}/multistrap.conf # Multistrap config use relative paths, so
> ensure that we are in the right folder cd ${TOPDIR}
>  
>      # Create root filesystem
> -    sudo multistrap -a ${DISTRO_ARCH} -d "${S}" -f
> "${WORKDIR}/multistrap.conf" || true
> +    sudo multistrap -a ${DISTRO_ARCH} -d "${IMAGE_ROOTFS}" -f
> "${WORKDIR}/multistrap.conf" || true 
>      # Configure root filesystem
> -    sudo chroot ${S} /configscript.sh ${MACHINE_SERIAL}
> ${BAUDRATE_TTY} \
> +    sudo chroot ${IMAGE_ROOTFS} /configscript.sh ${MACHINE_SERIAL}
> ${BAUDRATE_TTY} \ ${ROOTFS_DEV}
> -    sudo rm ${S}/configscript.sh
> +    sudo rm ${IMAGE_ROOTFS}/configscript.sh
>  }
>  
>  addtask rootfs before do_populate
> diff --git a/meta/classes/ext4-img.bbclass
> b/meta/classes/ext4-img.bbclass index 8588626..9af781f 100644
> --- a/meta/classes/ext4-img.bbclass
> +++ b/meta/classes/ext4-img.bbclass
> @@ -6,7 +6,7 @@ ROOTFS_EXTRA ?= "64"
>  
>  EXT4_IMAGE_FILE =
> "${DEPLOY_DIR_IMAGE}/${PN}-${MACHINE}-${DISTRO}.ext4.img" 
> -do_ext4_image[stamp-extra-info] = "${MACHINE}-${DISTRO}"
> +do_ext4_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>  
>  # Generate ext4 filesystem image
>  do_ext4_image() {
> @@ -14,23 +14,23 @@ do_ext4_image() {
>  
>      rm -f ${EXT4_IMAGE_FILE}
>  
> -    ROOTFS_SIZE=`sudo du -sm ${S} |  awk '{print $1 +
> ${ROOTFS_EXTRA};}'`
> +    ROOTFS_SIZE=`sudo du -sm ${IMAGE_ROOTFS} |  awk '{print $1 +
> ${ROOTFS_EXTRA};}'` dd if=/dev/zero of=${EXT4_IMAGE_FILE} bs=1M
> count=${ROOTFS_SIZE}
>
>      sudo mkfs.ext4 -F ${EXT4_IMAGE_FILE}
>  
>      mkdir -p ${WORKDIR}/mnt
>      sudo mount -o loop ${EXT4_IMAGE_FILE} ${WORKDIR}/mnt
> -    sudo cp -r ${S}/* ${WORKDIR}/mnt
> +    sudo cp -r ${IMAGE_ROOTFS}/* ${WORKDIR}/mnt
>      sudo umount ${WORKDIR}/mnt
>      rm -r ${WORKDIR}/mnt
>  
>      if [ -n "${KERNEL_IMAGE}" ]; then
> -        cp ${S}/boot/${KERNEL_IMAGE} ${DEPLOY_DIR_IMAGE}
> +        cp ${IMAGE_ROOTFS}/boot/${KERNEL_IMAGE} ${DEPLOY_DIR_IMAGE}
>      fi
>  
>      if [ -n "${INITRD_IMAGE}" ]; then
> -        cp ${S}/boot/${INITRD_IMAGE} ${DEPLOY_DIR_IMAGE}
> +        cp ${IMAGE_ROOTFS}/boot/${INITRD_IMAGE} ${DEPLOY_DIR_IMAGE}
>      fi
>  }
>  
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index a7f0d74..fe06352 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -9,20 +9,22 @@ IMAGE_TYPE    ?= "ext4-img"
>  
>  inherit ${IMAGE_TYPE}
>  
> -do_populate[stamp-extra-info] = "${MACHINE}-${DISTRO}"
> +IMAGE_ROOTFS = "${WORKDIR}/rootfs"
> +
> +do_populate[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>  
>  # Install Debian packages, that were built from sources
>  do_populate() {
>      if [ -n "${IMAGE_INSTALL}" ]; then
> -        sudo mkdir -p ${S}/deb
> +        sudo mkdir -p ${IMAGE_ROOTFS}/deb
>  
>          for p in ${IMAGE_INSTALL}; do
> -            sudo cp ${DEPLOY_DIR_DEB}/${p}_*.deb ${S}/deb
> +            sudo cp ${DEPLOY_DIR_DEB}/${p}_*.deb ${IMAGE_ROOTFS}/deb
>          done
>  
> -        sudo chroot ${S} /usr/bin/dpkg -i -R /deb
> +        sudo chroot ${IMAGE_ROOTFS} /usr/bin/dpkg -i -R /deb
>  
> -        sudo rm -rf ${S}/deb
> +        sudo rm -rf ${IMAGE_ROOTFS}/deb
>      fi
>  }
>  
> diff --git a/meta/conf/isar-bitbake.conf b/meta/conf/isar-bitbake.conf
> index f85f5cc..5a26743 100644
> --- a/meta/conf/isar-bitbake.conf
> +++ b/meta/conf/isar-bitbake.conf
> @@ -18,9 +18,10 @@
>  # ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
> OR # OTHER DEALINGS IN THE SOFTWARE.
>  
> +WORKDIR = "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/${PF}"
>  DEPLOY_DIR_DEB = "${TMPDIR}/deploy/deb/${MACHINE}"
>  SSTATE_DIR ?= "${TMPDIR}/sstate-cache"
> -BUILDCHROOT_DIR =
> "${TOPDIR}/tmp/work/buildchroot/${DISTRO}-${DISTRO_ARCH}/rootfs"
> +BUILDCHROOT_DIR =
> "${TMPDIR}/work/${DISTRO}-${DISTRO_ARCH}/buildchroot/rootfs" # Setup
> our default hash policy BB_SIGNATURE_HANDLER ?= "noop"
> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb
> b/meta/recipes-devtools/buildchroot/buildchroot.bb index
> ccba683..155bd3f 100644 ---
> a/meta/recipes-devtools/buildchroot/buildchroot.bb +++
> b/meta/recipes-devtools/buildchroot/buildchroot.bb @@ -9,6 +9,7 @@
> LICENSE = "gpl-2.0" LIC_FILES_CHKSUM =
> "file://${LAYERDIR_isar}/licenses/COPYING.GPLv2;md5=751419260aa954499f7abaabaa882bbe" 
>  PV = "1.0"
> +PF = "${PN}"

I think this is to remove the package version from WORKDIR, i suggest
to do WORKDIR= instead of overwriting such an essential variable.
Writing to PF will i.e. also affect FILESPATH and STAMP.
This line deserves a comment!

>  
>  BUILDCHROOT_PREINSTALL ?= "gcc \
>                             make \
> @@ -21,8 +22,6 @@ BUILDCHROOT_PREINSTALL ?= "gcc \
>                             apt \
>                             automake"
>  
> -WORKDIR = "${TMPDIR}/work/${PF}/${DISTRO}"
> -
>  do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>  
>  do_build() {
> @@ -40,9 +39,9 @@ do_build() {
>      sed -i 's|##DISTRO_APT_SOURCE##|${DISTRO_APT_SOURCE}|'
> ${WORKDIR}/multistrap.conf sed -i
> 's|##DISTRO_SUITE##|${DISTRO_SUITE}|' ${WORKDIR}/multistrap.conf sed
> -i 's|##DISTRO_COMPONENTS##|${DISTRO_COMPONENTS}|'
> ${WORKDIR}/multistrap.conf
> -    sed -i
> 's|##CONFIG_SCRIPT##|./tmp/work/${PF}/${DISTRO}/configscript.sh|'
> ${WORKDIR}/multistrap.conf
> -    sed -i 's|##SETUP_SCRIPT##|./tmp/work/${PF}/${DISTRO}/setup.sh|'
> ${WORKDIR}/multistrap.conf
> -    sed -i
> 's|##DIR_HOOKS##|./tmp/work/${PF}/${DISTRO}/hooks_multistrap|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##CONFIG_SCRIPT##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/configscript.sh|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##SETUP_SCRIPT##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/setup.sh|'
> ${WORKDIR}/multistrap.conf
> +    sed -i
> 's|##DIR_HOOKS##|./tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/hooks_multistrap|'
> ${WORKDIR}/multistrap.conf 

The pattern tmp/work/${DISTRO}-${DISTRO_ARCH}/${PF}/ shows up all the
time, can that be derived from $WORKDIR instead of writing it down
again and again?

Henning

> # Multistrap config use relative paths, so
> ensure that we are in the right folder cd ${TOPDIR}