Rearchitect how 'bootloader' key works

10 files changed, 82 insertions, 66 deletions

diff --git a/devel/script/2_keys.xml b/devel/script/2_keys.xml
index 9ffeba8..bd8469c 100644
--- a/devel/script/2_keys.xml
+++ b/devel/script/2_keys.xml
@@ -707,11 +707,11 @@ signingkey https://packages/builder@ourcompany.net.pub
             </formalpara>
             <formalpara id="bootloader.format">
                 <title>Format</title>
-                <para>The <literal>bootloader</literal> key is a string and/or boolean value.  If the value is <literal>false</literal>, no bootloader will be installed on the target computer and boot configuration must be performed externally.  If the value is <literal>true</literal>, Horizon will install the platform's preferred bootloader.  If the value is a string value, Horizon will attempt to install the specified bootloader.  If the string value does not correspond to a bootloader available for the target computer, script execution will fail.</para>
+                <para>The <literal>bootloader</literal> key is a space-separated list of the desired bootloader configuration.  The first item, which is required, determines the block device that the bootloader will be installed on.  The second item determines which bootloader to install.  If the second value is not specified, or if the value is <literal>true</literal>, Horizon will install the platform's preferred bootloader.  If the value is a string value, Horizon will attempt to install the specified bootloader.  If the string value does not correspond to a bootloader available for the target computer, script execution will fail.</para>
             </formalpara>
             <formalpara id="bootloader.default">
                 <title>Default</title>
-                <para>If no <literal>bootloader</literal> key is specified, the system will behave as if <literal>false</literal> was given.</para>
+                <para>If no <literal>bootloader</literal> key is specified, no bootloader will be installed on the target computer and boot configuration must be performed externally.</para>
             </formalpara>
             <formalpara id="bootloader.example">
                 <title>Example</title>
@@ -719,9 +719,9 @@ signingkey https://packages/builder@ourcompany.net.pub
                     <example>
                         <title>The <literal>bootloader</literal> Key</title>
                         <programlisting>
-bootloader true
+bootloader /dev/sda
                         </programlisting>
-                        <para>This will configure the target computer with the platform's preferred bootloader.</para>
+                        <para>This will configure the target computer with the platform's preferred bootloader installed to <literal>/dev/sda</literal>.</para>
                     </example>
                 </para>
             </formalpara>
diff --git a/hscript/meta.cc b/hscript/meta.cc
index 7552ab9..a0f958c 100644
--- a/hscript/meta.cc
+++ b/hscript/meta.cc
@@ -692,18 +692,6 @@ bool Version::execute() const {
 }
 
 
