[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH 2/5] Detect and select NewWorld bootstrap partitions (adapted from d-i/yaboot-installer).



On 11/06/2017 12:14 PM, Frank Scheiner wrote:
> ---
>  debian/changelog                |   4 +-
>  debian/grub-installer.templates |  19 ++++++
>  grub-installer                  | 126 +++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 147 insertions(+), 2 deletions(-)
> 
> diff --git a/debian/changelog b/debian/changelog
> index 0f1dc54..752849e 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -3,7 +3,9 @@ grub-installer (1.147) UNRELEASED; urgency=medium
>    [ Frank Scheiner ]
>    * Execute EFI related code only for installations of grub-efi* packages.
>    * Make d-i/grub-installer work for NewWorld Power Macs:
> -    * Install hfsutils in-target for NewWorld Power Macs (adapted from
> +    - Install hfsutils in-target for NewWorld Power Macs (adapted from
> +      d-i/yaboot-installer).
> +    - Detect and select NewWorld bootstrap partitions (adapted from
>        d-i/yaboot-installer).

Please strip the "(adapted from d-i/yaboot-installer)." from the
changelog entry and commit message. Also, please no periods in
commit messages.

>   -- Frank Scheiner <frank.scheiner@web.de>  Thu, 06 Nov 2017 08:42:00 +0200
> diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
> index 5f82c04..0c0b7f9 100644
> --- a/debian/grub-installer.templates
> +++ b/debian/grub-installer.templates
> @@ -1,3 +1,22 @@
> +Template: grub-installer/nopart_newworld
> +Type: error
> +# :sl4:
> +_Description: No NewWorld boot partition found
> + No hard disks were found which have a "NewWorld boot partition".
> + You must create a 10-Megabyte partition of type "NewWorld boot partition".
> +
> +Template: grub-installer/bootdev_newworld
> +Type: select
> +Choices: ${DEVICES}
> +Default: invaliddevice
> +# :sl4:
> +_Description: Device for boot loader installation:
> + GRUB needs to be installed on a "NewWorld boot partition" in order for
> + your system to be bootable. Please choose the destination partition
> + from among these partitions that have the bootable flag set.
> + .
> + Warning: this will erase all data on the selected partition!
> +
>  Template: grub-installer/apt_install_hfsutils
>  Type: text
>  # :sl2:
> diff --git a/grub-installer b/grub-installer
> index a45b616..f65842a 100755
> --- a/grub-installer
> +++ b/grub-installer
> @@ -37,6 +37,128 @@ debug () {
>  	[ -z "${DEBCONF_DEBUG}" ] || log "debug: $@"
>  }
>  
> +# taken from yaboot-installer's postinst script
> +die() {
> +    template="$1"
> +    shift
> +
> +    error "$@"
> +    db_input critical "$template" || [ $? -eq 30 ]
> +    db_go || true
> +    db_progress STOP
> +    exit 1
> +}

We shouldn't be introducing die() here but rather adopt the existing
log() and error() functions. Please drop die() here and replace it
with calls to log() and error(), similar to how it's used in update_grub():

        if ! $in_target $update_grub; then
                error "Running '$update_grub' failed." 1>&2
                db_input critical grub-installer/update-grub-failed || [
$? -eq 30 ]
                db_go || true
                db_progress STOP
                exit 1
        fi

> +# taken from yaboot-installer
> +partitions_with_flag () {
> +	/usr/lib/partconf/find-partitions --colons --flag "$1" 2>/dev/null || true
> +}
> +
> +# taken and adapted from yaboot-installer's `postinst` script
> +# nw is short for NewWorld
> +nw_select_offs_part()
> +{
> +	local self="nw_select_offs_part"
> +
> +	# Find the boot partition
> +
> +	# Telling parted to create an Apple_Bootstrap partition doesn't work as well
> +	# as we might like: parted's probe function isn't intelligent enough to know
> +	# about this, and reports the partition as containing whatever filesystem
> +	# was there beforehand.
> +	#
> +	# As a workaround, we only check for partitions with the boot flag set, and
> +	# don't bother checking the filesystem. However, this means that we *must*
> +	# ask the user to confirm the bootstrap partition, otherwise we might
> +	# mistakenly overwrite some other partition that happened to be flagged as
> +	# bootable.
> +
> +	PARTITIONS=
> +	DEFAULT=
> +	bootdev_priority=critical
> +
> +	case "$ARCH" in
> +	    powerpc/powermac_newworld|ppc64/powermac_newworld)

I'm not sure whether this statement makes sense here. We are calling
nw_select_offs_part() only for */powermac_newworld) anyway further
below.

