mbox series

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

Message ID cover.1703028229.git.w@wmeyer.eu
Headers
Series guix import cpan improvements |

Message

Wilko Meyer Dec. 19, 2023, 11:42 p.m. UTC 
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

Comments

Ludovic Courtès Dec. 23, 2023, 10:13 a.m. UTC | #1 
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’.
Wilko Meyer Dec. 23, 2023, 11:49 a.m. UTC | #2 
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.
Ludovic Courtès Jan. 8, 2024, 4:33 p.m. UTC | #3 
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’.