[PATCH 0/4] Perl build system and importer

[PATCH 0/4] Perl build system and importer

[Eric Bavier]

The following patches make some updates to perl-build-system and the cpan
importer.  The first patch is necessary for several Perl modules that I've
encountered which don't use MakeMaker in their build system, which
perl-build-system current assumes, but instead use the newer Module::Build
system.  Rather than creating an entirely new build-system, I found it much
easier to just ammend the current perl-build-system.

Eric Bavier (4):
  build-system/perl: Use Build.PL for builds if present.
  guix: licenses: Add Artistic 2.0 license.
  import: cpan: Use corelist to filter dependencies.
  import: cpan: Sort inputs, use mirror url, adjust licenses.

 doc/guix.texi                    |    7 ++--
 guix/build-system/perl.scm       |    4 ++
 guix/build/perl-build-system.scm |   59 ++++++++++++++++++++++++------
 guix/import/cpan.scm             |   75 ++++++++++++++++++++++++++++----------
 guix/licenses.scm                |    7 +++-
 5 files changed, 117 insertions(+), 35 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] build-system/perl: Use Build.PL for builds if present.

[Eric Bavier]

* guix/build/perl-build-system.scm (configure): Use Build.PL if present.
  (build, check, install): New procedures.
  (%standard-phases): Replace build, check, and install phases.
* guix/build-system/perl (perl-build): Add make-maker? and module-build-flags
  arguments.

squash: aab6f19: Include make-maker? argument.
---
 guix/build-system/perl.scm       |    4 +++
 guix/build/perl-build-system.scm |   59 ++++++++++++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/guix/build-system/perl.scm b/guix/build-system/perl.scm
