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

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

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

Hello!

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

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)