[PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Federico Beffa]

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

Generalize the Python procedure to recursively replace compiler/interpreter.

[-- Attachment #2: 0001-build-Generalize-package-with-explicit-python.patch --]
[-- Type: text/x-patch, Size: 10830 bytes --]

From a8220b430d196e5bb079e23ac63b1acd16fdaaee Mon Sep 17 00:00:00 2001
From: Federico Beffa <beffa@fbengineering.ch>
Date: Wed, 8 Feb 2017 18:55:32 +0100
Subject: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

* guix/build-system/python.scm (package-with-explicit-python): Remove it and
  replace it with the generalized procedure 'package-with-explicit-compiler'
  in the new file. (package-with-python2): Adapt it.
* guix/build-system/utils.scm: New file with the generalized procedure.
* Makefile.am (MODULES): Add new file.
---
 Makefile.am                  |   1 +
 guix/build-system/python.scm |  71 +++--------------------------
 guix/build-system/utils.scm  | 105 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 112 insertions(+), 65 deletions(-)
 create mode 100644 guix/build-system/utils.scm

diff --git a/Makefile.am b/Makefile.am
index 360c356f1..2d2544a7b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -76,6 +76,7 @@ MODULES =					\
   guix/build-system/r.scm			\
   guix/build-system/ruby.scm			\
   guix/build-system/trivial.scm			\
+  guix/build-system/utils.scm			\
   guix/ftp-client.scm				\
   guix/http-client.scm				\
   guix/gnupg.scm				\
diff --git a/guix/build-system/python.scm b/guix/build-system/python.scm
index 17173f121..8b3eb4008 100644
--- a/guix/build-system/python.scm
+++ b/guix/build-system/python.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2017 Federico Beffa <beffa@fbengineering.ch>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -27,6 +28,7 @@
   #:use-module (guix search-paths)
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system utils)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-26)
@@ -70,75 +72,14 @@ extension, such as '.tar.gz'."
   (let ((python (resolve-interface '(gnu packages python))))
     (module-ref python 'python-2)))
 
-(define* (package-with-explicit-python python old-prefix new-prefix
-                                       #:key variant-property)
-  "Return a procedure of one argument, P.  The procedure creates a package with
-the same fields as P, which is assumed to use PYTHON-BUILD-SYSTEM, such that
-it is compiled with PYTHON instead.  The inputs are changed recursively
-accordingly.  If the name of P starts with OLD-PREFIX, this is replaced by
-NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name.
-
-When VARIANT-PROPERTY is present, it is used as a key to search for
-pre-defined variants of this transformation recorded in the 'properties' field
-of packages.  The property value must be the promise of a package.  This is a
-convenient way for package writers to force the transformation to use
-pre-defined variants."
-  (define transform
-    ;; Memoize the transformations.  Failing to do that, we would build a huge
-    ;; object graph with lots of duplicates, which in turns prevents us from
-    ;; benefiting from memoization in 'package-derivation'.
-    (mlambdaq (p)
-      (let* ((rewrite-if-package
-              (lambda (content)
-                ;; CONTENT may be a file name, in which case it is returned,
-                ;; or a package, which is rewritten with the new PYTHON and
-                ;; NEW-PREFIX.
-                (if (package? content)
-                    (transform content)
-                    content)))
-             (rewrite
-              (match-lambda
-                ((name content . rest)
-                 (append (list name (rewrite-if-package content)) rest)))))
-
-        (cond
-         ;; If VARIANT-PROPERTY is present, use that.
-         ((and variant-property
-               (assoc-ref (package-properties p) variant-property))
-          => force)
-
-         ;; Otherwise build the new package object graph.
-         ((eq? (package-build-system p) python-build-system)
-          (package
-            (inherit p)
-            (location (package-location p))
-            (name (let ((name (package-name p)))
-                    (string-append new-prefix
-                                   (if (string-prefix? old-prefix name)
-                                       (substring name
-                                                  (string-length old-prefix))
-                                       name))))
-            (arguments
-             (let ((python (if (promise? python)
-                               (force python)
-                               python)))
-               (ensure-keyword-arguments (package-arguments p)
-                                         `(#:python ,python))))
-            (inputs (map rewrite (package-inputs p)))
-            (propagated-inputs (map rewrite (package-propagated-inputs p)))
-            (native-inputs (map rewrite (package-native-inputs p)))))
-         (else
-          p)))))
-
-  transform)
-
 (define package-with-python2
   ;; Note: delay call to 'default-python2' until after the 'arguments' field
   ;; of packages is accessed to avoid a circular dependency when evaluating
   ;; the top-level of (gnu packages python).
-  (package-with-explicit-python (delay (default-python2))
-                                "python-" "python2-"
-                                #:variant-property 'python2-variant))
+  (package-with-explicit-compiler (delay (default-python2))
+                                  'python
+                                  "python-" "python2-"
+                                  #:variant-property 'python2-variant))
 
 (define (strip-python2-variant p)
   "Remove the 'python2-variant' property from P."
diff --git a/guix/build-system/utils.scm b/guix/build-system/utils.scm
new file mode 100644
index 000000000..400696030
--- /dev/null
+++ b/guix/build-system/utils.scm
@@ -0,0 +1,105 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
+;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2017 Federico Beffa <beffa@fbengineering.ch>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix build-system utils)
+  #:use-module (guix build-system)
+  #:use-module (guix memoization)
+  #:use-module (guix packages)
+  #:use-module (guix ui)
+  #:use-module (guix utils)
+  #:use-module (ice-9 match)
+  #:export (package-with-explicit-compiler))
+
+;; Note: delay call to 'compiler' until after the 'arguments' field of
+;; packages is accessed to avoid a circular dependency when evaluating the
+;; top-level of (gnu packages python).  For a similar reason we pass the name
+;; of the build-system and not the build-system itself.
+(define* (package-with-explicit-compiler compiler bs-name
+                                         old-prefix new-prefix
+                                         #:key variant-property)
+  "Return a procedure of one argument, P.  The procedure creates a package
+with the same fields as P, which is assumed a build-system named BS-NAME, such
+that it is compiled with COMPILER instead.  The inputs are changed recursively
+accordingly.  If the name of P starts with OLD-PREFIX, this is replaced by
+NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name.
+
+When VARIANT-PROPERTY is present, it is used as a key to search for
+pre-defined variants of this transformation recorded in the 'properties' field
+of packages.  The property value must be the promise of a package.  This is a
+convenient way for package writers to force the transformation to use
+pre-defined variants."
+  (define (maybe-force maybe-promise)
+    (if (promise? maybe-promise)
+        (force maybe-promise)
+        maybe-promise))
+
+  (define compiler-keyword
+    (case bs-name
+      ((haskell python emacs perl r ruby) (symbol->keyword bs-name))
+      (else
+       (leave (_ "Operation not supported by the build system: ~A~%") bs-name))))
+
+  (define transform
+    ;; Memoize the transformations.  Failing to do that, we would build a huge
+    ;; object graph with lots of duplicates, which in turns prevents us from
+    ;; benefiting from memoization in 'package-derivation'.
+    (mlambdaq (p)
+      (let* ((rewrite-if-package
+              (lambda (content)
+                ;; CONTENT may be a file name, in which case it is returned,
+                ;; or a package, which is rewritten with the new COMPILER and
+                ;; NEW-PREFIX.
+                (if (package? content)
+                    (transform content)
+                    content)))
+             (rewrite
+              (match-lambda
+                ((name content . rest)
+                 (append (list name (rewrite-if-package content)) rest)))))
+
+        (cond
+         ;; If VARIANT-PROPERTY is present, use that.
+         ((and variant-property
+               (assoc-ref (package-properties p) variant-property))
+          => force)
+
+         ;; Otherwise build the new package object graph.
+         ((eq? (build-system-name (package-build-system p)) bs-name)
+          (package
+            (inherit p)
+            (location (package-location p))
+            (name (let ((name (package-name p)))
+                    (string-append new-prefix
+                                   (if (string-prefix? old-prefix name)
+                                       (substring name
+                                                  (string-length old-prefix))
+                                       name))))
+            (arguments
+             (let ((compiler (maybe-force compiler)))
+               (ensure-keyword-arguments (package-arguments p)
+                                         `(,compiler-keyword ,compiler))))
+            (inputs (map rewrite (package-inputs p)))
+            (propagated-inputs (map rewrite (package-propagated-inputs p)))
+            (native-inputs (map rewrite (package-native-inputs p)))))
+         (else
+          p)))))
+
+  transform)
-- 
2.11.0

Re: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Ricardo Wurmus]

Federico Beffa <beffa@ieee.org> writes:

> Generalize the Python procedure to recursively replace compiler/interpreter.
> From a8220b430d196e5bb079e23ac63b1acd16fdaaee Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 8 Feb 2017 18:55:32 +0100
> Subject: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.
>
> * guix/build-system/python.scm (package-with-explicit-python): Remove it and
>   replace it with the generalized procedure 'package-with-explicit-compiler'
>   in the new file. (package-with-python2): Adapt it.
> * guix/build-system/utils.scm: New file with the generalized procedure.
> * Makefile.am (MODULES): Add new file.

The goal here is to override packages in build system arguments, right?
This is great and we really need something like this to make building
package variants simpler.  (I ran into a problem that would be solved by
this when I tried to build Perl modules for different variants of Perl.)

Looking over the patch it feels inelegant to have a whitelist of
permitted arguments, as in “compiler-keyword”.  We already have a
procedure called “package-input-rewriting” that replaces any (explicit
input-) package in the dependency graph with a different package.  What
I like about that is that it is generic.  Is there a way we could extend
it to include packages pulled in by the build system, e.g. by making it
work at the level of bags?

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net

Re: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Federico Beffa]

On Wed, Feb 8, 2017 at 8:09 PM, Ricardo Wurmus <rekado@elephly.net> wrote:
>
> Federico Beffa <beffa@ieee.org> writes:
>
>> Generalize the Python procedure to recursively replace compiler/interpreter.
>> From a8220b430d196e5bb079e23ac63b1acd16fdaaee Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Wed, 8 Feb 2017 18:55:32 +0100
>> Subject: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.
>>
>> * guix/build-system/python.scm (package-with-explicit-python): Remove it and
>>   replace it with the generalized procedure 'package-with-explicit-compiler'
>>   in the new file. (package-with-python2): Adapt it.
>> * guix/build-system/utils.scm: New file with the generalized procedure.
>> * Makefile.am (MODULES): Add new file.
>
> The goal here is to override packages in build system arguments, right?
> This is great and we really need something like this to make building
> package variants simpler.  (I ran into a problem that would be solved by
> this when I tried to build Perl modules for different variants of Perl.)

Hi Ricardo,

The goal is to exploit the compiler keyword argument of many
build-systems (such as #:python, #:haskell, ...) to pass the desired
compiler instead of the default one.

> Looking over the patch it feels inelegant to have a whitelist of
> permitted arguments, as in “compiler-keyword”.  We already have a
> procedure called “package-input-rewriting” that replaces any (explicit
> input-) package in the dependency graph with a different package.  What
> I like about that is that it is generic.  Is there a way we could extend
> it to include packages pulled in by the build system, e.g. by making it
> work at the level of bags?

The procedure was not written by me: 99% of it comes from the
python-build-system.  The only thing that I did is to replace the
hard-coded '#:python' keyword with a variable one such that it can be
used with some other build systems. This seemed to me as one step in
the right direction, but if you have a better idea then please go
ahead and write it.  I will not feel offended ;-)

Regards,
Fede

Re: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Myles English]

on [2017-02-09] at 07:58 Federico Beffa writes:

> On Wed, Feb 8, 2017 at 8:09 PM, Ricardo Wurmus <rekado@elephly.net> wrote:
>>
>> Federico Beffa <beffa@ieee.org> writes:
>>
>>> Generalize the Python procedure to recursively replace compiler/interpreter.
>>> From a8220b430d196e5bb079e23ac63b1acd16fdaaee Mon Sep 17 00:00:00 2001
>>> From: Federico Beffa <beffa@fbengineering.ch>
>>> Date: Wed, 8 Feb 2017 18:55:32 +0100
>>> Subject: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.
>>>
>>> * guix/build-system/python.scm (package-with-explicit-python): Remove it and
>>>   replace it with the generalized procedure 'package-with-explicit-compiler'
>>>   in the new file. (package-with-python2): Adapt it.
>>> * guix/build-system/utils.scm: New file with the generalized procedure.
>>> * Makefile.am (MODULES): Add new file.
>>
>> The goal here is to override packages in build system arguments, right?
>> This is great and we really need something like this to make building
>> package variants simpler.  (I ran into a problem that would be solved by
>> this when I tried to build Perl modules for different variants of Perl.)
>
> Hi Ricardo,
>
> The goal is to exploit the compiler keyword argument of many
> build-systems (such as #:python, #:haskell, ...) to pass the desired
> compiler instead of the default one.
>
>> Looking over the patch it feels inelegant to have a whitelist of
>> permitted arguments, as in “compiler-keyword”.  We already have a
>> procedure called “package-input-rewriting” that replaces any (explicit
>> input-) package in the dependency graph with a different package.  What
>> I like about that is that it is generic.  Is there a way we could extend
>> it to include packages pulled in by the build system, e.g. by making it
>> work at the level of bags?
>
> The procedure was not written by me: 99% of it comes from the
> python-build-system.  The only thing that I did is to replace the
> hard-coded '#:python' keyword with a variable one such that it can be
> used with some other build systems. This seemed to me as one step in
> the right direction, but if you have a better idea then please go
> ahead and write it.  I will not feel offended ;-)

Thanks a lot for doing this it seems to work fine so far.  I think it
might cause a warning:

guix build: warning: deprecated NAME-VERSION syntax; use NAME@VERSION
instead

(I have reached a blockage to do with Haskell package dependencies, I am
trying to build hledger, here is my effort so far, 27 new packages!:
http://paste.lisp.org/+79B8 ).

Myles

Re: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Ludovic Courtès]

Hello!

Federico Beffa <beffa@ieee.org> skribis:

> +(define* (package-with-explicit-compiler compiler bs-name
> +                                         old-prefix new-prefix
> +                                         #:key variant-property)
> +  "Return a procedure of one argument, P.  The procedure creates a package
> +with the same fields as P, which is assumed a build-system named BS-NAME, such
> +that it is compiled with COMPILER instead.  The inputs are changed recursively
> +accordingly.  If the name of P starts with OLD-PREFIX, this is replaced by
> +NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name.
> +
> +When VARIANT-PROPERTY is present, it is used as a key to search for
> +pre-defined variants of this transformation recorded in the 'properties' field
> +of packages.  The property value must be the promise of a package.  This is a
> +convenient way for package writers to force the transformation to use
> +pre-defined variants."

Great idea, along with --with-compiler.

However, I think this:

> +  (define compiler-keyword
> +    (case bs-name
> +      ((haskell python emacs perl r ruby) (symbol->keyword bs-name))
> +      (else
> +       (leave (_ "Operation not supported by the build system: ~A~%") bs-name))))

and this:

> +         ;; Otherwise build the new package object graph.
> +         ((eq? (build-system-name (package-build-system p)) bs-name)

are kind of hacks (the build system name is supposed to be used for
debugging purposes only, and not for identification) and not very
extensible (we’d have to extend the ‘case’ above for every new build
system.)

What we’re trying to achieve here is similar to
‘package-input-rewriting’ and --with-input, except that it should
additionally take into account implicit inputs.

For that reason I’d suggest a new ‘package-input-rewriting*’ that would:

  1. Call ‘package-input-rewriting’;

  2. Change the ‘build-system’ field of the result and inject a “proxy”
     build system in order to be able to change the inputs of the bag
     (which includes the package’s implicit inputs).

Then we could change --with-input to use this new procedure, and
--with-input=r=my-r would DTRT.

WDYT?

Thanks!
Ludo’.

Re: [PATCH 1/2] build: Generalize 'package-with-explicit-python'.

[Federico Beffa]

On Fri, Feb 10, 2017 at 5:18 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Hello!
>
> Federico Beffa <beffa@ieee.org> skribis:
>
>> +(define* (package-with-explicit-compiler compiler bs-name
>> +                                         old-prefix new-prefix
>> +                                         #:key variant-property)
>> +  "Return a procedure of one argument, P.  The procedure creates a package
>> +with the same fields as P, which is assumed a build-system named BS-NAME, such
>> +that it is compiled with COMPILER instead.  The inputs are changed recursively
>> +accordingly.  If the name of P starts with OLD-PREFIX, this is replaced by
>> +NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name.
>> +
>> +When VARIANT-PROPERTY is present, it is used as a key to search for
>> +pre-defined variants of this transformation recorded in the 'properties' field
>> +of packages.  The property value must be the promise of a package.  This is a
>> +convenient way for package writers to force the transformation to use
>> +pre-defined variants."
>
> Great idea, along with --with-compiler.
>
> However, I think this:
>
>> +  (define compiler-keyword
>> +    (case bs-name
>> +      ((haskell python emacs perl r ruby) (symbol->keyword bs-name))
>> +      (else
>> +       (leave (_ "Operation not supported by the build system: ~A~%") bs-name))))
>
> and this:
>
>> +         ;; Otherwise build the new package object graph.
>> +         ((eq? (build-system-name (package-build-system p)) bs-name)
>
> are kind of hacks (the build system name is supposed to be used for
> debugging purposes only, and not for identification) and not very
> extensible (we’d have to extend the ‘case’ above for every new build
> system.)
>
> What we’re trying to achieve here is similar to
> ‘package-input-rewriting’ and --with-input, except that it should
> additionally take into account implicit inputs.
>
> For that reason I’d suggest a new ‘package-input-rewriting*’ that would:
>
>   1. Call ‘package-input-rewriting’;
>
>   2. Change the ‘build-system’ field of the result and inject a “proxy”
>      build system in order to be able to change the inputs of the bag
>      (which includes the package’s implicit inputs).
>
> Then we could change --with-input to use this new procedure, and
> --with-input=r=my-r would DTRT.

I agree with everything you said and that's a nice plan forward.  But
there are other considerations as well.

We have many outdated Haskell packages which do not work with the latest
GHC.  I'm using it and would love to see the ecosystem being upgraded,
but currently can't invest enough time to do the job myself.  Now,
somebody offered to do the job (that's upgrading a couple 100s of
packages!) and asked if tools are available to test compatibility with
a new compiler.  Given that we are using 'package-with-explicit-python'
since 2013 and that with a simple hack we can make it work with several
other build-systems, I thought that offering it as an interim solution
would be simple and acceptable.

As far I can see, this hack doesn't have widespread consequences and
offers an immediate aid.  Once the elegant and super general solution will be
ready we can easily replace it.

Regards,
Fede