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

["Ludovic Courtès" <ludo@gnu.org>]

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

> From: Jakub Kądziołka <kuba@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@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’.