unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Danny Milosavljevic <dannym@scratchpost.org>
To: Stefan <stefan-guix@vodafonemail.de>
Cc: 41066@debbugs.gnu.org, Mathieu Othacehe <othacehe@gnu.org>,
	Efraim Flashner <efraim@flashner.co.il>,
	Maxim Cournoyer <maxim.cournoyer@gmail.com>
Subject: [bug#41066] [PATCH] gnu: bootloader: Support for chain loading.
Date: Tue, 17 Nov 2020 16:47:55 +0100	[thread overview]
Message-ID: <20201117164755.1a27422b@scratchpost.org> (raw)
In-Reply-To: <EF2E13C3-8DE6-4264-AAA0-B985333C1FB2@vodafonemail.de>

[-- Attachment #1: Type: text/plain, Size: 3877 bytes --]

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.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-11-17 15:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-03 23:34 [bug#41066] [PATCH] gnu: grub: Support for chain loading Stefan
2020-05-24 11:13 ` Danny Milosavljevic
2020-05-24 13:21   ` Stefan
2020-10-04 16:31     ` [bug#41066] [PATCH] gnu: bootloader: " Stefan
2020-10-10  9:31       ` Stefan
2020-10-18 11:20         ` Stefan
2020-10-18 11:21           ` Stefan
2020-10-22 17:46             ` Danny Milosavljevic
2020-10-23 12:48               ` Ludovic Courtès
2020-10-24  1:30                 ` Danny Milosavljevic
2020-10-24 16:22                   ` Ludovic Courtès
2020-10-25  0:33                     ` Danny Milosavljevic
2020-10-25 16:58                       ` Stefan
2020-10-25 16:59                         ` Stefan
2020-11-02 15:42                           ` Danny Milosavljevic
2020-11-02 16:21                             ` Mathieu Othacehe
2020-11-03  9:07                               ` Ludovic Courtès
2020-11-03  9:32                                 ` Mathieu Othacehe
2020-11-07 21:14                                   ` Stefan
2020-11-07 21:15                                     ` Stefan
2020-10-26 10:37                       ` Ludovic Courtès
2020-11-16  9:33             ` bug#41066: " Danny Milosavljevic
2020-11-17 14:26               ` [bug#41066] " Stefan
2020-11-17 15:47                 ` Danny Milosavljevic [this message]
2020-11-17 16:17                   ` Danny Milosavljevic
2020-11-17 20:27                   ` Stefan
2020-11-18 18:05                     ` Danny Milosavljevic
2020-11-18 18:20                       ` Stefan
2020-11-28 22:14                         ` [bug#41066] [PATCH] gnu: bootloader: Improve support " Stefan
2020-12-12 17:14                           ` Stefan
2020-12-13 14:42                             ` Danny Milosavljevic
2020-12-13 17:24                               ` Stefan
2020-12-13 19:28                                 ` Stefan
2020-12-28 19:02                                   ` Stefan
2021-03-27 16:48                                     ` bug#41066: " Stefan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201117164755.1a27422b@scratchpost.org \
    --to=dannym@scratchpost.org \
    --cc=41066@debbugs.gnu.org \
    --cc=efraim@flashner.co.il \
    --cc=maxim.cournoyer@gmail.com \
    --cc=othacehe@gnu.org \
    --cc=stefan-guix@vodafonemail.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).