-Key *Bootloader::parseFromData(const std::string &data,
-                               const ScriptLocation &pos, int *errors, int *,
-                               const Script *script) {
-    if(data.find_first_of(" ") != std::string::npos) {
-        if(errors) *errors += 1;
-        output_error(pos, "bootloader: invalid bootloader", data);
-        return nullptr;
-    }
-
-    return new Bootloader(script, pos, data);
-}
-
 const std::string my_arch(const Horizon::Script *script) {
     const Key *arch_key = script->getOneValue("arch");
     if(arch_key != nullptr) {
@@ -740,38 +728,77 @@ const std::string my_arch(const Horizon::Script *script) {
     }
 }
 
+Key *Bootloader::parseFromData(const std::string &data,
+                               const ScriptLocation &pos, int *errors, int *,
+                               const Script *script) {
+    const std::string arch = my_arch(script);
+    std::string device, boot = "true";
+    std::string::size_type space = data.find_first_of(" ");
+
+    if(space == std::string::npos) {
+        device = data;
+    } else {
+        device = data.substr(0, space);
+        boot = data.substr(space + 1);
+
+        if(boot.find_first_of(" ") != std::string::npos) {
+            if(errors) *errors += 1;
+            output_error(pos, "bootloader: invalid bootloader", data);
+            return nullptr;
+        }
+    }
+
+    if(boot == "true") {
+        if(arch == "ppc64" || arch == "ppc") {
+            boot = "grub-ieee1275";
+        } else if(arch == "aarch64") {
+            boot = "grub-efi";
+        } else if(arch == "x86_64" || arch == "pmmx") {
+#ifdef HAS_INSTALL_ENV
+            if(fs::exists("/sys/firmware/efi")) {
+                boot = "grub-efi";
+            } else
+#endif  /* HAS_INSTALL_ENV */
+                boot = "grub-bios";
+        } else {
+            output_error(pos, "bootloader: no default for architecture", arch);
+            return nullptr;
+        }
+    }
+
+    return new Bootloader(script, pos, device, boot);
+}
+
 bool Bootloader::validate() const {
     const std::string arch = my_arch(script);
     bool valid_selection;
-
-    /* 'true' and 'false' are always valid. */
-    if(_value == "true" || _value == "false") return true;
+    const std::string candidate = this->bootloader();
 
     if(arch == "ppc64") {
         const static std::set<std::string> valid_ppc64 = {"grub-ieee1275"};
-        valid_selection = valid_ppc64.find(this->value()) != valid_ppc64.end();
+        valid_selection = valid_ppc64.find(candidate) != valid_ppc64.end();
     } else if(arch == "ppc") {
         const static std::set<std::string> valid_ppc = {"grub-ieee1275",
                                                         "iquik"};
-        valid_selection = valid_ppc.find(this->value()) != valid_ppc.end();
+        valid_selection = valid_ppc.find(candidate) != valid_ppc.end();
     } else if(arch == "aarch64") {
         const static std::set<std::string> valid_arm64 = {"grub-efi"};
-        valid_selection = valid_arm64.find(this->value()) != valid_arm64.end();
+        valid_selection = valid_arm64.find(candidate) != valid_arm64.end();
     } else if(arch == "armv7") {
         const static std::set<std::string> valid_arm = {};
-        valid_selection = valid_arm.find(this->value()) != valid_arm.end();
+        valid_selection = valid_arm.find(candidate) != valid_arm.end();
     } else if(arch == "pmmx") {
         const static std::set<std::string> valid_pmmx = {"grub-bios",
                                                          "grub-efi"};
-        valid_selection = valid_pmmx.find(this->value()) != valid_pmmx.end();
+        valid_selection = valid_pmmx.find(candidate) != valid_pmmx.end();
     } else if(arch == "x86_64") {
         const static std::set<std::string> valid_x86 = {"grub-bios",
                                                         "grub-efi"};
-        valid_selection = valid_x86.find(this->value()) != valid_x86.end();
+        valid_selection = valid_x86.find(candidate) != valid_x86.end();
     } else if(arch == "mips64" || arch == "mips" ||
               arch == "mips64el" || arch == "mipsel") {
         const static std::set<std::string> valid_mips = {};
-        valid_selection = valid_mips.find(this->value()) != valid_mips.end();
+        valid_selection = valid_mips.find(candidate) != valid_mips.end();
     } else {
         output_error(pos, "bootloader: unknown architecture", arch);
         return false;
@@ -779,38 +806,15 @@ bool Bootloader::validate() const {
 
     if(!valid_selection) {
         output_error(pos, "bootloader: architecture does not support loader",
-                     this->value());
+                     candidate);
     }
 
     return valid_selection;
 }
 
 bool Bootloader::execute() const {
-    /* Nothing to do. */
-    if(_value == "false") return true;
-
     const std::string arch = my_arch(script);
-    std::string method;
-
-    if(_value == "true") {
-        if(arch == "ppc64" || arch == "ppc") {
-            method = "grub-ieee1275";
-        } else if(arch == "aarch64") {
-            method = "grub-efi";
-        } else if(arch == "x86_64" || arch == "pmmx") {
-#ifdef HAS_INSTALL_ENV
-            if(fs::exists("/sys/firmware/efi")) {
-                method = "grub-efi";
-            } else
-#endif  /* HAS_INSTALL_ENV */
-                method = "grub-bios";
-        } else {
-            output_error(pos, "bootloader: no default for architecture", arch);
-            return false;
-        }
-    } else {
-        method = _value;
-    }
+    std::string method = _bootloader;
 
     if(method == "grub-efi") {
         if(script->options().test(Simulate)) {
@@ -818,7 +822,7 @@ bool Bootloader::execute() const {
                       << " --keys-dir etc/apk/keys add grub-efi"
                       << std::endl
                       << "chroot " << script->targetDirectory()
-                      << " grub-install" << std::endl;
+                      << " grub-install " << _device << std::endl;
             return true;
         }
 #ifdef HAS_INSTALL_ENV
@@ -836,7 +840,8 @@ bool Bootloader::execute() const {
               MS_REMOUNT | MS_NOEXEC | MS_NODEV | MS_NOSUID | MS_RELATIME,
               nullptr);
 
-        if(run_command("chroot", {script->targetDirectory(), "grub-install"})
+        if(run_command("chroot",
+                       {script->targetDirectory(), "grub-install", _device})
                 != 0) {
             output_error(pos, "bootloader: failed to install GRUB");
             return false;
@@ -855,7 +860,7 @@ bool Bootloader::execute() const {
                       << " --keys-dir etc/apk/keys add grub-bios"
                       << std::endl
                       << "chroot " << script->targetDirectory()
-                      << " grub-install" << std::endl;
+                      << " grub-install " << _device << std::endl;
             return true;
         }
 #ifdef HAS_INSTALL_ENV
@@ -865,7 +870,8 @@ bool Bootloader::execute() const {
             output_error(pos, "bootloader: couldn't add package");
             return false;
         }
-        if(run_command("chroot", {script->targetDirectory(), "grub-install"})
+        if(run_command("chroot",
+                       {script->targetDirectory(), "grub-install", _device})
                 != 0) {
             output_error(pos, "bootloader: failed to install GRUB");
             return false;
@@ -883,7 +889,8 @@ bool Bootloader::execute() const {
                       << " --keys-dir etc/apk/keys add grub-ieee1275"
                       << std::endl
                       << "chroot " << script->targetDirectory()
-                      << " grub-install" << std::endl;
+                      << " grub-install --macppc-directory=/boot/grub "
+                      << _device << std::endl;
             return true;
         }
 #ifdef HAS_INSTALL_ENV
@@ -893,7 +900,9 @@ bool Bootloader::execute() const {
             output_error(pos, "bootloader: couldn't add package");
             return false;
         }
-        if(run_command("chroot", {script->targetDirectory(), "grub-install"})
+        if(run_command("chroot",
+                       {script->targetDirectory(), "grub-install",
+                        "--macppc-directory=/boot/grub", _device})
                 != 0) {
             output_error(pos, "bootloader: failed to install GRUB");
             return false;
diff --git a/hscript/meta.hh b/hscript/meta.hh
index 8d56abb..32f8eb7 100644
--- a/hscript/meta.hh
+++ b/hscript/meta.hh
@@ -146,13 +146,20 @@ public:
     bool execute() const override;
 };
 
-class Bootloader : public StringKey {
+class Bootloader : public Key {
 private:
+    const std::string _device;
+    const std::string _bootloader;
+
     Bootloader(const Script *_s, const ScriptLocation &_p,
-               const std::string &_v) : StringKey(_s, _p, _v) {}
+               const std::string &_d, const std::string &_b) : Key(_s, _p),
+        _device(_d), _bootloader(_b) {}
 public:
     static Key *parseFromData(const std::string &, const ScriptLocation &,
                               int *, int *, const Script *);
+
+    const std::string device() const { return this->_device; }
+    const std::string bootloader() const { return this->_bootloader; }
     bool validate() const override;
     bool execute() const override;
 };
diff --git a/hscript/script_i.hh b/hscript/script_i.hh
index b82b807..12ec133 100644
--- a/hscript/script_i.hh
+++ b/hscript/script_i.hh
@@ -300,7 +300,7 @@ struct Script::ScriptPrivate {
     bool store_bootloader(Key *obj, const ScriptLocation &pos, int *errors,
                           int *, const ScriptOptions &) {
         if(boot) {
-            DUPLICATE_ERROR(boot, "bootloader", boot->value())
+            DUPLICATE_ERROR(boot, "bootloader", boot->bootloader())
             return false;
         }
         std::unique_ptr<Bootloader> b(dynamic_cast<Bootloader *>(obj));
diff --git a/tests/fixtures/0240-bootloader-invalid.installfile b/tests/fixtures/0240-bootloader-invalid.installfile
index 210fe7f..7f14f5a 100644
--- a/tests/fixtures/0240-bootloader-invalid.installfile
+++ b/tests/fixtures/0240-bootloader-invalid.installfile
@@ -4,4 +4,4 @@ pkginstall adelie-base
 rootpw $6$gumtLGmHwOVIRpQR$2M9PUO24hy5mofzWWf9a.YLbzOgOlUby1g0hDj.wG67E2wrrvys59fq02PPdxBdbgkLZFtjfEx6MHZwMBamwu/
 mount /dev/sda1 /
 arch ppc64
-bootloader grub-efi
+bootloader /dev/sda grub-efi
diff --git a/tests/fixtures/0249-bootloader-ppc.installfile b/tests/fixtures/0249-bootloader-ppc.installfile
index 3124cda..f734df0 100644
--- a/tests/fixtures/0249-bootloader-ppc.installfile
+++ b/tests/fixtures/0249-bootloader-ppc.installfile
@@ -4,4 +4,4 @@ pkginstall adelie-base
 rootpw $6$gumtLGmHwOVIRpQR$2M9PUO24hy5mofzWWf9a.YLbzOgOlUby1g0hDj.wG67E2wrrvys59fq02PPdxBdbgkLZFtjfEx6MHZwMBamwu/
 mount /dev/sda1 /
 arch ppc
-bootloader yaboot
+bootloader /dev/sda yaboot
diff --git a/tests/fixtures/0250-bootloader-x86efi.installfile b/tests/fixtures/0250-bootloader-x86efi.installfile
index bb18a9a..5d64d3e 100644
--- a/tests/fixtures/0250-bootloader-x86efi.installfile
+++ b/tests/fixtures/0250-bootloader-x86efi.installfile
@@ -4,4 +4,4 @@ pkginstall adelie-base
 rootpw $6$gumtLGmHwOVIRpQR$2M9PUO24hy5mofzWWf9a.YLbzOgOlUby1g0hDj.wG67E2wrrvys59fq02PPdxBdbgkLZFtjfEx6MHZwMBamwu/
 mount /dev/sda1 /
 arch pmmx
-bootloader grub-efi
+bootloader /dev/sda grub-efi
diff --git a/tests/fixtures/0251-bootloader-x86bios.installfile b/tests/fixtures/0251-bootloader-x86bios.installfile
index dbc461d..895c5b4 100644
--- a/tests/fixtures/0251-bootloader-x86bios.installfile
+++ b/tests/fixtures/0251-bootloader-x86bios.installfile
@@ -4,4 +4,4 @@ pkginstall adelie-base
 rootpw $6$gumtLGmHwOVIRpQR$2M9PUO24hy5mofzWWf9a.YLbzOgOlUby1g0hDj.wG67E2wrrvys59fq02PPdxBdbgkLZFtjfEx6MHZwMBamwu/
 mount /dev/sda1 /
 arch x86_64
-bootloader grub-bios
+bootloader /dev/sda grub-bios
diff --git a/tests/fixtures/0252-bootloader-alpha.installfile b/tests/fixtures/0252-bootloader-alpha.installfile
index d522901..2faf21d 100644
--- a/tests/fixtures/0252-bootloader-alpha.installfile
+++ b/tests/fixtures/0252-bootloader-alpha.installfile
@@ -4,4 +4,4 @@ pkginstall adelie-base
 rootpw $6$gumtLGmHwOVIRpQR$2M9PUO24hy5mofzWWf9a.YLbzOgOlUby1g0hDj.wG67E2wrrvys59fq02PPdxBdbgkLZFtjfEx6MHZwMBamwu/
 mount /dev/sda1 /
 arch alpha
-bootloader aboot
+bootloader /dev/sda aboot
diff --git a/ui/qt5/horizonwizard.cc b/ui/qt5/horizonwizard.cc
index 92b81db..d97ab3b 100644
--- a/ui/qt5/horizonwizard.cc
+++ b/ui/qt5/horizonwizard.cc
@@ -654,7 +654,7 @@ QString HorizonWizard::toHScript() {
     lines << "pkginstall openrc";
 
     if(this->grub) {
-        lines << "bootloader true";
+        lines << "bootloader " << QString::fromStdString(chosen_disk);
     }
 
     switch(this->binsh) {