| Message ID | 20200527072420.26140-6-othacehe@gnu.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | image: Add MBR based boot support. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
Mathieu Othacehe <m.othacehe@gmail.com> skribis: > * guix/utils.scm (target-intel?): New exported procedure. > * gnu/build/image.scm (initialize-efi-partition): Rename bootloader-package > argument to grub-efi. > * gnu/system/image.scm (system-disk-image): Adapt accordingly to pass > grub-efi package. Make sure that grub-efi is not built if we are not targeting > an Intel system. [...] > +++ b/gnu/build/image.scm > @@ -146,10 +146,10 @@ deduplicates files common to CLOSURE and the rest of PREFIX." > > (define* (initialize-efi-partition root > #:key > - bootloader-package > + grub-efi > #:allow-other-keys) > "Install in ROOT directory, an EFI loader using BOOTLOADER-PACKAGE." > - (install-efi-loader bootloader-package root)) > + (install-efi-loader grub-efi root)) Does it have to be GRUB? > +++ b/gnu/system/image.scm > @@ -235,6 +235,10 @@ used in the image." > #:references-graphs '#$graph > #:deduplicate? #f > #:system-directory #$os > + #:grub-efi #$(let-system (system target) > + (and (target-intel? > + (or target system)) > + grub-efi)) Some AArch64 systems such as the SoftIron OverDrive 1000 use EFI too. So I don’t think the above is correct. > +(define* (target-intel? #:optional (target (or (%current-target-system) > + (%current-system)))) > + (any (cut string-prefix? <> target) '("x86_64" "i686"))) Shouldn’t it include i[345]6 as well? Also, I think no 32-bit Intel systems use EFI. Actually, why do we need to guess, can’t we just use the bootloader specified in the <operating-system> record? (Naïve question, I haven’t checked…)
>> "Install in ROOT directory, an EFI loader using BOOTLOADER-PACKAGE." >> - (install-efi-loader bootloader-package root)) >> + (install-efi-loader grub-efi root)) > > Does it have to be GRUB? No any EFI compatible bootloader would do the trick. However, for now "install-efi-loader" calls "grub-mkstandalone", so maybe we can keep it that way for now? > >> +++ b/gnu/system/image.scm >> @@ -235,6 +235,10 @@ used in the image." >> #:references-graphs '#$graph >> #:deduplicate? #f >> #:system-directory #$os >> + #:grub-efi #$(let-system (system target) >> + (and (target-intel? >> + (or target system)) >> + grub-efi)) > > Some AArch64 systems such as the SoftIron OverDrive 1000 use EFI too. > So I don’t think the above is correct. Yes you're correct. Actually, I think that I was overthinking this thing. Using "#+grub-efi" should work. > >> +(define* (target-intel? #:optional (target (or (%current-target-system) >> + (%current-system)))) >> + (any (cut string-prefix? <> target) '("x86_64" "i686"))) > > Shouldn’t it include i[345]6 as well? So I removed this bit. > > Also, I think no 32-bit Intel systems use EFI. > > Actually, why do we need to guess, can’t we just use the bootloader > specified in the <operating-system> record? (Naïve question, I haven’t > checked…) If we take the example of "installation-os", we use "grub-bootloader", but we expect this image to be EFI ready. That's why "grub-efi" usage is forced here. As this is already the case without this serie, I think we can proceed. Thanks, Mathieu
diff --git a/gnu/build/image.scm b/gnu/build/image.scm index 49faeab466..a8594e202b 100644 --- a/gnu/build/image.scm +++ b/gnu/build/image.scm @@ -146,10 +146,10 @@ deduplicates files common to CLOSURE and the rest of PREFIX." (define* (initialize-efi-partition root #:key - bootloader-package + grub-efi #:allow-other-keys) "Install in ROOT directory, an EFI loader using BOOTLOADER-PACKAGE." - (install-efi-loader bootloader-package root)) + (install-efi-loader grub-efi root)) (define* (initialize-root-partition root #:key diff --git a/gnu/system/image.scm b/gnu/system/image.scm index 7ac998d861..a706f872a8 100644 --- a/gnu/system/image.scm +++ b/gnu/system/image.scm @@ -235,6 +235,10 @@ used in the image." #:references-graphs '#$graph #:deduplicate? #f #:system-directory #$os + #:grub-efi #$(let-system (system target) + (and (target-intel? + (or target system)) + grub-efi)) #:bootloader-package #+(bootloader-package bootloader) #:bootloader-installer diff --git a/guix/utils.scm b/guix/utils.scm index d7b197fa44..fb3b233286 100644 --- a/guix/utils.scm +++ b/guix/utils.scm @@ -74,6 +74,7 @@ %current-target-system package-name->name+version target-mingw? + target-intel? target-arm32? target-aarch64? target-arm? @@ -490,6 +491,10 @@ a character other than '@'." (and target (string-suffix? "-mingw32" target))) +(define* (target-intel? #:optional (target (or (%current-target-system) + (%current-system)))) + (any (cut string-prefix? <> target) '("x86_64" "i686"))) + (define* (target-arm32? #:optional (target (or (%current-target-system) (%current-system)))) (string-prefix? "arm" target))