public inbox for isar-users@googlegroups.com
 help / color / mirror / Atom feed
From: Alexander Smirnov <asmirnov@ilbers.de>
To: Henning Schild <henning.schild@siemens.com>
Cc: isar-users@googlegroups.com
Subject: Re: [PATCH 1/2] isar: Change build folders tree
Date: Fri, 8 Sep 2017 15:36:09 +0300	[thread overview]
Message-ID: <06b45946-1011-4bc4-8eff-e0ef1a242dfe@ilbers.de> (raw)
In-Reply-To: <20170908134756.4fec08c6@md1em3qc>

> 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}"
> 

No problem, if general approach is Ok, I'm going to split it into pieces.

> 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.
> 

Ok.

>>   
>>       # 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
> 

Ok.

>> -
>> -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.

Ok.

>    
>>   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!

I'm still thinking about this...

My idea is that buildchroot and image recipes don't need version at all, 
because there is no source code provider, that could track the code 
revision. The line:

  PV = "1.0"

is confusing and makes really no sense, I can't imagine by which event 
it should be updated. Any changes in this recipe should be reflected in 
whole Isar version update. So the proposal is to drop it, but by 
dropping it we will get broken ${PF} variable, it will look like: 
"buildchroot-".

So, the desired goal - to have buildchroot and image WORKDIRs without 
version suffixes:

1. If drop PV, the PF should be updated
2. If do not drop PV, WORKDIR should be updated

> 
>>   
>>   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?

That's mad, but mulstistrap understands relative paths only. Any attempt 
to replace it by absolute path lead to errors. That's why it was done in 
this way, because WORKDIR is absolute path. :-(

Alex

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

  reply	other threads:[~2017-09-08 12:36 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-08  9:48 [PATCH 0/2] Unify work space for packages Alexander Smirnov
2017-09-08  9:48 ` [PATCH 1/2] isar: Change build folders tree Alexander Smirnov
2017-09-08 11:47   ` Henning Schild
2017-09-08 12:36     ` Alexander Smirnov [this message]
2017-09-08 12:59       ` Henning Schild
2017-09-08 13:07       ` Henning Schild
2017-09-08 13:20         ` Alexander Smirnov
2017-09-08 13:36           ` Henning Schild
2017-09-09  9:39       ` Claudius Heine
2017-09-11  7:28         ` Alexander Smirnov
2017-09-11  8:27   ` Claudius Heine
2017-09-11  8:41     ` Alexander Smirnov
2017-09-08  9:48 ` [PATCH 2/2] class/dpkg: Unify workplace for packages Alexander Smirnov
2017-09-08 11:29   ` Henning Schild
2017-09-08 10:01 ` [PATCH 0/2] Unify work space " Alexander Smirnov

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=06b45946-1011-4bc4-8eff-e0ef1a242dfe@ilbers.de \
    --to=asmirnov@ilbers.de \
    --cc=henning.schild@siemens.com \
    --cc=isar-users@googlegroups.com \
    /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