GNU bug report logs - #62551
Added new transformation option: --with-configure-flag

Previous Next

Package: guix-patches;

Reported by: Sarthak Shah <shahsarthakw <at> gmail.com>

Date: Thu, 30 Mar 2023 19:53:02 UTC

Severity: normal

Tags: patch

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

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 62551 in the body.
You can then email your comments to 62551 AT debbugs.gnu.org in the normal way.

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Thu, 30 Mar 2023 19:53:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Sarthak Shah <shahsarthakw <at> gmail.com>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 30 Mar 2023 19:53:02 GMT) Full text and rfc822 format available.

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

From: Sarthak Shah <shahsarthakw <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: Added new transformation option: --with-configure-flag
Date: Fri, 31 Mar 2023 01:22:41 +0530
[Message part 1 (text/plain, inline)]
This patch adds a new transformation option that lets the user specify a
package to add an extra #:configure-flags value to. Functionally, it is
quite similar to the --with-patch transform.
Currently, I've generated a list of build systems that do not support
#:configure-flags that this transform checks against, but this could be
improved by making the transform check if any given transform has the
#:configure-flags keyword in its arguments.
Please test this patch if possible; my preliminary testing indicates that
it is working, however it could fail for edge cases I have not considered.
CCing Ludovic as he might be interested in this.

