Re: [PATCH 3/5] Handle HFS file systems (format, mount, unmount, permanent mount). Override $bootfs, $bootfstype, $bootdev and $state for NewWorld Power Macs.

[John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>]

On 11/06/2017 12:15 PM, Frank Scheiner wrote:
> ---
>  debian/changelog |  1 +
>  grub-installer   | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/debian/changelog b/debian/changelog
> index 752849e..4c056d3 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -7,6 +7,7 @@ grub-installer (1.147) UNRELEASED; urgency=medium
>        d-i/yaboot-installer).
>      - Detect and select NewWorld bootstrap partitions (adapted from
>        d-i/yaboot-installer).
> +    - Handle HFS file systems (format, mount, unmount, permanent mount).

This should be "fstab entry", not "permanent mount".

Also, please strip ". Override $bootfs, $bootfstype, $bootdev and $state
for NewWorld Power Macs." from the commit message. If you need to add
more information to the commit message, create a separate paragraph
after an empty line for that.

>   -- Frank Scheiner <frank.scheiner@web.de>  Thu, 06 Nov 2017 08:42:00 +0200
>  
> diff --git a/grub-installer b/grub-installer
> index f65842a..f606752 100755
> --- a/grub-installer
> +++ b/grub-installer
> @@ -19,6 +19,8 @@ fi
>  newline="
>  "
>  
> +NW_OFFS_MOUNT_POINT="/boot/grub"
> +
>  db_capb backup
>  
>  log() {
> @@ -159,6 +161,84 @@ nw_select_offs_part()
>  	echo "$boot"
>  	return 0
>  }
> +
> +# adapted from ybin's mkoffs()
> +nw_format_offs()
> +{
> +	local nw_boot_partition="$1"
> +
> +	local self="nw_format_offs"
> +
> +	mount | grep "^$nw_boot_partition\>" > /dev/null
> +	if [ $? = 0 ] ; then
> +		error "$self: $nw_boot_partition appears to be mounted! Aborting." 1>&2
> +		return 1
> +	fi
> +
> +	info "$self: Creating HFS filesystem on $nw_boot_partition..."
> +	in-target hformat -l bootstrap "$nw_boot_partition" > /dev/null
> +	if [ $? != 0 ] ; then
> +		error "$self: HFS filesystem creation failed!" 1>&2
> +		return 1
> +	fi
> +	in-target humount "$nw_boot_partition" ## otherwise we might get confused.
> +
> +	return 0
> +}
> +
> +nw_include_offs_in_fstab()
> +{
> +	local nw_boot_partition="$1"
> +	local fstab="$2"
> +
> +	local self="nw_include_offs_in_fstab"
> +	local mount_point="$NW_OFFS_MOUNT_POINT"
> +
> +	if ! grep "$nw_boot_partition $mount_point hfs defaults 0 0" "$fstab" 1>/dev/null 2>&1; then
> +		if echo "$nw_boot_partition $mount_point hfs defaults 0 0" >> $fstab; then
> +			info "$self: In-target mount of $nw_boot_partition on $mount_point is now permanent."
> +			return 0
> +		else
> +			error "$self: Problem accessing in-target $fstab." 1>&2
> +			return 1
> +		fi
> +	else
> +		info "$self: In-target mount of $nw_boot_partition on $mount_point was already permanent."
> +	fi
> +}
> +
> +nw_mount_offs()
> +{
> +	local nw_boot_partition="$1"
> +
> +	local self="nw_mount_offs"
> +	local mount_point="$NW_OFFS_MOUNT_POINT"
> +
> +	in-target mkdir -p "$mount_point"
> +
> +	if mount -t hfs "$nw_boot_partition" "${ROOT}${mount_point}" 1>/dev/null 2>&1; then
> +		info "$self: Mount of $nw_boot_partition on ${ROOT}${mount_point} succeeded."
> +		return 0
> +	else
> +		error "$self: Mount of $nw_boot_partition on ${ROOT}${mount_point} failed." 1>&2
> +		return 1
> +	fi
> +}
> +
> +nw_unmount_offs()
> +{
> +	local nw_boot_partition="$1"
> +
> +	local self="nw_unmount_offs"
> +
> +	if umount "$nw_boot_partition" 1>/dev/null 2>&1; then
> +		info "$self: Unmount of $nw_boot_partition succeeded."
> +		return 0
> +	else
> +		error "$self: Unmount of $nw_boot_partition failed." 1>&2
> +		return 1
> +	fi
> +}
>  ARCH="$(archdetect)"
>  info "architecture: $ARCH"

These functions are using info() and error() again instead of die(), so
the use of die() in the previous patch would just be inconsistent.

Otherwise, this looks fine.

> @@ -363,8 +443,16 @@ case $ARCH in
>      powerpc/powermac_newworld|ppc64/powermac_newworld)
>  	info "$ARCH selected."
>  	offs_part=$( nw_select_offs_part )
> +	nw_format_offs "$offs_part" || exit 1
> +	nw_mount_offs "$offs_part" || exit 1
> +	nw_include_offs_in_fstab "$offs_part" "$ROOT/etc/fstab" || exit 1
>  	offs=$(findfs /boot/grub)
> -	[ -n "$offs" ] || error "GRUB requires that the OF partition is mounted in /boot/grub" 1>&2
> +	if [ -n "$offs" ]; then
> +		bootfs=$offs
> +		bootfstype=$(findfstype /boot/grub)
> +	else
> +		error "GRUB requires that the OF partition is mounted in /boot/grub" 1>&2
> +	fi
>  	;;
>  esac

Looks fine.

> @@ -777,6 +865,10 @@ case $ARCH:$grub_package in
>  	bootdev="$wipe_bootdev"
>  	state=3
>  	;;
> +    powerpc/powermac_newworld:grub-ieee1275|ppc64/powermac_newworld:grub-ieee1275)
> +	bootdev="$bootfs"
> +	state=3
> +	;;
>      *)
>  	# No need for install device selection on other platforms.
>  	bootdev=dummy
> 

Dito.

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913