Merge ~mwhudson/ubiquity:lp-1847721 into ubiquity:master

[Michael Hudson-Doyle]

This depends on

https://code.launchpad.net/~mwhudson/ubuntu/+source/partman-efi/+git/partman-efi/+merge/384067

and

https://code.launchpad.net/~mwhudson/ubuntu/+source/grub-installer/+git/grub-installer/+merge/384068

and has not been tested (will test it soon though)

[Michael Hudson-Doyle (mwhudson)]

Ah this doesn't work, I think perhaps instead of hacking grub-installer like this, it would be easier to just install the packages and run chroot /target grub-install --target=x86_64-efi in the ubiquity code. Open to input here :)

[Dimitri John Ledkov (xnox)]

So this depends on our goal.

At the moment, it is not possible to install _both_ grub-efi-amd64 and grub-pc, and keep both maintained correctly.

We can install both grub-pc-bin & grub-efi-amd64-bin (& grub-efi-amd64-signed) but only one of them will be updated.

So imho, for now, if we booted in bios mode we should install grub-pc + grub-efi-amd64-bin, run grub-install --target x86_64-efi, and then install grub-efi-amd64-signed.

[Steve Langasek (vorlon)]

> So imho, for now, if we booted in bios mode we should install
> grub-pc + grub-efi-amd64-bin, run grub-install --target x86_64-efi,
> and then install grub-efi-amd64-signed.

We ought to be installing shim-signed, which pulls in grub-efi-amd64-signed as a dependency; and this will run grub-install --target x86_64-efi for us with the right options. Is there any reason to run grub-install --target x86_64-efi manually?

[Dimitri John Ledkov (xnox)]

I'm putting this review on hold for now, until we have the spec done and have a coherent implementation.

[Michael Hudson-Doyle (mwhudson)]

With some updates to my grub-installer MP I can do an on-rails legacy boot install and then boot it under UEFI. I'm about to push an update towards getting other scenarios to work with some XXX comments I'd love some help with :)

[Michael Hudson-Doyle (mwhudson)]

with the recent partman-efi changes, the UI complains if you don't create an ESP even when booted in legacy mode, so I think that aspect is OK. But when booted under UEFI, the default partitioning does not create a bios_grub partition and there is no complaint if you don't create one manually. So I think I'll take the "install so legacy boot works when UEFI booted" bits out of this and my grub-installer MP and we can make that work in separate changes.

diff --git a/d-i/source/grub-installer/grub-installer b/d-i/source/grub-installer/grub-installer
index 8879dbe..2329f0e 100755
--- a/d-i/source/grub-installer/grub-installer
+++ b/d-i/source/grub-installer/grub-installer
@@ -8,11 +8,16 @@ set -e
 if [ "$1" ]; then
 	ROOT="$1"
 	chroot=chroot
+	shift
 else
 	ROOT=
 	chroot=
 fi
 
+if [ "$1" ]; then
+	ARCH="$1"
+fi
+
 . /usr/share/grub-installer/functions.sh
 . /usr/share/grub-installer/otheros.sh
 
@@ -37,7 +42,7 @@ debug () {
 	[ -z "${DEBCONF_DEBUG}" ] || log "debug: $@"
 }
 
-ARCH="$(archdetect)"
+ARCH="${ARCH:-$(archdetect)}"
 info "architecture: $ARCH"
 
 umount_dirs=
@@ -494,17 +499,7 @@ case $ARCH in
 	if [ -f /var/lib/partman/ignore_uefi ]; then
 		grub_package="grub-pc"
 	else
-		grub_package="grub-efi"
-		# Override the package choice if we can figure out the
-		# right package to use directly
-		if [ -f /sys/firmware/efi/fw_platform_size ] ; then
-			SIZE=$(cat /sys/firmware/efi/fw_platform_size)
-			if [ $SIZE -eq 64 ] ; then
-				grub_package="grub-efi-amd64-signed"
-			elif [ $SIZE -eq 32 ] ; then
-				grub_package="grub-efi-ia32"
-			fi
-		fi
+		grub_package="grub-efi-amd64-signed"
 		if [ ! -d /target/boot/efi ]; then
 			# No EFI System Partition, so presumably the partitioner
 			# believed this to be unnecessary, perhaps because we're
@@ -1094,6 +1089,10 @@ EOF
 			grub_install_params="$grub_install_params --force"
 		fi
 
+		if [ "$ARCH" = "amd64/efi" ] ; then
+		        grub_install_params="$grub_install_params --target x86_64-efi"
+                fi
+
 		CODE=0
 		case $ARCH:$grub_package in
 		    *:grub|*:grub-pc|*:grub-efi*|sparc:grub-ieee1275|ppc64el/*:grub-ieee1275)
diff --git a/scripts/plugininstall.py b/scripts/plugininstall.py
index 53461ad..e9acdc9 100755
--- a/scripts/plugininstall.py
+++ b/scripts/plugininstall.py
@@ -945,6 +945,13 @@ class Install(install_misc.InstallBase):
                                 self.db.set('grub-installer/bootdev', response)
                         else:
                             break
+                    if arch == 'amd64' and subarch != 'efi':
+                        dbfilter = grubinstaller.GrubInstaller(
+                            None, self.db, extra_args=['amd64/efi'])
+                        ret = dbfilter.run_command(auto_process=True)
+                        if ret != 0:
+                            raise install_misc.InstallStepError(
+                                "GrubInstaller failed with code %d" % ret)
                 else:
                     raise install_misc.InstallStepError(
                         "No bootloader installer found")
diff --git a/ubiquity/components/grubinstaller.py b/ubiquity/components/grubinstaller.py
index 8c1af85..774d7cb 100644
--- a/ubiquity/components/grubinstaller.py
+++ b/ubiquity/components/grubinstaller.py
@@ -22,8 +22,15 @@ from ubiquity.filteredcommand import FilteredCommand
 
 
 class GrubInstaller(FilteredCommand):
+    def __init__(self, frontend, db=None, ui=None, extra_args=None):
+        super().__init__(frontend, db, ui)
+        self.extra_args = extra_args
+
     def prepare(self):
-        return (['/usr/share/grub-installer/grub-installer', '/target'],
+        cmd = ['/usr/share/grub-installer/grub-installer', '/target']
+        if self.extra_args is not None:
+            cmd.extend(self.extra_args)
+        return (cmd,
                 ['^grub-installer/bootdev$', 'ERROR'],
                 {'OVERRIDE_UNSUPPORTED_OS': '1'})