Hi Ulrich,
I'm happy with this "as is", but there may be a few improvements still.
By the way: This improves the situation for mounted ro /boot by moving
the check from preinst to pretend.
For noauto /boot (I believe the default and recommended) this fixes things.
This is the reason I decided to rather go with mounting /boot but as ro
instead of not mounting at all.
May I also suggest we start recommended read-only /boot instead of not
mounted at all in order to avoid similar issues from recurring?
Kind Regards,
Jaco
On 2019/12/07 11:10, Ulrich Müller wrote:
> The eclass failed to remount a read-only mounted /boot, because package
> collision sanity checks in recent Portage versions prevented it from
> reaching pkg_preinst() at all. Furthermore, with the "mount-sandbox"
> feature enabled, the mount won't be propagated past pkg_preinst() and
> installed files would end up under the (shadowed) mount point.
>
> Therefore don't even attempt to mount /boot ourselves, but error out
> if it isn't mounted read/write and ask the user to mount /boot.
>
> Also clean up and simplify. (For example, awk is a grown-up program
> which doesn't need any help from egrep or sed. :-)
>
> Closes: https://bugs.gentoo.org/532264
> See-also: https://bugs.gentoo.org/274130#c5
> Signed-off-by: Ulrich Müller
Acked-by: Jaco Kroon
>
> ---
> v3: Exit awk commands on first match.
>
> v4: Added die statements after awk commands
> Fixed typo in mount-boot_is_disabled function documentation
> Reverted renaming of I_KNOW_WHAT_I_AM_DOING variable
>
> eclass/mount-boot.eclass | 144 +--
> 1 file changed, 48 insertions(+), 96 deletions(-)
>
> diff --git a/eclass/mount-boot.eclass b/eclass/mount-boot.eclass
> index 938df6732f43..ca27aca7efbd 100644
> --- a/eclass/mount-boot.eclass
> +++ b/eclass/mount-boot.eclass
> @@ -1,156 +1,108 @@
> -# Copyright 1999-2015 Gentoo Foundation
> +# Copyright 1999-2019 Gentoo Authors
> # Distributed under the terms of the GNU General Public License v2
>
> # @ECLASS: mount-boot.eclass
> # @MAINTAINER:
> # base-sys...@gentoo.org
> # @BLURB: functions for packages that install files into /boot
> # @DESCRIPTION:
> # This eclass is really only useful for bootloaders.
> #
> # If the live system has a separate /boot partition configured, then this
> # function tries to ensure that it's mounted in rw mode, exiting with an
> -# error if it can't. It does nothing if /boot isn't a separate partition.
> +# error if it can't. It does nothing if /boot isn't a separate
partition.
> +
> +case ${EAPI:-0} in
> + 4|5|6|7) ;;
> + *) die "${ECLASS}: EAPI ${EAPI:-0} not supported" ;;
> +esac
>
> EXPORT_FUNCTIONS pkg_pretend pkg_preinst pkg_postinst pkg_prerm
pkg_postrm
>
> -# @FUNCTION: mount-boot_disabled
> +# @FUNCTION: mount-boot_is_disabled
> # @INTERNAL
> # @DESCRIPTION:
> # Detect whether the current environment/build settings are such that
we do not
> # want to mess with any mounts.
> mount-boot_is_disabled() {
> - # Since this eclass only deals with /boot, skip things when ROOT
is active.
> - if [[ "${ROOT:-/}" != "/" ]] ; then
> + # Since this eclass only deals with /boot, skip things when EROOT
is active.
> + if [[ ${EROOT:-/} != / ]] ; then
> return 0
> fi
I don't use spaces in path names ... but what happens here if ROOT or
EPREFIX (and by implication EROOT) contains a space?
What about just checking "${EROOT}/boot" instead?
Would that even be possible ... ?
>
>
> # If we're only building a package, then there's no need to check
things.
> - if [[ "${MERGE_TYPE}" == "buildonly" ]] ; then
> + if [[ ${MERGE_TYPE} == buildonly ]] ; then
> return 0
> fi
>
> # The user wants us to leave things be.
> if [[ -n ${DONT_MOUNT_BOOT} ]] ; then
> return 0
> fi
>
> # OK, we want to handle things ourselves.
> return 1
> }
>
> # @FUNCTION: mount-boot_check_status
> # @INTERNAL
> # @DESCRIPTION:
> -# Figure out what kind of work we need to do in order to have /boot
be sane.
> -# Return values are:
> -# 0 - Do nothing at all!
> -# 1 - It's mounted, but is currently ro, so need to remount rw.
> -# 2 - It's not mounted, so need to mount it rw.
> +# Check if /boot is sane, i.e., mounted read/write if on a separate
> +# partition. Die if conditions are not fulfilled.
> mount-boot_check_status() {
> # Get out fast if possible.
> - mount-boot_is_disabled && return 0
> + mount-boot_is_disabled && return
>
> # note that /dev/BOOT is in the Gentoo default /etc/fstab file
> - local fstabstate=$(awk '!/^#|^[[:blank:]]+#|^\/dev\/BOOT/ {print
$2}' /etc/fstab | egrep "^/boot$" )
> - local procstate=$(awk '$2 ~ /^\/boot$/ {print $2}' /proc/mounts)
> - local proc_ro=$(awk '{ print $2 " ," $4 "," }' /proc/mounts | sed
-n '/^\/boot .*,ro,/p')
> -
> - if [ -n "${fstabstate}" ] && [ -n "${procstate}" ] ; then
> - if [ -n "${proc_ro}" ] ; th
