* [PATCH 0/2] Fail on the first error inside sudo
@ 2021-12-10 8:10 Anton Mikanovich
2021-12-10 8:10 ` [PATCH 1/2] rootfs: Execute rmdir only if needed Anton Mikanovich
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Anton Mikanovich @ 2021-12-10 8:10 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
There in no need in further execution if any command failed. But inside
the sudo sections (also with chroot) we didn't have 'set -e' declared.
It leads to incorrect execution of the next code in case of failures.
So adding 'set -e' to exit immediately.
Also fix rmdir which was failed (but masked) on execution if there were
no base-apt used.
Anton Mikanovich (2):
rootfs: Execute rmdir only if needed
sudo: Fail on the first error
meta/classes/deb-dl-dir.bbclass | 2 ++
meta/classes/image-locales-extension.bbclass | 1 +
meta/classes/image.bbclass | 9 +++++----
meta/classes/rootfs.bbclass | 2 ++
meta/classes/vm-img.bbclass | 1 +
meta/classes/wic-img.bbclass | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 1 +
7 files changed, 13 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] rootfs: Execute rmdir only if needed
2021-12-10 8:10 [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
@ 2021-12-10 8:10 ` Anton Mikanovich
2021-12-10 8:10 ` [PATCH 2/2] sudo: Fail on the first error Anton Mikanovich
2021-12-17 11:04 ` [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
2 siblings, 0 replies; 5+ messages in thread
From: Anton Mikanovich @ 2021-12-10 8:10 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
We should not remove dirs if they are not unmounted or even exist.
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
meta/classes/image.bbclass | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index d757256..95c14e5 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -198,12 +198,12 @@ do_rootfs_finalize() {
-maxdepth 1 -name 'qemu-*-static' -type f -delete
mountpoint -q '${ROOTFSDIR}/isar-apt' && \
- umount -l ${ROOTFSDIR}/isar-apt
- rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
+ umount -l ${ROOTFSDIR}/isar-apt && \
+ rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
mountpoint -q '${ROOTFSDIR}/base-apt' && \
- umount -l ${ROOTFSDIR}/base-apt
- rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
+ umount -l ${ROOTFSDIR}/base-apt && \
+ rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
mountpoint -q '${ROOTFSDIR}/dev' && \
umount -l ${ROOTFSDIR}/dev
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] sudo: Fail on the first error
2021-12-10 8:10 [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
2021-12-10 8:10 ` [PATCH 1/2] rootfs: Execute rmdir only if needed Anton Mikanovich
@ 2021-12-10 8:10 ` Anton Mikanovich
2021-12-17 12:05 ` Henning Schild
2021-12-17 11:04 ` [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
2 siblings, 1 reply; 5+ messages in thread
From: Anton Mikanovich @ 2021-12-10 8:10 UTC (permalink / raw)
To: isar-users; +Cc: Anton Mikanovich
The code execution inside sudo section should be stopped on the first
command failure because the next commands can cause incorrect behavior.
Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
---
meta/classes/deb-dl-dir.bbclass | 2 ++
meta/classes/image-locales-extension.bbclass | 1 +
meta/classes/image.bbclass | 1 +
meta/classes/rootfs.bbclass | 2 ++
meta/classes/vm-img.bbclass | 1 +
meta/classes/wic-img.bbclass | 1 +
meta/recipes-devtools/buildchroot/buildchroot.inc | 1 +
7 files changed, 9 insertions(+)
diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index 29bf45b..ffbff07 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -22,6 +22,7 @@ is_not_part_of_current_build() {
debsrc_do_mounts() {
sudo -s <<EOSUDO
+ set -e
mkdir -p "${1}/deb-src"
mountpoint -q "${1}/deb-src" || \
mount --bind "${DEBSRCDIR}" "${1}/deb-src"
@@ -30,6 +31,7 @@ EOSUDO
debsrc_undo_mounts() {
sudo -s <<EOSUDO
+ set -e
mkdir -p "${1}/deb-src"
mountpoint -q "${1}/deb-src" && \
umount -l "${1}/deb-src"
diff --git a/meta/classes/image-locales-extension.bbclass b/meta/classes/image-locales-extension.bbclass
index 0f0d0ca..25af540 100644
--- a/meta/classes/image-locales-extension.bbclass
+++ b/meta/classes/image-locales-extension.bbclass
@@ -59,6 +59,7 @@ __EOF__
# Install configuration into image:
sudo -E -s <<'EOSUDO'
+ set -e
cat '${WORKDIR}/locale.gen' >> '${ROOTFSDIR}/etc/locale.gen'
cat '${WORKDIR}/locale.default' > '${ROOTFSDIR}/etc/default/locale'
cat '${WORKDIR}/locale.nopurge' > '${ROOTFSDIR}/etc/locale.nopurge'
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 95c14e5..ea49354 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -189,6 +189,7 @@ addtask deploy before do_build after do_image
do_rootfs_finalize() {
sudo -s <<'EOSUDO'
+ set -e
test -e "${ROOTFSDIR}/chroot-setup.sh" && \
"${ROOTFSDIR}/chroot-setup.sh" "cleanup" "${ROOTFSDIR}"
rm -f "${ROOTFSDIR}/chroot-setup.sh"
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index e0604e0..6ecb39d 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -30,6 +30,7 @@ export LC_ALL = "C"
rootfs_do_mounts[weight] = "3"
rootfs_do_mounts() {
sudo -s <<'EOSUDO'
+ set -e
mountpoint -q '${ROOTFSDIR}/dev' || \
mount --rbind /dev '${ROOTFSDIR}/dev'
mount --make-rslave '${ROOTFSDIR}/dev'
@@ -80,6 +81,7 @@ ROOTFS_CONFIGURE_COMMAND += "rootfs_configure_isar_apt"
rootfs_configure_isar_apt[weight] = "2"
rootfs_configure_isar_apt() {
sudo -s <<'EOSUDO'
+ set -e
mkdir -p '${ROOTFSDIR}/etc/apt/sources.list.d'
echo 'deb [trusted=yes] file:///isar-apt ${DEBDISTRONAME} main' > \
diff --git a/meta/classes/vm-img.bbclass b/meta/classes/vm-img.bbclass
index b230af2..c75a544 100644
--- a/meta/classes/vm-img.bbclass
+++ b/meta/classes/vm-img.bbclass
@@ -95,6 +95,7 @@ do_create_ova() {
image_do_mounts
sudo -Es chroot --userspec=$( id -u ):$( id -g ) ${BUILDCHROOT_DIR} <<'EOSUDO'
+ set -e
export DISK_SIZE_BYTES=$(qemu-img info -f vmdk "${VIRTUAL_MACHINE_DISK}" \
| gawk 'match($0, /^virtual size:.*\(([0-9]+) bytes\)/, a) {print a[1]}')
export DISK_UUID=$(uuidgen)
diff --git a/meta/classes/wic-img.bbclass b/meta/classes/wic-img.bbclass
index 80ca5f7..7537a27 100644
--- a/meta/classes/wic-img.bbclass
+++ b/meta/classes/wic-img.bbclass
@@ -156,6 +156,7 @@ wic_do_mounts() {
buildchroot_do_mounts
sudo -s <<'EOSUDO'
( flock 9
+ set -e
for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR} ${BITBAKEDIR}; do
mkdir -p ${BUILDCHROOT_DIR}/$dir
if ! mountpoint ${BUILDCHROOT_DIR}/$dir >/dev/null 2>&1; then
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc b/meta/recipes-devtools/buildchroot/buildchroot.inc
index 726c7bb..6d9ced0 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.inc
+++ b/meta/recipes-devtools/buildchroot/buildchroot.inc
@@ -44,6 +44,7 @@ BUILDCHROOT_PREINSTALL_COMMON = " \
rootfs_do_mounts_append() {
sudo -s <<'EOSUDO'
+ set -e
mkdir -p '${BUILDCHROOT_DIR}/downloads'
mountpoint -q '${BUILDCHROOT_DIR}/downloads' || \
mount --bind '${DL_DIR}' '${BUILDCHROOT_DIR}/downloads'
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] Fail on the first error inside sudo
2021-12-10 8:10 [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
2021-12-10 8:10 ` [PATCH 1/2] rootfs: Execute rmdir only if needed Anton Mikanovich
2021-12-10 8:10 ` [PATCH 2/2] sudo: Fail on the first error Anton Mikanovich
@ 2021-12-17 11:04 ` Anton Mikanovich
2 siblings, 0 replies; 5+ messages in thread
From: Anton Mikanovich @ 2021-12-17 11:04 UTC (permalink / raw)
To: isar-users
10.12.2021 11:10, Anton Mikanovich wrote:
> There in no need in further execution if any command failed. But inside
> the sudo sections (also with chroot) we didn't have 'set -e' declared.
> It leads to incorrect execution of the next code in case of failures.
> So adding 'set -e' to exit immediately.
>
> Also fix rmdir which was failed (but masked) on execution if there were
> no base-apt used.
>
> Anton Mikanovich (2):
> rootfs: Execute rmdir only if needed
> sudo: Fail on the first error
>
> meta/classes/deb-dl-dir.bbclass | 2 ++
> meta/classes/image-locales-extension.bbclass | 1 +
> meta/classes/image.bbclass | 9 +++++----
> meta/classes/rootfs.bbclass | 2 ++
> meta/classes/vm-img.bbclass | 1 +
> meta/classes/wic-img.bbclass | 1 +
> meta/recipes-devtools/buildchroot/buildchroot.inc | 1 +
> 7 files changed, 13 insertions(+), 4 deletions(-)
>
Applied to next.
--
Anton Mikanovich
Promwad Ltd.
External service provider of ilbers GmbH
Maria-Merian-Str. 8
85521 Ottobrunn, Germany
+49 (89) 122 67 24-0
Commercial register Munich, HRB 214197
General Manager: Baurzhan Ismagulov
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] sudo: Fail on the first error
2021-12-10 8:10 ` [PATCH 2/2] sudo: Fail on the first error Anton Mikanovich
@ 2021-12-17 12:05 ` Henning Schild
0 siblings, 0 replies; 5+ messages in thread
From: Henning Schild @ 2021-12-17 12:05 UTC (permalink / raw)
To: Anton Mikanovich; +Cc: isar-users
Nice catch. I think we do have several other subshell places that could
benefit from that as well.
i.e. "sh -c ..." could become "sh -e -c ..."
regards,
Henning
Am Fri, 10 Dec 2021 11:10:54 +0300
schrieb Anton Mikanovich <amikan@ilbers.de>:
> The code execution inside sudo section should be stopped on the first
> command failure because the next commands can cause incorrect
> behavior.
>
> Signed-off-by: Anton Mikanovich <amikan@ilbers.de>
> ---
> meta/classes/deb-dl-dir.bbclass | 2 ++
> meta/classes/image-locales-extension.bbclass | 1 +
> meta/classes/image.bbclass | 1 +
> meta/classes/rootfs.bbclass | 2 ++
> meta/classes/vm-img.bbclass | 1 +
> meta/classes/wic-img.bbclass | 1 +
> meta/recipes-devtools/buildchroot/buildchroot.inc | 1 +
> 7 files changed, 9 insertions(+)
>
> diff --git a/meta/classes/deb-dl-dir.bbclass
> b/meta/classes/deb-dl-dir.bbclass index 29bf45b..ffbff07 100644
> --- a/meta/classes/deb-dl-dir.bbclass
> +++ b/meta/classes/deb-dl-dir.bbclass
> @@ -22,6 +22,7 @@ is_not_part_of_current_build() {
>
> debsrc_do_mounts() {
> sudo -s <<EOSUDO
> + set -e
> mkdir -p "${1}/deb-src"
> mountpoint -q "${1}/deb-src" || \
> mount --bind "${DEBSRCDIR}" "${1}/deb-src"
> @@ -30,6 +31,7 @@ EOSUDO
>
> debsrc_undo_mounts() {
> sudo -s <<EOSUDO
> + set -e
> mkdir -p "${1}/deb-src"
> mountpoint -q "${1}/deb-src" && \
> umount -l "${1}/deb-src"
> diff --git a/meta/classes/image-locales-extension.bbclass
> b/meta/classes/image-locales-extension.bbclass index 0f0d0ca..25af540
> 100644 --- a/meta/classes/image-locales-extension.bbclass
> +++ b/meta/classes/image-locales-extension.bbclass
> @@ -59,6 +59,7 @@ __EOF__
>
> # Install configuration into image:
> sudo -E -s <<'EOSUDO'
> + set -e
> cat '${WORKDIR}/locale.gen' >> '${ROOTFSDIR}/etc/locale.gen'
> cat '${WORKDIR}/locale.default' >
> '${ROOTFSDIR}/etc/default/locale' cat '${WORKDIR}/locale.nopurge' >
> '${ROOTFSDIR}/etc/locale.nopurge' diff --git
> a/meta/classes/image.bbclass b/meta/classes/image.bbclass index
> 95c14e5..ea49354 100644 --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -189,6 +189,7 @@ addtask deploy before do_build after do_image
>
> do_rootfs_finalize() {
> sudo -s <<'EOSUDO'
> + set -e
> test -e "${ROOTFSDIR}/chroot-setup.sh" && \
> "${ROOTFSDIR}/chroot-setup.sh" "cleanup" "${ROOTFSDIR}"
> rm -f "${ROOTFSDIR}/chroot-setup.sh"
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index e0604e0..6ecb39d 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -30,6 +30,7 @@ export LC_ALL = "C"
> rootfs_do_mounts[weight] = "3"
> rootfs_do_mounts() {
> sudo -s <<'EOSUDO'
> + set -e
> mountpoint -q '${ROOTFSDIR}/dev' || \
> mount --rbind /dev '${ROOTFSDIR}/dev'
> mount --make-rslave '${ROOTFSDIR}/dev'
> @@ -80,6 +81,7 @@ ROOTFS_CONFIGURE_COMMAND +=
> "rootfs_configure_isar_apt" rootfs_configure_isar_apt[weight] = "2"
> rootfs_configure_isar_apt() {
> sudo -s <<'EOSUDO'
> + set -e
>
> mkdir -p '${ROOTFSDIR}/etc/apt/sources.list.d'
> echo 'deb [trusted=yes] file:///isar-apt ${DEBDISTRONAME} main'
> > \ diff --git a/meta/classes/vm-img.bbclass
> > b/meta/classes/vm-img.bbclass
> index b230af2..c75a544 100644
> --- a/meta/classes/vm-img.bbclass
> +++ b/meta/classes/vm-img.bbclass
> @@ -95,6 +95,7 @@ do_create_ova() {
> image_do_mounts
>
> sudo -Es chroot --userspec=$( id -u ):$( id -g )
> ${BUILDCHROOT_DIR} <<'EOSUDO'
> + set -e
> export DISK_SIZE_BYTES=$(qemu-img info -f vmdk
> "${VIRTUAL_MACHINE_DISK}" \ | gawk 'match($0, /^virtual
> size:.*\(([0-9]+) bytes\)/, a) {print a[1]}') export
> DISK_UUID=$(uuidgen) diff --git a/meta/classes/wic-img.bbclass
> b/meta/classes/wic-img.bbclass index 80ca5f7..7537a27 100644
> --- a/meta/classes/wic-img.bbclass
> +++ b/meta/classes/wic-img.bbclass
> @@ -156,6 +156,7 @@ wic_do_mounts() {
> buildchroot_do_mounts
> sudo -s <<'EOSUDO'
> ( flock 9
> + set -e
> for dir in ${BBLAYERS} ${STAGING_DIR} ${SCRIPTSDIR}
> ${BITBAKEDIR}; do mkdir -p ${BUILDCHROOT_DIR}/$dir
> if ! mountpoint ${BUILDCHROOT_DIR}/$dir >/dev/null 2>&1;
> then diff --git a/meta/recipes-devtools/buildchroot/buildchroot.inc
> b/meta/recipes-devtools/buildchroot/buildchroot.inc index
> 726c7bb..6d9ced0 100644 ---
> a/meta/recipes-devtools/buildchroot/buildchroot.inc +++
> b/meta/recipes-devtools/buildchroot/buildchroot.inc @@ -44,6 +44,7 @@
> BUILDCHROOT_PREINSTALL_COMMON = " \
> rootfs_do_mounts_append() {
> sudo -s <<'EOSUDO'
> + set -e
> mkdir -p '${BUILDCHROOT_DIR}/downloads'
> mountpoint -q '${BUILDCHROOT_DIR}/downloads' || \
> mount --bind '${DL_DIR}' '${BUILDCHROOT_DIR}/downloads'
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-17 12:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 8:10 [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
2021-12-10 8:10 ` [PATCH 1/2] rootfs: Execute rmdir only if needed Anton Mikanovich
2021-12-10 8:10 ` [PATCH 2/2] sudo: Fail on the first error Anton Mikanovich
2021-12-17 12:05 ` Henning Schild
2021-12-17 11:04 ` [PATCH 0/2] Fail on the first error inside sudo Anton Mikanovich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox