From: Jelle Licht <jlicht@fsfe.org>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: Timothy Sample <samplet@ngyro.com>,
Josselin Poiret <dev@jpoiret.xyz>,
Tobias Geerinckx-Rice <me@tobias.gr>,
Simon Tournier <zimon.toutoune@gmail.com>,
Mathieu Othacehe <othacehe@gnu.org>,
Christopher Baines <mail@cbaines.net>,
Lars-Dominik Braun <lars@6xq.net>,
62375@debbugs.gnu.org, Ricardo Wurmus <rekado@elephly.net>
Subject: [bug#62375] [PATCH] import: Add binary npm importer.
Date: Sat, 08 Apr 2023 20:29:18 +0200 [thread overview]
Message-ID: <871qkurzwh.fsf@fsfe.org> (raw)
In-Reply-To: <87tty4g9kj.fsf@gnu.org>
Ludovic Courtès <ludo@gnu.org> writes:
> Hello Jelle!
>
> jlicht@fsfe.org skribis:
>
>> From: Jelle Licht <jlicht@fsfe.org>
>>
>> * guix/scripts/import.scm: (importers): Add "npm-binary".
>> * guix/import/npm-binary.scm: New file.
>> * guix/scripts/import/npm-binary.scm: New file.
>> * Makefile.am: Add them.
>>
>> Co-authored-by: Timothy Sample <samplet@ngyro.com>
>> Co-authored-by: Lars-Dominik Braun <lars@6xq.net>
>
> Woohoo! I think it’ll be useful to many, even if we know it’s far from
> “ideal” by our standards.
>
> We’ll need doc under “Invoking guix import” and tests in
> ‘tests/npm-binary.scm’, similar to what’s done for the other importers.
> The doc should clarify why it’s called that way and what the limitations
> are.
>
> For tests, I’d recommend mocking the npmjs.org HTTP servers using
> ‘with-http-server’ as in ‘tests/cpan.scm’.
>
> Also, please add docstrings to top-level procedures.
>
>> +;; TODO: Support other registries
>> +(define *registry* "https://registry.npmjs.org")
>> +(define *default-page* "https://www.npmjs.com/package")
>
> The convention currently is more like ‘%registry’.
>
>> +(define (http-error-code arglist)
>> + (match arglist
>> + (('http-error _ _ _ (code)) code)
>> + (_ #f)))
>
> Unused. :-)
>
>> +(define (hash-url url)
>> + "Downloads the resource at URL and computes the base32 hash for it."
>> + (call-with-temporary-output-file
>> + (lambda (temp port)
>> + (begin ((@ (guix import utils) url-fetch) url temp)
>> + (guix-hash-url temp)))))
>
> Maybe something more like: (port-sha256 (http-fetch …)) ?
>
>> +(define (npm-package->package-sexp npm-package)
>> + "Return the `package' s-expression for an NPM-PACKAGE."
>> + (define (new-or-existing-inputs resolved-deps)
>> + (map package-revision->input resolved-deps))
>> +
>> + (match npm-package
>> + (($ <package-revision> name version home-page dependencies dev-dependencies peer-dependencies license description dist)
>
> Please use ‘match-record’ instead and keep lines below 80 chars. :-)
The records generated by `define-json-mapping' through
`define-record-type' seem to not work with `match-record'.
I could define our very own `define-json-mapping*' that works /w
`define-record-type*' instead (and through that, `match-record'), but I
thought I'd ask if you had something else in mind first :).
>> + (let* ((name (npm-name->name name))
>> + (url (dist-tarball dist))
>> + (home-page (if (string? home-page)
>> + home-page
>> + (string-append *default-page* "/" (uri-encode name))))
>> + (synopsis description)
>> + (resolved-deps (map (match-lambda (($ <versioned-package> name version)
>> + (resolve-package name (string->semver-range version)))) (append dependencies peer-dependencies)))
>
> Likewise.
>
> That’s it! Could you send an updated patch?
Will do, once I got some proper test cases set up.
Thanks again for the review!
- Jelle
next prev parent reply other threads:[~2023-04-08 18:30 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 11:25 [bug#62375] [PATCH 0/1] npm binary importer jlicht
2023-03-22 11:27 ` [bug#62375] [PATCH] import: Add binary npm importer jlicht
2023-03-28 15:49 ` Ludovic Courtès
2023-04-08 18:29 ` Jelle Licht [this message]
2023-04-17 21:14 ` [bug#62375] [PATCH 0/1] npm binary importer Ludovic Courtès
2023-06-18 21:03 ` Ludovic Courtès
2023-06-22 9:39 ` Jelle Licht
2024-02-08 0:59 ` Nicolas Graves via Guix-patches via
2024-03-24 14:54 ` [bug#62375] Continue the npm-binary importer Pablo Zamora
2024-03-31 19:57 ` Jelle Licht
2024-03-31 22:03 ` Pablo Zamora
2024-03-31 19:46 ` [bug#62375] [PATCH v2] import: Add binary npm importer jlicht
2024-03-31 20:37 ` [bug#62375] [PATCH v3] " jlicht
2024-04-01 20:41 ` Ludovic Courtès
2024-04-02 14:12 ` Jelle Licht
2024-04-01 22:01 ` [bug#62375] [PATCH 0/1] npm binary importer Jonathan Brielmaier via Guix-patches via
2024-04-02 14:16 ` Jelle Licht
2024-04-02 14:13 ` [bug#62375] [PATCH v4] import: Add binary npm importer jlicht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871qkurzwh.fsf@fsfe.org \
--to=jlicht@fsfe.org \
--cc=62375@debbugs.gnu.org \
--cc=dev@jpoiret.xyz \
--cc=lars@6xq.net \
--cc=ludo@gnu.org \
--cc=mail@cbaines.net \
--cc=me@tobias.gr \
--cc=othacehe@gnu.org \
--cc=rekado@elephly.net \
--cc=samplet@ngyro.com \
--cc=zimon.toutoune@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.