Added new transformation option: --with-configure-flag

  • Done
  • quality assurance status badge
Details
5 participants
  • Ludovic Courtès
  • Ludovic Courtès
  • Bruno Victal
  • pelzflorian (Florian Pelz)
  • Sarthak Shah
Owner
unassigned
Submitted by
Sarthak Shah
Severity
normal
S
S
Sarthak Shah wrote on 30 Mar 2023 21:52
(address . guix-patches@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
CADBZEVnRLzReTO9+z0kFiwOA97V-_O4O7Bjchsr3Rdbzh0_4Xw@mail.gmail.com
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(+)

Toggle diff (111 lines)
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
Attachment: file
S
S
Sarthak Shah wrote on 30 Mar 2023 21:56
(updated with changelog formatting)
(address . 62551@debbugs.gnu.org)
CADBZEVnXqNK5Z9A76qhqH+DZZpfMNQVk225C3rowS9cssFpVuw@mail.gmail.com
* 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(+)

Toggle diff (111 lines)
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
Attachment: file
B
B
Bruno Victal wrote on 31 Mar 2023 01:00
control-msg
(name . control)(address . control@debbugs.gnu.org)
74912071-f536-c8bc-4711-db295b4da0e6@makinata.eu
# done with 5c131aff691fa1cb0fafe71b5f2795902ae056a7
close 55819

# control fail?
close 45449

# no longer relevant
close 33078

# tags
tags 62551 patch
tags 62503 patch
tags 62461 patch
tags 62443 patch
tags 62428 patch
tags 61226 patch
tags 59893 patch
tags 59852 patch
tags 49451 patch
tags 49207 patch
tags 44258 patch


quit
L
L
Ludovic Courtès wrote on 31 Mar 2023 14:34
Re: bug#62551: Added new transformation option: --with-configure-flag
(name . Sarthak Shah)(address . shahsarthakw@gmail.com)(address . 62551@debbugs.gnu.org)
87355l5cbt.fsf@gnu.org
Hello!

Sarthak Shah <shahsarthakw@gmail.com> skribis:

Toggle quote (11 lines)
> 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!

Toggle quote (9 lines)
> + (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)

Toggle quote (3 lines)
> + (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.)

Toggle quote (4 lines)
> + (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
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’.
S
S
Sarthak Shah wrote on 1 Apr 2023 02:17
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 62551@debbugs.gnu.org)
CADBZEVnoCcEdDDzm90gmAuGsiahURMyg7VL8zS_uKVQ9J=7PPw@mail.gmail.com
Hey Ludovic,
Thanks for the comments!

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

Toggle quote (1 lines)
> 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.

Toggle quote (1 lines)
> 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.

Toggle quote (1 lines)
> 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.

Toggle quote (3 lines)
> 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)
Attachment: file
S
S
Sarthak Shah wrote on 25 Apr 2023 14:22
Updated patch - with changes, documentation and tests.
(address . 62551@debbugs.gnu.org)
CADBZEVn1kTD3cMs_Pj9wvEjOE=b2E2bsCqv0f49wHx4P+vjHYA@mail.gmail.com
* 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(-)

Toggle diff (155 lines)
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
Attachment: file
L
L
Ludovic Courtès wrote on 4 May 2023 16:25
Re: bug#62551: Added new transformation option: --with-configure-flag
(name . Sarthak Shah)(address . shahsarthakw@gmail.com)(address . 62551@debbugs.gnu.org)
87y1m4p40i.fsf_-_@gnu.org
Hi Sarthak,

Sarthak Shah <shahsarthakw@gmail.com> skribis:

Toggle quote (8 lines)
> * 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’.
Attachment: file
L
L
Ludovic Courtès wrote on 4 May 2023 16:25
control message for bug #62551
(address . control@debbugs.gnu.org)
87wn1op409.fsf@gnu.org
close 62551
quit
P
P
pelzflorian (Florian Pelz) wrote on 5 May 2023 13:59
Re: [bug#62551] Added new transformation option: --with-configure-flag
(name . Ludovic Courtès)(address . ludovic.courtes@inria.fr)
87ttwr3s6o.fsf@pelzflorian.de
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
?