index c488adb..e0f8643 100644
--- a/guix/build-system/perl.scm
+++ b/guix/build-system/perl.scm
@@ -75,7 +75,9 @@
                      (tests? #t)
                      (parallel-build? #t)
                      (parallel-tests? #t)
+                     (make-maker? #f)
                      (make-maker-flags ''())
+                     (module-build-flags ''())
                      (phases '(@ (guix build perl-build-system)
                                  %standard-phases))
                      (outputs '("out"))
@@ -101,7 +103,9 @@ provides a `Makefile.PL' file as its build system."
                                 source))
                    #:search-paths ',(map search-path-specification->sexp
                                          search-paths)
+                   #:make-maker? ,make-maker?
                    #:make-maker-flags ,make-maker-flags
+                   #:module-build-flags ,module-build-flags
                    #:phases ,phases
                    #:system ,system
                    #:test-target "test"
diff --git a/guix/build/perl-build-system.scm b/guix/build/perl-build-system.scm
index 904daf7..7eb944c 100644
--- a/guix/build/perl-build-system.scm
+++ b/guix/build/perl-build-system.scm
@@ -29,22 +29,57 @@
 ;;
 ;; Code:
 
-(define* (configure #:key outputs (make-maker-flags '())
+(define* (configure #:key outputs make-maker?
+                    (make-maker-flags '()) (module-build-flags '())
                     #:allow-other-keys)
   "Configure the given Perl package."
-  (let ((out (assoc-ref outputs "out")))
-    (if (file-exists? "Makefile.PL")
-        (let ((args `("Makefile.PL" ,(string-append "PREFIX=" out)
-                      "INSTALLDIRS=site" ,@make-maker-flags)))
-          (format #t "running `perl' with arguments ~s~%" args)
-          (zero? (apply system* "perl" args)))
-        (error "no Makefile.PL found"))))
+  (let* ((out (assoc-ref outputs "out"))
+         (args (cond
+                ;; Prefer to use Module::Build unless otherwise told
+                ((and (file-exists? "Build.PL")
+                      (not make-maker?))
+                 `("Build.PL" ,(string-append "--prefix=" out)
+                   "--installdirs=site" ,@module-build-flags))
+                ((file-exists? "Makefile.PL")
+                 `("Makefile.PL" ,(string-append "PREFIX=" out)
+                   "INSTALLDIRS=site" ,@make-maker-flags))
+                (else (error "no Build.PL or Makefile.PL found")))))
+    (format #t "running `perl' with arguments ~s~%" args)
+    (zero? (apply system* "perl" args))))
+
+(define-syntax-rule (define-w/gnu-fallback* (name args ...) body ...)
+  (define* (name args ... #:rest rest)
+    (if (access? "Build" X_OK)
+        (begin body ...)
+        (apply (assoc-ref gnu:%standard-phases 'name) rest))))
+
+(define-w/gnu-fallback* (build)
+  (zero? (system* "./Build")))
+
+(define-w/gnu-fallback* (check #:key target
+                               (tests? (not target)) (test-flags '())
+                               #:allow-other-keys)
+  (if tests?
+      (zero? (apply system* "./Build" "test" test-flags))
+      (begin
+        (format #t "test suite not run~%")
+        #t)))
+
+(define-w/gnu-fallback* (install)
+  (zero? (system* "./Build" "install")))
 
 (define %standard-phases
-  ;; Everything is as with the GNU Build System except for the `configure'
-  ;; phase.
-  (alist-replace 'configure configure
-                 gnu:%standard-phases))
+  ;; Everything is as with the GNU Build System except for the `configure',
+  ;; `build', `check', and `install' phases.
+  (alist-replace
+   'configure configure
+   (alist-replace
+    'build build
+    (alist-replace
+     'check check
+     (alist-replace
+      'install install
+      gnu:%standard-phases)))))
 
 (define* (perl-build #:key inputs (phases %standard-phases)
                      #:allow-other-keys #:rest args)
-- 
1.7.9.5

Re: [PATCH 1/4] build-system/perl: Use Build.PL for builds if present.

[Ludovic Courtès]

Eric Bavier <bavier@member.fsf.org> skribis:

> * guix/build/perl-build-system.scm (configure): Use Build.PL if present.
>   (build, check, install): New procedures.
>   (%standard-phases): Replace build, check, and install phases.
> * guix/build-system/perl (perl-build): Add make-maker? and module-build-flags
>   arguments.
>
> squash: aab6f19: Include make-maker? argument.
   ^^^
Hm?

LGTM.  Could you also update guix.texi before pushing?

Thanks,
Ludo’.

[PATCH 2/4] guix: licenses: Add Artistic 2.0 license.

[Eric Bavier]

* guix/licenses.scm (artistic2.0): New variable.
---
 guix/licenses.scm |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/guix/licenses.scm b/guix/licenses.scm
index 86f3ae4..ef3f446 100644
--- a/guix/licenses.scm
+++ b/guix/licenses.scm
@@ -28,7 +28,7 @@
             cc0
             cddl1.0
             cecill-c
-            clarified-artistic
+            artistic2.0 clarified-artistic
             cpl1.0
             epl1.0
             expat
@@ -129,6 +129,11 @@ which may be a file:// URI pointing the package's tree."
            "http://www.cecill.info/licences/Licence_CeCILL-C_V1-en.html"
            "https://www.gnu.org/licenses/license-list.html#CeCILL"))
 
+(define artistic2.0
+  (license "Artistic License 2.0"
+           "http://www.perlfoundation.org/artistic_license_2_0"
+           "http://www.gnu.org/licenses/license-list.html#ArtisticLicense2"))
+
 (define clarified-artistic
   (license "Clarified Artistic"
            ;; http://directory.fsf.org/wiki/User:Jgay/license-categorization#Clarified_Artistic_License
-- 
1.7.9.5

Re: [PATCH 2/4] guix: licenses: Add Artistic 2.0 license.

[Ludovic Courtès]

Eric Bavier <bavier@member.fsf.org> skribis:

> * guix/licenses.scm (artistic2.0): New variable.

OK!

Ludo'.

[PATCH 3/4] import: cpan: Use corelist to filter dependencies.

[Eric Bavier]

* guix/import/cpan.scm (%corelist): New variable.
  (module->dist-name, core-module?): New procedures.
  (cpan-module->sexp)[convert-inputs]: Use them.  Include "test" dependencies
  in converted inputs.
* doc/guix.texi (Invoking guix import)[cpan]: Mention corelist filtering.
---
 doc/guix.texi        |    7 ++++---
 guix/import/cpan.scm |   50 ++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 7febee4..a14f7af 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -2965,9 +2965,10 @@ guix import pypi itsdangerous
 Import meta-data from @uref{https://www.metacpan.org/, MetaCPAN}.
 Information is taken from the JSON-formatted meta-data provided through
 @uref{https://api.metacpan.org/, MetaCPAN's API} and includes most
-relevant information.  License information should be checked closely.
-Package dependencies are included but may in some cases needlessly
-include core Perl modules.
+relevant information, such as module dependencies.  License information
+should be checked closely.  If Perl is available in the store, then the
+@code{corelist} utility will be used to filter core modules out of the
+list of dependencies.
 
 The command command below imports meta-data for the @code{Acme::Boolean}
 Perl module:
diff --git a/guix/import/cpan.scm b/guix/import/cpan.scm
index 5f4602a..c1b0006 100644
--- a/guix/import/cpan.scm
+++ b/guix/import/cpan.scm
@@ -19,6 +19,8 @@
 (define-module (guix import cpan)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
+  #:use-module ((ice-9 popen) #:select (open-pipe* close-pipe))
+  #:use-module ((ice-9 rdelim) #:select (read-line))
   #:use-module (srfi srfi-1)
   #:use-module (json)
   #:use-module (guix hash)
@@ -27,6 +29,9 @@
   #:use-module ((guix download) #:select (download-to-store))
   #:use-module (guix import utils)
   #:use-module (guix import json)
+  #:use-module (guix packages)
+  #:use-module (guix derivations)
+  #:use-module (gnu packages perl)
   #:export (cpan->guix-package))
 
 ;;; Commentary:
@@ -71,6 +76,14 @@
   "Transform a 'module' name into a 'release' name"
   (regexp-substitute/global #f "::" module 'pre "-" 'post))
 
+(define (module->dist-name module)
+  "Return the base distribution module for a given module.  E.g. the 'ok'
+module is distributed with 'Test::Simple', so (module->dist-name \"ok\") would
+return \"Test-Simple\""
+  (assoc-ref (json-fetch (string-append "http://api.metacpan.org/module/"
+                                        module))
+             "distribution"))
+
 (define (cpan-fetch module)
   "Return an alist representation of the CPAN metadata for the perl module MODULE,
 or #f on failure.  MODULE should be e.g. \"Test::Script\""
@@ -84,6 +97,14 @@ or #f on failure.  MODULE should be e.g. \"Test::Script\""
 (define (cpan-home name)
   (string-append "http://search.cpan.org/dist/" name))
 
+(define %corelist
+  (let* ((perl (with-store store
+                 (derivation->output-path
+                  (package-derivation store perl))))
+         (core (string-append perl "/bin/corelist")))
+    (and (access? core X_OK)
+         core)))
+
 (define (cpan-module->sexp meta)
   "Return the `package' s-expression for a CPAN module from the metadata in
 META."
@@ -98,6 +119,17 @@ META."
   (define version
     (assoc-ref meta "version"))
 
+  (define (core-module? name)
+    (and %corelist
+         (parameterize ((current-error-port (%make-void-port "w")))
+           (let* ((corelist (open-pipe* OPEN_READ %corelist name)))
+             (let loop ((line (read-line corelist)))
+               (if (eof-object? line)
+                   (begin (close-pipe corelist) #f)
+                   (if (string-contains line "first released with perl")
+                       (begin (close-pipe corelist) #t)
+                       (loop (read-line corelist)))))))))
+
   (define (convert-inputs phases)
     ;; Convert phase dependencies into a list of name/variable pairs.
     (match (flatten
@@ -112,15 +144,13 @@ META."
        (delete-duplicates
         ;; Listed dependencies may include core modules.  Filter those out.
         (filter-map (match-lambda
-                     ((or (module . "0") ("perl" . _))
-                      ;; TODO: A stronger test might to run MODULE through
-                      ;; `corelist' from our perl package.  This current test
-                      ;; seems to be only a loose convention.
+                     (("perl" . _)      ;implicit dependency
                       #f)
                      ((module . _)
-                      (let ((name (guix-name (module->name module))))
-                        (list name
-                              (list 'unquote (string->symbol name))))))
+                      (and (not (core-module? module))
+                           (let ((name (guix-name (module->dist-name module))))
+                             (list name
+                                   (list 'unquote (string->symbol name)))))))
                     inputs)))))
 
   (define (maybe-inputs guix-name inputs)
@@ -147,12 +177,12 @@ META."
                    ,(bytevector->nix-base32-string (file-sha256 tarball))))))
        (build-system perl-build-system)
        ,@(maybe-inputs 'native-inputs
-                       ;; "runtime" and "test" may also be needed here.  See
+                       ;; "runtime" may also be needed here.  See
                        ;; https://metacpan.org/pod/CPAN::Meta::Spec#Phases,
                        ;; which says they are required during building.  We
                        ;; have not yet had a need for cross-compiled perl
-                       ;; modules, however, so we leave them out.
-                       (convert-inputs '("configure" "build")))
+                       ;; modules, however, so we leave it out.
+                       (convert-inputs '("configure" "build" "test")))
        ,@(maybe-inputs 'inputs
                        (convert-inputs '("runtime")))
        (home-page ,(string-append "http://search.cpan.org/dist/" name))
-- 
1.7.9.5

Re: [PATCH 3/4] import: cpan: Use corelist to filter dependencies.

[Ludovic Courtès]

Eric Bavier <bavier@member.fsf.org> skribis:

> * guix/import/cpan.scm (%corelist): New variable.
>   (module->dist-name, core-module?): New procedures.
>   (cpan-module->sexp)[convert-inputs]: Use them.  Include "test" dependencies
>   in converted inputs.
> * doc/guix.texi (Invoking guix import)[cpan]: Mention corelist filtering.

[...]

> +(define %corelist
> +  (let* ((perl (with-store store
> +                 (derivation->output-path
> +                  (package-derivation store perl))))
> +         (core (string-append perl "/bin/corelist")))
> +    (and (access? core X_OK)
> +         core)))

What about adding:

  (warning (_ "the 'corelist' command is unavailable, not filtering modules~%"))

?

Or perhaps it would be even better to take it from $PATH, otherwise the
situation could be confusing: even if Perl is installed, the user could
get an “unavailable” warning just because it’s not the exact same Perl.

WDYT?

Thanks,
Ludo’.

Re: [PATCH 3/4] import: cpan: Use corelist to filter dependencies.

[Eric Bavier]

On Sat, 14 Feb 2015 00:15:22 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Eric Bavier <bavier@member.fsf.org> skribis:
> 
> > * guix/import/cpan.scm (%corelist): New variable.
> >   (module->dist-name, core-module?): New procedures.
> >   (cpan-module->sexp)[convert-inputs]: Use them.  Include "test" dependencies
> >   in converted inputs.
> > * doc/guix.texi (Invoking guix import)[cpan]: Mention corelist filtering.
> 
> [...]
> 
> > +(define %corelist
> > +  (let* ((perl (with-store store
> > +                 (derivation->output-path
> > +                  (package-derivation store perl))))
> > +         (core (string-append perl "/bin/corelist")))
> > +    (and (access? core X_OK)
> > +         core)))
> 
> What about adding:
> 
>   (warning (_ "the 'corelist' command is unavailable, not filtering modules~%"))
> 
> ?

I like this idea.

> Or perhaps it would be even better to take it from $PATH, otherwise the
> situation could be confusing: even if Perl is installed, the user could
> get an “unavailable” warning just because it’s not the exact same Perl.
> 
> WDYT?

This might be reasonable if we check first that the perl versions match
up. Otherwise the core modules might differ enough to not be helpful.
Maybe we could try finding 'corelist' in $PATH if the current perl is
not available in the store.  In practice we should usually have the
most up-to-date 'corelist' in the store, if the packager is using `guix
build` to check that the resulting recipe builds, correct?

Thanks for the review,
`~Eric

Re: [PATCH 3/4] import: cpan: Use corelist to filter dependencies.

[Ludovic Courtès]

Eric Bavier <bavier@member.fsf.org> skribis:

> On Sat, 14 Feb 2015 00:15:22 +0100
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Eric Bavier <bavier@member.fsf.org> skribis:
>> 
>> > * guix/import/cpan.scm (%corelist): New variable.
>> >   (module->dist-name, core-module?): New procedures.
>> >   (cpan-module->sexp)[convert-inputs]: Use them.  Include "test" dependencies
>> >   in converted inputs.
>> > * doc/guix.texi (Invoking guix import)[cpan]: Mention corelist filtering.
>> 
>> [...]
>> 
>> > +(define %corelist
>> > +  (let* ((perl (with-store store
>> > +                 (derivation->output-path
>> > +                  (package-derivation store perl))))
>> > +         (core (string-append perl "/bin/corelist")))
>> > +    (and (access? core X_OK)
>> > +         core)))
>> 
>> What about adding:
>> 
>>   (warning (_ "the 'corelist' command is unavailable, not filtering modules~%"))
>> 
>> ?
>
> I like this idea.
>
>> Or perhaps it would be even better to take it from $PATH, otherwise the
>> situation could be confusing: even if Perl is installed, the user could
>> get an “unavailable” warning just because it’s not the exact same Perl.
>> 
>> WDYT?
>
> This might be reasonable if we check first that the perl versions match
> up. Otherwise the core modules might differ enough to not be helpful.
> Maybe we could try finding 'corelist' in $PATH if the current perl is
> not available in the store.  In practice we should usually have the
> most up-to-date 'corelist' in the store, if the packager is using `guix
> build` to check that the resulting recipe builds, correct?

That’s right, so perhaps my suggestion does not make much sense.

Thanks,
Ludo’.

[PATCH 4/4] import: cpan: Sort inputs, use mirror url, adjust licenses.

[Eric Bavier]

* guix/import/cpan.scm (string->license): Add artistic2.0.  Use
  '(package-license perl) for "perl_5" as in our convention.
  (cpan-module->sexp)[convert-inputs]: Sort returned list of inputs.
  [source-url]: Substitute cpan mirror url.
---
 guix/import/cpan.scm |   35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/guix/import/cpan.scm b/guix/import/cpan.scm
index c1b0006..3dc6eda 100644
--- a/guix/import/cpan.scm
+++ b/guix/import/cpan.scm
@@ -49,7 +49,7 @@
    ;; apache_1_1
    ("apache_2_0" 'asl2.0)
    ;; artistic_1_0
-   ;; artistic_2_0
+   ("artistic_2_0" 'artistic2.0)
    ("bsd" 'bsd-3)
    ("freebsd" 'bsd-2)
    ;; gfdl_1_2
@@ -63,7 +63,7 @@
    ;; mozilla_1_0
    ("mozilla_1_1" 'mpl1.1)
    ("openssl" 'openssl)
-   ("perl_5" 'gpl1+)                    ;and Artistic 1
+   ("perl_5" '(package-license perl))   ;GPL1+ and Artistic 1
    ("qpl_1_0" 'qpl)
    ;; ssleay
    ;; sun
@@ -141,17 +141,22 @@ META."
       (#f
        '())
       ((inputs ...)
-       (delete-duplicates
-        ;; Listed dependencies may include core modules.  Filter those out.
-        (filter-map (match-lambda
-                     (("perl" . _)      ;implicit dependency
-                      #f)
-                     ((module . _)
-                      (and (not (core-module? module))
-                           (let ((name (guix-name (module->dist-name module))))
-                             (list name
-                                   (list 'unquote (string->symbol name)))))))
-                    inputs)))))
+       (sort
+        (delete-duplicates
+         ;; Listed dependencies may include core modules.  Filter those out.
+         (filter-map (match-lambda
+                      (("perl" . _)     ;implicit dependency
+                       #f)
+                      ((module . _)
+                       (and (not (core-module? module))
+                            (let ((name (guix-name (module->dist-name module))))
+                              (list name
+                                    (list 'unquote (string->symbol name)))))))
+                     inputs))
+        (lambda args
+          (match args
+            (((a _ ...) (b _ ...))
+             (string<? a b))))))))
 
   (define (maybe-inputs guix-name inputs)
     (match inputs
@@ -162,7 +167,9 @@ META."
                    (list 'quasiquote inputs))))))
 
   (define source-url
-    (assoc-ref meta "download_url"))
+    (regexp-substitute/global #f "http://cpan.metacpan.org"
+                              (assoc-ref meta "download_url")
+                              'pre "mirror://cpan" 'post))
 
   (let ((tarball (with-store store
                    (download-to-store store source-url))))
-- 
1.7.9.5

Re: [PATCH 4/4] import: cpan: Sort inputs, use mirror url, adjust licenses.

[Ludovic Courtès]

Eric Bavier <bavier@member.fsf.org> skribis:

> * guix/import/cpan.scm (string->license): Add artistic2.0.  Use
>   '(package-license perl) for "perl_5" as in our convention.
>   (cpan-module->sexp)[convert-inputs]: Sort returned list of inputs.
>   [source-url]: Substitute cpan mirror url.

LGTM, but please commit as 3 different patches since the 3 changes are
unrelated.

Thanks,
Ludo’.