[PATCH 0/3] Add a generalized `wrap-language-programs`

[PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Hello,

Following on from discussion earlier in the week, please find attached 3
patches that implement a generalized `wrap-language-programs` procedure that
can be used to add a `wrap` phase to build-systems.

The first patch adds the procedure, and the following 2 patches add `wrap`
phase implementations using that procedure.

I have tested Perl’s wrap phase, and that seems to work fine.  I was
unfortunately not able to test Python’s wrap phase, as I did not know of an
appropriate test candidate (I tried to build offlineimap, but this failed
because Guile@2.013 failed at least one of it’s tests).

Feedback welcome!

Ludovic: you mentioned pushing this patch series to a separate branch so we
can test the builds on the build farm.  What branch would you propose?

Best wishes,

Alex

Alex Sassmannshausen (3):
  build/utils: Add `wrap-language-programs`.
  build/perl-build-system: Add `wrap` phase.
  build/python-build-system: Refactor `wrap`.

 guix/build/perl-build-system.scm   | 15 +++++++++++++--
 guix/build/python-build-system.scm | 36 ++++++++----------------------------
 guix/build/utils.scm               | 36 ++++++++++++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 30 deletions(-)

-- 
2.10.2

[PATCH 1/3] build/utils: Add `wrap-language-programs`.

[Alex Sassmannshausen]

* guix/build/utils.scm (wrap-language-programs): New procedure.
---
 guix/build/utils.scm | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index bc6f114..b14f96b 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -75,6 +75,7 @@
             patch-/usr/bin/file
             fold-port-matches
             remove-store-references
+            wrap-language-programs
             wrap-program
 
             locale-category->string))
