Subject: Re: [bug#42338] [PATCH 01/34] guix: import: Add composer importer.

From: Ludovic Courtès <[email-masked]>
Date: Fri, 25 Sep 2020 12:27:01 +0200

Hi Julien,

Julien Lepiller <[email-masked]> skribis:

> From 70b9cb2bb389f3e5f9dcc75a44d7d60c28f997bc Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <[email-masked]>
> Date: Tue, 29 Oct 2019 08:07:38 +0100
> Subject: [PATCH 01/34] guix: import: Add composer importer.
>
> * guix/import/composer.scm: New file.
> * guix/scripts/import/composer.scm: New file.
> * guix/tests/composer.scm: New file.
> * Makefile.am: Add them.
> * guix/scripts/import.scm: Add composer importer.
> * doc/guix.texi (Invoking guix import): Mention it.

[...]

> +@cindex PHP
> +Import metadat from the @uref{https://getcomposer.org/, Composer} package
                ^
Typo.

> +(define* (composer-fetch name #:optional version)
> +  "Return an alist representation of the Composer metadata for the package NAME,
> +or #f on failure."
> +  (let ((package (json-fetch
> +                   (string-append (%composer-base-url) "/p/" name ".json"))))
> +    (if package
> +        (let* ((packages (assoc-ref package "packages"))
> +               (package (or (assoc-ref packages name) package))
> +               (versions (filter
> +                           (lambda (version)
> +                             (and (not (string-contains version "dev"))
> +                                  (not (string-contains version "beta"))))
> +                           (map car package)))
> +               (version (or (if (null? version) #f version)
> +                            (latest-version versions))))
> +          (assoc-ref package version))
> +        #f)))

I think this should directly return a <composer-package> since the all
the callers pass the alist through ‘json->composer-package’.  The idea
is that alists should be converted to records as soon as they enter the
process.

Also it’s weird that ‘package’ above has a “packages” (plural) entry.
Perhaps we’re missing another JSON mapping?

[...]

> +++ b/guix/scripts/import/composer.scm
> @@ -0,0 +1,107 @@
> +;;; GNU Guix --- Functional package management for GNU
> +;;; Copyright © 2015 David Thompson <[email-masked]>
> +;;; Copyright © 2018 Oleg Pykhalov <[email-masked]>

You can preserve these two lines if you think it’s relevant, but I’d
suggest adding one for yourself.

OK to push with changes along these lines.

Thanks for taking the time to convert to ‘define-json-mapping’ and all!

Ludo’.