[PATCH] meta: refactored flock usage

[claudius....@siemens.com]

From: Claudius Heine <c...@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 <c...@denx.de>
---
meta/classes/buildchroot.bbclass | 6 ++++--
meta/classes/wic-img.bbclass | 6 ++++--
meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 7 +++++--
3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/meta/classes/buildchroot.bbclass b/meta/classes/buildchroot.bbclass
index 0d4ff4e..0ee6ca9 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..febc5dc 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..b385825 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -214,7 +214,8 @@ isar_bootstrap() {
fi
fi
E="${@bb.utils.export_proxies(d)}"
- sudo -E flock "${ISAR_BOOTSTRAP_LOCK}" -c "\
+ sudo -E -s <<EOSUDO
+ ( flock 9
set -e
if [ ! -e "${DEPLOY_ISAR_BOOTSTRAP}" ]; then
rm -rf "${ROOTFSDIR}"
@@ -295,7 +296,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"
--
2.20.1

[Claudius Heine]

On 14/02/2019 13.18, [ext] claudius....@siemens.com wrote:
> From: Claudius Heine <c...@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.

Ok, there are some issues with that patch. I will investigate.

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: c...@denx.de

[claudius....@siemens.com]

From: Claudius Heine <c...@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 <c...@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}"
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() {

[Claudius Heine]

ping

[Baurzhan Ismagulov]

On Thu, Mar 07, 2019 at 02:23:21PM +0100, Claudius Heine wrote:
> ping

Maxim will return next week and look at this (IIUC, this is included in
Andreas's apt-key series) among other things. AFAIK, he wanted to make the
release first.

With kind regards,
Baurzhan.

[Jan Kiszka]

Are you sure that the removal of '' is correct here as well? What binds that
package list to --include=?

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

[Claudius Heine]

Hi Jan,

Quoting Jan Kiszka (2019-03-08 18:49:48)

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

> --
> 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+...@googlegroups.com.
> To post to this group, send email to isar-...@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: c...@denx.de

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

[Claudius Heine]

Quoting Baurzhan Ismagulov (2019-03-08 18:38:20)

> On Thu, Mar 07, 2019 at 02:23:21PM +0100, Claudius Heine wrote:
> > ping
>
> Maxim will return next week and look at this (IIUC, this is included in
> Andreas's apt-key series) among other things. AFAIK, he wanted to make the
> release first.

Well that patch is now nearly a month old and its a fix not a feature. But
do with that as you will.

Claudius

>
> With kind regards,
> Baurzhan.

>
> --
> 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+...@googlegroups.com.
> To post to this group, send email to isar-...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20190308173820.GA8986%40yssyq.m.ilbers.de.

> 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: c...@denx.de

[Andreas Reichel]

Please fix this before the release. The patch from Claudius is a fix.
The code as is without his fix is not maintainable, since we have an extremely
huge area embedded in double quotes which can break everything in between.
All quotes used inside are reversed, since the first double quote CLOSES
instead of OPENS a quoted string. This is bad coding style and should
not go into any release.

> --
> 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+...@googlegroups.com.
> To post to this group, send email to isar-...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20190308173820.GA8986%40yssyq.m.ilbers.de.
> For more options, visit https://groups.google.com/d/optout.

--

Andreas Reichel
Dipl.-Phys. (Univ.)
Software Consultant

Andreas...@tngtech.com, +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterfoehring
Geschaeftsfuehrer: Henrik Klagges, Dr. Robert Dahlke, Gerhard Mueller
Sitz: Unterfoehring * Amtsgericht Muenchen * HRB 135082

[Jan Kiszka]

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

Jan

[Claudius Heine]

debootstrap manpage:

--include=alpha,beta
Comma separated list of packages which will be added to
download and extract lists.


If it does, than that is AFAIK undocumented and should not be done.

Claudius

[Maxim Yu. Osipov]

On 2/14/19 5:08 PM, claudius....@siemens.com wrote:
> From: Claudius Heine <c...@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.

Applied to the 'next',

Thanks,
Maxim.

Maxim Osipov
ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn
Germany
+49 (151) 6517 6917
mos...@ilbers.de
http://ilbers.de/
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov