* [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 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
* 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
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