GNU bug report logs - #42634
[PATCH 0/3] Add image-type support.

Previous Next

Package: guix-patches;

Reported by: Mathieu Othacehe <m.othacehe <at> gmail.com>

Date: Fri, 31 Jul 2020 14:49:01 UTC

Severity: normal

Tags: patch

Done: Mathieu Othacehe <othacehe <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 42634 in the body.
You can then email your comments to 42634 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Fri, 31 Jul 2020 14:49:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mathieu Othacehe <m.othacehe <at> gmail.com>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Fri, 31 Jul 2020 14:49:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Mathieu Othacehe <othacehe <at> gnu.org>
Subject: [PATCH 0/3] Add image-type support.
Date: Fri, 31 Jul 2020 16:48:25 +0200
Hello,

This is the missing piece for the new image API. It has been previously
discussed here[1].

I think this is close to what Ludo suggested and it addresses janneke concerns
about composability.

The idea is to introduce the concept of "image-type". An image type is a
converter from an <operating-system> record to an <image> record.

I have created in this serie 4 new image type records:
- raw
- iso9660
- uncompressed-iso9660
- hurd-raw

I also adapted the "guix system" command by removing the "file-system-type"
argument and replaced it by the "image-type" argument.

The default is still to create a raw disk-image, but one can now call:

guix system disk-image -t iso9660 my-config.scm
guix system disk-image -t uncompressed-iso9660 my-config.scm
guix system disk-image -t hurd-raw my-config.scm

and so on. Maybe we should also rename "disk-image" command to "image" that
would be somehow more accurate.

The only missing bit is the documentation update, as I prefer to wait for this
change to be approved first.

WDYT?

Thanks,

Mathieu

[1]: https://lists.gnu.org/archive/html/guix-devel/2020-05/msg00417.html

Mathieu Othacehe (3):
  image: Add image-type support.
  system: image: Add image-type support.
  scripts: system: Add support for image-type.

 gnu/image.scm              | 29 ++++++++++++++-
 gnu/system/image.scm       | 75 ++++++++++++++++++++++++++++++++++++--
 gnu/system/images/hurd.scm | 13 +++++--
 guix/scripts/system.scm    | 56 +++++++++++++++++-----------
 4 files changed, 145 insertions(+), 28 deletions(-)

-- 
2.26.2





Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Fri, 31 Jul 2020 14:50:03 GMT) Full text and rfc822 format available.

Message #8 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: 42634 <at> debbugs.gnu.org
Cc: Mathieu Othacehe <othacehe <at> gnu.org>
Subject: [PATCH 1/3] image: Add image-type support.
Date: Fri, 31 Jul 2020 16:49:27 +0200
* gnu/image.scm (<image-type>): New record,
(image-type, image-type?, image-type-name,
image-type-constructor, os->image): new procedures.
---
 gnu/image.scm | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/gnu/image.scm b/gnu/image.scm
index dc66f2c533..6f8f4828ac 100644
--- a/gnu/image.scm
+++ b/gnu/image.scm
@@ -39,7 +39,14 @@
             image-partitions
             image-compression?
             image-volatile-root?
-            image-substitutable?))
+            image-substitutable?
+
+            image-type
+            image-type?
+            image-type-name
+            image-type-constructor
+
+            os->image))
 
 
 ;;;
@@ -84,3 +91,23 @@
                       (default #t))
   (substitutable?     image-substitutable? ;boolean
                       (default #t)))
+
+
+;;;
+;;; Image type.
+;;;
+
+(define-record-type* <image-type>
+  image-type make-image-type
+  image-type?
+  (name           image-type-name) ;string
+  (constructor    image-type-constructor)) ;<operating-system> -> <image>
+
+
+;;;
+;;; Image creation.
+;;;
+
+(define* (os->image os #:key type)
+  (let ((constructor (image-type-constructor type)))
+    (constructor os)))
-- 
2.26.2





Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Fri, 31 Jul 2020 14:50:03 GMT) Full text and rfc822 format available.

Message #11 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: 42634 <at> debbugs.gnu.org
Cc: Mathieu Othacehe <othacehe <at> gnu.org>
Subject: [PATCH 2/3] system: image: Add image-type support.
Date: Fri, 31 Jul 2020 16:49:28 +0200
* gnu/system/image.scm (image-with-os): New macro. Rename the old
"image-with-os" procedure to ...
(image-with-os*): ... this new procedure,
(system-image): adapt according,
(raw-image-type, iso-image-type, uncompressed-iso-image-type
%image-types): new variables,
(lookup-image-type-by-name): new procedure.
(find-image): remove it.
* gnu/system/images/hurd.scm (hurd-image-type): New variable,
use it to define ...
(hurd-disk-image): ... this variable, using "os->image" procedure.
* gnu/tests/install.scm (run-install): Rename
installation-disk-image-file-system-type parameter to installation-image-type,
use os->config instead of find-image to compute the image passed to system-image,
(%test-iso-image-installer) adapt accordingly,
(guided-installation-test): ditto.
---
 gnu/system/image.scm       | 88 ++++++++++++++++++++++++++++++--------
 gnu/system/images/hurd.scm | 13 ++++--
 gnu/tests/install.scm      | 46 ++++++++++----------
 3 files changed, 103 insertions(+), 44 deletions(-)

diff --git a/gnu/system/image.scm b/gnu/system/image.scm
index 36f56e237d..deee8a6412 100644
--- a/gnu/system/image.scm
+++ b/gnu/system/image.scm
@@ -18,6 +18,7 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu system image)
+  #:use-module (guix discovery)
   #:use-module (guix gexp)
   #:use-module (guix modules)
   #:use-module (guix monads)
@@ -62,8 +63,15 @@
             efi-disk-image
             iso9660-image
 
-            find-image
-            system-image))
+            image-with-os
+            raw-image-type
+            iso-image-type
+            uncompressed-iso-image-type
+
+            system-image
+
+            %image-types
+            lookup-image-type-by-name))
 
 
 ;;;
@@ -110,6 +118,37 @@
            (label "GUIX_IMAGE")
            (flags '(boot)))))))
 
+
+;;;
+;;; Images types.
+;;;
+
+(define-syntax-rule (image-with-os base-image os)
+  "Return an image inheriting from BASE-IMAGE, with the operating-system field
+set to the given OS."
+  (image
+   (inherit base-image)
+   (operating-system os)))
+
+(define raw-image-type
+  (image-type
+   (name "raw")
+   (constructor (cut image-with-os efi-disk-image <>))))
+
+(define iso-image-type
+  (image-type
+   (name "iso9660")
+   (constructor (cut image-with-os iso9660-image <>))))
+
+(define uncompressed-iso-image-type
+  (image-type
+   (name "uncompressed-iso9660")
+   (constructor (cut image-with-os
+                 (image
+                  (inherit iso9660-image)
+                  (compression? #f))
+                 <>))))
+
 
 ;;
 ;; Helpers.
@@ -426,7 +465,7 @@ used in the image. "
       image-size)
      (else root-size))))
 
-(define* (image-with-os base-image os)
+(define* (image-with-os* base-image os)
   "Return an image based on BASE-IMAGE but with the operating-system field set
 to OS.  Also set the UUID and the size of the root partition."
   (define root-file-system
@@ -507,7 +546,7 @@ image, depending on IMAGE format."
 
   (with-parameters ((%current-target-system target))
     (let* ((os (operating-system-for-image image))
-           (image* (image-with-os image os))
+           (image* (image-with-os* image os))
            (register-closures? (has-guix-service-type? os))
            (bootcfg (operating-system-bootcfg os))
            (bootloader (bootloader-configuration-bootloader
@@ -539,18 +578,33 @@ image, depending on IMAGE format."
           #:grub-mkrescue-environment
           '(("MKRESCUE_SED_MODE" . "mbr_only"))))))))
 
-(define (find-image file-system-type target)
-  "Find and return an image built that could match the given FILE-SYSTEM-TYPE,
-built for TARGET.  This is useful to adapt to interfaces written before the
-addition of the <image> record."
-  (match file-system-type
-    ("iso9660" iso9660-image)
-    (_ (cond
-        ((and target
-              (hurd-triplet? target))
-         (module-ref (resolve-interface '(gnu system images hurd))
-                     'hurd-disk-image))
-        (else
-         efi-disk-image)))))
+
+;;
+;; Image detection.
+;;
+
+(define (image-modules)
+  "Return the list of image modules."
+  (cons (resolve-interface '(gnu system image))
+        (all-modules (map (lambda (entry)
+                            `(,entry . "gnu/system/images/"))
+                          %load-path)
+                     #:warn warn-about-load-error)))
+
+(define %image-types
+  ;; The list of publically-known image types.
+  (delay (fold-module-public-variables (lambda (obj result)
+                                         (if (image-type? obj)
+                                             (cons obj result)
+                                             result))
+                                       '()
+                                       (image-modules))))
+
+(define (lookup-image-type-by-name name)
+  "Return the image type called NAME."
+  (or (srfi-1:find (lambda (image-type)
+                     (string=? name (image-type-name image-type)))
+                   (force %image-types))
+      (leave (G_ "~a: no such image type.~%") name)))
 
 ;;; image.scm ends here
diff --git a/gnu/system/images/hurd.scm b/gnu/system/images/hurd.scm
index d87640e8e3..67f657d289 100644
--- a/gnu/system/images/hurd.scm
+++ b/gnu/system/images/hurd.scm
@@ -29,8 +29,10 @@
   #:use-module (gnu system file-systems)
   #:use-module (gnu system hurd)
   #:use-module (gnu system image)
+  #:use-module (srfi srfi-26)
   #:export (hurd-barebones-os
             hurd-disk-image
+            hurd-image-type
             hurd-barebones-disk-image))
 
 (define hurd-barebones-os
@@ -82,8 +84,13 @@
            (flags '(boot))
            (initializer hurd-initialize-root-partition))))))
 
+(define hurd-image-type
+  (image-type
+   (name "hurd-raw")
+   (constructor (cut image-with-os hurd-disk-image <>))))
+
 (define hurd-barebones-disk-image
   (image
-   (inherit hurd-disk-image)
-   (name 'hurd-barebones-disk-image)
-   (operating-system hurd-barebones-os)))
+   (inherit
+    (os->image hurd-barebones-os #:type hurd-image-type))
+   (name 'hurd-barebones-disk-image)))
diff --git a/gnu/tests/install.scm b/gnu/tests/install.scm
index 9656e5f41f..0be9ee2892 100644
--- a/gnu/tests/install.scm
+++ b/gnu/tests/install.scm
@@ -218,7 +218,7 @@ reboot\n")
                            #:imported-modules '((gnu services herd)
                                                 (gnu installer tests)
                                                 (guix combinators))))
-                      (installation-disk-image-file-system-type "ext4")
+                      (installation-image-type "raw")
                       (install-size 'guess)
                       (target-size (* 2200 MiB)))
   "Run SCRIPT (a shell script following the system installation procedure) in
@@ -228,10 +228,6 @@ packages defined in installation-os."
 
   (mlet* %store-monad ((_      (set-grafting #f))
                        (system (current-system))
-                       (target (current-target-system))
-                       (base-image -> (find-image
-                                       installation-disk-image-file-system-type
-                                       target))
 
                        ;; Since the installation system has no network access,
                        ;; we cheat a little bit by adding TARGET to its GC
@@ -239,18 +235,20 @@ packages defined in installation-os."
                        ;; succeed.  Also add guile-final, which is pulled in
                        ;; through provenance.drv and may not always be present.
                        (target (operating-system-derivation target-os))
+                       (base-image ->
+                                   (os->image
+                                    (operating-system-with-gc-roots
+                                     os (list target guile-final))
+                                    #:type (lookup-image-type-by-name
+                                            installation-image-type)))
                        (image ->
-                        (system-image
-                         (image
-                          (inherit base-image)
-                          (size install-size)
-                          (operating-system
-                            (operating-system-with-gc-roots
-                             os (list target guile-final)))
-                          ;; Do not compress to speed-up the tests.
-                          (compression? #f)
-                          ;; Don't provide substitutes; too big.
-                          (substitutable? #f)))))
+                              (system-image
+                               (image
+                                (inherit base-image)
+                                (size install-size)
+
+                                ;; Don't provide substitutes; too big.
+                                (substitutable? #f)))))
     (define install
       (with-imported-modules '((guix build utils)
                                (gnu build marionette))
@@ -270,16 +268,16 @@ packages defined in installation-os."
                  "-no-reboot"
                  "-m" "1200"
                  #$@(cond
-                     ((string=? "ext4" installation-disk-image-file-system-type)
+                     ((string=? "raw" installation-image-type)
                       #~("-drive"
                          ,(string-append "file=" #$image
                                          ",if=virtio,readonly")))
-                     ((string=? "iso9660" installation-disk-image-file-system-type)
+                     ((string-contains installation-image-type "iso9660")
                       #~("-cdrom" #$image))
                      (else
                       (error
-                       "unsupported installation-disk-image-file-system-type:"
-                       installation-disk-image-file-system-type)))
+                       "unsupported installation-image-type:"
+                       installation-image-type)))
                  "-drive"
                  ,(string-append "file=" #$output ",if=virtio")
                  ,@(if (file-exists? "/dev/kvm")
@@ -443,8 +441,8 @@ reboot\n")
                                    %minimal-os-on-vda-source
                                    #:script
                                    %simple-installation-script-for-/dev/vda
-                                   #:installation-disk-image-file-system-type
-                                   "iso9660"))
+                                   #:installation-image-type
+                                   "uncompressed-iso9660"))
                          (command (qemu-command/writable-image image)))
       (run-basic-test %minimal-os-on-vda command name)))))
 
@@ -1309,8 +1307,8 @@ build (current-guix) and then store a couple of full system images.")
                                #:os installation-os-for-gui-tests
                                #:install-size install-size
                                #:target-size target-size
-                               #:installation-disk-image-file-system-type
-                               "iso9660"
+                               #:installation-image-type
+                               "uncompressed-iso9660"
                                #:gui-test
                                (lambda (marionette)
                                  (gui-test-program
-- 
2.26.2





Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Fri, 31 Jul 2020 14:50:03 GMT) Full text and rfc822 format available.

Message #14 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <m.othacehe <at> gmail.com>
To: 42634 <at> debbugs.gnu.org
Cc: Mathieu Othacehe <othacehe <at> gnu.org>
Subject: [PATCH 3/3] scripts: system: Add support for image-type.
Date: Fri, 31 Jul 2020 16:49:29 +0200
* guix/scripts/system.scm (list-image-types): New procedure,
(%options): add "image-type" and "list-image-types" options, remove
"file-system-type" option,
(show-help): adapt accordingly,
(%default-options): also adapt, and set the default "image-type" to "raw",
(perform-action): add image-type argument and remove file-system-type argument,
(process-action):  adapt perform-action call,
(system-derivation-for-action): remove base-image
argument, add image-type argument, and use it to create the image passed to
"system-image".
---
 guix/scripts/system.scm | 56 +++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/guix/scripts/system.scm b/guix/scripts/system.scm
index bfd50c7a79..4962401f36 100644
--- a/guix/scripts/system.scm
+++ b/guix/scripts/system.scm
@@ -659,8 +659,8 @@ checking this by themselves in their 'check' procedure."
 ;;; Action.
 ;;;
 
-(define* (system-derivation-for-action os base-image action
-                                       #:key image-size file-system-type
+(define* (system-derivation-for-action os action
+                                       #:key image-size image-type
                                        full-boot? container-shared-network?
                                        mappings)
   "Return as a monadic value the derivation for OS according to ACTION."
@@ -686,9 +686,8 @@ checking this by themselves in their 'check' procedure."
      (lower-object
       (system-image
        (image
-        (inherit base-image)
-        (size image-size)
-        (operating-system os)))))
+        (inherit (os->image os #:type image-type))
+        (size image-size)))))
     ((docker-image)
      (system-docker-image os #:shared-network? container-shared-network?))))
 
@@ -741,16 +740,17 @@ and TARGET arguments."
                          install-bootloader?
                          dry-run? derivations-only?
                          use-substitutes? bootloader-target target
-                         image-size file-system-type full-boot?
-                         container-shared-network?
+                         image-size image-type
+                         full-boot? container-shared-network?
                          (mappings '())
                          (gc-root #f))
   "Perform ACTION for OS.  INSTALL-BOOTLOADER? specifies whether to install
 bootloader; BOOTLOADER-TAGET is the target for the bootloader; TARGET is the
 target root directory; IMAGE-SIZE is the size of the image to be built, for
-the 'vm-image' and 'disk-image' actions.  The root file system is created as a
-FILE-SYSTEM-TYPE file system.  FULL-BOOT? is used for the 'vm' action; it
-determines whether to boot directly to the kernel or to the bootloader.
+the 'vm-image' and 'disk-image' actions.  IMAGE-TYPE is the type of image to
+be built.
+FULL-BOOT? is used for the 'vm' action; it determines whether to boot directly
+to the kernel or to the bootloader.
 CONTAINER-SHARED-NETWORK? determines if the container will use a separate
 network namespace.
 
@@ -792,10 +792,8 @@ static checks."
       (check-initrd-modules os)))
 
   (mlet* %store-monad
-      ((target*   (current-target-system))
-       (image ->  (find-image file-system-type target*))
-       (sys       (system-derivation-for-action os image action
-                                                #:file-system-type file-system-type
+       ((sys       (system-derivation-for-action os action
+                                                #:image-type image-type
                                                 #:image-size image-size
                                                 #:full-boot? full-boot?
                                                 #:container-shared-network? container-shared-network?
@@ -876,6 +874,17 @@ upgrade, and restart each service that was not automatically restarted.\n"))))))
                   #:node-type (shepherd-service-node-type shepherds)
                   #:reverse-edges? #t)))
 
+
+;;;
+;;; Images.
+;;;
+
+(define (list-image-types)
+  "Print the available image types."
+  (display (G_ "The available image types are:\n"))
+  (newline)
+  (format #t "~{   - ~a ~%~}" (map image-type-name (force %image-types))))
+
 
 ;;;
 ;;; Options.
@@ -935,9 +944,9 @@ Some ACTIONS support additional ARGS.\n"))
                          apply STRATEGY (one of nothing-special, backtrace,
                          or debug) when an error occurs while reading FILE"))
   (display (G_ "
-      --file-system-type=TYPE
-                         for 'disk-image', produce a root file system of TYPE
-                         (one of 'ext4', 'iso9660')"))
+      --list-image-types list available image types"))
+  (display (G_ "
+  -t, --image-type=TYPE  for 'disk-image', produce an image of TYPE"))
   (display (G_ "
       --image-size=SIZE  for 'vm-image', produce an image of SIZE"))
   (display (G_ "
@@ -994,10 +1003,14 @@ Some ACTIONS support additional ARGS.\n"))
                  (lambda (opt name arg result)
                    (alist-cons 'on-error (string->symbol arg)
                                result)))
-         (option '(#\t "file-system-type") #t #f
+         (option '(#\t "image-type") #t #f
                  (lambda (opt name arg result)
-                   (alist-cons 'file-system-type arg
+                   (alist-cons 'image-type arg
                                result)))
+         (option '("list-image-types") #f #f
+                 (lambda (opt name arg result)
+                   (list-image-types)
+                   (exit 0)))
          (option '("image-size") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'image-size (size->number arg)
@@ -1063,7 +1076,7 @@ Some ACTIONS support additional ARGS.\n"))
     (debug . 0)
     (verbosity . #f)                              ;default
     (validate-reconfigure . ,ensure-forward-reconfigure)
-    (file-system-type . "ext4")
+    (image-type . "raw")
     (image-size . guess)
     (install-bootloader? . #t)))
 
@@ -1150,7 +1163,8 @@ resulting from command-line parsing."
                                (assoc-ref opts 'skip-safety-checks?)
                                #:validate-reconfigure
                                (assoc-ref opts 'validate-reconfigure)
-                               #:file-system-type (assoc-ref opts 'file-system-type)
+                               #:image-type (lookup-image-type-by-name
+                                             (assoc-ref opts 'image-type))
                                #:image-size (assoc-ref opts 'image-size)
                                #:full-boot? (assoc-ref opts 'full-boot?)
                                #:container-shared-network?
-- 
2.26.2





Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Thu, 24 Sep 2020 15:35:01 GMT) Full text and rfc822 format available.

Message #17 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, 42634 <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 0/3] Add image-type support.
Date: Thu, 24 Sep 2020 17:34:20 +0200
Hi!

Mathieu Othacehe <m.othacehe <at> gmail.com> skribis:

> I think this is close to what Ludo suggested and it addresses janneke concerns
> about composability.
>
> The idea is to introduce the concept of "image-type". An image type is a
> converter from an <operating-system> record to an <image> record.
>
> I have created in this serie 4 new image type records:
> - raw
> - iso9660
> - uncompressed-iso9660
> - hurd-raw
>
> I also adapted the "guix system" command by removing the "file-system-type"
> argument and replaced it by the "image-type" argument.
>
> The default is still to create a raw disk-image, but one can now call:
>
> guix system disk-image -t iso9660 my-config.scm
> guix system disk-image -t uncompressed-iso9660 my-config.scm
> guix system disk-image -t hurd-raw my-config.scm

Neat!

> and so on. Maybe we should also rename "disk-image" command to "image" that
> would be somehow more accurate.

Yes.  We can do that separately, but I agree that it would make sense.

I wonder if ‘docker-image’ could also fit in there.

Apologies for taking so long to reply!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Thu, 24 Sep 2020 15:36:02 GMT) Full text and rfc822 format available.

Message #20 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, 42634 <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 1/3] image: Add image-type support.
Date: Thu, 24 Sep 2020 17:35:21 +0200
Mathieu Othacehe <m.othacehe <at> gmail.com> skribis:

> * gnu/image.scm (<image-type>): New record,
> (image-type, image-type?, image-type-name,
> image-type-constructor, os->image): new procedures.

[...]

> +(define-record-type* <image-type>
> +  image-type make-image-type
> +  image-type?
> +  (name           image-type-name) ;string

It’s subjective, but I’d make it a symbol, like we do for service types
for instance.

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Thu, 24 Sep 2020 15:39:01 GMT) Full text and rfc822 format available.

Message #23 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, 42634 <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 2/3] system: image: Add image-type support.
Date: Thu, 24 Sep 2020 17:37:53 +0200
Mathieu Othacehe <m.othacehe <at> gmail.com> skribis:

> * gnu/system/image.scm (image-with-os): New macro. Rename the old
> "image-with-os" procedure to ...
> (image-with-os*): ... this new procedure,
> (system-image): adapt according,
> (raw-image-type, iso-image-type, uncompressed-iso-image-type
> %image-types): new variables,
> (lookup-image-type-by-name): new procedure.
> (find-image): remove it.
> * gnu/system/images/hurd.scm (hurd-image-type): New variable,
> use it to define ...
> (hurd-disk-image): ... this variable, using "os->image" procedure.
> * gnu/tests/install.scm (run-install): Rename
> installation-disk-image-file-system-type parameter to installation-image-type,
> use os->config instead of find-image to compute the image passed to system-image,
> (%test-iso-image-installer) adapt accordingly,
> (guided-installation-test): ditto.

[...]

> +(define (lookup-image-type-by-name name)
> +  "Return the image type called NAME."
> +  (or (srfi-1:find (lambda (image-type)
> +                     (string=? name (image-type-name image-type)))
> +                   (force %image-types))
> +      (leave (G_ "~a: no such image type.~%") name)))

I’d raise a ‘&formatted-message’ condition instead.  That allows you to
remove (guix ui) from the loop.

You can also remove the period from the message here sine it’s not a
sentence.

Otherwise LGTM!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Thu, 24 Sep 2020 15:40:02 GMT) Full text and rfc822 format available.

Message #26 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <m.othacehe <at> gmail.com>
Cc: Mathieu Othacehe <othacehe <at> gnu.org>, 42634 <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 3/3] scripts: system: Add support for
 image-type.
Date: Thu, 24 Sep 2020 17:39:33 +0200
Mathieu Othacehe <m.othacehe <at> gmail.com> skribis:

> * guix/scripts/system.scm (list-image-types): New procedure,
> (%options): add "image-type" and "list-image-types" options, remove
> "file-system-type" option,
> (show-help): adapt accordingly,
> (%default-options): also adapt, and set the default "image-type" to "raw",
> (perform-action): add image-type argument and remove file-system-type argument,
> (process-action):  adapt perform-action call,
> (system-derivation-for-action): remove base-image
> argument, add image-type argument, and use it to create the image passed to
> "system-image".

LGTM (with corresponding doc)!

Perhaps you can also add a ‘guix system list-image-types’ command to
tests/guix-system.sh for good measure.  That’d at least catch broken
modules and similar.

Thanks,
Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#42634; Package guix-patches. (Wed, 30 Sep 2020 09:51:02 GMT) Full text and rfc822 format available.

Message #29 received at 42634 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 42634 <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 0/3] Add image-type support.
Date: Wed, 30 Sep 2020 11:50:33 +0200
Hey Ludo,

> I wonder if ‘docker-image’ could also fit in there.

Yes I think that 'docker-image' and 'vm-image' can both be absorbed by
the upcoming 'image' option.  Turns out, 'hurd-qcow2' image type already
produces what would be expected from 'vm-image'.

Thanks for reviewing :)

Mathieu




Reply sent to Mathieu Othacehe <othacehe <at> gnu.org>:
You have taken responsibility. (Wed, 30 Sep 2020 09:52:01 GMT) Full text and rfc822 format available.

Notification sent to Mathieu Othacehe <m.othacehe <at> gmail.com>:
bug acknowledged by developer. (Wed, 30 Sep 2020 09:52:02 GMT) Full text and rfc822 format available.

Message #34 received at 42634-done <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 42634-done <at> debbugs.gnu.org
Subject: Re: [bug#42634] [PATCH 3/3] scripts: system: Add support for
 image-type.
Date: Wed, 30 Sep 2020 11:51:33 +0200
> Perhaps you can also add a ‘guix system list-image-types’ command to
> tests/guix-system.sh for good measure.  That’d at least catch broken
> modules and similar.

Sure, fixed. I pushed this serie with the according documentation
update.

Thanks,

Mathieu




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 28 Oct 2020 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 178 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.