[bug#62551] Added new transformation option: --with-configure-flag

["Ludovic Courtès" <ludovic.courtes@inria.fr>]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

Hi Sarthak,

Sarthak Shah <shahsarthakw@gmail.com> skribis:

> * doc/guix.texi (--with-configure-flag): Added documentation for
> --with-configure-flag
> * guix/transformations.scm (transform-package-configure-flag): New
> function, changes to %transformations and
> show-transformation-options-help/detailed
> * tests/transformations.scm (test-equal "options-transformation,
> with-configure-flag"): Added a test for --with-configure-flag

Nice!

I made the superficial changes below, which can be summarized like this:

  • Allow for equal signs in the configure flag itself.

  • Remove build system check: we don’t do that for the other options;
    instead, document the limitation and leave it up to the user.

  • Tweak the style of the system test to avoid ‘cadr’ (info "(guix)
    Data Types and Pattern Matching").

  • Add a CMake example in the manual and tweak wording.

I’ll follow up with a news entry so people who run ‘guix pull’ can learn
about the new option.

Great work, thank you!

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 6650 bytes --]

diff --git a/doc/guix.texi b/doc/guix.texi
index ff0e62053b..55221a10c3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12919,23 +12919,33 @@ Package Transformation Options
 In this example, glibc itself as well as everything that leads to
 Coreutils in the dependency graph is rebuilt.
 
-@item --with-configure-flag=@var{package}=@var{configure-flag}
-Add @var{configure-flag} to the list of configure-flags applied to
-the arguments of @var{package}, where @var{package} is a spec such as
-@code{guile@@3.1} or @code{glibc}. The build system of @var{package}
-must support the argument @code{#:configure-flags}.
+@cindex configure flags, changing them
+@item --with-configure-flag=@var{package}=@var{flag}
+Append @var{flag} to the configure flags of @var{package}, where
+@var{package} is a spec such as @code{guile@@3.0} or @code{glibc}.  The
+build system of @var{package} must support the @code{#:configure-flags}
+argument.
 
-For example, the command below builds GNU Hello with the
-configure-flag @code{--disable-nls}:
+For example, the command below builds GNU@tie{}Hello with the
+configure flag @code{--disable-nls}:
 
 @example
 guix build hello --with-configure-flag=hello=--disable-nls
 @end example
 
-@quotation Warning
-Currently, there is a primitive check for whether the build system
-supports the argument @code{#:configure-flags} or not, however
-users should not rely on it.
+The following command passes an extra flag to @command{cmake} as it
+builds @code{lapack}:
+
+@example
+guix build lapack \
+  --with-configure-flag=lapack=-DBUILD_COMPLEX=OFF
+@end example
+
+@quotation Note
+Under the hood, this option works by passing the
+@samp{#:configure-flags} argument to the build system of the package of
+interest (@pxref{Build Systems}).  Most build systems support that
+option but some do not.  In that case, an error is raised.
 @end quotation
 
 @cindex upstream, latest version
diff --git a/guix/transformations.scm b/guix/transformations.scm
index 9cbac5df9e..943b2768c6 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -677,49 +677,42 @@ (define (transform-package-tests specs)
         obj)))
 
 (define (transform-package-configure-flag specs)
-  "Return a procedure that, when passed a package and a flag, adds the flag to #:configure-flags in the package's
-'arguments' field."
+  "Return a procedure that, when passed a package and a flag, adds the flag to
+#:configure-flags in the package's 'arguments' field."
   (define (package-with-configure-flag p extra-flag)
     (package/inherit p
       (arguments
        (substitute-keyword-arguments (package-arguments p)
-         ((#:configure-flags list-of-flags (quote '()))
-          #~(cons* #$extra-flag #$list-of-flags))))))
+         ((#:configure-flags flags #~'())
+          ;; Add EXTRA-FLAG to the end so it can potentially override FLAGS.
+          #~(append #$flags '(#$extra-flag)))))))
 
-
-  (define %BUILD-SYSTEMS-WITHOUT-CONFIGURE-FLAGS
-    ;; These build systems do not have a #:configure-flags parameter
-'(android-ndk asdf/sbcl asdf/ecl asdf/source cargo channel chicken clojure copy dub dune elm emacs go guile julia linux-module maven minetest-mod minify node perl rakudo rebar ruby scons texlive tree-sitter trivial))
-
-  (define (build-system-supports-flags? spec)
-    ;; XXX: a more sophisticated approach could be added that checks the given build system for a configure-flags option
-    ;; if a new build system is added, it needs to be added to the %BUILD-SYSTEMS-WITHOUT-CONFIGURE-FLAGS list manually
-    (not (member (build-system-name (package-build-system spec))
-                 %BUILD-SYSTEMS-WITHOUT-CONFIGURE-FLAGS)))
-
-  (define cflags
+  (define configure-flags
     ;; Spec/flag alist.
-     (map (lambda (spec)
-            (match (string-tokenize spec %not-equal)
-              ((spec flag)
-               (cons spec flag))
-              (_
-               (raise (formatted-message
-                       (G_ "~a: invalid package configure-flags specification")
-                       spec)))))
-          specs))
+    (map (lambda (spec)
+           ;; Split SPEC on the first equal sign (the configure flag might
+           ;; contain equal signs, as in '-DINTSIZE=32').
+           (let ((equal (string-index spec #\=)))
+             (match (and equal
+                         (list (string-take spec equal)
+                               (string-drop spec (+ 1 equal))))
+               ((spec flag)
+                (cons spec flag))
+               (_
+                (raise (formatted-message
+                        (G_ "~a: invalid package configure flag specification")
+                        spec))))))
+         specs))
 
   (define rewrite
     (package-input-rewriting/spec
      (map (match-lambda
             ((spec . flags)
              (cons spec (cut package-with-configure-flag <> flags))))
-          cflags)))
+          configure-flags)))
 
   (lambda (obj)
-    (if (and
-         (package? obj)
-         (build-system-supports-flags? obj))
+    (if (package? obj)
         (rewrite obj)
         obj)))
 
@@ -1004,7 +997,7 @@ (define (show-transformation-options-help/detailed)
                          add FILE to the list of patches of PACKAGE"))
   (display (G_ "
       --with-configure-flag=PACKAGE=FLAG
-                         add FLAG to the list of #:configure-flags of PACKAGE"))
+                         append FLAG to the configure flags of PACKAGE"))
   (display (G_ "
       --with-latest=PACKAGE
                          use the latest upstream release of PACKAGE"))
diff --git a/tests/transformations.scm b/tests/transformations.scm
index 31fd042d31..704818b9ed 100644
--- a/tests/transformations.scm
+++ b/tests/transformations.scm
@@ -409,12 +409,15 @@ (define* (depends-on-toolchain? p #:optional (toolchain "gcc-toolchain"))
               (package-arguments (package-replacement dep0))))))))
 
 (test-equal "options->transformation, with-configure-flag"
-  '(cons* "--flag" '())
+  '(append '() '("--flag=42"))
   (let* ((p   (dummy-package "foo"
                 (build-system gnu-build-system)))
-         (t   (options->transformation '((with-configure-flag . "foo=--flag")))))
+         (t   (options->transformation
+               '((with-configure-flag . "foo=--flag=42")))))
     (let ((new (t p)))
-      (gexp->approximate-sexp (cadr (memq #:configure-flags (package-arguments new)))))))
+      (match (package-arguments new)
+        ((#:configure-flags flags)
+         (gexp->approximate-sexp flags))))))
 
 (test-assert "options->transformation, without-tests"
   (let* ((dep (dummy-package "dep"))