GNU bug report logs - #42146
[PATCH core-updates 1/?] build: substitute: Don't fail silently.

Previous Next

To reply to this bug, email your comments to 42146 AT debbugs.gnu.org.

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

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: guix-patches <at> gnu.org
Cc: mbakke <at> fastmail.com
Subject: [PATCH core-updates 1/?] build: substitute: Don't fail silently.
Date: Wed,  1 Jul 2020 00:09:13 +0200

* guix/build/utils.scm (substitute, substitute*)[require-matches?]: New
  argument.
---
Of course, this will require some changes in the build recipes. However,
many of the bugs I have seen before could've been prevented by this
behavior, so I believe it is worth it.

I am currently running 'guix build hello' with these changes; I will
send any follow-up fixes needed to this bug#.

 guix/build/utils.scm | 47 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 12 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index dc55c6745d..1bfb774c60 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -6,6 +6,7 @@
 ;;; Copyright © 2018 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2020 Efraim Flashner <efraim <at> flashner.co.il>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -747,29 +748,42 @@ PROC's result is returned."
       (lambda (key . args)
         (false-if-exception (delete-file template))))))
 
-(define (substitute file pattern+procs)
+(define* (substitute file pattern+procs #:key (require-matches? #t))
   "PATTERN+PROCS is a list of regexp/two-argument-procedure pairs.  For each
 line of FILE, and for each PATTERN that it matches, call the corresponding
 PROC as (PROC LINE MATCHES); PROC must return the line that will be written as
 a substitution of the original line.  Be careful about using '$' to match the
-end of a line; by itself it won't match the terminating newline of a line."
-  (let ((rx+proc  (map (match-lambda
-                        (((? regexp? pattern) . proc)
-                         (cons pattern proc))
-                        ((pattern . proc)
-                         (cons (make-regexp pattern regexp/extended)
-                               proc)))
-                       pattern+procs)))
+end of a line; by itself it won't match the terminating newline of a line.
+
+By default, SUBSTITUTE will raise a &message condition if one of the patterns
+fails to match. If a lack of matches is acceptable, pass #:require-matches? #f
+to disable this check."
+  (let ((rx+proc (map (match-lambda
+                       (((? regexp? pattern) . proc)
+                        (cons* #f "<unknown>" pattern proc))
+                       ((pattern . proc)
+                        (cons* #f pattern (make-regexp pattern regexp/extended)
+                              proc)))
+                      pattern+procs)))
     (with-atomic-file-replacement file
       (lambda (in out)
         (let loop ((line (read-line in 'concat)))
           (if (eof-object? line)
-              #t
+              (when require-matches?
+                (for-each
+                  (match-lambda
+                    ((#f pat . _)
+                     (raise (condition
+                             (&message
+                              (message (format #f "substitute: ~a: pattern failed to match: ~a" file pat))))))
+                    ((#t . _) #t))
+                  rx+proc))
               (let ((line (fold (lambda (r+p line)
                                   (match r+p
-                                    ((regexp . proc)
+                                    ((_ _ regexp . proc)
                                      (match (list-matches regexp line)
                                        ((and m+ (_ _ ...))
+                                        (set-car! r+p #t)
                                         (proc line m+))
                                        (_ line)))))
                                 line
@@ -814,9 +828,17 @@ match substring.
 Alternatively, FILE may be a list of file names, in which case they are
 all subject to the substitutions.
 
+By default, SUBSTITUTE* will raise a &message condition if one of the patterns
+fails to match on one of the files. If a lack of matches is acceptable,
+add #:require-matches? #f after FILE to disable this check.
+
 Be careful about using '$' to match the end of a line; by itself it won't
 match the terminating newline of a line."
     ((substitute* file ((regexp match-var ...) body ...) ...)
+     (substitute* file #:require-matches? #t
+                  ((regexp match-var ...) body ...) ...))
+    ((substitute* file #:require-matches? require-matches?
+                  ((regexp match-var ...) body ...) ...)
      (let ()
        (define (substitute-one-file file-name)
          (substitute
@@ -840,7 +862,8 @@ match the terminating newline of a line."
                                       (begin body ...)
                                       (substring l o (match:start m))
                                       r))))))))
-                ...)))
+                ...)
+          #:require-matches? require-matches?))
 
        (match file
          ((files (... ...))
-- 
2.26.2

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: [PATCH 2/?] build: bootstrap-configure: Allow lack of matches in
 substitute.
Date: Wed,  1 Jul 2020 00:11:21 +0200

Matches are not required here, as not every file will use every
variable.

* guix/build/gnu-bootstrap.scm (bootstrap-configure): Pass
  #:require-matches? #f to substitute*.
---
 guix/build/gnu-bootstrap.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/guix/build/gnu-bootstrap.scm b/guix/build/gnu-bootstrap.scm
index 1cb9dc5512..6ee520e301 100644
--- a/guix/build/gnu-bootstrap.scm
+++ b/guix/build/gnu-bootstrap.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -49,7 +50,7 @@ and object directories."
                   (format #t "Configuring ~a~%" template)
                   (let ((target (string-drop-right template 3)))
                     (copy-file template target)
-                    (substitute* target
+                    (substitute* target #:require-matches? #f
                       (("@VERSION@") version))))
                 (find-files modules
                             (lambda (fn st)
@@ -58,7 +59,7 @@ and object directories."
                   (format #t "Configuring ~a~%" template)
                   (let ((target (string-drop-right template 3)))
                     (copy-file template target)
-                    (substitute* target
+                    (substitute* target #:require-matches? #f
                       (("@GUILE@") guile)
                       (("@MODDIR@") moddir)
                       (("@GODIR@") godir))
-- 
2.26.2

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: [PATCH 3/?] gnu: commencement: Build fix for %bootstrap-mes-rewired.
Date: Wed,  1 Jul 2020 00:11:34 +0200

Not every file has to refer to all dependencies.

* gnu/packages/commencement.scm (%bootstrap-mes-rewired)[builder]: Allow
  lack of matches in substitute*.
---
 gnu/packages/commencement.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index ef250e037b..1c7cf3d810 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2018, 2019, 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
 ;;; Copyright © 2019, 2020 Marius Bakke <mbakke <at> fastmail.com>
 ;;; Copyright © 2020 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -283,7 +284,7 @@ pure Scheme to Tar and decompression in one easy step.")
                           (mescc (string-append bin "/mescc"))
                           (module (string-append out "/share/mes/module")))
                      (define (rewire file)
-                       (substitute* file
+                       (substitute* file #:require-matches? #f
                          ((mes) out)
                          (("/gnu/store[^ ]+mes-minimal-[^/)}\"]*") out)
                          (("/gnu/store[^ ]+guile-[^/]*/bin/guile") guile)
-- 
2.26.2

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: Re: [PATCH core-updates 1/?] build: substitute: Don't fail silently.
Date: Wed, 1 Jul 2020 00:50:07 +0200

[Message part 1 (text/plain, inline)]

The error messages I'm getting from this are quite indirect - there are
wrapped in quite a few layers. Is there a better way to report errors
inside a build utility?

ice-9/eval.scm:387:11: In procedure eval:
ice-9/eval.scm:387:11: Throw to key `srfi-34' with args `(#<condition &message [message: "substit ute: install.sh: pattern failed to match:  -xf"] 11aa480>)'.
builder for `/gnu/store/wdyzzh7rkg47hfp434w72ly9nay1yva1-mes-boot-0.22.drv' failed with exit code 1

Regards,
Jakub Kądziołka

[signature.asc (application/pgp-signature, inline)]

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: [PATCH 4/?] gnu: mes-boot: Use verbosity settings integrated into
 buildsystem
Date: Wed,  1 Jul 2020 02:42:34 +0200

* gnu/packages/commencement.scm (mes-boot): Set the V environment
  variable. Don't substitute in the install script.
---
 gnu/packages/commencement.scm | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 1c7cf3d810..295a378d11 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -407,6 +407,7 @@ $MES -e '(mescc)' module/mescc.scm -- \"$@\"
                        (string-append
                         mes "/share/mes/module"
                         ":" dir "/nyacc-0.99.0/module"))
+               (setenv "V" "1")
                (invoke "gash" "configure.sh"
                        (string-append "--prefix=" out)
                        (string-append "--host=i686-linux-gnu")))))
@@ -416,11 +417,6 @@ $MES -e '(mescc)' module/mescc.scm -- \"$@\"
          (delete 'check)
          (replace 'install
            (lambda _
-             (substitute* "install.sh"  ; show some progress
-               ((" -xf") " -xvf")
-               (("^( *)((cp|mkdir|tar) [^']*[^\\])\n" all space cmd)
-                (string-append space "echo '" cmd "'\n"
-                               space cmd "\n")))
              (invoke "sh" "install.sh")
              ;; Keep ASCII output, for friendlier comparison and bisection
              (let* ((out (assoc-ref %outputs "out"))
-- 
2.26.2

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: [PATCH 5/?] build-system/gnu: Allow lack of matches in substitution
 phases.
Date: Wed,  1 Jul 2020 02:42:42 +0200

* guix/build/utils.scm (patch-makefile-SHELL, patch-/usr/bin/file),
  guix/build/gnu-build-system.scm (patch-dot-desktop-files):
  Allow lack of matches in substitute*.
---
 guix/build/gnu-build-system.scm | 3 ++-
 guix/build/utils.scm            | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
index 2e7dff2034..4b96761233 100644
--- a/guix/build/gnu-build-system.scm
+++ b/guix/build/gnu-build-system.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2018 Mark H Weaver <mhw <at> netris.org>
 ;;; Copyright © 2020 Brendan Tildesley <mail <at> brendan.scot>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -724,7 +725,7 @@ which cannot be found~%"
                      ;; '.desktop' files contain translations and are always
                      ;; UTF-8-encoded.
                      (with-fluids ((%default-port-encoding "UTF-8"))
-                       (substitute* files
+                       (substitute* files #:require-matches? #f
                          (("^Exec=([^/[:blank:]\r\n]*)(.*)$" _ binary rest)
                           (string-append "Exec=" (which binary) rest))
                          (("^TryExec=([^/[:blank:]\r\n]*)(.*)$" _ binary rest)
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 1bfb774c60..a8218a7743 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -1005,7 +1005,7 @@ When KEEP-MTIME? is true, the atime/mtime of FILE are kept unchanged."
   (let ((st (stat file)))
     ;; Consider FILE is using an 8-bit encoding to avoid errors.
     (with-fluids ((%default-port-encoding #f))
-      (substitute* file
+      (substitute* file #:require-matches? #f
         (("^ *SHELL[[:blank:]]*:?=[[:blank:]]*([[:graph:]]*/)([[:graph:]]+)(.*)$"
           _ dir shell args)
          (let* ((old (string-append dir shell))
@@ -1033,7 +1033,7 @@ no replacement 'file' command, doing nothing~%")
       (let ((st (stat file)))
         ;; Consider FILE is using an 8-bit encoding to avoid errors.
         (with-fluids ((%default-port-encoding #f))
-          (substitute* file
+          (substitute* file #:require-matches? #f
             (("/usr/bin/file")
              (begin
                (format (current-error-port)
-- 
2.26.2

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

From: Jakub Kądziołka <kuba <at> kadziolka.net>
To: 42146 <at> debbugs.gnu.org
Subject: [PATCH 6/6] gnu: glibc-mesboot0: Fixup the fixup-configure phase.
Date: Wed,  1 Jul 2020 11:55:23 +0200

* gnu/packages/commencement.scm (glibc-mesboot0)[fixup-configure]: Don't
  repeat substitution. Fix indentation.
---
This is an interesting cleanup commit. I believe this concludes a nice
sampling of the consequences of this patch, and thus subsequent fixups
are unlikely to benefit from a review. I will be pushing my work to

https://github.com/NieDzejkob/guix/tree/core-updates

 gnu/packages/commencement.scm | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 295a378d11..837f4e2ecc 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -1418,19 +1418,17 @@ ac_cv_c_float_format='IEEE (little-endian)'
              (format (current-error-port)
                      "running ./configure ~a\n" (string-join configure-flags))
              (apply invoke "./configure" configure-flags)))
-                  (add-after 'configure 'fixup-configure
-                    (lambda _
-                      (let* ((out (assoc-ref %outputs "out"))
-                             (bash (assoc-ref %build-inputs "bash"))
-                             (shell (string-append bash "/bin/bash")))
-                        (substitute* "config.make"
-                          (("INSTALL = scripts/") "INSTALL = $(..)./scripts/"))
-                        (substitute* "config.make"
-                          (("INSTALL = scripts/") "INSTALL = $(..)./scripts/")
-                          (("BASH = ") (string-append
-                                        "SHELL = " shell "
-         BASH = ")))
-                        #t))))))))
+         (add-after 'configure 'fixup-configure
+           (lambda _
+             (let* ((out (assoc-ref %outputs "out"))
+                    (bash (assoc-ref %build-inputs "bash"))
+                    (shell (string-append bash "/bin/bash")))
+               (substitute* "config.make"
+                 (("INSTALL = scripts/") "INSTALL = $(..)./scripts/")
+                 (("BASH = ") (string-append
+                               "SHELL = " shell "
+BASH = ")))
+               #t))))))))
 
 (define gcc-mesboot0
   (package
-- 
2.26.2

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Jakub Kądziołka <kuba <at> kadziolka.net>
Cc: 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Thu, 19 Oct 2023 09:14:51 -0400

Hi Jakub,

Sorry for the late reply; nice series!

Jakub Kądziołka <kuba <at> kadziolka.net> writes:

> The error messages I'm getting from this are quite indirect - there are
> wrapped in quite a few layers. Is there a better way to report errors
> inside a build utility?
>
> ice-9/eval.scm:387:11: In procedure eval:
> ice-9/eval.scm:387:11: Throw to key `srfi-34' with args `(#<condition &message [message: "substit ute: install.sh: pattern failed to match:  -xf"] 11aa480>)'.
> builder for `/gnu/store/wdyzzh7rkg47hfp434w72ly9nay1yva1-mes-boot-0.22.drv' failed with exit code 1

I don't think it's possible currently; I guess we'd have to serialize
the exceptions in the builder to recover them on the host to present
them in a nicer fashion.

-- 
Thanks,
Maxim

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 1/3] build: Relocate <regexp*> record and associated
 procedures here.
Date: Thu, 19 Oct 2023 16:33:32 -0400

* etc/teams.scm.in (<regexp*>, make-regexp*, regexp*-exec): Move to...
* guix/build/utils.scm: ... here.
(list-matches*): New procedure.

Change-Id: I566ac372f7d8ba08de94e19b54dcc68da2106a23
---
 etc/teams.scm.in     | 19 +------------------
 guix/build/utils.scm | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index 55242caad1..8af25b9802 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -34,29 +34,12 @@
              (srfi srfi-9)
              (srfi srfi-26)
              (ice-9 format)
-             (ice-9 regex)
              (ice-9 match)
              (ice-9 rdelim)
+             (guix build utils)
              (guix ui)
              (git))
 
-(define-record-type <regexp*>
-  (%make-regexp* pat flag rx)
-  regexp*?
-  (pat regexp*-pattern)
-  (flag regexp*-flag)
-  (rx regexp*-rx))
-
-;;; Work around regexp implementation.
-;;; This record allows to track the regexp pattern and then display it.
-(define* (make-regexp* pat #:optional (flag regexp/extended))
-  "Alternative to `make-regexp' producing annotated <regexp*> objects."
-  (%make-regexp* pat flag (make-regexp pat flag)))
-
-(define (regexp*-exec rx* str)
-  "Execute the RX* regexp, a <regexp*> object."
-  (regexp-exec (regexp*-rx rx*) str))
-
 (define-record-type <team>
   (make-team id name description members scope)
   team?
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 8e630ad586..2b3a8e278b 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2020, 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
+;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;; Copyright © 2023 Carlo Zancanaro <carlo <at> zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -28,6 +29,7 @@
 
 (define-module (guix build utils)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -55,6 +57,14 @@ (define-module (guix build utils)
             package-name->name+version
             parallel-job-count
 
+            make-regexp*
+            regexp*-exec
+            regexp*?
+            regexp*-pattern
+            regexp*-flag
+            regexp*-rx
+            list-matches*
+
             compressor
             tarball?
             %xz-parallel-args
@@ -163,6 +173,35 @@ (define-syntax-rule (define-constant name val)
    (module-replace! (current-module) '(setvbuf)))
   (else #f))
 
+
+;;;
+;;; Extend regexp objects with a pattern field.
+;;;
+(define-record-type <regexp*>
+  (%make-regexp* pat flag rx)
+  regexp*?
+  (pat regexp*-pattern)                 ;the regexp pattern, a string
+  (flag regexp*-flag)                   ;regexp flags
+  (rx regexp*-rx))                      ;the compiled regexp object
+
+;;; Work around regexp implementation.
+;;; This record allows to track the regexp pattern and then display it.
+(define* (make-regexp* pat #:optional (flag regexp/extended))
+  "Alternative to `make-regexp' producing annotated <regexp*> objects."
+  (%make-regexp* pat flag (make-regexp pat flag)))
+
+(define (regexp*-exec rx* str)
+  "Execute the RX* regexp, a <regexp*> object."
+  (regexp-exec (regexp*-rx rx*) str))
+
+(define* (list-matches* regexp str #:optional (flags regexp/extended))
+  "Like 'list-matches', but also accepting a regexp* as REGEXP."
+  (match regexp
+    ((or (? string?) (? regexp?))
+     (list-matches regexp str flags))
+    ((? regexp*?)
+     (list-matches (regexp*-rx regexp) str flags))))
+
 
 ;;;
 ;;; Compression helpers.

base-commit: d59653b7c9e43ebdbba20e2ca071429507f94c67
-- 
2.41.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 2/3] build: substitute: Error when no substitutions were done.
Date: Thu, 19 Oct 2023 16:33:33 -0400

From: Jakub Kądziołka <kuba <at> kadziolka.net>

* guix/build/utils.scm (substitute, substitute*)
[require-matches?]: New argument.
* tests/build-utils.scm ("substitute*"): New test group.
("substitute*, no match error")
("substitute*, partial no match error"): New tests.

Co-authored-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Change-Id: I66ed33d72aa73cd35e5642521efec70bf756f86e
---
 guix/build/utils.scm  | 94 +++++++++++++++++++++++++++++++++----------
 tests/build-utils.scm | 63 +++++++++++++++++++----------
 2 files changed, 114 insertions(+), 43 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 2b3a8e278b..7bfb6560e1 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -6,7 +6,8 @@
 ;;; Copyright © 2018, 2022 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2020 Efraim Flashner <efraim <at> flashner.co.il>
-;;; Copyright © 2020, 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
+;;; Copyright © 2020, 2021, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
 ;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
@@ -971,24 +972,53 @@ (define (replace-char c1 c2 s)
                         c))
                   s)))
 
-(define (substitute file pattern+procs)
+(define* (substitute file pattern+procs #:key (require-matches? #t))
   "PATTERN+PROCS is a list of regexp/two-argument-procedure pairs.  For each
 line of FILE, and for each PATTERN that it matches, call the corresponding
 PROC as (PROC LINE MATCHES); PROC must return the line that will be written as
 a substitution of the original line.  Be careful about using '$' to match the
-end of a line; by itself it won't match the terminating newline of a line."
-  (let ((rx+proc  (map (match-lambda
-                        (((? regexp? pattern) . proc)
+end of a line; by itself it won't match the terminating newline of a line.
+
+By default, SUBSTITUTE will raise a &message condition if one of the patterns
+fails to match.  REQUIRE-MATCHES? can be set to false when lack of matches is
+acceptable (e.g. if you have multiple potential patterns not guaranteed to be
+found in FILE)."
+  (define (rx->pattern m)
+    (match m
+      ((? regexp? pattern)
+       "<unknown pattern (regexp)>")
+      ((? regexp*? pattern)
+       (regexp*-pattern pattern))
+      ((? string? pattern)
+       pattern)))
+
+  (define (make-condition failed-matches)
+    (condition
+     (&message
+      (message (format #f "substitute: `~a': no match for patterns `~a'"
+                       file failed-matches)))))
+
+  (let ((rx+proc (map (match-lambda
+                        (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
                          (cons pattern proc))
                         ((pattern . proc)
-                         (cons (make-regexp pattern regexp/extended)
-                               proc)))
-                       pattern+procs)))
+                         (cons (make-regexp* pattern regexp/extended) proc)))
+                      pattern+procs)))
     (with-atomic-file-replacement file
       (lambda (in out)
-        (let loop ((line (read-line in 'concat)))
+        (let loop ((line (read-line in 'concat))
+                   (ok-matches '())
+                   (failed-matches '()))
           (if (eof-object? line)
-              #t
+              (when require-matches?
+                (let ((failed-patterns (lset-difference
+                                        string=?
+                                        (delete-duplicates
+                                         (map rx->pattern failed-matches))
+                                        (delete-duplicates
+                                         (map rx->pattern ok-matches)))))
+                  (when (not (null? failed-patterns))
+                    (raise (make-condition failed-patterns)))))
               ;; Work around the fact that Guile's regexp-exec does not handle
               ;; NUL characters (a limitation of the underlying GNU libc's
               ;; regexec) by temporarily replacing them by an unused private
@@ -998,19 +1028,30 @@ (define (substitute file pattern+procs)
                                     (unused-private-use-code-point line))
                                #\nul))
                      (line* (replace-char #\nul nul* line))
-                     (line1* (fold (lambda (r+p line)
-                                     (match r+p
-                                       ((regexp . proc)
-                                        (match (list-matches regexp line)
-                                          ((and m+ (_ _ ...))
-                                           (proc line m+))
-                                          (_ line)))))
-                                   line*
-                                   rx+proc))
+                     (results           ;line, ok-matches and failed-matches
+                      (fold (lambda (r+p results)
+                              (let ((line (first results))
+                                    (ok-matches (second results))
+                                    (failed-matches (third results)))
+                                (match r+p
+                                  ((regexp . proc)
+                                   (match (list-matches* regexp line)
+                                     ((and m+ (_ _ ...))
+                                      (list (proc line m+)
+                                            (cons regexp ok-matches)
+                                            failed-matches))
+                                     (_
+                                      (list line
+                                            ok-matches
+                                            (cons regexp failed-matches))))))))
+                            (list line* '() '())
+                            rx+proc))
+                     (line1* (first results))
+                     (ok-matches (second results))
+                     (failed-matches (third results))
                      (line1 (replace-char nul* #\nul line1*)))
                 (display line1 out)
-                (loop (read-line in 'concat)))))))))
-
+                (loop (read-line in 'concat) ok-matches failed-matches))))))))
 
 (define-syntax let-matches
   ;; Helper macro for `substitute*'.
@@ -1048,9 +1089,17 @@ (define-syntax substitute*
 Alternatively, FILE may be a list of file names, in which case they are
 all subject to the substitutions.
 
+By default, SUBSTITUTE* will raise a &message condition if one of the patterns
+fails to match on one of the files; REQUIRE-MATCHES? may be set to false to
+avoid an error being raised in such condition.
+
 Be careful about using '$' to match the end of a line; by itself it won't
 match the terminating newline of a line."
     ((substitute* file ((regexp match-var ...) body ...) ...)
+     (substitute* file #:require-matches? #t
+                  ((regexp match-var ...) body ...) ...))
+    ((substitute* file #:require-matches? require-matches?
+                  ((regexp match-var ...) body ...) ...)
      (let ()
        (define (substitute-one-file file-name)
          (substitute
@@ -1074,7 +1123,8 @@ (define-syntax substitute*
                                       (begin body ...)
                                       (substring l o (match:start m))
                                       r))))))))
-                ...)))
+                ...)
+          #:require-matches? require-matches?))
 
        (match file
          ((files (... ...))
diff --git a/tests/build-utils.scm b/tests/build-utils.scm
index 3babf5d544..890fbca16f 100644
--- a/tests/build-utils.scm
+++ b/tests/build-utils.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2015, 2016, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado <at> elephly.net>
-;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
 ;;;
@@ -289,26 +289,47 @@ (define (arg-test bash-args)
 (test-assert "wrap-script, argument handling, bash --norc"
   (arg-test " --norc"))
 
-(test-equal "substitute*, text contains a NUL byte, UTF-8"
-  "c\0d"
-  (with-fluids ((%default-port-encoding "UTF-8")
-                (%default-port-conversion-strategy 'error))
-    ;; The GNU libc is locale sensitive.  Depending on the value of LANG, the
-    ;; test could fail with "string contains #\\nul character: ~S" or "cannot
-    ;; convert wide string to output locale".
-    (setlocale LC_ALL "en_US.UTF-8")
-    (call-with-temporary-output-file
-     (lambda (file port)
-       (format port "a\0b")
-       (flush-output-port port)
-
-       (substitute* file
-         (("a") "c")
-         (("b") "d"))
-
-       (with-input-from-file file
-         (lambda _
-           (get-string-all (current-input-port))))))))
+(test-group "substitute*"
+  (define-syntax-rule (define-substitute*-test test-type name expected
+                        content clauses ...)
+    (test-type
+     name
+     expected
+     (with-fluids ((%default-port-encoding "UTF-8")
+                   (%default-port-conversion-strategy 'error))
+       ;; The GNU libc is locale sensitive.  Depending on the value of LANG,
+       ;; the test could fail with "string contains #\\nul character: ~S" or
+       ;; "cannot convert wide string to output locale".
+       (setlocale LC_ALL "en_US.UTF-8")
+       (call-with-temporary-output-file
+        (lambda (file port)
+          (format port content)
+          (flush-output-port port)
+
+          (substitute* file
+            clauses ...)
+
+          (with-input-from-file file
+            (lambda _
+              (get-string-all (current-input-port)))))))))
+
+  (define-substitute*-test test-equal
+    "substitute*, text contains a NUL byte, UTF-8"
+    "c\0d"                              ;expected
+    "a\0b"                              ;content
+    (("a") "c")
+    (("b") "d"))
+
+  (define-substitute*-test test-error "substitute*, no match error"
+    #t                                  ;expected
+    "a\0b"                              ;content
+    (("Oops!") "c"))
+
+  (define-substitute*-test test-error "substitute*, partial no match error"
+    #t                                  ;expected
+    "a\0b"                              ;content
+    (("a") "c"
+     ("Oops!") "c")))
 
 (test-equal "search-input-file: exception if not found"
   `((path)
-- 
2.41.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH 3/3] build: bootstrap-configure: Allow lack of matches in
 substitute.
Date: Thu, 19 Oct 2023 16:33:34 -0400

From: Jakub Kądziołka <kuba <at> kadziolka.net>

Matches are not required here, as not every file will use every
variable.

* guix/build/gnu-bootstrap.scm (bootstrap-configure): Pass
  #:require-matches? #f to substitute*.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
---
 guix/build/gnu-bootstrap.scm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/guix/build/gnu-bootstrap.scm b/guix/build/gnu-bootstrap.scm
index b4257a3717..d044c8acd9 100644
--- a/guix/build/gnu-bootstrap.scm
+++ b/guix/build/gnu-bootstrap.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020, 2022 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -51,6 +52,7 @@ (define (bootstrap-configure name version modules scripts)
                   (let ((target (string-drop-right template 3)))
                     (copy-file template target)
                     (substitute* target
+                      #:require-matches? #f
                       (("@PACKAGE_NAME@") name)
                       (("@VERSION@") version))))
                 (append-map (lambda (dir) (find-files dir "\\.in$"))
@@ -60,14 +62,14 @@ (define (bootstrap-configure name version modules scripts)
                   (let ((target (string-drop-right template 3)))
                     (copy-file template target)
                     (substitute* target
+                      #:require-matches? #f
                       (("@GUILE@") guile)
                       (("@MODDIR@") moddir)
                       (("@GODIR@") godir))
                     (chmod target #o755)))
                 (find-files scripts
                             (lambda (fn st)
-                              (string-suffix? ".in" fn))))
-      #t)))
+                              (string-suffix? ".in" fn)))))))
 
 (define (bootstrap-build modules)
   "Create a procedure that builds an early bootstrap package.  The
-- 
2.41.0

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Thu, 19 Oct 2023 22:40:45 +0200

Hi,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> * etc/teams.scm.in (<regexp*>, make-regexp*, regexp*-exec): Move to...
> * guix/build/utils.scm: ... here.
> (list-matches*): New procedure.


[...]

> +++ b/guix/build/utils.scm

[...]

> +;;;
> +;;; Extend regexp objects with a pattern field.
> +;;;
> +(define-record-type <regexp*>
> +  (%make-regexp* pat flag rx)
> +  regexp*?
> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
> +  (flag regexp*-flag)                   ;regexp flags
> +  (rx regexp*-rx))                      ;the compiled regexp object
> +
> +;;; Work around regexp implementation.
> +;;; This record allows to track the regexp pattern and then display it.
> +(define* (make-regexp* pat #:optional (flag regexp/extended))

I’m skeptical about the concrete benefits.  I would not include it in
(guix build utils), or at least not in this patch series.

(I tend to be super conservative about (guix build utils) because we
rarely get a chance to change it.)

Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Thu, 19 Oct 2023 22:49:44 +0200

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> From: Jakub Kądziołka <kuba <at> kadziolka.net>
>
> * guix/build/utils.scm (substitute, substitute*)
> [require-matches?]: New argument.
> * tests/build-utils.scm ("substitute*"): New test group.
> ("substitute*, no match error")
> ("substitute*, partial no match error"): New tests.
>
> Co-authored-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
> Change-Id: I66ed33d72aa73cd35e5642521efec70bf756f86e

[...]

> -(define (substitute file pattern+procs)
> +(define* (substitute file pattern+procs #:key (require-matches? #t))
>    "PATTERN+PROCS is a list of regexp/two-argument-procedure pairs.  For each
>  line of FILE, and for each PATTERN that it matches, call the corresponding

As discussed on IRC recently, I’d suggest:

  #:key (require-matches? (%substitute-requires-matches?))

where:

  (define %substitute-requires-matches?
    (make-parameter #t))

That way it’ll be easier to change the default for entire builds if we
need to.

> +              (when require-matches?
> +                (let ((failed-patterns (lset-difference
> +                                        string=?
> +                                        (delete-duplicates
> +                                         (map rx->pattern failed-matches))
> +                                        (delete-duplicates
> +                                         (map rx->pattern ok-matches)))))

That’s potentially costly.

Would it be enough to thread a list of unmatched regexps, and to
(delq rx unmatched) every time RX is matched?  (This is O(N) but N, the
number of regexps, is typically a handful.)

Then at the end, we’d check whether UNMATCHED is empty.

> +                  (when (not (null? failed-patterns))
> +                    (raise (make-condition failed-patterns)))))

SRFI-35 ‘make-condition’ expects different arguments.

Should probably be: (raise (condition (&substitute-error …))).

> +    ((substitute* file #:require-matches? require-matches?
> +                  ((regexp match-var ...) body ...) ...)

Maybe rather:

   (substitute* file
                ((regexp match-var ...) body ...) ...
                #:require-matches? require-matches?)

That way we formatting remains unchanged.

> +(test-group "substitute*"

I’d avoid groups: they’re not super useful and the output with the
Automake driver is terrible.

Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Thu, 19 Oct 2023 22:51:40 +0200

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> From: Jakub Kądziołka <kuba <at> kadziolka.net>
>
> Matches are not required here, as not every file will use every
> variable.
>
> * guix/build/gnu-bootstrap.scm (bootstrap-configure): Pass
>   #:require-matches? #f to substitute*.
>
> Signed-off-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

To me these two special cases suggest we may discover other more cases
where it’s okay to not match and we’d think.  We’ll need a dedicated
branch built on ci.guix to assess that.

Thanks for working on it!

Ludo’.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Thu, 19 Oct 2023 19:54:53 -0400

Hi Ludo,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> * etc/teams.scm.in (<regexp*>, make-regexp*, regexp*-exec): Move to...
>> * guix/build/utils.scm: ... here.
>> (list-matches*): New procedure.
>
>
> [...]
>
>> +++ b/guix/build/utils.scm
>
> [...]
>
>> +;;;
>> +;;; Extend regexp objects with a pattern field.
>> +;;;
>> +(define-record-type <regexp*>
>> +  (%make-regexp* pat flag rx)
>> +  regexp*?
>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>> +  (flag regexp*-flag)                   ;regexp flags
>> +  (rx regexp*-rx))                      ;the compiled regexp object
>> +
>> +;;; Work around regexp implementation.
>> +;;; This record allows to track the regexp pattern and then display it.
>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>
> I’m skeptical about the concrete benefits.  I would not include it in
> (guix build utils), or at least not in this patch series.
>
> (I tend to be super conservative about (guix build utils) because we
> rarely get a chance to change it.)

The original users are substitute* and the teams.scm script.  Since
substitute* is from (guix build utils), it makes sense to add it there
as well, since they are coupled.

The benefit is concrete: it makes it possible to show which regexp
pattern failed to match (its textual representation), instead of
something much less useful such as a generic placeholder such as
"<unknown> (regexp)".

It's in actual use if you look at the definition of substitute:

--8<---------------cut here---------------start------------->8---

  (let ((rx+proc (map (match-lambda
                        (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
                         (cons pattern proc))
                        ((pattern . proc)
                         (cons (make-regexp* pattern regexp/extended) proc)))
                      pattern+procs)))
--8<---------------cut here---------------end--------------->8---

The previous version followed a different approach, annotating the
rx+proc list with the raw pattern; I think the approach here is a bit
cleaner, and it should also enable users to pass pre-computed regexp*
objects to substitute* and have useful error messages produced.

-- 
Thanks,
Maxim

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 2/3] build: substitute: Error when no substitutions were
 done.
Date: Thu, 19 Oct 2023 20:57:39 -0400

From: Jakub Kądziołka <kuba <at> kadziolka.net>

* guix/build/utils.scm (substitute, substitute*)
[require-matches?]: New argument.
* tests/build-utils.scm ("substitute*"): New test group.
("substitute*, no match error")
("substitute*, partial no match error"): New tests.

Co-authored-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Change-Id: I66ed33d72aa73cd35e5642521efec70bf756f86e
---
 guix/build/utils.scm  | 93 +++++++++++++++++++++++++++++++++----------
 tests/build-utils.scm | 68 +++++++++++++++++++++----------
 2 files changed, 118 insertions(+), 43 deletions(-)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 2b3a8e278b..8e4b8321dd 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -6,7 +6,8 @@
 ;;; Copyright © 2018, 2022 Arun Isaac <arunisaac <at> systemreboot.net>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado <at> elephly.net>
 ;;; Copyright © 2020 Efraim Flashner <efraim <at> flashner.co.il>
-;;; Copyright © 2020, 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
+;;; Copyright © 2020, 2021, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
 ;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
@@ -111,8 +112,14 @@ (define-module (guix build utils)
             modify-phases
 
             with-atomic-file-replacement
+            %substitute-requires-matches?
             substitute
             substitute*
+            &substitute-error
+            substitute-error?
+            substitute-error-file
+            substitute-error-patterns
+
             dump-port
             set-file-time
             patch-shebang
@@ -971,24 +978,51 @@ (define (replace-char c1 c2 s)
                         c))
                   s)))
 
-(define (substitute file pattern+procs)
+(define-condition-type &substitute-error &error
+  substitute-error?
+  (file substitute-error-file)
+  (patterns substitute-error-patterns))
+
+(define %substitute-requires-matches?
+  (make-parameter #t))
+
+(define* (substitute file pattern+procs
+                     #:key (require-matches? (%substitute-requires-matches?)))
   "PATTERN+PROCS is a list of regexp/two-argument-procedure pairs.  For each
 line of FILE, and for each PATTERN that it matches, call the corresponding
 PROC as (PROC LINE MATCHES); PROC must return the line that will be written as
 a substitution of the original line.  Be careful about using '$' to match the
-end of a line; by itself it won't match the terminating newline of a line."
-  (let ((rx+proc  (map (match-lambda
-                        (((? regexp? pattern) . proc)
+end of a line; by itself it won't match the terminating newline of a line.
+
+By default, SUBSTITUTE will raise a &substitute-error condition if one of the
+patterns fails to match.  REQUIRE-MATCHES? can be set to false when lack of
+matches is acceptable (e.g. if you have multiple potential patterns not
+guaranteed to be found in FILE)."
+  (define (rx->pattern m)
+    (match m
+      ((? regexp? pattern)
+       "<unknown pattern (regexp)>")
+      ((? regexp*? pattern)
+       (regexp*-pattern pattern))
+      ((? string? pattern)
+       pattern)))
+
+  (let ((rx+proc (map (match-lambda
+                        (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
                          (cons pattern proc))
                         ((pattern . proc)
-                         (cons (make-regexp pattern regexp/extended)
-                               proc)))
-                       pattern+procs)))
+                         (cons (make-regexp* pattern regexp/extended) proc)))
+                      pattern+procs)))
     (with-atomic-file-replacement file
       (lambda (in out)
-        (let loop ((line (read-line in 'concat)))
+        (let loop ((line (read-line in 'concat))
+                   (unmatched-regexps (map first rx+proc)))
           (if (eof-object? line)
-              #t
+              (when (and require-matches? (not (null? unmatched-regexps)))
+                (raise (condition
+                        (&substitute-error
+                         (file file)
+                         (patterns (map rx->pattern unmatched-regexps))))))
               ;; Work around the fact that Guile's regexp-exec does not handle
               ;; NUL characters (a limitation of the underlying GNU libc's
               ;; regexec) by temporarily replacing them by an unused private
@@ -998,19 +1032,23 @@ (define (substitute file pattern+procs)
                                     (unused-private-use-code-point line))
                                #\nul))
                      (line* (replace-char #\nul nul* line))
-                     (line1* (fold (lambda (r+p line)
-                                     (match r+p
-                                       ((regexp . proc)
-                                        (match (list-matches regexp line)
-                                          ((and m+ (_ _ ...))
-                                           (proc line m+))
-                                          (_ line)))))
-                                   line*
-                                   rx+proc))
+                     (results           ;line, unmatched-regexps
+                      (fold (lambda (r+p results)
+                              (let ((line (first results))
+                                    (unmatched (second results)))
+                                (match r+p
+                                  ((regexp . proc)
+                                   (match (list-matches* regexp line)
+                                     ((and m+ (_ _ ...))
+                                      (list (proc line m+)
+                                            (delq regexp unmatched)))
+                                     (_ (list line unmatched)))))))
+                            (list line* unmatched-regexps)
+                            rx+proc))
+                     (line1* (first results))
                      (line1 (replace-char nul* #\nul line1*)))
                 (display line1 out)
-                (loop (read-line in 'concat)))))))))
-
+                (loop (read-line in 'concat) (second results)))))))))
 
 (define-syntax let-matches
   ;; Helper macro for `substitute*'.
@@ -1048,9 +1086,19 @@ (define-syntax substitute*
 Alternatively, FILE may be a list of file names, in which case they are
 all subject to the substitutions.
 
+By default, SUBSTITUTE* will raise a &message condition if one of the patterns
+fails to match on one of the files; REQUIRE-MATCHES? may be set to false to
+avoid an error being raised in such condition.
+
 Be careful about using '$' to match the end of a line; by itself it won't
 match the terminating newline of a line."
     ((substitute* file ((regexp match-var ...) body ...) ...)
+     (substitute* file
+       ((regexp match-var ...) body ...) ...
+       #:require-matches? #t))
+    ((substitute* file
+       ((regexp match-var ...) body ...) ...
+       #:require-matches? require-matches?)
      (let ()
        (define (substitute-one-file file-name)
          (substitute
@@ -1074,7 +1122,8 @@ (define-syntax substitute*
                                       (begin body ...)
                                       (substring l o (match:start m))
                                       r))))))))
-                ...)))
+                ...)
+          #:require-matches? require-matches?))
 
        (match file
          ((files (... ...))
diff --git a/tests/build-utils.scm b/tests/build-utils.scm
index 3babf5d544..35c66faa3c 100644
--- a/tests/build-utils.scm
+++ b/tests/build-utils.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2015, 2016, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2019 Ricardo Wurmus <rekado <at> elephly.net>
-;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
+;;; Copyright © 2021, 2023 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
 ;;;
@@ -289,26 +289,52 @@ (define (arg-test bash-args)
 (test-assert "wrap-script, argument handling, bash --norc"
   (arg-test " --norc"))
 
-(test-equal "substitute*, text contains a NUL byte, UTF-8"
-  "c\0d"
-  (with-fluids ((%default-port-encoding "UTF-8")
-                (%default-port-conversion-strategy 'error))
-    ;; The GNU libc is locale sensitive.  Depending on the value of LANG, the
-    ;; test could fail with "string contains #\\nul character: ~S" or "cannot
-    ;; convert wide string to output locale".
-    (setlocale LC_ALL "en_US.UTF-8")
-    (call-with-temporary-output-file
-     (lambda (file port)
-       (format port "a\0b")
-       (flush-output-port port)
-
-       (substitute* file
-         (("a") "c")
-         (("b") "d"))
-
-       (with-input-from-file file
-         (lambda _
-           (get-string-all (current-input-port))))))))
+(define-syntax-rule (define-substitute*-test test-type name expected
+                      content clauses ...)
+  (test-type
+   name
+   expected
+   (with-fluids ((%default-port-encoding "UTF-8")
+                 (%default-port-conversion-strategy 'error))
+     ;; The GNU libc is locale sensitive.  Depending on the value of LANG,
+     ;; the test could fail with "string contains #\\nul character: ~S" or
+     ;; "cannot convert wide string to output locale".
+     (setlocale LC_ALL "en_US.UTF-8")
+     (call-with-temporary-output-file
+      (lambda (file port)
+        (format port content)
+        (flush-output-port port)
+
+        (substitute* file
+          clauses ...)
+
+        (with-input-from-file file
+          (lambda _
+            (get-string-all (current-input-port)))))))))
+
+(define-substitute*-test test-equal
+  "substitute*, text contains a NUL byte, UTF-8"
+  "c\0d"                              ;expected
+  "a\0b"                              ;content
+  (("a") "c")
+  (("b") "d"))
+
+(define-substitute*-test test-error "substitute*, no match error"
+  #t                                  ;expected
+  "a\0b"                              ;content
+  (("Oops!") "c"))
+
+(define-substitute*-test test-equal "substitute*, no match, ignored"
+  "abc"                                 ;expected
+  "abc"                                 ;content
+  (("Oops!") "c")
+  #:require-matches? #f)
+
+(define-substitute*-test test-error "substitute*, partial no match error"
+  #t                                  ;expected
+  "a\0b"                              ;content
+  (("a") "c"
+   ("Oops!") "c"))
 
 (test-equal "search-input-file: exception if not found"
   `((path)
-- 
2.41.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 1/3] build: Relocate <regexp*> record and associated
 procedures here.
Date: Thu, 19 Oct 2023 20:57:38 -0400

* etc/teams.scm.in (<regexp*>, make-regexp*, regexp*-exec): Move to...
* guix/build/utils.scm: ... here.
(list-matches*): New procedure.

Change-Id: I566ac372f7d8ba08de94e19b54dcc68da2106a23
---
 etc/teams.scm.in     | 19 +------------------
 guix/build/utils.scm | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/etc/teams.scm.in b/etc/teams.scm.in
index 55242caad1..8af25b9802 100644
--- a/etc/teams.scm.in
+++ b/etc/teams.scm.in
@@ -34,29 +34,12 @@
              (srfi srfi-9)
              (srfi srfi-26)
              (ice-9 format)
-             (ice-9 regex)
              (ice-9 match)
              (ice-9 rdelim)
+             (guix build utils)
              (guix ui)
              (git))
 
-(define-record-type <regexp*>
-  (%make-regexp* pat flag rx)
-  regexp*?
-  (pat regexp*-pattern)
-  (flag regexp*-flag)
-  (rx regexp*-rx))
-
-;;; Work around regexp implementation.
-;;; This record allows to track the regexp pattern and then display it.
-(define* (make-regexp* pat #:optional (flag regexp/extended))
-  "Alternative to `make-regexp' producing annotated <regexp*> objects."
-  (%make-regexp* pat flag (make-regexp pat flag)))
-
-(define (regexp*-exec rx* str)
-  "Execute the RX* regexp, a <regexp*> object."
-  (regexp-exec (regexp*-rx rx*) str))
-
 (define-record-type <team>
   (make-team id name description members scope)
   team?
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 8e630ad586..2b3a8e278b 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2020, 2021 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
 ;;; Copyright © 2021, 2022 Maxime Devos <maximedevos <at> telenet.be>
 ;;; Copyright © 2021 Brendan Tildesley <mail <at> brendan.scot>
+;;; Copyright © 2022 Simon Tournier <zimon.toutoune <at> gmail.com>
 ;;; Copyright © 2023 Carlo Zancanaro <carlo <at> zancanaro.id.au>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -28,6 +29,7 @@
 
 (define-module (guix build utils)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module (srfi srfi-34)
@@ -55,6 +57,14 @@ (define-module (guix build utils)
             package-name->name+version
             parallel-job-count
 
+            make-regexp*
+            regexp*-exec
+            regexp*?
+            regexp*-pattern
+            regexp*-flag
+            regexp*-rx
+            list-matches*
+
             compressor
             tarball?
             %xz-parallel-args
@@ -163,6 +173,35 @@ (define-syntax-rule (define-constant name val)
    (module-replace! (current-module) '(setvbuf)))
   (else #f))
 
+
+;;;
+;;; Extend regexp objects with a pattern field.
+;;;
+(define-record-type <regexp*>
+  (%make-regexp* pat flag rx)
+  regexp*?
+  (pat regexp*-pattern)                 ;the regexp pattern, a string
+  (flag regexp*-flag)                   ;regexp flags
+  (rx regexp*-rx))                      ;the compiled regexp object
+
+;;; Work around regexp implementation.
+;;; This record allows to track the regexp pattern and then display it.
+(define* (make-regexp* pat #:optional (flag regexp/extended))
+  "Alternative to `make-regexp' producing annotated <regexp*> objects."
+  (%make-regexp* pat flag (make-regexp pat flag)))
+
+(define (regexp*-exec rx* str)
+  "Execute the RX* regexp, a <regexp*> object."
+  (regexp-exec (regexp*-rx rx*) str))
+
+(define* (list-matches* regexp str #:optional (flags regexp/extended))
+  "Like 'list-matches', but also accepting a regexp* as REGEXP."
+  (match regexp
+    ((or (? string?) (? regexp?))
+     (list-matches regexp str flags))
+    ((? regexp*?)
+     (list-matches (regexp*-rx regexp) str flags))))
+
 
 ;;;
 ;;; Compression helpers.

base-commit: d59653b7c9e43ebdbba20e2ca071429507f94c67
-- 
2.41.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 42146 <at> debbugs.gnu.org
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH v3 3/3] build: bootstrap-configure: Allow lack of matches in
 substitute.
Date: Thu, 19 Oct 2023 20:57:40 -0400

From: Jakub Kądziołka <kuba <at> kadziolka.net>

Matches are not required here, as not every file will use every
variable.

* guix/build/gnu-bootstrap.scm (bootstrap-configure): Pass
  #:require-matches? #f to substitute*.

Signed-off-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Change-Id: I7eee433a3af5fa310b3f2e8b8b58ac9befb4b56a
---
 guix/build/gnu-bootstrap.scm | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/guix/build/gnu-bootstrap.scm b/guix/build/gnu-bootstrap.scm
index b4257a3717..b097f410ad 100644
--- a/guix/build/gnu-bootstrap.scm
+++ b/guix/build/gnu-bootstrap.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2020, 2022 Timothy Sample <samplet <at> ngyro.com>
+;;; Copyright © 2020 Jakub Kądziołka <kuba <at> kadziolka.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -52,7 +53,8 @@ (define (bootstrap-configure name version modules scripts)
                     (copy-file template target)
                     (substitute* target
                       (("@PACKAGE_NAME@") name)
-                      (("@VERSION@") version))))
+                      (("@VERSION@") version)
+                      #:require-matches? #f)))
                 (append-map (lambda (dir) (find-files dir "\\.in$"))
                             modules))
       (for-each (lambda (template)
@@ -62,12 +64,12 @@ (define (bootstrap-configure name version modules scripts)
                     (substitute* target
                       (("@GUILE@") guile)
                       (("@MODDIR@") moddir)
-                      (("@GODIR@") godir))
+                      (("@GODIR@") godir)
+                      #:require-matches? #f)
                     (chmod target #o755)))
                 (find-files scripts
                             (lambda (fn st)
-                              (string-suffix? ".in" fn))))
-      #t)))
+                              (string-suffix? ".in" fn)))))))
 
 (define (bootstrap-build modules)
   "Create a procedure that builds an early bootstrap package.  The
-- 
2.41.0

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

From: Simon Tournier <zimon.toutoune <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>, Maxim Cournoyer
 <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: [bug#42146] [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Fri, 20 Oct 2023 17:11:05 +0200

Hi Ludo,

On Thu, 19 Oct 2023 at 22:40, Ludovic Courtès <ludo <at> gnu.org> wrote:

>> +;;;
>> +;;; Extend regexp objects with a pattern field.
>> +;;;
>> +(define-record-type <regexp*>
>> +  (%make-regexp* pat flag rx)
>> +  regexp*?
>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>> +  (flag regexp*-flag)                   ;regexp flags
>> +  (rx regexp*-rx))                      ;the compiled regexp object
>> +
>> +;;; Work around regexp implementation.
>> +;;; This record allows to track the regexp pattern and then display it.
>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>
> I’m skeptical about the concrete benefits.  I would not include it in
> (guix build utils), or at least not in this patch series.
>
> (I tend to be super conservative about (guix build utils) because we
> rarely get a chance to change it.)

If I remember correctly, the record was introduced in #58660 [1].
Basically, if you have,

              (make-regexp "^gnu/packages/python(-.+|)\\.scm$")

then you only have access to some #<regexp 7f6315fb3500>.  Other said,
you lost the human-readable "^gnu/packages/python(-.+|)\\.scm$" regexp
pattern.  The workaround just stores this human-readable regexp pattern.
Later, it is thus possible to display it; for debugging or else.

For the location of such feature, I do not have an opinion.  For the
concrete benefits, I have one. :-)

Well, maybe the feature – keep an access to the human-readable regexp
pattern – could be implemented on Guile-side.

Or maybe there is another simpler way that I am not aware of?

Cheers,
simon


1: https://issues.guix.gnu.org/58660

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Simon Tournier <zimon.toutoune <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Ludovic Courtès <ludo <at> gnu.org>, 42146 <at> debbugs.gnu.org
Subject: Re: [bug#42146] [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Fri, 20 Oct 2023 11:39:04 -0400

Hi,

Simon Tournier <zimon.toutoune <at> gmail.com> writes:

> Hi Ludo,
>
> On Thu, 19 Oct 2023 at 22:40, Ludovic Courtès <ludo <at> gnu.org> wrote:
>
>>> +;;;
>>> +;;; Extend regexp objects with a pattern field.
>>> +;;;
>>> +(define-record-type <regexp*>
>>> +  (%make-regexp* pat flag rx)
>>> +  regexp*?
>>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>>> +  (flag regexp*-flag)                   ;regexp flags
>>> +  (rx regexp*-rx))                      ;the compiled regexp object
>>> +
>>> +;;; Work around regexp implementation.
>>> +;;; This record allows to track the regexp pattern and then display it.
>>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>>
>> I’m skeptical about the concrete benefits.  I would not include it in
>> (guix build utils), or at least not in this patch series.
>>
>> (I tend to be super conservative about (guix build utils) because we
>> rarely get a chance to change it.)
>
> If I remember correctly, the record was introduced in #58660 [1].
> Basically, if you have,
>
>               (make-regexp "^gnu/packages/python(-.+|)\\.scm$")
>
> then you only have access to some #<regexp 7f6315fb3500>.  Other said,
> you lost the human-readable "^gnu/packages/python(-.+|)\\.scm$" regexp
> pattern.  The workaround just stores this human-readable regexp pattern.
> Later, it is thus possible to display it; for debugging or else.
>
> For the location of such feature, I do not have an opinion.  For the
> concrete benefits, I have one. :-)
>
> Well, maybe the feature – keep an access to the human-readable regexp
> pattern – could be implemented on Guile-side.

Agreed, for the long haul (as with many cool bits that are Guix specific
currently).

-- 
Thanks,
Maxim

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

From: Bruno Victal <mirai <at> makinata.eu>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#42146] [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Fri, 20 Oct 2023 19:50:15 +0100

Hi Ludo’,

On 2023-10-19 21:49, Ludovic Courtès wrote:
>> +(test-group "substitute*"
> 
> I’d avoid groups: they’re not super useful and the output with the
> Automake driver is terrible.

I personally like using groups for structuring the logic of the tests.

The output issue sounds either like a bug to me that should be fixed
or a place for improvement.

-- 
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 Ludovic Courtès <ludo <at> gnu.org>, 42146 <at> debbugs.gnu.org
Subject: Re: [bug#42146] [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Fri, 20 Oct 2023 17:11:05 -0400

Hi,

Bruno Victal <mirai <at> makinata.eu> writes:

> Hi Ludo’,
>
> On 2023-10-19 21:49, Ludovic Courtès wrote:
>>> +(test-group "substitute*"
>> 
>> I’d avoid groups: they’re not super useful and the output with the
>> Automake driver is terrible.
>
> I personally like using groups for structuring the logic of the tests.
>
> The output issue sounds either like a bug to me that should be fixed
> or a place for improvement.

Ludo, could you refresh my memory about what is wrong with the Automake
SRFI-64 driver when it comes to tests?  I think it used to masks the
tests in its output, but I'm not seeing this now?

Consider the following tests, where the "elm->package-name" and
"infer-elm-package-name" tests are in the "round trip" test group itself
nested in the "elm->package-name and infer-elm-package-name" test group.

tests/configuration.scm contains a "duplicated/conflicting entries" test
group that contains "duplicate sanitizer", "duplicate serializer" and
"conflicting use of serializer + empty-serializer":

--8<---------------cut here---------------start------------->8---
make check TESTS='tests/services/configuration.scm tests/elm.scm' \
  SCM_LOG_DRIVER_FLAGS="--brief=no"
--8<---------------cut here---------------end--------------->8---

PASS: tests/services/configuration.scm - default value, no serialization
PASS: tests/services/configuration.scm - wrong type for a field
PASS: tests/services/configuration.scm - default value, custom serializer
PASS: tests/services/configuration.scm - no default value, provided
PASS: tests/services/configuration.scm - no default value, not provided
PASS: tests/services/configuration.scm - serialize-configuration
PASS: tests/services/configuration.scm - serialize-configuration [deprecated]
PASS: tests/services/configuration.scm - serialize-configuration with no-serialization
PASS: tests/services/configuration.scm - serialize-configuration with prefix
PASS: tests/services/configuration.scm - default value, sanitizer
PASS: tests/services/configuration.scm - string value, sanitized to number
PASS: tests/services/configuration.scm - default value, serializer literal
PASS: tests/services/configuration.scm - empty-serializer as literal
PASS: tests/services/configuration.scm - empty-serializer as procedure
PASS: tests/services/configuration.scm - default value, sanitizer, permutation
PASS: tests/services/configuration.scm - default value, serializer, permutation
PASS: tests/services/configuration.scm - string value sanitized to number, permutation
PASS: tests/services/configuration.scm - default value, sanitizer, permutation 2
PASS: tests/services/configuration.scm - default value, serializer, permutation 2
PASS: tests/services/configuration.scm - duplicate sanitizer
PASS: tests/services/configuration.scm - duplicate serializer
PASS: tests/services/configuration.scm - conflicting use of serializer + empty-serializer
PASS: tests/services/configuration.scm - Mix of bare serializer and new syntax
PASS: tests/services/configuration.scm - Mix of bare serializer and new syntax, permutation)
PASS: tests/services/configuration.scm - maybe value serialization
PASS: tests/services/configuration.scm - maybe value serialization of the instance
PASS: tests/services/configuration.scm - maybe value serialization of the instance, unspecified
PASS: tests/services/configuration.scm - symbol maybe value serialization, unspecified
PASS: tests/services/configuration.scm - maybe value without serialization no procedure bound
PASS: tests/services/configuration.scm - maybe type, no default
PASS: tests/services/configuration.scm - maybe type, with default
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infer-elm-package-name
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm->package-name
PASS: tests/elm.scm - infers other name
PASS: tests/elm.scm - infered name round-trips
PASS: tests/elm.scm - elm
PASS: tests/elm.scm - guile
PASS: tests/elm.scm - gcc-toolchain
PASS: tests/elm.scm - font-adobe-source-sans-pro
PASS: tests/elm.scm - (elm->guix-package "elm/core")
PASS: tests/elm.scm - (elm-recursive-import "elm-guix/demo")
============================================================================
Testsuite summary for GNU Guix 1.3.0.48706-b42e6315-dirty
============================================================================
# TOTAL: 85
# PASS:  85
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0
============================================================================

Observation: the output says nothing about the groups, but at least the
nested tests are correctly listed.

-- 
Thanks,
Maxim

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Mon, 23 Oct 2023 10:42:08 +0200

Hi Maxim,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> Ludovic Courtès <ludo <at> gnu.org> writes:

[...]

>>> +;;; Extend regexp objects with a pattern field.
>>> +;;;
>>> +(define-record-type <regexp*>
>>> +  (%make-regexp* pat flag rx)
>>> +  regexp*?
>>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>>> +  (flag regexp*-flag)                   ;regexp flags
>>> +  (rx regexp*-rx))                      ;the compiled regexp object
>>> +
>>> +;;; Work around regexp implementation.
>>> +;;; This record allows to track the regexp pattern and then display it.
>>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>>
>> I’m skeptical about the concrete benefits.  I would not include it in
>> (guix build utils), or at least not in this patch series.

[...]

> The benefit is concrete: it makes it possible to show which regexp
> pattern failed to match (its textual representation), instead of
> something much less useful such as a generic placeholder such as
> "<unknown> (regexp)".

Yes, okay.

Can we keep the <regexp*> interface private, then?  To me it’s an
implementation detail and perhaps a bit of a hack that I’d rather not
commit to maintaining.

The cost might be to duplicate it in teams.scm, but that’s an acceptable
cost IMO.

> It's in actual use if you look at the definition of substitute:
>
>
>   (let ((rx+proc (map (match-lambda
>                         (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
>                          (cons pattern proc))
>                         ((pattern . proc)
>                          (cons (make-regexp* pattern regexp/extended) proc)))
>                       pattern+procs)))
>
> The previous version followed a different approach, annotating the
> rx+proc list with the raw pattern; I think the approach here is a bit
> cleaner, and it should also enable users to pass pre-computed regexp*
> objects to substitute* and have useful error messages produced.

Note that ‘substitute*’ only takes string literals as patterns, not
regexp objects.  The pattern part of clauses has sometimes been abused
to pass code, typically ‘string-append’ calls, but that’s definitely not
the spirit.

Another approach would have been, in ‘substitute*’, to capture the
regexp-as-string as well as other useful debugging info, such as its
source location.

Thanks,
Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Mon, 23 Oct 2023 10:48:12 +0200

Hi,

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:

> From: Jakub Kądziołka <kuba <at> kadziolka.net>
>
> * guix/build/utils.scm (substitute, substitute*)
> [require-matches?]: New argument.
> * tests/build-utils.scm ("substitute*"): New test group.
> ("substitute*, no match error")
> ("substitute*, partial no match error"): New tests.
>
> Co-authored-by: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
> Change-Id: I66ed33d72aa73cd35e5642521efec70bf756f86e

[…]

> +(define-condition-type &substitute-error &error
> +  substitute-error?
> +  (file substitute-error-file)
> +  (patterns substitute-error-patterns))
> +
> +(define %substitute-requires-matches?
> +  (make-parameter #t))

[…]

> @@ -1048,9 +1086,19 @@ (define-syntax substitute*
>  Alternatively, FILE may be a list of file names, in which case they are
>  all subject to the substitutions.
>  
> +By default, SUBSTITUTE* will raise a &message condition if one of the patterns

I think it’s ‘&substitute-error’ rather than ‘&message’.

> +(define-substitute*-test test-error "substitute*, partial no match error"
> +  #t                                  ;expected
> +  "a\0b"                              ;content
> +  (("a") "c"
> +   ("Oops!") "c"))

Maybe it’s not all that important here, but I think ‘test-error’ tests
for any exception, whereas we’re looking specifically for a
‘&substitute-error’ condition filled with the right values.

Ludo’.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Mon, 23 Oct 2023 10:51:09 +0200

BTW, as far as merging is concerned, how about doing an
x86_64-linux-only build of a branch containing these patches on ci.guix
first, as proposed on IRC a few days ago?

That would allow us to see, within less than 24h, whether there’s a
massive fallout that we didn’t think of, for instance because key
packages rely on silent match failures.

Thanks,
Ludo’.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Mon, 23 Oct 2023 11:05:31 -0400

Hi Ludovic,

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi Maxim,
>
> Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>
>> Ludovic Courtès <ludo <at> gnu.org> writes:
>
> [...]
>
>>>> +;;; Extend regexp objects with a pattern field.
>>>> +;;;
>>>> +(define-record-type <regexp*>
>>>> +  (%make-regexp* pat flag rx)
>>>> +  regexp*?
>>>> +  (pat regexp*-pattern)                 ;the regexp pattern, a string
>>>> +  (flag regexp*-flag)                   ;regexp flags
>>>> +  (rx regexp*-rx))                      ;the compiled regexp object
>>>> +
>>>> +;;; Work around regexp implementation.
>>>> +;;; This record allows to track the regexp pattern and then display it.
>>>> +(define* (make-regexp* pat #:optional (flag regexp/extended))
>>>
>>> I’m skeptical about the concrete benefits.  I would not include it in
>>> (guix build utils), or at least not in this patch series.
>
> [...]
>
>> The benefit is concrete: it makes it possible to show which regexp
>> pattern failed to match (its textual representation), instead of
>> something much less useful such as a generic placeholder such as
>> "<unknown> (regexp)".
>
> Yes, okay.
>
> Can we keep the <regexp*> interface private, then?  To me it’s an
> implementation detail and perhaps a bit of a hack that I’d rather not
> commit to maintaining.

If you can think of a more elegant way to define it, I'm all ears.
Ideally, as Simon pointed out, that's something that Guile would support
natively (the ability to pull back the raw pattern from a regexp
object).  In Python for example, the re.Pattern object has a 'pattern'
property [0]

[0]  https://docs.python.org/3/library/re.html?highlight=regex#re.Pattern.pattern

Otherwise I don't think "the bit of a hack" is substantiated enough :-).

> The cost might be to duplicate it in teams.scm, but that’s an acceptable
> cost IMO.

I dislike copying bits around just to "hide" them from the "official"
interface, and have no fear maintaining that simple piece of code, so
I'd rather keep it at one place and expose it as a public API.

>> It's in actual use if you look at the definition of substitute:
>>
>>
>>   (let ((rx+proc (map (match-lambda
>>                         (((or (? regexp? pattern) (? regexp*? pattern)) . proc)
>>                          (cons pattern proc))
>>                         ((pattern . proc)
>>                          (cons (make-regexp* pattern regexp/extended) proc)))
>>                       pattern+procs)))
>>
>> The previous version followed a different approach, annotating the
>> rx+proc list with the raw pattern; I think the approach here is a bit
>> cleaner, and it should also enable users to pass pre-computed regexp*
>> objects to substitute* and have useful error messages produced.
>
> Note that ‘substitute*’ only takes string literals as patterns, not
> regexp objects.  The pattern part of clauses has sometimes been abused
> to pass code, typically ‘string-append’ calls, but that’s definitely not
> the spirit.
>

It's true that 'substitute*' only accepts literal patterns, but that's
not the case with 'substitute' itself, which is also exposed in the API
of (guix build utils): it accepts regexp objects as well, and thus
there's value in this change -- users could in theory use make-regexp*
objects and pass that to 'substitute' and benefit from more precises
error messages.

> Another approach would have been, in ‘substitute*’, to capture the
> regexp-as-string as well as other useful debugging info, such as its
> source location.

That'd be a nice solution, but it doesn't take into account
'substitute', as mentioned above.  As it has exactly zero (!) users
outside of substitute*, perhaps it could be yanked from the public API
and then we could consider doing the above for substitute* ?

-- 
Thanks,
Maxim

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Jakub Kądziołka <kuba <at> kadziolka.net>,
 42146 <at> debbugs.gnu.org
Subject: Re: bug#42146: [PATCH core-updates 1/?] build: substitute: Don't
 fail silently.
Date: Mon, 23 Oct 2023 12:25:27 -0400

Hi Ludo,

Ludovic Courtès <ludo <at> gnu.org> writes:

[...]

>> @@ -1048,9 +1086,19 @@ (define-syntax substitute*
>>  Alternatively, FILE may be a list of file names, in which case they are
>>  all subject to the substitutions.
>>  
>> +By default, SUBSTITUTE* will raise a &message condition if one of the patterns
>
> I think it’s ‘&substitute-error’ rather than ‘&message’.

Good catch.  Fixed.

>> +(define-substitute*-test test-error "substitute*, partial no match error"
>> +  #t                                  ;expected
>> +  "a\0b"                              ;content
>> +  (("a") "c"
>> +   ("Oops!") "c"))
>
> Maybe it’s not all that important here, but I think ‘test-error’ tests
> for any exception, whereas we’re looking specifically for a
> ‘&substitute-error’ condition filled with the right values.

I thought test-error was limited in srfi-64 that made it not work with
condition types, but it seems that's not the case and it actually does
work:

--8<---------------cut here---------------start------------->8---
modified   tests/build-utils.scm
@@ -320,8 +320,8 @@ (define-substitute*-test test-equal
   (("b") "d"))
 
 (define-substitute*-test test-error "substitute*, no match error"
-  #t                                  ;expected
-  "a\0b"                              ;content
+  &substitute-error                     ;expected
+  "a\0b"                                ;content
   (("Oops!") "c"))
 
 (define-substitute*-test test-equal "substitute*, no match, ignored"
@@ -331,8 +331,8 @@ (define-substitute*-test test-equal "substitute*, no match, ignored"
   #:require-matches? #f)
 
 (define-substitute*-test test-error "substitute*, partial no match error"
-  #t                                  ;expected
-  "a\0b"                              ;content
+  &substitute-error                     ;expected
+  "a\0b"                                ;content
   (("a") "c"
    ("Oops!") "c"))
--8<---------------cut here---------------end--------------->8---

and all tests still pass :-).  I'll send a v4.

-- 
Thanks,
Maxim

Previous Next

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