Package: guix-patches;
Reported by: Stefan <stefan-guix <at> vodafonemail.de>
Date: Sun, 9 May 2021 15:33:02 UTC
Severity: normal
Tags: patch
Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Bug is archived. No further changes may be made.
Message #218 received at 48314 <at> debbugs.gnu.org (full text, mbox):
From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com> To: Stefan <stefan-guix <at> vodafonemail.de> Cc: Vagrant Cascadian <vagrant <at> debian.org>, Danny Milosavljevic <dannym <at> scratchpost.org>, Ludovic Courtès <ludo <at> gnu.org>, phodina <phodina <at> protonmail.com>, 48314 <at> debbugs.gnu.org Subject: Re: bug#48314: [PATCH] Install guix system on Raspberry Pi Date: Thu, 01 Dec 2022 11:22:41 -0500
Hi,
Stefan <stefan-guix <at> 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.