Subject: Re: [PATCH] gnu: bootloader: Support for chain loading.

From: Danny Milosavljevic <[email-masked]>
Date: Tue, 17 Nov 2020 16:47:55 +0100

Hi Stefan,

oops.

I've reviewed and pushed the first part (s;HOOK;HOOKS;) now as guix master
commit ede4117f7f18e118003f2599f5c8e985dfbdf9a5.

But I'm not sure whether the COPY-FILES? thing is a nice API (and I mean not just
the flag).

I would prefer if the user would just change the INSTALLER in the case he wants
to not use the profile (which is kinda weird?!) or pack it or whatever.

In case the user wanted to index the profile content, the user would use a HOOK
to do that.

It really depends on what exactly efi-bootloader-chain is being presented as.

Is it a profile ?  Then it shouldn't have weird flags like
that--if possible--and instead use the standard methods.

For example you could do instead (I cut&pasted to show the idea--untested!):

(define* (efi-bootloader-chain files
                               final-bootloader
                               #:key
                               (hooks '())
                               installer)

  (let* ((final-installer (or installer
                              (lambda (bootloader target mount-point)
                                ((bootloader-installer bootloader) bootloader target mount-point)
            (let* ((mount-point/target (string-append mount-point target))
                     ;; When installing Guix, it's common to mount TARGET below
                     ;; MOUNT-POINT rather than below the root directory.
                     (bootloader-target (if (file-exists? mount-point/target)
                                            mount-point/target
                                            target)))
              (when (eq? (and=> (stat bootloader-target #f) stat:type)
                        'directory)
                (copy-recursively (string-append bootloader "/collection")
                                  bootloader-target
                                  #:follow-symlinks? #t
                                  #:log (%make-void-port "w")))))))))))))
         (profile (efi-bootloader-profile files
                                          (bootloader-package final-bootloader)
                                          (if (list? hooks)
                                              hooks
                                              (list hooks)))))
    (bootloader
     (inherit final-bootloader)
     (package profile)
     (installer
      #~(lambda (bootloader target mount-point)
          (#$final-installer bootloader target mount-point))))

This way the weird flag COPY-FILES? is gone with no loss of functionality to
the customizer.  It's possible for the customizer to read the bootloader
package (profile), so it's still possible to copy stuff if it's required
(pass a custom INSTALLER which does the copying and also some custom
installation).

I have a few questions:

(1) Why is there a $output/collection subdirectory?  Is there something
else than that in the profile output?  
If there are no good reasons to do it like that, I'd just put the
profile into $output directly instead--it's easier to understand, and also it's
how other profiles are being used.

(2) The COPY-FILES? flag is kinda weird.
I would prefer if INSTALLER just defaulted to a procedure that: does copy
files, and then calls the final bootloader installer.
If the user doesn't want it then the user could still pass a INSTALLER
that doesn't (for example the user could pass #:installer
(bootloader-installer final-bootloader)).

(3) Why isn't the final bootloader installed last?  I would have expected
it to be installed last so that if it does packing of the profile contents
in order to quickly find it at boot, it would have to have all the files
of the profiles already, no?

Could you explain what this is for in your use case?  I don't yet understand
the reason for this complexity.