> +		if db_get partman-newworld/boot_partitions && [ "$RET" ]; then
> +			OLDIFS="$IFS"
> +			IFS=,
> +			for part in $RET; do
> +				IFS="$OLDIFS"
> +				mappart="$(mapdevfs "$part")"
> +				if [ -z "$PARTITIONS" ]; then
> +					DEFAULT="$mappart"
> +					PARTITIONS="$mappart"
> +				else
> +					PARTITIONS="$PARTITIONS, $mappart"
> +				fi
> +				IFS=,
> +			done
> +			IFS="$OLDIFS"
> +			info "$self: partman-supplied bootstrap partitions: $PARTITIONS"
> +			info "$self: partman-supplied default bootstrap partition: $DEFAULT"
> +			if [ "$PARTITIONS" ] && [ "$DEFAULT" = "$PARTITIONS" ]; then
> +				# We have explicit information from partman-newworld that only one
> +				# bootstrap partition is available, so it's safe to bypass this
> +				# question.
> +				bootdev_priority=medium
> +			fi
> +		fi
> +		;;
> +	esac
> +
> +	if [ -z "$PARTITIONS" ]; then
> +		case $ARCH in
> +		    powerpc/powermac_newworld|ppc64/powermac_newworld)
> +			PARTED_FLAGS='boot'
> +			;;
> +		    *)
> +			error "$self: unknown subarchitecture; allowing any bootable partition" 1>&2
> +			PARTED_FLAGS='boot'
> +			;;
> +		esac
> +
> +		for flag in $PARTED_FLAGS; do
> +			for part in $(partitions_with_flag "$flag" | cut -d: -f1); do
> +			mappart="$(mapdevfs "$part")"
> +			if [ -z "$PARTITIONS" ]; then
> +				DEFAULT="$mappart"
> +				PARTITIONS="$mappart"
> +			else
> +				PARTITIONS="$PARTITIONS, $mappart"
> +			fi
> +			done
> +		done
> +		if [ "$PARTED_FLAGS" ]; then
> +			info "$self: guessed bootstrap partitions: $PARTITIONS"
> +			info "$self: guessed default bootstrap partition: $DEFAULT"
> +		fi
> +	fi
> +
> +	if [ "$PARTED_FLAGS" ] && [ -z "$PARTITIONS" ]; then
> +		# error: no viable boot partitions found; fall over
> +		die grub-installer/nopart_newworld 'No NewWorld bootstrap partitions found'
> +	fi
> +
> +	if [ "$PARTITIONS" ]; then
> +		db_subst grub-installer/bootdev_newworld DEVICES "$PARTITIONS"
> +		db_set grub-installer/bootdev_newworld "$DEFAULT"
> +		db_input "$bootdev_priority" grub-installer/bootdev_newworld || [ $? -eq 30 ]
> +		if ! db_go; then
> +			db_progress STOP
> +			exit 10 # back up to menu
> +		fi
> +
> +		db_get grub-installer/bootdev_newworld
> +		if [ "$RET" = false ]; then
> +			die grub-installer/nopart_newworld 'No bootstrap partition selected (?)'
> +		fi
> +		# already devfs-mapped
> +		boot="$RET"
> +	fi
> +
> +	echo "$boot"
> +	return 0
> +}
>  ARCH="$(archdetect)"
>  info "architecture: $ARCH"
>  
> @@ -238,7 +360,9 @@ case $ARCH in
>  	;;
>      ppc64/chrp|ppc64/chrp_rs6k|ppc64/chrp_ibm|ppc64/cell)
>  	;;
> -    powerpc/*|ppc64/*)
> +    powerpc/powermac_newworld|ppc64/powermac_newworld)
> +	info "$ARCH selected."
> +	offs_part=$( nw_select_offs_part )
>  	offs=$(findfs /boot/grub)
>  	[ -n "$offs" ] || error "GRUB requires that the OF partition is mounted in /boot/grub" 1>&2
>  	;;

-- 
 .''`.  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


Reply to: