[bug#48314] [PATCH] Install guix system on Raspberry Pi

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

Hi,

Stefan <stefan-guix@vodafonemail.de> writes:

[...]

> new file mode 100644
> index 0000000000..0cfaffe056
> --- /dev/null
> +++ b/guix/build/kconfig.scm

[...]

> +(define-module (guix build kconfig)
> +  #:use-module  (ice-9 rdelim)
> +  #:use-module  (ice-9 regex)
> +  #:use-module  (srfi srfi-1)
> +  #:use-module  (srfi srfi-26)
> +  #:export (modify-defconfig
> +            verify-config))
> +
> +;; Commentary:
> +;;
> +;; Builder-side code to modify configurations for the Kconfig build system as
> +;; used by Linux and U-Boot.
> +;;
> +;; Code:
> +
> +(define (config-string->pair config-string)
> +  "Parse a configuration string like \"CONFIG_EXAMPLE=m\" into a key-value pair.
> +An error is thrown for invalid configurations.
> +
> +\"CONFIG_A=y\"            -> '(\"CONFIG_A\" . \"y\")
> +\"CONFIG_B=\\\"\\\"\"         -> '(\"CONFIG_B\" . \"\\\"\\\"\")
> +\"CONFIG_C=\"             -> '(\"CONFIG_C\" . \"\")
> +\"# CONFIG_E is not set\" -> '(\"CONFIG_E\" . #f)
> +\"CONFIG_D\"              -> '(\"CONFIG_D\" . #f)
> +\"# Any comment\"         -> '(#f . \"# Any comment\")
> +\"\"                      -> '(#f . \"\")
> +\"# CONFIG_E=y\"          -> (error \"Invalid configuration\")
> +\"CONFIG_E is not set\"   -> (error \"Invalid configuration\")
> +\"Anything else\"         -> (error \"Invalid configuration\")"
> +  (define config-regexp
> +    (make-regexp
> +     ;; (match:substring (string-match "=(.*)" "=") 1) returns "", but the
> +     ;; pattern "=(.+)?" makes it return #f instead.  From a "CONFIG_A=" we like
> +     ;; to get "", which later emits "CONFIG_A=" again.
> +     "^ *(#[\\t ]*)?(CONFIG_[a-zA-Z0-9_]+)([\\t ]*=[\\t ]*(.*)|([\\t ]+is[\\t ]+not[\\t ]+set))?$"))
> +
> +  (define config-comment-regexp
> +    (make-regexp "^([\\t ]*(#.*)?)$"))
> +
> +  (let ((match (regexp-exec config-regexp (string-trim-right config-string))))
> +    (if match
> +        (let* ((comment (match:substring match 1))
> +               (key (match:substring match 2))
> +               (unset (match:substring match 5))
> +               (value (and (not comment)
> +                           (not unset)
> +                           (match:substring match 4))))
> +          (if (eq? (not comment) (not unset))
> +              ;; The key is uncommented and set or commented and unset.
> +              (cons key value)
> +              ;; The key is set or unset ambigiously.
> +              (error (format #f "Invalid configuration, did you mean \"~a\"?"
> +                             (pair->config-string (cons key #f)))
> +                     config-string)))
> +        ;; This is not a valid or ambigious config-string, but mayby a comment.
> +        (if (regexp-exec config-comment-regexp config-string)
> +            ;; We keep valid comments.
> +            (cons #f config-string)
> +            (error "Invalid configuration" config-string)))))
> +
> +(define (pair->config-string pair)
> +  "Convert a PAIR back to a config-string."
> +  (let* ((key (first pair))
> +         (value (cdr pair)))
> +    (if (string? key)
> +        (if (string? value)
> +            (string-append key "=" value)
> +            (string-append "# " key " is not set"))
> +        value)))
> +
> +(define (defconfig->alist defconfig)
> +  "Convert the content of a DEFCONFIG (or .config) file into an alist."
> +  (with-input-from-file defconfig
> +    (lambda ()
> +      (let loop ((alist '())
> +                 (line (read-line)))
> +        (if (eof-object? line)
> +            ;; Building the alist is done, now check for duplicates.
> +            (let loop ((keys (map first (filter first alist)))
                                           ^
What is this filter used for here? EDIT: saw later, it's used to filter
out comments.

[...]

> +(define (verify-config config defconfig)
> +  "Verify that the CONFIG file contains all configurations from the DEFCONFIG
> +file and return #t in this case. Otherwise throw an error with the mismatching
> +keys and their values."
> +  (let* ((config-pairs (defconfig->alist config))
> +         (defconfig-pairs (defconfig->alist defconfig))
> +         (mismatching-pairs
> +          (remove (lambda (pair)
> +                    ;; Remove all configurations, whose values are #f and whose
> +                    ;; keys are not in config-pairs, as not in config-pairs
> +                    ;; means unset, …
> +                    (and (not (cdr pair))
> +                         (not (assoc-ref config-pairs (first pair)))))
> +                  ;; … from the defconfig-pairs different to config-pairs.

So, it finds mismatched configurations that exist in both CONFIG and
DEFCONFIG, but it doesn't error when there are configs that exist in
DEFCONFIG but missing from CONFIG, right?  Should it?

> +                  (lset-difference equal?
> +                                   ;; Remove comments by filtering with first.
> +                                   (filter first defconfig-pairs)
> +                                   config-pairs))))
> +    (if (null? mismatching-pairs)
> +        #t
> +        (error (format #f
> +                       "Mismatching configurations in ~a and ~a"
> +                       config
> +                       defconfig)
> +               (map (lambda (mismatching-pair)
> +                      (let* ((key (first mismatching-pair))
> +                             (defconfig-value (cdr mismatching-pair))
> +                             (config-value (assoc-ref config-pairs key)))
> +                        (cons key (list (list config-value defconfig-value)))))
> +                    mismatching-pairs)))))
>
>

I've made the following mostly cosmetic changes:

--8<---------------cut here---------------start------------->8---
2 files changed, 43 insertions(+), 45 deletions(-)
gnu/packages/bootloaders.scm | 18 +++++++++---------
guix/build/kconfig.scm       | 70 ++++++++++++++++++++++++++++++++++------------------------------------

modified   gnu/packages/bootloaders.scm
@@ -792,9 +792,9 @@ (define*-public (make-u-boot-package board triplet #:key defconfig configs)
                                                      "_" "-")))
       (native-inputs
        `(,@(if (not (same-arch?))
-             `(("cross-gcc" ,(cross-gcc triplet))
-               ("cross-binutils" ,(cross-binutils triplet)))
-             `())
+               `(("cross-gcc" ,(cross-gcc triplet))
+                 ("cross-binutils" ,(cross-binutils triplet)))
+               `())
          ,@(package-native-inputs u-boot)))
       (arguments
        `(#:modules ((ice-9 ftw)
@@ -808,8 +808,8 @@ (define*-public (make-u-boot-package board triplet #:key defconfig configs)
          #:make-flags
          (list "HOSTCC=gcc"
                ,@(if (not (same-arch?))
-                   `((string-append "CROSS_COMPILE=" ,triplet "-"))
-                   '()))
+                     `((string-append "CROSS_COMPILE=" ,triplet "-"))
+                     '()))
          #:phases
          (modify-phases %standard-phases
            (replace 'configure
@@ -828,7 +828,7 @@ (define*-public (make-u-boot-package board triplet #:key defconfig configs)
                        (apply invoke "make" `(,@make-flags ,config-name))
                        (verify-config ".config" config-file))
                      (begin
-                       (display "Invalid board name. Valid board names are:"
+                       (display "invalid board name; valid board names are:"
                                 (current-error-port))
                        (let ((suffix-len (string-length "_defconfig"))
                              (entries (scandir "configs")))
@@ -839,7 +839,7 @@ (define*-public (make-u-boot-package board triplet #:key defconfig configs)
                                                (string-drop-right file-name
                                                                   suffix-len))))
                                    (sort entries string-ci<)))
-                       (error "Invalid boardname ~s." ,board))))))
+                       (error "invalid boardname ~s" ,board))))))
            (add-after 'configure 'disable-tools-libcrypto
              ;; Disable libcrypto due to GPL and OpenSSL license
              ;; incompatibilities
@@ -881,8 +881,8 @@ (define-public u-boot-malta

 (define-public u-boot-am335x-boneblack
   (let ((base (make-u-boot-package "am335x_evm" "arm-linux-gnueabihf"
-               ;; Patch out other device trees to build image small enough to
-               ;; fit within typical partitioning schemes where the first
+               ;; Patch out other device trees to build an image small enough
+               ;; to fit within typical partitioning schemes where the first
                ;; partition begins at sector 2048.
                #:configs '("CONFIG_OF_LIST=\"am335x-evm am335x-boneblack\""))))
     (package
modified   guix/build/kconfig.scm
@@ -50,7 +50,8 @@ (define config-regexp
      ;; (match:substring (string-match "=(.*)" "=") 1) returns "", but the
      ;; pattern "=(.+)?" makes it return #f instead.  From a "CONFIG_A=" we like
      ;; to get "", which later emits "CONFIG_A=" again.
-     "^ *(#[\\t ]*)?(CONFIG_[a-zA-Z0-9_]+)([\\t ]*=[\\t ]*(.*)|([\\t ]+is[\\t ]+not[\\t ]+set))?$"))
+     (string-append "^ *(#[\\t ]*)?(CONFIG_[a-zA-Z0-9_]+)([\\t ]*="
+                    "[\\t ]*(.*)|([\\t ]+is[\\t ]+not[\\t ]+set))?$")))

   (define config-comment-regexp
     (make-regexp "^([\\t ]*(#.*)?)$"))
@@ -67,13 +68,13 @@ (define config-comment-regexp
               ;; The key is uncommented and set or commented and unset.
               (cons key value)
               ;; The key is set or unset ambigiously.
-              (error (format #f "Invalid configuration, did you mean \"~a\"?"
+              (error (format #f "invalid configuration, did you mean \"~a\"?"
                              (pair->config-string (cons key #f)))
                      config-string)))
-        ;; This is not a valid or ambigious config-string, but mayby a comment.
+        ;; This is not a valid or ambigious config-string, but maybe a
+        ;; comment.
         (if (regexp-exec config-comment-regexp config-string)
-            ;; We keep valid comments.
-            (cons #f config-string)
+            (cons #f config-string)     ;keep valid comments
             (error "Invalid configuration" config-string)))))

 (define (pair->config-string pair)
@@ -94,6 +95,7 @@ (define (defconfig->alist defconfig)
                  (line (read-line)))
         (if (eof-object? line)
             ;; Building the alist is done, now check for duplicates.
+            ;; Note: the filter invocation is used to remove comments.
             (let loop ((keys (map first (filter first alist)))
                        (duplicates '()))
               (if (null? keys)
@@ -102,11 +104,11 @@ (define (defconfig->alist defconfig)
                   (if (null? duplicates)
                       alist
                       (error
-                       (format #f "Duplicate configurations in ~a" defconfig)
+                       (format #f "duplicate configurations in ~a" defconfig)
                        duplicates))
                   ;; Continue the search for duplicates.
                   (loop (cdr keys)
-                        (if (member (first keys) (cdr keys) equal?)
+                        (if (member (first keys) (cdr keys))
                             (cons (first keys) duplicates)
                             duplicates))))
             ;; Build the alist.
@@ -127,13 +129,13 @@ (define (modify-defconfig defconfig configs)
   \"CONFIG_D=m\"
   \"CONFIG_E=\"
   \"# CONFIG_G is not set\"
-  ;; For convinience this abbrevation can be used for not set configurations.
+  ;; For convenience this abbrevation can be used for not set configurations.
   \"CONFIG_F\")

-Instead of a list, CONFGIS can be a string with one configuration per line."
-  (let* (;; Split the configs into a list of single configuations.
-         ;; To minimize mistakes, we support a string and a list of strings,
-         ;; each with newlines to separate configurations.
+Instead of a list, CONFIGS can be a string with one configuration per line."
+  (let* (;; Split the configs into a list of single configurations.  Both a
+         ;; string and or a list of strings is supported, each with newlines
+         ;; to separate configurations.
          (config-pairs (map config-string->pair
                             (append-map (cut string-split <>  #\newline)
                                         (if (string? configs)
@@ -141,45 +143,41 @@ (define (modify-defconfig defconfig configs)
                                             configs))))
          ;; Generate a blocklist from all valid keys in config-pairs.
          (blocklist (delete #f (map first config-pairs)))
-         ;; Generate an alist from the defconifg without the keys in blocklist.
+         ;; Generate an alist from the defconfig without the keys in blocklist.
          (filtered-defconfig-pairs (remove (lambda (pair)
                                              (member (first pair) blocklist))
                                            (defconfig->alist defconfig))))
     (with-output-to-file defconfig
       (lambda ()
-        (for-each
-           (lambda (pair)
-             (display (pair->config-string pair))
-             (newline))
-           (append filtered-defconfig-pairs config-pairs))))))
+        (for-each (lambda (pair)
+                    (display (pair->config-string pair))
+                    (newline))
+                  (append filtered-defconfig-pairs config-pairs))))))

 (define (verify-config config defconfig)
   "Verify that the CONFIG file contains all configurations from the DEFCONFIG
-file and return #t in this case. Otherwise throw an error with the mismatching
-keys and their values."
+file.  When the verification fails, raise an error with the mismatching keys
+and their values."
   (let* ((config-pairs (defconfig->alist config))
          (defconfig-pairs (defconfig->alist defconfig))
          (mismatching-pairs
           (remove (lambda (pair)
-                    ;; Remove all configurations, whose values are #f and whose
-                    ;; keys are not in config-pairs, as not in config-pairs
-                    ;; means unset, …
+                    ;; Remove all configurations, whose values are #f and
+                    ;; whose keys are not in config-pairs, as not in
+                    ;; config-pairs means unset, ...
                     (and (not (cdr pair))
                          (not (assoc-ref config-pairs (first pair)))))
-                  ;; … from the defconfig-pairs different to config-pairs.
+                  ;; ... from the defconfig-pairs different to config-pairs.
                   (lset-difference equal?
                                    ;; Remove comments by filtering with first.
                                    (filter first defconfig-pairs)
                                    config-pairs))))
-    (if (null? mismatching-pairs)
-        #t
-        (error (format #f
-                       "Mismatching configurations in ~a and ~a"
-                       config
-                       defconfig)
-               (map (lambda (mismatching-pair)
-                      (let* ((key (first mismatching-pair))
-                             (defconfig-value (cdr mismatching-pair))
-                             (config-value (assoc-ref config-pairs key)))
-                        (cons key (list (list config-value defconfig-value)))))
-                    mismatching-pairs)))))
+    (unless (null? mismatching-pairs)
+      (error (format #f "Mismatching configurations in ~a and ~a"
+                     config defconfig)
+             (map (lambda (mismatching-pair)
+                    (let* ((key (first mismatching-pair))
+                           (defconfig-value (cdr mismatching-pair))
+                           (config-value (assoc-ref config-pairs key)))
+                      (cons key (list (list config-value defconfig-value)))))
+                  mismatching-pairs)))))
--8<---------------cut here---------------end--------------->8---

And streamlined the commit messages as:

--8<---------------cut here---------------start------------->8---
build: kconfig: Add new module to modify defconfig files.

* guix/build/kconfig.scm: New file.
* Makefile.am: Register it.
* gnu/packages/bootloaders.scm (make-u-boot-package)
(make-u-boot-sunxi64-package): Add DEFCONFIGS and CONFIGS arguments.
(u-boot-am335x-boneblack, u-boot-pinebook)
(u-boot-novena,u-boot-rockpro64-rk3399): Simplify packages by using the new
keyword arguments.
--8<---------------cut here---------------end--------------->8---

Explanations don't go in the GNU ChangeLog, they go ideally in comments
in the code or *before* the GNU ChangeLog, if some rationale helps
understanding the change, so you can keep things dry there.

I'll push this change shortly.

-- 
Thanks,
Maxim