@@ -918,6 +919,41 @@ known as `nuke-refs' in Nixpkgs."
                              (put-u8 out (char->integer char))
                              result))))))
 
+(define (wrap-language-programs path-proc env-var)
+  "Return a procedure, which, invoked as part of a `wrap' phase, is capable of
+wrapping executables inside an environment in which ENV-VAR is correctly set.
+
+The string ENV-VAR is the name of the environmental variable we are setting
+for the executable we are wrapping.  PATH-PROC is a procedure of 2 arguments,
+`inputs' and `outputs', returning the value that we should send ENV-VAR to.
+
+This is a specialized version of `wrap-program' below, intended specifically
+to grant all executables that are part of our output access to all libraries
+that were declared in our inputs.  This is of use for languages such as Perl,
+Python and Guile."
+  (define (list-of-files dir)
+    (map (cut string-append dir "/" <>)
+         (or (scandir dir (lambda (f)
+                            (let ((s (stat (string-append dir "/" f))))
+                              (eq? 'regular (stat:type s)))))
+             '())))
+  (lambda* (#:key inputs outputs #:allow-other-keys)
+    (define bindirs
+      (append-map (match-lambda
+                    ((_ . dir)
+                     (list (string-append dir "/bin")
+                           (string-append dir "/sbin"))))
+                  outputs))
+    (define vars
+      `(,env-var prefix ,(cons (path-proc inputs outputs)
+                               (search-path-as-string->list
+                                (or (getenv env-var) "")))))
+    (for-each (lambda (dir)
+                (let ((files (list-of-files dir)))
+                  (for-each (cut wrap-program <> vars)
+                            files)))
+              bindirs)))
+
 (define* (wrap-program prog #:rest vars)
   "Make a wrapper for PROG.  VARS should look like this:
 
-- 
2.10.2

Re: [PATCH 1/3] build/utils: Add `wrap-language-programs`.

[Ludovic Courtès]

Hey!

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> * guix/build/utils.scm (wrap-language-programs): New procedure.

Neat!

> +(define (wrap-language-programs path-proc env-var)

Maybe ‘program-wrapper’ or ‘program-wrapping-procedure’ or
‘program-wrapping-phase’ to suggest that it returns a procedure?  (I’m
not sure “language” is helpful.)

> +  "Return a procedure, which, invoked as part of a `wrap' phase, is capable of
> +wrapping executables inside an environment in which ENV-VAR is correctly set.
> +
> +The string ENV-VAR is the name of the environmental variable we are setting
> +for the executable we are wrapping.  PATH-PROC is a procedure of 2 arguments,
> +`inputs' and `outputs', returning the value that we should send ENV-VAR to.
> +
> +This is a specialized version of `wrap-program' below, intended specifically
> +to grant all executables that are part of our output access to all libraries
> +that were declared in our inputs.  This is of use for languages such as Perl,
> +Python and Guile."

Nitpick: the GCS now recommend straight quotes in ASCII, like 'this'.
:-)

Otherwise this and the 2 subsequent patches LGTM.

Could you make sure that the resulting wrapper is the same, either by
rebuilding one way or another as discussed earlier, or by running the
code “by hand” on an example, maybe in ‘guix environment’?

After that you could commit in ‘core-updates’.

Thank you!

Ludo’.

[PATCH 2/3] build/perl-build-system: Add `wrap` phase.

[Alex Sassmannshausen]

* guix/build/perl-build-system.scm (wrap): New procedure.
  (%standard-phases): Add `wrap` phase.
---
 guix/build/perl-build-system.scm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/guix/build/perl-build-system.scm b/guix/build/perl-build-system.scm
index 8f480ea..861d75c 100644
--- a/guix/build/perl-build-system.scm
+++ b/guix/build/perl-build-system.scm
@@ -19,7 +19,7 @@
 (define-module (guix build perl-build-system)
   #:use-module ((guix build gnu-build-system) #:prefix gnu:)
   #:use-module (guix build utils)
-  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (%standard-phases
             perl-build))
 
@@ -47,6 +47,15 @@
     (format #t "running `perl' with arguments ~s~%" args)
     (zero? (apply system* "perl" args))))
 
+;; Use `wrap-language-program' to return an executable wrapper for perl.
+(define wrap
+  (wrap-language-programs
+   (lambda (inputs outputs)
+     (string-append (assoc-ref outputs "out") "/lib/perl5/site_perl/"
+                    ;; As in python, assume version at end of `perl' string.
+                    (last (string-split (assoc-ref inputs "perl") #\-))))
+   "PERL5LIB"))
+
 (define-syntax-rule (define-w/gnu-fallback* (name args ...) body ...)
   (define* (name args ... #:rest rest)
     (if (access? "Build" X_OK)
@@ -70,9 +79,11 @@
 
 (define %standard-phases
   ;; Everything is as with the GNU Build System except for the `configure',
-  ;; `build', `check', and `install' phases.
+  ;; `build', `check', and `install' phases.  We also add a `wrap' phase to
+  ;; wrap perl binaries with a complete PERL5LIB path.
   (modify-phases gnu:%standard-phases
     (replace 'install install)
+    (add-after 'install 'wrap wrap)
     (replace 'check check)
     (replace 'build build)
     (replace 'configure configure)))
-- 
2.10.2

[PATCH 3/3] build/python-build-system: Refactor `wrap`.

[Alex Sassmannshausen]

* guix/build/python-build-system.scm (wrap): Use
  `wrap-language-program`.
---
 guix/build/python-build-system.scm | 36 ++++++++----------------------------
 1 file changed, 8 insertions(+), 28 deletions(-)

diff --git a/guix/build/python-build-system.scm b/guix/build/python-build-system.scm
index 9109fb4..8e3ea41 100644
--- a/guix/build/python-build-system.scm
+++ b/guix/build/python-build-system.scm
@@ -80,34 +80,14 @@
                                add-path))
         (call-setuppy "install" params)))
 
-(define* (wrap #:key inputs outputs #:allow-other-keys)
-  (define (list-of-files dir)
-    (map (cut string-append dir "/" <>)
-         (or (scandir dir (lambda (f)
-                            (let ((s (stat (string-append dir "/" f))))
-                              (eq? 'regular (stat:type s)))))
-             '())))
-
-  (define bindirs
-    (append-map (match-lambda
-                 ((_ . dir)
-                  (list (string-append dir "/bin")
-                        (string-append dir "/sbin"))))
-                outputs))
-
-  (let* ((out  (assoc-ref outputs "out"))
-         (python (assoc-ref inputs "python"))
-         (var `("PYTHONPATH" prefix
-                ,(cons (string-append out "/lib/python"
-                                      (get-python-version python)
-                                      "/site-packages")
-                       (search-path-as-string->list
-                        (or (getenv "PYTHONPATH") ""))))))
-    (for-each (lambda (dir)
-                (let ((files (list-of-files dir)))
-                  (for-each (cut wrap-program <> var)
-                            files)))
-              bindirs)))
+;; Use `wrap-language-program' to return an executable wrapper for python.
+(define wrap
+  (wrap-language-programs
+   (lambda (inputs outputs)
+     (string-append (assoc-ref outputs "out") "/lib/python"
+                    (get-python-version (assoc-ref inputs "python"))
+                    "/site-packages"))
+   "PYTHONPATH"))
 
 (define* (rename-pth-file #:key name inputs outputs #:allow-other-keys)
   "Rename easy-install.pth to NAME.pth to avoid conflicts between packages
-- 
2.10.2

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Hello

> You may try e.g. scons, pip, sphinx, or youtube-dl.

Hartmut was kind enough to suggest the above python applications to test
this `wrap-language-programs` patch set against, however I have been
stuck in "Rebuild the world"-hell for the last day and a half setting up
testcases.

I end up feeling a bit desesperated and incompetent, as I'm sure it's
not supposed to be this hard to create an appropriate test scenario here
— so I was wondering whether anyone has any pointers in avoiding
constantly rebuilding the world?

For details:
- I run on i686
- using GuixSD
- I rebased my patch set on master 2 days ago and tried to test from
there
- this resulted in a bunch of "401 - Not Found" for a number of the
binary substitute dependencies.
- and from then I have been rebuilding those dependencies

I guess part of the problem is that the substitute servers won't
necessarily have substitutes for a particular program from master yet?

I guess an additional problem is that some of the packages fail tests on
my setup intermittently (they fail using
`guix package -i youtube-dl --fallback`, for instance, but would then
succeed if I build using `guix build $dependency`).

But would people have recommendations to ensure some level of substitute
stability or some such?

Best wishes,

Alex

Hartmut Goebel writes:

> Am 16.11.2016 um 14:27 schrieb Alex Sassmannshausen:
>> as I did not know of an
>> appropriate test candidate (I tried to build offlineimap, but this failed
>> because Guile@2.013 failed at least one of it’s tests).
>
> You may try e.g. scons, pip, sphinx, or youtube-dl.

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Fwiw, I have now succeeded at building offlineimap, and as a result, it
seems youtube-dl also was able to be built easily (presumably because
they share a great number of dependencies).

In both cases, the programs work fine with the generalized patch.

Alex

Alex Sassmannshausen writes:

> Hello
>
>> You may try e.g. scons, pip, sphinx, or youtube-dl.
>
> Hartmut was kind enough to suggest the above python applications to test
> this `wrap-language-programs` patch set against, however I have been
> stuck in "Rebuild the world"-hell for the last day and a half setting up
> testcases.
>
> I end up feeling a bit desesperated and incompetent, as I'm sure it's
> not supposed to be this hard to create an appropriate test scenario here
> — so I was wondering whether anyone has any pointers in avoiding
> constantly rebuilding the world?
>
> For details:
> - I run on i686
> - using GuixSD
> - I rebased my patch set on master 2 days ago and tried to test from
> there
> - this resulted in a bunch of "401 - Not Found" for a number of the
> binary substitute dependencies.
> - and from then I have been rebuilding those dependencies
>
> I guess part of the problem is that the substitute servers won't
> necessarily have substitutes for a particular program from master yet?
>
> I guess an additional problem is that some of the packages fail tests on
> my setup intermittently (they fail using
> `guix package -i youtube-dl --fallback`, for instance, but would then
> succeed if I build using `guix build $dependency`).
>
> But would people have recommendations to ensure some level of substitute
> stability or some such?
>
> Best wishes,
>
> Alex
>
> Hartmut Goebel writes:
>
>> Am 16.11.2016 um 14:27 schrieb Alex Sassmannshausen:
>>> as I did not know of an
>>> appropriate test candidate (I tried to build offlineimap, but this failed
>>> because Guile@2.013 failed at least one of it’s tests).
>>
>> You may try e.g. scons, pip, sphinx, or youtube-dl.

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Hartmut Goebel]

Am 17.11.2016 um 18:09 schrieb Alex Sassmannshausen:
> In both cases, the programs work fine with the generalized patch.
Fine :-)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Leo Famulari]

On Thu, Nov 17, 2016 at 04:58:45PM +0100, Alex Sassmannshausen wrote:
> 
> Hello
> 
> > You may try e.g. scons, pip, sphinx, or youtube-dl.
> 
> Hartmut was kind enough to suggest the above python applications to test
> this `wrap-language-programs` patch set against, however I have been
> stuck in "Rebuild the world"-hell for the last day and a half setting up
> testcases.
> 
> I end up feeling a bit desesperated and incompetent, as I'm sure it's
> not supposed to be this hard to create an appropriate test scenario here
> — so I was wondering whether anyone has any pointers in avoiding
> constantly rebuilding the world?

Most of the Guix distribution depends on Perl, and most of the rest
depends on Python ;)

Try `guix refresh -l perl`.

So, changing the build systems for these languages will require you to
rebuild almost everything.

Changes like this will need to go on the core-updates branch, where we
make changes to the core of the system.

You could create a separate perl-test-build-system and use it for your
test packages.

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Hi Leo,

Leo Famulari writes:

> On Thu, Nov 17, 2016 at 04:58:45PM +0100, Alex Sassmannshausen wrote:
>> 
>> Hello
>> 
>> > You may try e.g. scons, pip, sphinx, or youtube-dl.
>> 
>> Hartmut was kind enough to suggest the above python applications to test
>> this `wrap-language-programs` patch set against, however I have been
>> stuck in "Rebuild the world"-hell for the last day and a half setting up
>> testcases.
>> 
>> I end up feeling a bit desesperated and incompetent, as I'm sure it's
>> not supposed to be this hard to create an appropriate test scenario here
>> — so I was wondering whether anyone has any pointers in avoiding
>> constantly rebuilding the world?
>
> Most of the Guix distribution depends on Perl, and most of the rest
> depends on Python ;)
>
> Try `guix refresh -l perl`.
>
> So, changing the build systems for these languages will require you to
> rebuild almost everything.

Thank you for pointing this out.  As I took a walk yesterday evening it
slowly started to dawn on me that something like this might be going on!
It's super useful to have this confirmed as the cause though: it gave me
that satisfying feeling of pieces of a larger puzzle falling into place
in my head :-)

> Changes like this will need to go on the core-updates branch, where we
> make changes to the core of the system.

Makes sense, I will push it to core-updates tonight.

> You could create a separate perl-test-build-system and use it for your
> test packages.

I guess this would mean that 'the world' would still depend on the
unchanged build system, thus being substitutable, and only my test
packages needing to be rebuilt with the changed build system.  That
makes a lot of sense.  Cheers!

With a new sense of clarity,

Alex

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Leo Famulari]

On Fri, Nov 18, 2016 at 09:02:10AM +0100, Alex Sassmannshausen wrote:
> Leo Famulari writes:
> > Changes like this will need to go on the core-updates branch, where we
> > make changes to the core of the system.
> 
> Makes sense, I will push it to core-updates tonight.

To clarify, I didn't read all the patches yet, and I'm not the best
person to review them. I was just trying to explain why it might take a
long time to test this kind of change.

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Leo Famulari writes:

> On Fri, Nov 18, 2016 at 09:02:10AM +0100, Alex Sassmannshausen wrote:
>> Leo Famulari writes:
>> > Changes like this will need to go on the core-updates branch, where we
>> > make changes to the core of the system.
>> 
>> Makes sense, I will push it to core-updates tonight.
>
> To clarify, I didn't read all the patches yet, and I'm not the best
> person to review them. I was just trying to explain why it might take a
> long time to test this kind of change.

OK, sorry, I misunderstood.  Also in light of Hartmut's latest email, I
will hold off so we can reach a plan of action.

Alex

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Hartmut Goebel]

Am 17.11.2016 um 22:15 schrieb Leo Famulari:
> So, changing the build systems for these languages will require you to
> rebuild almost everything.

We now have thee build-systems which are changed: python (branch
python-build-system), cmake (branch staging, a simple change only) and perl.

What about consolidating the efforts and avoid rebuilding the world
three times? (How long will it take? The Lord took seven days for one
world :-)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Ludovic Courtès]

Hi Alex,

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

>> You may try e.g. scons, pip, sphinx, or youtube-dl.
>
> Hartmut was kind enough to suggest the above python applications to test
> this `wrap-language-programs` patch set against, however I have been
> stuck in "Rebuild the world"-hell for the last day and a half setting up
> testcases.
>
> I end up feeling a bit desesperated and incompetent, as I'm sure it's
> not supposed to be this hard to create an appropriate test scenario here
> — so I was wondering whether anyone has any pointers in avoiding
> constantly rebuilding the world?

The patch set you posted modifies (guix build utils), which is a module
every single derivation uses.  That’s why modifying it leads to a full
rebuild.

To test ‘wrap-language-programs’ while avoiding a full rebuild, you
could for instance add that procedure to (guix build
python-build-system), in which case you’d have to rebuild “only” Python
packages (but not Python itself), or you could introduce a new module
and use that selectively.

I hope this clarifies and helps a bit!

Ludo’.

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Alex Sassmannshausen]

Hi Ludo,

Ludovic Courtès writes:

> Hi Alex,
>
> Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:
>
>>> You may try e.g. scons, pip, sphinx, or youtube-dl.
>>
>> Hartmut was kind enough to suggest the above python applications to test
>> this `wrap-language-programs` patch set against, however I have been
>> stuck in "Rebuild the world"-hell for the last day and a half setting up
>> testcases.
>>
>> I end up feeling a bit desesperated and incompetent, as I'm sure it's
>> not supposed to be this hard to create an appropriate test scenario here
>> — so I was wondering whether anyone has any pointers in avoiding
>> constantly rebuilding the world?
>
> The patch set you posted modifies (guix build utils), which is a module
> every single derivation uses.  That’s why modifying it leads to a full
> rebuild.

That too, makes a whole load of sense.  Thank you for explaining this to
me!

> To test ‘wrap-language-programs’ while avoiding a full rebuild, you
> could for instance add that procedure to (guix build
> python-build-system), in which case you’d have to rebuild “only” Python
> packages (but not Python itself), or you could introduce a new module
> and use that selectively.

Right, that seems absolutely clear now with your explanation above :-)

> I hope this clarifies and helps a bit!

Indeed it does, thank you!

Alex

Re: [PATCH 0/3] Add a generalized `wrap-language-programs`

[Ludovic Courtès]

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> I have tested Perl’s wrap phase, and that seems to work fine.  I was
> unfortunately not able to test Python’s wrap phase, as I did not know of an
> appropriate test candidate (I tried to build offlineimap, but this failed
> because Guile@2.013 failed at least one of it’s tests).

Oops.  I’ve fixed the Guile issue in commit
8134dc1f74c2df2d42e1510fd3ac35234a4d89df.

Thanks,
Ludo’.