---
 guix/transformations.scm | 70 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index 8ff472ad21..8f260807dc 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -676,6 +676,70 @@ (define rewrite
         (rewrite obj)
         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."
+  (define (package-with-configure-flag p extra-flag)
+    (package/inherit p
+      (arguments
+       (substitute-keyword-arguments (package-arguments p)
+         ((#:configure-flags list-of-flags (quote '()))
+          ;; here extra-flag takes the form (--extra-flag)
+          ;; hence it must be spliced to avoid eval errors
+          `(cons* ,@extra-flag ,list-of-flags))))))
+
+  (define (coalesce-alist alist)
+    ;; Coalesce multiple occurrences of the same key in ALIST.
+    (let loop ((alist alist)
+               (keys '())
+               (mapping vlist-null))
+      (match alist
+        (()
+         (map (lambda (key)
+                (cons key (vhash-fold* cons '() key mapping)))
+              (delete-duplicates (reverse keys))))
+        (((key . value) . rest)
+         (loop rest
+               (cons key keys)
+               (vhash-cons key value mapping))))))
+
+  (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
+    ;; Spec/flag alist.
+    (coalesce-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)))
+
+  (define rewrite
+    (package-input-rewriting/spec
+     (map (match-lambda
+            ((spec . flags)
+             (cons spec (cut package-with-configure-flag <> flags))))
+          cflags)))
+
+  (lambda (obj)
+    (if (and
+         (package? obj)
+         (build-system-supports-flags? obj))
+        (rewrite obj)
+        obj)))
+
 (define (patched-source name source patches)
   "Return a file-like object with the given NAME that applies PATCHES to
 SOURCE.  SOURCE must itself be a file-like object of any type, including
@@ -845,6 +909,7 @@ (define %transformations
     (tune . ,transform-package-tuning)
     (with-debug-info . ,transform-package-with-debug-info)
     (without-tests . ,transform-package-tests)
+    (with-configure-flag . ,transform-package-configure-flag)
     (with-patch  . ,transform-package-patches)
     (with-latest . ,transform-package-latest)
     (with-version . ,transform-package-version)))
@@ -915,6 +980,8 @@ (define micro-architecture
                   (parser 'with-debug-info))
           (option '("without-tests") #t #f
                   (parser 'without-tests))
+          (option '("with-configure-flag") #t #f
+                  (parser 'with-configure-flag))
           (option '("with-patch") #t #f
                   (parser 'with-patch))
           (option '("with-latest") #t #f
@@ -952,6 +1019,9 @@ (define (show-transformation-options-help/detailed)
   (display (G_ "
       --with-patch=PACKAGE=FILE
                          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"))
   (display (G_ "
       --with-latest=PACKAGE
                          use the latest upstream release of PACKAGE"))
-- 
2.40.0
[Message part 2 (text/html, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Thu, 30 Mar 2023 19:58:02 GMT) Full text and rfc822 format available.

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

From: Sarthak Shah <shahsarthakw <at> gmail.com>
To: 62551 <at> debbugs.gnu.org
Subject: (updated with changelog formatting)
Date: Fri, 31 Mar 2023 01:26:43 +0530
[Message part 1 (text/plain, inline)]
* guix/transformations.scm (transform-package-configure-flag): New
function, changes to %transformations and
show-transformation-options-help/detailed
---
 guix/transformations.scm | 70 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)

diff --git a/guix/transformations.scm b/guix/transformations.scm
index 8ff472ad21..8f260807dc 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -676,6 +676,70 @@ (define rewrite
         (rewrite obj)
         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."
+  (define (package-with-configure-flag p extra-flag)
+    (package/inherit p
+      (arguments
+       (substitute-keyword-arguments (package-arguments p)
+         ((#:configure-flags list-of-flags (quote '()))
+          ;; here extra-flag takes the form (--extra-flag)
+          ;; hence it must be spliced to avoid eval errors
+          `(cons* ,@extra-flag ,list-of-flags))))))
+
+  (define (coalesce-alist alist)
+    ;; Coalesce multiple occurrences of the same key in ALIST.
+    (let loop ((alist alist)
+               (keys '())
+               (mapping vlist-null))
+      (match alist
+        (()
+         (map (lambda (key)
+                (cons key (vhash-fold* cons '() key mapping)))
+              (delete-duplicates (reverse keys))))
+        (((key . value) . rest)
+         (loop rest
+               (cons key keys)
+               (vhash-cons key value mapping))))))
+
+  (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
+    ;; Spec/flag alist.
+    (coalesce-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)))
+
+  (define rewrite
+    (package-input-rewriting/spec
+     (map (match-lambda
+            ((spec . flags)
+             (cons spec (cut package-with-configure-flag <> flags))))
+          cflags)))
+
+  (lambda (obj)
+    (if (and
+         (package? obj)
+         (build-system-supports-flags? obj))
+        (rewrite obj)
+        obj)))
+
 (define (patched-source name source patches)
   "Return a file-like object with the given NAME that applies PATCHES to
 SOURCE.  SOURCE must itself be a file-like object of any type, including
@@ -845,6 +909,7 @@ (define %transformations
     (tune . ,transform-package-tuning)
     (with-debug-info . ,transform-package-with-debug-info)
     (without-tests . ,transform-package-tests)
+    (with-configure-flag . ,transform-package-configure-flag)
     (with-patch  . ,transform-package-patches)
     (with-latest . ,transform-package-latest)
     (with-version . ,transform-package-version)))
@@ -915,6 +980,8 @@ (define micro-architecture
                   (parser 'with-debug-info))
           (option '("without-tests") #t #f
                   (parser 'without-tests))
+          (option '("with-configure-flag") #t #f
+                  (parser 'with-configure-flag))
           (option '("with-patch") #t #f
                   (parser 'with-patch))
           (option '("with-latest") #t #f
@@ -952,6 +1019,9 @@ (define (show-transformation-options-help/detailed)
   (display (G_ "
       --with-patch=PACKAGE=FILE
                          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"))
   (display (G_ "
       --with-latest=PACKAGE
                          use the latest upstream release of PACKAGE"))
-- 
2.40.0
[Message part 2 (text/html, inline)]

Added tag(s) patch. Request was from Bruno Victal <mirai <at> makinata.eu> to control <at> debbugs.gnu.org. (Thu, 30 Mar 2023 23:01:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Fri, 31 Mar 2023 12:35:02 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Sarthak Shah <shahsarthakw <at> gmail.com>
Cc: 62551 <at> debbugs.gnu.org
Subject: Re: bug#62551: Added new transformation option: --with-configure-flag
Date: Fri, 31 Mar 2023 14:34:30 +0200
Hello!

Sarthak Shah <shahsarthakw <at> gmail.com> skribis:

> This patch adds a new transformation option that lets the user specify a
> package to add an extra #:configure-flags value to. Functionally, it is
> quite similar to the --with-patch transform.
> Currently, I've generated a list of build systems that do not support
> #:configure-flags that this transform checks against, but this could be
> improved by making the transform check if any given transform has the
> #:configure-flags keyword in its arguments.
> Please test this patch if possible; my preliminary testing indicates that
> it is working, however it could fail for edge cases I have not considered.
> CCing Ludovic as he might be interested in this.

Nice, that’s a great start!

> +  (define (package-with-configure-flag p extra-flag)
> +    (package/inherit p
> +      (arguments
> +       (substitute-keyword-arguments (package-arguments p)
> +         ((#:configure-flags list-of-flags (quote '()))
> +          ;; here extra-flag takes the form (--extra-flag)
> +          ;; hence it must be spliced to avoid eval errors
> +          `(cons* ,@extra-flag ,list-of-flags))))))

“Nowadays” we’d use gexps, like so:

  #~(cons* #$extra-flags #$list-of-flags)

> +  (define (coalesce-alist alist)
> +    ;; Coalesce multiple occurrences of the same key in ALIST.

This seems to be pasted from somewhere else; we might want to factorize
it (not your fault of course, but something to keep in mind.)

> +  (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

In general, the ‘name’ field of build systems is purely informational
and I would suggest not relying on it.

Actually, I would probably not perform any check at all.  After all, the
“contract” we have with users of transformation options is that it’s “at
their own risk”: these options build a new package that may or may not
“work”, broadly speaking.

Then we could think of a more sophisticated approach where build systems
would have a field listing supported flags or something along these
lines.  But again, I would not bother about it for now.

The rest looks great!

Have you been able to test it on actual packages? (I haven’t taken the
time yet.)

What we’d like to have, in addition to this, is two things:

  1. Unit tests in ‘tests/transformations.scm’, similar to those of
     other transformations.  Check out
     <https://guix.gnu.org/manual/devel/en/html_node/Running-the-Test-Suite.html>
     on how to run the test suite.

  2. A few lines in ‘doc/guix.texi’ describing the new option, under
     “Package Transformation Options”.

Could you give it a try?

Anyways, kudos for this first step!

Ludo’.




Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Sat, 01 Apr 2023 00:18:01 GMT) Full text and rfc822 format available.

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

From: Sarthak Shah <shahsarthakw <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 62551 <at> debbugs.gnu.org
Subject: Re: bug#62551: Added new transformation option: --with-configure-flag
Date: Sat, 1 Apr 2023 05:47:00 +0530
[Message part 1 (text/plain, inline)]
Hey Ludovic,
Thanks for the comments!

> “Nowadays” we’d use gexps, like so:
>  #~(cons* #$extra-flags #$list-of-flags)
Noted, I will follow this in the updated patch.

> This seems to be pasted from somewhere else; we might want to factorize
it (not your fault of course, but something to keep in mind.)
It was indeed copied over from the with-patches segment, as I thought it
would be useful to check if a configure-flag is being passed again. I think
it is not particularly necessary as we assume that the user knows what they
are doing when they are using transforms, so I will omit it in the updated
patch.

> In general, the ‘name’ field of build systems is purely informational and
I would suggest not relying on it.
Yes, and I've factored that in in the current patch- I have obtained the
actual 'name' parameters of each of the given build systems through
grepping. However, I agree with you in thinking that it might not be
necessary at all- I wrote this as a 'stopgap' of sorts anyways. I would
like to update it with a sophisticated checking mechanism at a later date
that actually checks if the build system supports configure-flags if
necessary.

> Have you been able to test it on actual packages? (I haven’t taken the
time yet.)
This is the part where I've been having the most trouble actually; I
haven't been able to find suitable methods for testing this, so for now
I've used two methods for testing if it works:
1) printing the arguments of the rewritten package record with display
2) comparing the hashes of patches built with and without configure-flags
Both tests seem to agree that it is working, however I would really
appreciate more rigorous testing by someone else or suggestions on how to
test it more rigorously.
For one, I have been unable to actually check if a feature is getting
added/removed by adding configure-flags because I haven't been able to find
a suitable package to test it with.
If possible, that would be a very clear indication of it working.

> What we’d like to have, in addition to this, is two things:
> ...
> Could you give it a try?
Sure, I will include these changes with the updated patch.

I will try to submit it in about a week, as I would like to test it more
rigorously first.

Happy hacking!
Sarthak (cel7t)
[Message part 2 (text/html, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Tue, 25 Apr 2023 12:23:02 GMT) Full text and rfc822 format available.

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

From: Sarthak Shah <shahsarthakw <at> gmail.com>
To: 62551 <at> debbugs.gnu.org
Subject: Updated patch - with changes, documentation and tests.
Date: Tue, 25 Apr 2023 17:52:10 +0530
[Message part 1 (text/plain, inline)]
* 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
---
 doc/guix.texi             | 19 ++++++++++++++
 guix/transformations.scm  | 53 +++++++++++++++++++++++++++++++++++++++
 tests/transformations.scm | 10 +++++++-
 3 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index c49e51b72e..627a468b62 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -12859,6 +12859,25 @@ guix build coreutils
--with-patch=glibc=./glibc-frob.patch
 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}.
+
+For example, the command below builds GNU 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.
+@end quotation
+
 @cindex upstream, latest version
 @item --with-latest=@var{package}
 @itemx --with-version=@var{package}=@var{version}
diff --git a/guix/transformations.scm b/guix/transformations.scm
index 8ff472ad21..27fb0cb646 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -676,6 +676,53 @@ (define rewrite
         (rewrite obj)
         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."
+  (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))))))
+
+
+  (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
+    ;; 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))
+
+  (define rewrite
+    (package-input-rewriting/spec
+     (map (match-lambda
+            ((spec . flags)
+             (cons spec (cut package-with-configure-flag <> flags))))
+          cflags)))
+
+  (lambda (obj)
+    (if (and
+         (package? obj)
+         (build-system-supports-flags? obj))
+        (rewrite obj)
+        obj)))
+
 (define (patched-source name source patches)
   "Return a file-like object with the given NAME that applies PATCHES to
 SOURCE.  SOURCE must itself be a file-like object of any type, including
@@ -845,6 +892,7 @@ (define %transformations
     (tune . ,transform-package-tuning)
     (with-debug-info . ,transform-package-with-debug-info)
     (without-tests . ,transform-package-tests)
+    (with-configure-flag . ,transform-package-configure-flag)
     (with-patch  . ,transform-package-patches)
     (with-latest . ,transform-package-latest)
     (with-version . ,transform-package-version)))
@@ -915,6 +963,8 @@ (define micro-architecture
                   (parser 'with-debug-info))
           (option '("without-tests") #t #f
                   (parser 'without-tests))
+          (option '("with-configure-flag") #t #f
+                  (parser 'with-configure-flag))
           (option '("with-patch") #t #f
                   (parser 'with-patch))
           (option '("with-latest") #t #f
@@ -952,6 +1002,9 @@ (define (show-transformation-options-help/detailed)
   (display (G_ "
       --with-patch=PACKAGE=FILE
                          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"))
   (display (G_ "
       --with-latest=PACKAGE
                          use the latest upstream release of PACKAGE"))
diff --git a/tests/transformations.scm b/tests/transformations.scm
index 1fa2c0bba8..31fd042d31 100644
--- a/tests/transformations.scm
+++ b/tests/transformations.scm
@@ -33,7 +33,7 @@ (define-module (test-transformations)
   #:use-module ((guix gexp)
                 #:select (local-file? local-file-file
                           computed-file? computed-file-gexp
-                          gexp-input-thing))
+                          gexp-input-thing gexp->approximate-sexp))
   #:use-module (guix ui)
   #:use-module (guix utils)
   #:use-module (guix git)
@@ -408,6 +408,14 @@ (define (package-name* obj)
                         (package-full-name grep))
               (package-arguments (package-replacement dep0))))))))

+(test-equal "options->transformation, with-configure-flag"
+  '(cons* "--flag" '())
+  (let* ((p   (dummy-package "foo"
+                (build-system gnu-build-system)))
+         (t   (options->transformation '((with-configure-flag .
"foo=--flag")))))
+    (let ((new (t p)))
+      (gexp->approximate-sexp (cadr (memq #:configure-flags
(package-arguments new)))))))
+
 (test-assert "options->transformation, without-tests"
   (let* ((dep (dummy-package "dep"))
          (p   (dummy-package "foo"
-- 
2.40.0
[Message part 2 (text/html, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Thu, 04 May 2023 14:26:03 GMT) Full text and rfc822 format available.

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

From: Ludovic Courtès <ludovic.courtes <at> inria.fr>
To: Sarthak Shah <shahsarthakw <at> gmail.com>
Cc: 62551 <at> debbugs.gnu.org
Subject: Re: bug#62551: Added new transformation option: --with-configure-flag
Date: Thu, 04 May 2023 16:25:49 +0200
[Message part 1 (text/plain, inline)]
Hi Sarthak,

Sarthak Shah <shahsarthakw <at> 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’.

[Message part 2 (text/x-patch, inline)]
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 <at> 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"))

bug closed, send any further explanations to 62551 <at> debbugs.gnu.org and Sarthak Shah <shahsarthakw <at> gmail.com> Request was from Ludovic Courtès <ludo <at> gnu.org> to control <at> debbugs.gnu.org. (Thu, 04 May 2023 14:27:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#62551; Package guix-patches. (Fri, 05 May 2023 12:00:02 GMT) Full text and rfc822 format available.

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

From: "pelzflorian (Florian Pelz)" <pelzflorian <at> pelzflorian.de>
To: Ludovic Courtès <ludovic.courtes <at> inria.fr>
Cc: 62551 <at> debbugs.gnu.org, Sarthak Shah <shahsarthakw <at> gmail.com>
Subject: Re: [bug#62551] Added new transformation option: --with-configure-flag
Date: Fri, 05 May 2023 13:59:11 +0200
Hi all.  This option indeed is great work, however when testing the
CMake example from the news entry, it fails to build.  Is it just me?
It seems like it should work.  Ludo, are you maybe using a lapack from
another channel?

Regards,
Florian




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 03 Jun 2023 11:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 299 days ago.

Previous Next


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