Re: [PATCH v2] meta: refactored flock usage

[Jan Kiszka <jan.kiszka@siemens.com>]

On 09.03.19 20:08, Claudius Heine wrote:
> Hi Jan,
> 
> Quoting Jan Kiszka (2019-03-08 18:49:48)
>> On 14.02.19 17:08, [ext] claudius.heine.ext@siemens.com wrote:
>>> From: Claudius Heine <ch@denx.de>
>>>
>>> Currently much care has to be taken in order to correctly escape strings
>>> inside flock commands. And there is also on instance where this was
>>> incorrectly used (isar-bootstrap.inc).
>>>
>>> The usage of flock was changed to no longer require single or double
>>> ticks. Instead commands are run inside a subshell.
>>>
>>> Signed-off-by: Claudius Heine <ch@denx.de>
>>> ---
>>>    meta/classes/buildchroot.bbclass                    |  6 ++++--
>>>    meta/classes/wic-img.bbclass                        |  6 ++++--
>>>    meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 10 +++++++---
>>>    3 files changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
>>> index 0d4ff4e..c017b25 100644
>>> --- a/meta/classes/buildchroot.bbclass
>>> +++ b/meta/classes/buildchroot.bbclass
>>> @@ -22,7 +22,8 @@ python __anonymous() {
>>>    MOUNT_LOCKFILE = "${BUILDCHROOT_DIR}/mount.lock"
>>>    
>>>    buildchroot_do_mounts() {
>>> -    sudo flock ${MOUNT_LOCKFILE} -c ' \
>>> +    sudo -s <<'EOSUDO'
>>> +        ( flock 9
>>>            set -e
>>>            if ! grep -q ${BUILDCHROOT_DIR}/isar-apt /proc/mounts; then
>>>                mount --bind ${REPO_ISAR_DIR}/${DISTRO} ${BUILDCHROOT_DIR}/isar-apt
>>> @@ -36,5 +37,6 @@ buildchroot_do_mounts() {
>>>    
>>>            # Refresh /etc/resolv.conf at this chance
>>>            cp -L /etc/resolv.conf ${BUILDCHROOT_DIR}/etc
>>> -        '
>>> +        ) 9>${MOUNT_LOCKFILE}
>>> +EOSUDO
>>>    }
>>> diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
>>> index 76602d8..5367a37 100644
>>> --- a/meta/classes/wic-img.bbclass
>>> +++ b/meta/classes/wic-img.bbclass
>>> @@ -87,14 +87,16 @@ do_build[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}"
>>>    
>>>    do_wic_image() {
>>>        buildchroot_do_mounts
>>> -    sudo flock ${MOUNT_LOCKFILE} -c ' \
>>> +    sudo -s <<'EOSUDO'
>>> +        ( flock 9
>>>            for dir in ${BBLAYERS} ${STAGING_DIR} ${ISARROOT}/scripts; do
>>>                mkdir -p ${BUILDCHROOT_DIR}/$dir
>>>                if ! mountpoint ${BUILDCHROOT_DIR}/$dir >/dev/null 2>&1; then
>>>                    mount --bind --make-private $dir ${BUILDCHROOT_DIR}/$dir
>>>                fi
>>>            done
>>> -        '
>>> +        ) 9>${MOUNT_LOCKFILE}
>>> +EOSUDO
>>>        export FAKEROOTCMD=${FAKEROOTCMD}
>>>        export BUILDDIR=${BUILDDIR}
>>>        export MTOOLS_SKIP_CHECK=1
>>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>> index 234d339..9c82184 100644
>>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>>> @@ -207,14 +207,16 @@ isar_bootstrap() {
>>>            esac
>>>            shift
>>>        done
>>> -    debootstrap_args="--verbose --variant=minbase --include='${DISTRO_BOOTSTRAP_BASE_PACKAGES}'"
>>> +    debootstrap_args="--verbose --variant=minbase --include=${DISTRO_BOOTSTRAP_BASE_PACKAGES}"
>>
>> Are you sure that the removal of '' is correct here as well? What binds that
>> package list to --include=?
> 
> Well the package list should be ',' seperated otherwise debootstrap
> would not accept it. With my change it would look at "'locales'" (with
> ') and would not find that package, since it does not exist. Having
> those ' there is pretty misleading and wrong.

But the package list may contain spaces as well, no?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux