bug#22437: Fixing package-with-python2

bug#22437: Fixing package-with-python2

[Ludovic Courtès]

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

ludo@gnu.org (Ludovic Courtès) skribis:

> An idea I haven’t taken the time to test yet would be to use
> ‘properties’:
>
>   (define python-foobar   ;with Python 3
>     (package
>       (name "foobar")
>       ;; Specify which Python 2 variant to use.
>       (properties `((python2-variant . ,(delay python2-foobar))))))
>
>   (define python2-foobar
>     (package (inherit python-foobar)
>       ;; … stuff beyond the mechanical python 2→3 switch…
>       ))
>
> ‘package-with-python2’ would honor this ‘python2-variant’ property.

Here’s a first stab at this.

As an example, I modified ‘python-matplotlib’ to use that feature, so we
can test that ‘python2-scipy’ is using the right ‘python2-matplotlib’:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix build python2-matplotlib -d 2>/dev/null
/gnu/store/07zr2pqg61b742czb2aqyisml2i90r4a-python2-matplotlib-1.4.3.drv
$ ./pre-inst-env guix build python2-scipy -d 2>/dev/null
/gnu/store/8yhxdbyjvx2xwynpqqcrj9ilksd2pb01-python2-scipy-0.16.0.drv
$ guix gc --references /gnu/store/8yhxdbyjvx2xwynpqqcrj9ilksd2pb01-python2-scipy-0.16.0.drv | grep python2-matplotlib
/gnu/store/07zr2pqg61b742czb2aqyisml2i90r4a-python2-matplotlib-1.4.3.drv
--8<---------------cut here---------------end--------------->8---

This will trigger rebuilds (but with an identical result) because in
manually-written variants we would use “python2-foo” as the label of
inputs, whereas the automatic transformations keeps the original
“python-foo” label.

What do people think?

I can apply this patch of the approach sounds good to you.  I think we
should probably do one commit per rewrite for clarity.

We should probably start with the lowest level, like python2-pycairo and
python2-pygobject and even python itself, because if we fix them then
some of the higher-level stuff won’t even need their own
‘python2-variant’ property.  For instance, if python, pycairo, and
pygobject have their ‘python2-variant’ set, then we no longer need this:

--8<---------------cut here---------------start------------->8---
(define-public python2-matplotlib
  (let ((matplotlib (package-with-python2 %python-matplotlib)))
    (package (inherit matplotlib)
      ;; Make sure to use special packages for Python 2 instead
      ;; of those automatically rewritten by package-with-python2.
      (propagated-inputs
       `(("python2-pycairo" ,python2-pycairo)
         ("python2-pygobject-2" ,python2-pygobject-2)
         ("python2-tkinter" ,python-2 "tk")
         ,@(fold alist-delete (package-propagated-inputs matplotlib)
                 '("python-pycairo" "python-pygobject" "python-tkinter")))))))
--8<---------------cut here---------------end--------------->8---

… and as a consequence, we don’t need a ‘python2-variant’ in
‘python-matplotlib’.

Does that make sense?  Any takers?  (This can be done incrementally.)

Thanks,
Ludo’.


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

diff --git a/gnu/packages/python.scm b/gnu/packages/python.scm
index 48f65b5..b43f539 100644
--- a/gnu/packages/python.scm
+++ b/gnu/packages/python.scm
@@ -2913,7 +2913,7 @@ is designed to have a low barrier to entry.")
 (define-public python2-rq
   (package-with-python2 python-rq))
 
-(define-public python-cython
+(define %python-cython
   (package
     (name "python-cython")
     (version "0.23.4")
@@ -2946,8 +2946,13 @@ programming language and the extended Cython programming language.  It makes
 writing C extensions for Python as easy as Python itself.")
     (license asl2.0)))
 
+(define-public python-cython
+  (package
+    (inherit %python-cython)
+    (properties `((python2-variant . ,(delay python2-cython))))))
+
 (define-public python2-cython
-  (package (inherit (package-with-python2 python-cython))
+  (package (inherit (package-with-python2 %python-cython))
     (name "python2-cython")
     (inputs
      `(("python-2" ,python-2))))) ; this is not automatically changed
@@ -3117,13 +3122,7 @@ association studies (GWAS) on extremely large data sets.")
               ,phases)))))))
 
 (define-public python2-numpy
-  (let ((numpy (package-with-python2 python-numpy)))
-    (package (inherit numpy)
-      ;; Make sure we use exactly PYTHON2-MATPLOTLIB, which is customized for
-      ;; Python 2.
-      (inputs `(("python2-matplotlib" ,python2-matplotlib)
-                ,@(alist-delete "python-matplotlib"
-                                (package-inputs numpy)))))))
+  (package-with-python2 python-numpy))
 
 (define-public python-pyparsing
   (package
@@ -3247,7 +3246,7 @@ transcendental functions).")
          ,@(alist-delete "python-numpy"
                          (package-propagated-inputs numexpr)))))))
 
-(define-public python-matplotlib
+(define %python-matplotlib
   (package
     (name "python-matplotlib")
     (version "1.4.3")
@@ -3323,7 +3322,7 @@ transcendental functions).")
               (lambda (port)
                 (format port "[directories]~%
 basedirlist = ~a,~a~%
-[rc_options]~%
+ [rc_options]~%
 backend = TkAgg~%"
                         (assoc-ref inputs "tcl")
                         (assoc-ref inputs "tk"))))))
@@ -3372,8 +3371,13 @@ ipython shell, web application servers, and six graphical user interface
 toolkits.")
     (license psfl)))
 
+(define-public python-matplotlib
+  (package
+    (inherit %python-matplotlib)
+    (properties `((python2-variant . ,(delay python2-matplotlib))))))
+
 (define-public python2-matplotlib
-  (let ((matplotlib (package-with-python2 python-matplotlib)))
+  (let ((matplotlib (package-with-python2 %python-matplotlib)))
     (package (inherit matplotlib)
       ;; Make sure to use special packages for Python 2 instead
       ;; of those automatically rewritten by package-with-python2.
@@ -3549,15 +3553,7 @@ routines such as routines for numerical integration and optimization.")
     (license bsd-3)))
 
 (define-public python2-scipy
-  (let ((scipy (package-with-python2 python-scipy)))
-    (package (inherit scipy)
-      ;; Use packages customized for python-2.
-      (propagated-inputs
-       `(("python2-matplotlib" ,python2-matplotlib)
-         ("python2-numpy" ,python2-numpy)
-         ,@(alist-delete "python-matplotlib"
-                         (alist-delete "python-numpy"
-                                       (package-propagated-inputs scipy))))))))
+  (package-with-python2 python-scipy))
 
 (define-public python-sqlalchemy
   (package
diff --git a/guix/build-system/python.scm b/guix/build-system/python.scm
index 9a80bd6..74ec854 100644
--- a/guix/build-system/python.scm
+++ b/guix/build-system/python.scm
@@ -65,12 +65,19 @@ 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)
+(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."
+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
@@ -90,26 +97,34 @@ NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name."
                  ((name content . rest)
                   (append (list name (rewrite-if-package content)) rest)))))
 
-         (if (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))))
-             p)))))
+         (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)
 
@@ -118,7 +133,8 @@ NEW-PREFIX; otherwise, NEW-PREFIX is prepended to the name."
   ;; 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-"))
+                                "python-" "python2-"
+                                #:variant-property 'python2-variant))
 
 (define* (lower name
                 #:key source inputs native-inputs outputs system target

bug#22437: Fixing package-with-python2

[Efraim Flashner]

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

On Wed, 03 Feb 2016 09:47:15 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> ludo@gnu.org (Ludovic Courtès) skribis:
> 
> > An idea I haven’t taken the time to test yet would be to use
> > ‘properties’:
> >
> >   (define python-foobar   ;with Python 3
> >     (package
> >       (name "foobar")
> >       ;; Specify which Python 2 variant to use.
> >       (properties `((python2-variant . ,(delay python2-foobar))))))

This part I don't get. What's the period for?

> >   (define python2-foobar
> >     (package (inherit python-foobar)
> >       ;; … stuff beyond the mechanical python 2→3 switch…
> >       ))
> >
> > ‘package-with-python2’ would honor this ‘python2-variant’ property.
> 
> Here’s a first stab at this.
> 
> As an example, I modified ‘python-matplotlib’ to use that feature, so we
> can test that ‘python2-scipy’ is using the right ‘python2-matplotlib’:
> 
> --8<---------------cut here---------------start------------->8---
> $ ./pre-inst-env guix build python2-matplotlib -d 2>/dev/null
> /gnu/store/07zr2pqg61b742czb2aqyisml2i90r4a-python2-matplotlib-1.4.3.drv
> $ ./pre-inst-env guix build python2-scipy -d 2>/dev/null
> /gnu/store/8yhxdbyjvx2xwynpqqcrj9ilksd2pb01-python2-scipy-0.16.0.drv
> $ guix gc --references /gnu/store/8yhxdbyjvx2xwynpqqcrj9ilksd2pb01-python2-scipy-0.16.0.drv | grep python2-matplotlib
> /gnu/store/07zr2pqg61b742czb2aqyisml2i90r4a-python2-matplotlib-1.4.3.drv
> --8<---------------cut here---------------end--------------->8---
> 
> This will trigger rebuilds (but with an identical result) because in
> manually-written variants we would use “python2-foo” as the label of
> inputs, whereas the automatic transformations keeps the original
> “python-foo” label.

rebuilds python-foo and python2-foo, or just the python2- variants?

> What do people think?

I like it. It keeps the logic in the build-system. In terms of a speed test
when figuring out the build/dependancy graph, how does it affect the time of
`guix graph python2-scipy python2-matplotlib`?

> I can apply this patch of the approach sounds good to you.  I think we
> should probably do one commit per rewrite for clarity.
> 
> We should probably start with the lowest level, like python2-pycairo and
> python2-pygobject and even python itself, because if we fix them then
> some of the higher-level stuff won’t even need their own
> ‘python2-variant’ property.  For instance, if python, pycairo, and
> pygobject have their ‘python2-variant’ set, then we no longer need this:
> 
> --8<---------------cut here---------------start------------->8---
> (define-public python2-matplotlib
>   (let ((matplotlib (package-with-python2 %python-matplotlib)))
>     (package (inherit matplotlib)
>       ;; Make sure to use special packages for Python 2 instead
>       ;; of those automatically rewritten by package-with-python2.
>       (propagated-inputs
>        `(("python2-pycairo" ,python2-pycairo)
>          ("python2-pygobject-2" ,python2-pygobject-2)
>          ("python2-tkinter" ,python-2 "tk")
>          ,@(fold alist-delete (package-propagated-inputs matplotlib)
>                  '("python-pycairo" "python-pygobject" "python-tkinter")))))))
> --8<---------------cut here---------------end--------------->8---
> 
> … and as a consequence, we don’t need a ‘python2-variant’ in
> ‘python-matplotlib’.
> 
> Does that make sense?  Any takers?  (This can be done incrementally.)

It fits our "one change per commit" policy, and if we don't start at the base
of the pyramid we'll be modifying and then removing the special variants. I
don't mind doing the conversion process.

> Thanks,
> Ludo’.
> 

Thinking aloud, I think for the time being we should keep the
python-setuptools that are already part of the python- variants where they
are and save that for another time.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

bug#22437: Fixing package-with-python2

[Andreas Enge]

Hello,

this looks really good, but I do not understand why we need the
additional private variable, for instance %python-cython:

On Wed, Feb 03, 2016 at 09:47:15AM +0100, Ludovic Courtès wrote:
> -(define-public python-cython
> +(define %python-cython
>    (package
>      (name "python-cython")
>      (version "0.23.4")
> @@ -2946,8 +2946,13 @@ programming language and the extended Cython programming language.  It makes
>  writing C extensions for Python as easy as Python itself.")
>      (license asl2.0)))
>  
> +(define-public python-cython
> +  (package
> +    (inherit %python-cython)
> +    (properties `((python2-variant . ,(delay python2-cython))))))
> +
>  (define-public python2-cython
> -  (package (inherit (package-with-python2 python-cython))
> +  (package (inherit (package-with-python2 %python-cython))
>      (name "python2-cython")
>      (inputs

If python2-cython inherits from (package-with-python2 python-cython),
is not the only difference that it keeps the properties field? And
would this not be harmless, as we are not going to call package-with-python2
again? Or would this create a circular dependency with (delay python2-cython)?
(In C or Pascal, this would not be a problem, one could simply declare
things before they are used, and that is it.)

It would be more elegant to drop the additional variable if possible.

Andreas

bug#22437: Fixing package-with-python2

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> this looks really good, but I do not understand why we need the
> additional private variable, for instance %python-cython:

Glad you noticed.  :-)

> On Wed, Feb 03, 2016 at 09:47:15AM +0100, Ludovic Courtès wrote:
>> -(define-public python-cython
>> +(define %python-cython
>>    (package
>>      (name "python-cython")
>>      (version "0.23.4")
>> @@ -2946,8 +2946,13 @@ programming language and the extended Cython programming language.  It makes
>>  writing C extensions for Python as easy as Python itself.")
>>      (license asl2.0)))
>>  
>> +(define-public python-cython
>> +  (package
>> +    (inherit %python-cython)
>> +    (properties `((python2-variant . ,(delay python2-cython))))))
>> +
>>  (define-public python2-cython
>> -  (package (inherit (package-with-python2 python-cython))
>> +  (package (inherit (package-with-python2 %python-cython))
>>      (name "python2-cython")
>>      (inputs
>
> If python2-cython inherits from (package-with-python2 python-cython),
> is not the only difference that it keeps the properties field? And
> would this not be harmless, as we are not going to call package-with-python2
> again? Or would this create a circular dependency with (delay python2-cython)?

So I thought!  In hindsight, I think this is unnecessary.

> (In C or Pascal, this would not be a problem

I can’t wait to see “Pasix”!  :-)

Ludo’.

bug#22437: Fixing package-with-python2

[Ricardo Wurmus]

Efraim Flashner <efraim@flashner.co.il> writes:

> On Wed, 03 Feb 2016 09:47:15 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> ludo@gnu.org (Ludovic Courtès) skribis:
>> 
>> > An idea I haven’t taken the time to test yet would be to use
>> > ‘properties’:
>> >
>> >   (define python-foobar   ;with Python 3
>> >     (package
>> >       (name "foobar")
>> >       ;; Specify which Python 2 variant to use.
>> >       (properties `((python2-variant . ,(delay python2-foobar))))))
>
> This part I don't get. What's the period for?

The “properties” field holds a regular alist.  Here the alist has one
entry (a pair) with a symbol “python2-variant” as the key, and “,(delay
python2-foobar)” as its value.

The period is needed for the “dotted list” syntax, which is used to
distinguish a pair (or an improper list) from a well-formed list.

~~ Ricardo

bug#22437: Fixing package-with-python2

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Efraim Flashner <efraim@flashner.co.il> writes:
>
>> On Wed, 03 Feb 2016 09:47:15 +0100
>> ludo@gnu.org (Ludovic Courtès) wrote:
>>
>>> ludo@gnu.org (Ludovic Courtès) skribis:
>>> 
>>> > An idea I haven’t taken the time to test yet would be to use
>>> > ‘properties’:
>>> >
>>> >   (define python-foobar   ;with Python 3
>>> >     (package
>>> >       (name "foobar")
>>> >       ;; Specify which Python 2 variant to use.
>>> >       (properties `((python2-variant . ,(delay python2-foobar))))))
>>
>> This part I don't get. What's the period for?
>
> The “properties” field holds a regular alist.  Here the alist has one
> entry (a pair) with a symbol “python2-variant” as the key, and “,(delay
> python2-foobar)” as its value.
>
> The period is needed for the “dotted list” syntax, which is used to
> distinguish a pair (or an improper list) from a well-formed list.

This is a relic of the old days and probably kind of confusing to
newcomers, but since we started making this field an alist, let’s keep
it this way.

Ludo’.

bug#22437: Fixing package-with-python2

[Ludovic Courtès]

Efraim Flashner <efraim@flashner.co.il> skribis:

> On Wed, 03 Feb 2016 09:47:15 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:

[...]

>> This will trigger rebuilds (but with an identical result) because in
>> manually-written variants we would use “python2-foo” as the label of
>> inputs, whereas the automatic transformations keeps the original
>> “python-foo” label.
>
> rebuilds python-foo and python2-foo, or just the python2- variants?

The latter.

>> What do people think?
>
> I like it. It keeps the logic in the build-system. In terms of a speed test
> when figuring out the build/dependancy graph, how does it affect the time of
> `guix graph python2-scipy python2-matplotlib`?

It might be slightly faster, but it’s already rather fast.  :-)

--8<---------------cut here---------------start------------->8---
$ time guix graph python2-scipy python2-matplotlib >/dev/null

real	0m0.664s
user	0m0.768s
sys	0m0.056s
--8<---------------cut here---------------end--------------->8---


[...]

>> Does that make sense?  Any takers?  (This can be done incrementally.)
>
> It fits our "one change per commit" policy, and if we don't start at the base
> of the pyramid we'll be modifying and then removing the special variants. I
> don't mind doing the conversion process.

OK.

> Thinking aloud, I think for the time being we should keep the
> python-setuptools that are already part of the python- variants where they
> are and save that for another time.

Agreed.

Thanks for your feedback!

Ludo’.

bug#22437: Fixing package-with-python2

[Ludovic Courtès]

I’ve pushed the ‘python2-variant’ trick as 1be8334.  Subsequent commits
use it for a bunch of packages.

I’ll let others handle the remaining packages where this would help.
:-)

Closing the bug!

Ludo’.