Re: [PATCH v2] meta: refactored flock usage

[Claudius Heine <ch@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 5540 bytes --]

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.

Claudius

> 
> Jan
> 
> >       if [ "${ISAR_USE_CACHED_BASE_REPO}" = "1" ]; then
> >           if [ -z "${BASE_REPO_KEY}" ] ; then
> >               debootstrap_args="$debootstrap_args --no-check-gpg"
> >           fi
> >       fi
> >       E="${@bb.utils.export_proxies(d)}"
> > -    sudo -E flock "${ISAR_BOOTSTRAP_LOCK}" -c "\
> > +    export IS_HOST debootstrap_args E
> > +    sudo -E -s <<'EOSUDO'
> > +        ( flock 9
> >           set -e
> >           if [ ! -e "${DEPLOY_ISAR_BOOTSTRAP}" ]; then
> >               rm -rf "${ROOTFSDIR}"
> > @@ -295,7 +297,9 @@ isar_bootstrap() {
> >   
> >               # Finalize debootstrap by setting the link in deploy
> >               ln -Tfsr "${ROOTFSDIR}" "${DEPLOY_ISAR_BOOTSTRAP}"
> > -        fi"
> > +        fi
> > +        ) 9>'${ISAR_BOOTSTRAP_LOCK}'
> > +EOSUDO
> >   }
> >   
> >   CLEANFUNCS = "clean_deploy"
> > 
> 
> -- 
> Siemens AG, Corporate Technology, CT RDA IOT SES-DE
> Corporate Competence Center Embedded Linux
> 
> -- 
> 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/2b7ea7d6-5c27-2b86-e05a-37d21f58358f%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

           PGP key: 6FF2 E59F 00C6 BC28 31D8 64C1 1173 CB19 9808 B153
                             Keyserver: hkp://pool.sks-keyservers.net

[-- Attachment #2: signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEb/LlnwDGvCgx2GTBEXPLGZgIsVMFAlyEDxYACgkQEXPLGZgI
sVN0bxAAmuezbUTMeFX1PmUYwJB5t0WHmRkrRyjop0mlGchBMVlkKYb0eS/HWrbH
BmmZmcYc7YrMMuHZDm/4utWWB0mLvnvZ0fCAmVH/RWMuj26lRJTpxKC2JsdHUSkK
XyEoJOXH7Jx3acJjkmxioIE0EyZZ+tRqK+tOlMiFCRT3O76y8pA7iPn4X8UD9jnL
poGRqu83JFr2VVRMwHaUpZTBsEKTIas2nt43WNRkxAxDILBVf5T8IVGNa163+mRw
ZhIcAk9oMNbo/fV1vwNVStaQAamXLoyko/8Y1sqTMioQDSVdSVfR0ADybTDXlzvI
oOJ8pS6dDGHRG14C2ixvvD5TDUsWQs5o83d3o2lajOe9SL+iXW+gbNa+AWS38rAb
g1g4zxEVqhwYsz0JF/N19gskdCTpcliVDmZunhGKT7kDL7Q7vNrlzbJHW5fO+U0I
22u1fnnCUbc6FAlQiUlEOUOrX16m2KTXnYjuCfYL89nYmHcaUxQKcHQ93U2ItqxC
blhvtv+zv6WeFyX67m57nm3cgCPFC3t+yBcIxi2g/ISBnTOCtHZQrdhcruPk62s7
NLkc6+a0hKyybxZCP7Oi7guO9NwxZdA83avoDiF7kJk5Iue4zPXmRqSW8q2oF2YA
5SzL7oxPhTH8BauV/Ma5Pwi++YLmGIJkHq0z1M2CpsVLDeLLjS4=
=hhKy
-----END PGP SIGNATURE-----