[bug#67917] [PATCH 0/2] guix import cpan improvements

[bug#67917] [PATCH 0/2] guix import cpan improvements

[Wilko Meyer]

Hi Guix,

While packaging perl-devel-repl[0] I noticed, that the 'guix import
cpan' output:

- doesn't use 'define-public' to declare the package variable
- doesn't prefix licenses with license: while (if I understand this part
  of our imports in perl.scm right) we import from the licenses module
  with a license: prefix:

  #:use-module ((guix licenses) #:prefix license:)

so I added the prefix where applicable and took care of adding
(define-public package-name ... ) to the sexp.

the diffstat is pretty huge compared to the actual changes as I had to
reindend parts of the package sexp. Let me know, if there's anything I
can improve/that requires improvement in this patch series.

Kind regards,

Wilko Meyer

[0]: https://issues.guix.gnu.org/67879

Wilko Meyer (2):
  import: cpan: Add 'license:' prefix to license matching.
  import: cpan: Add 'define-public' to package definition.

 guix/import/cpan.scm | 76 +++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 37 deletions(-)


base-commit: d987b75618a62c95c030e7ca53e0972e700c4f06
prerequisite-patch-id: 3eb8883867495d9f3b48dc56486e12784f94e935
-- 
2.41.0

[bug#67917] [PATCH 1/2] import: cpan: Add 'license:' prefix to license matching.

[Wilko Meyer]

* guix/import/cpan.scm (string->license): Add 'license:' prefix.

Change-Id: If28622edd49a85fac9d6ee83ea595ef419e180b7
---
 guix/import/cpan.scm | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/guix/import/cpan.scm b/guix/import/cpan.scm
index b87736eef6c..7dd89c02dc7 100644
--- a/guix/import/cpan.scm
+++ b/guix/import/cpan.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2020, 2021, 2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2022 Hartmut Goebel <h.goebel@crazy-compilers.com>
+;;; Copyright © 2023 Wilko Meyer <w@wmeyer.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -103,29 +104,29 @@ (define string->license
   (match-lambda
    ;; List of valid values from https://metacpan.org/pod/CPAN::Meta::Spec.
    ;; Some licenses are excluded based on their absense from (guix licenses).
-   ("agpl_3" 'agpl3)
+   ("agpl_3" 'license:agpl3)
    ;; apache_1_1
-   ("apache_2_0" 'asl2.0)
+   ("apache_2_0" 'license:asl2.0)
    ;; artistic_1
-   ("artistic_2" 'artistic2.0)
-   ("bsd" 'bsd-3)
-   ("freebsd" 'bsd-2)
+   ("artistic_2" 'license:artistic2.0)
+   ("bsd" 'license:bsd-3)
+   ("freebsd" 'license:bsd-2)
    ;; gfdl_1_2
-   ("gfdl_1_3" 'fdl1.3+)
-   ("gpl_1" 'gpl1)
-   ("gpl_2" 'gpl2)
-   ("gpl_3" 'gpl3)
-   ("lgpl_2_1" 'lgpl2.1)
-   ("lgpl_3_0" 'lgpl3)
-   ("mit" 'x11)
+   ("gfdl_1_3" 'license:fdl1.3+)
+   ("gpl_1" 'license:gpl1)
+   ("gpl_2" 'license:gpl2)
+   ("gpl_3" 'license:gpl3)
+   ("lgpl_2_1" 'license:lgpl2.1)
+   ("lgpl_3_0" 'license:lgpl3)
+   ("mit" 'license:x11)
    ;; mozilla_1_0
-   ("mozilla_1_1" 'mpl1.1)
-   ("openssl" 'openssl)
-   ("perl_5" 'perl-license)   ;GPL1+ and Artistic 1
-   ("qpl_1_0" 'qpl)
+   ("mozilla_1_1" 'license:mpl1.1)
+   ("openssl" 'license:openssl)
+   ("perl_5" 'license:perl-license)   ;GPL1+ and Artistic 1
+   ("qpl_1_0" 'license:qpl)
    ;; ssleay
    ;; sun
-   ("zlib" 'zlib)
+   ("zlib" 'license:zlib)
    (#(x) (string->license x))
    (#(lst ...) `(list ,@(map string->license lst)))
    (_ #f)))
-- 
2.41.0

[bug#67917] [PATCH 2/2] import: cpan: Add 'define-public' to package definition.

[Wilko Meyer]

* guix/import/cpan.scm (string->license): Add 'define-public' to package definition.

Change-Id: I9a0bcb53b40ab8f68579e99c0b8a86bbe9905db2
---
 guix/import/cpan.scm | 41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/guix/import/cpan.scm b/guix/import/cpan.scm
index 7dd89c02dc7..d5ac8e0cb90 100644
--- a/guix/import/cpan.scm
+++ b/guix/import/cpan.scm
@@ -288,26 +288,27 @@ (define (cpan-module->sexp release)
   (let ((tarball (with-store store
                    (download-to-store store source-url)))
         (inputs (cpan-module-inputs release)))
-    `(package
-       (name ,(cpan-name->downstream-name name))
-       (version ,version)
-       (source (origin
-                 (method url-fetch)
-                 (uri (string-append ,@(factorize-uri source-url version)))
-                 (sha256
-                  (base32
-                   ,(bytevector->nix-base32-string (file-sha256 tarball))))))
-       (build-system perl-build-system)
-       ,@(maybe-inputs 'native-inputs
-                       (filter (upstream-input-type-predicate 'native)
-                               inputs))
-       ,@(maybe-inputs 'propagated-inputs
-                       (filter (upstream-input-type-predicate 'propagated)
-                               inputs))
-       (home-page ,(cpan-home name))
-       (synopsis ,(cpan-release-abstract release))
-       (description fill-in-yourself!)
-       (license ,(string->license (cpan-release-license release))))))
+    `(define-public ,(string->symbol (cpan-name->downstream-name name))
+       (package
+        (name ,(cpan-name->downstream-name name))
+        (version ,version)
+        (source (origin
+                    (method url-fetch)
+                    (uri (string-append ,@(factorize-uri source-url version)))
+                    (sha256
+                    (base32
+                    ,(bytevector->nix-base32-string (file-sha256 tarball))))))
+        (build-system perl-build-system)
+        ,@(maybe-inputs 'native-inputs
+                        (filter (upstream-input-type-predicate 'native)
+                                inputs))
+        ,@(maybe-inputs 'propagated-inputs
+                        (filter (upstream-input-type-predicate 'propagated)
+                                inputs))
+        (home-page ,(cpan-home name))
+        (synopsis ,(cpan-release-abstract release))
+        (description fill-in-yourself!)
+        (license ,(string->license (cpan-release-license release)))))))
 
 (define (cpan->guix-package module-name)
   "Fetch the metadata for PACKAGE-NAME from metacpan.org, and return the
-- 
2.41.0

[bug#67917] [PATCH 0/2] guix import cpan improvements

[Ludovic Courtès]

Hi,

Wilko Meyer <w@wmeyer.eu> skribis:

> While packaging perl-devel-repl[0] I noticed, that the 'guix import
> cpan' output:
>
> - doesn't use 'define-public' to declare the package variable

I think this doesn’t belong here: right now, ‘define-public’ is added
when using ‘-r’ by ‘package->definition’ in (guix import utils).  This
is shared by most importers so we cannot just change it in a single
importer.

> - doesn't prefix licenses with license: while (if I understand this part
>   of our imports in perl.scm right) we import from the licenses module
>   with a license: prefix:
>
>   #:use-module ((guix licenses) #:prefix license:)

This one I’m not sure, but we should make sure importers are consistent
as well.

WDYT?

Ludo’.

[bug#67917] [PATCH 0/2] guix import cpan improvements

[Wilko Meyer]

Hi Ludo,

Thanks for reviewing this so quickly!

Ludovic Courtès <ludo@gnu.org> writes:

> I think this doesn’t belong here: right now, ‘define-public’ is added
> when using ‘-r’ by ‘package->definition’ in (guix import utils).  This
> is shared by most importers so we cannot just change it in a single
> importer.

This currently seems to be inconsistent among importers. crate and go
always add define-public independent of using '-r', while e.g. egg, gnu,
elpa etc. do not do that. I'll prepare a v2 of this patch series without
this change, as the cpan importer should stay at its default behaviour.

Speaking of '-r', the cpan importer doesn't offer recursively importing
packages. I could add this functionality to the cpan importer in v2 of
this patch series, WDYT?

> This one I’m not sure, but we should make sure importers are consistent
> as well.

Agreed.

-- 
Kind regards,

Wilko Meyer
w@wmeyer.eu

[bug#67917] [PATCH 0/2] guix import cpan improvements

[Ludovic Courtès]

Hi!

Wilko Meyer <w@wmeyer.eu> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> I think this doesn’t belong here: right now, ‘define-public’ is added
>> when using ‘-r’ by ‘package->definition’ in (guix import utils).  This
>> is shared by most importers so we cannot just change it in a single
>> importer.
>
> This currently seems to be inconsistent among importers. crate and go
> always add define-public independent of using '-r', while e.g. egg, gnu,
> elpa etc. do not do that. I'll prepare a v2 of this patch series without
> this change, as the cpan importer should stay at its default behaviour.

Indeed.

> Speaking of '-r', the cpan importer doesn't offer recursively importing
> packages. I could add this functionality to the cpan importer in v2 of
> this patch series, WDYT?

Would be nice!

Ludo’.