[bug#55030] [PATCH 00/30] gnu: elm: Update to 0.19.1. Add build system & importer.

["Ludovic Courtès" <ludo@gnu.org>]

Philip McGrath <philip@philipmcgrath.com> skribis:

> * gnu/packages/patches/elm-offline-package-registry.scm: New file.
> * gnu/local.mk (dist_patch_DATA): Add it.
> * gnu/packages/elm.scm (elm): Use it.
> * guix/build-system/elm.scm, guix/build/elm-build-system.scm,
> guix/import/elm.scm, guix/scripts/import/elm.scm: New files.
> * guix/scripts/import.scm (importers): Add "elm".

I think the custom would be to add the importer in a separate commit; if
you can do that, that’s great.

Could you add an entry for the importer under “Invoking guix import”,
and one for the build system under “Build Systems” in guix.texi?  You
can follow existing entries as a template.

It would be nice to have tests for the importer.  One way to do that is
like ‘tests/cpan.scm’, which spawns an HTTP server that mimics the real
registry.

> +;; COMMENTARY:

Nitpick: You can make that literally “;;; Commentary:”.   That’s what
(ice-9 documentation) expects.

> +;; CODE:

Likewise: “;;; Code:”.

> +(define elm-package-registry
> +  ;; It is much nicer to fetch this small (< 40 KB gzipped)
> +  ;; file once than to do many HTTP requests.
> +  (mlambda ()
> +    "Fetch the Elm package registry, represented as a vhash mapping package
> +names to lists of available versions, sorted from latest to oldest."
> +    (let ((url "https://package.elm-lang.org/all-packages"))
> +      (cond
> +       ((json-fetch url)
> +        => (lambda (alist)
> +             (fold (lambda (entry vh)
> +                     (match entry
> +                       ((name . vec)
> +                        (vhash-cons name
> +                                    (sort (vector->list vec) version>?)
> +                                    vh))))
> +                   vlist-null
> +                   alist)))
> +       (else
> +        (raise (formatted-message
> +                (G_ "error downloading Elm package registry from ~a")
> +                url)))))))
> +
> +(define (make-elm-package-sexp name version)
> +  "Return two values: the `package' s-expression for the Elm package with the
> +given NAME and VERSION, and a list of Elm packages it depends on."
> +  (define-values (checkout _commit _relation)
> +    ;; Elm requires that packages use this very specific format
> +    (update-cached-checkout (string-append "https://github.com/" name)
> +                            #:ref `(tag . ,version)))
> +  (define info
> +    (call-with-input-file (string-append checkout "/elm.json")
> +      json->scm))
> +  (define (get-deps key)
> +    (cond
> +     ((assoc-ref info key)
> +      => (cut map car <>))
> +     (else
> +      '())))

The way the importer fiddles with alists isn’t pretty IMO.  :-)

How about using ‘define-json-mapping’ (also from Guile-JSON) to “map”
JSON data structures to records?  See how pypi.scm and others do it.
The resulting code should be clearer.

Also, instead of or in addition to memoizing ‘elm-package-registry’,
would it make sense to use ‘http-fetch/cached’ to fetch that file?

Nitpick: Guile has multiple-value truncation, so you can write:

  (define checkout
    (update-cached-checkout …))

I haven’t looked into much detail at the build system, but I’m sure you
know what you’re doing.  :-)

Ludo’.