unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#62375] [PATCH 0/1] npm binary importer
@ 2023-03-22 11:25 jlicht
  2023-03-22 11:27 ` [bug#62375] [PATCH] import: Add binary npm importer jlicht
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: jlicht @ 2023-03-22 11:25 UTC (permalink / raw)
  To: 62375; +Cc: dev, me, zimon.toutoune, othacehe, ludo, mail, rekado,
	Jelle Licht

From: Jelle Licht <jlicht@fsfe.org>

Folks,

Here a revised patch to add the npm binary importer. To give some context,
'binary' here refers to the fact that this downloads archives straight from
the npm registry at https://registry.npmjs.org. Some of these downloaded
archives may not contain the original sources, so unless properly vetted the
output of this importer does not allow one to easily generate package
expressions for inclusion in guix's collection of packages.

It should work as-is for most simple NPM packages. As noted in an inline
comment somewhere, the way both npm and our very own node-build-system treats
peer dependencies may require some manual intervention from time to time to
hook in the right dependency at the right spot. The upside here is that when
this happens, it's either trivial for a human to spot and fix, or wholly
incompatible with our current approach so not fixable without writing a custom
build system to deal with dependency cycles :-).

Please test and review, let us get this merged as it might be useful to many
people for building out their personal channels and/or package expressions.

Special thanks to Timothy Sample and Lars-Dominik Braun for involvement in
realising this, way too long ago.


Jelle Licht (1):
  import: Add binary npm importer.

 Makefile.am                        |   2 +
 guix/import/npm-binary.scm         | 269 +++++++++++++++++++++++++++++
 guix/scripts/import.scm            |   2 +-
 guix/scripts/import/npm-binary.scm | 113 ++++++++++++
 4 files changed, 385 insertions(+), 1 deletion(-)
 create mode 100644 guix/import/npm-binary.scm
 create mode 100644 guix/scripts/import/npm-binary.scm

-- 
2.39.2





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH] import: Add binary npm importer.
  2023-03-22 11:25 [bug#62375] [PATCH 0/1] npm binary importer jlicht
@ 2023-03-22 11:27 ` jlicht
  2023-03-28 15:49   ` Ludovic Courtès
  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
  2 siblings, 1 reply; 9+ messages in thread
From: jlicht @ 2023-03-22 11:27 UTC (permalink / raw)
  To: 62375
  Cc: Timothy Sample, Josselin Poiret, Tobias Geerinckx-Rice,
	Simon Tournier, Mathieu Othacehe, Ludovic Courtès,
	Christopher Baines, Lars-Dominik Braun, Ricardo Wurmus,
	Jelle Licht

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>

---

 Makefile.am                        |   2 +
 guix/import/npm-binary.scm         | 269 +++++++++++++++++++++++++++++
 guix/scripts/import.scm            |   2 +-
 guix/scripts/import/npm-binary.scm | 113 ++++++++++++
 4 files changed, 385 insertions(+), 1 deletion(-)
 create mode 100644 guix/import/npm-binary.scm
 create mode 100644 guix/scripts/import/npm-binary.scm

diff --git a/Makefile.am b/Makefile.am
index 23b939b674..52def58ae2 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -288,6 +288,7 @@ MODULES =					\
   guix/import/kde.scm				\
   guix/import/launchpad.scm   			\
   guix/import/minetest.scm   			\
+  guix/import/npm-binary.scm			\
   guix/import/opam.scm				\
   guix/import/print.scm				\
   guix/import/pypi.scm				\
@@ -339,6 +340,7 @@ MODULES =					\
   guix/scripts/import/hexpm.scm			\
   guix/scripts/import/json.scm  		\
   guix/scripts/import/minetest.scm  		\
+  guix/scripts/import/npm-binary.scm		\
   guix/scripts/import/opam.scm			\
   guix/scripts/import/pypi.scm			\
   guix/scripts/import/stackage.scm		\
diff --git a/guix/import/npm-binary.scm b/guix/import/npm-binary.scm
new file mode 100644
index 0000000000..f9b54263e4
--- /dev/null
+++ b/guix/import/npm-binary.scm
@@ -0,0 +1,269 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2019, 2020 Timothy Sample <samplet@ngyro.com>
+;;; Copyright © 2020, 2023 Jelle Licht <jlicht@fsfe.org>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix import npm-binary)
+  #:use-module (guix import json)
+  #:use-module (guix import utils)
+  #:use-module (guix memoization)
+  #:use-module ((gnu services configuration) #:select (alist?))
+  #:use-module (guix utils)
+  #:use-module (gnu packages)
+  #:use-module (ice-9 match)
+  #:use-module (ice-9 regex)
+  #:use-module (ice-9 receive)
+  #:use-module (json)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
+  #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-41)
+  #:use-module (web client)
+  #:use-module (web response)
+  #:use-module (web uri)
+  #:export (npm-binary-recursive-import
+            npm-binary->guix-package
+            package-json->guix-package
+            make-versioned-package
+            name+version->symbol))
+
+;; Autoload Guile-Semver so we only have a soft dependency.
+(module-autoload! (current-module)
+		  '(semver) '(string->semver semver? semver->string semver=? semver>?))
+(module-autoload! (current-module)
+		  '(semver ranges) '(*semver-range-any* string->semver-range semver-range-contains?))
+
+;; Dist-tags
+(define-json-mapping <dist-tags> make-dist-tags dist-tags?
+  json->dist-tags
+  (latest dist-tags-latest "latest" string->semver))
+
+(define-record-type <versioned-package>
+  (make-versioned-package name version)
+  versioned-package?
+  (name  versioned-package-name)       ;string
+  (version versioned-package-version)) ;string
+
+(define (dependencies->versioned-packages entries)
+  (match entries
+    (((names . versions) ...)
+     (map make-versioned-package names versions))
+    (_ '())))
+
+(define (extract-license license-string)
+  (if (unspecified? license-string)
+      'unspecified!
+      (spdx-string->license license-string)))
+
+(define-json-mapping <dist> make-dist dist?
+  json->dist
+  (tarball dist-tarball))
+
+(define (empty-or-string s)
+  (if (string? s) s ""))
+
+(define-json-mapping <package-revision> make-package-revision package-revision?
+  json->package-revision
+  (name package-revision-name)
+  (version package-revision-version "version" string->semver) ;semver
+  (home-page package-revision-home-page "homepage")           ;string
+  (dependencies package-revision-dependencies "dependencies"  ;list of versioned-package
+                dependencies->versioned-packages)
+  (dev-dependencies package-revision-dev-dependencies         ;list of versioned-package
+                    "devDependencies" dependencies->versioned-packages)
+  (peer-dependencies package-revision-peer-dependencies       ;list of versioned-package
+                    "peerDependencies" dependencies->versioned-packages)
+  (license package-revision-license "license"                 ;license | #f
+           (match-lambda
+             ((? unspecified?) #f)
+             ((? string? str) (spdx-string->license str))
+             ((? alist? alist)
+              (match (assoc "type" alist)
+                ((_ . (? string? type))
+                 (spdx-string->license type))
+                (_ #f)))))
+  (description package-revision-description                   ; string
+               "description" empty-or-string)
+  (dist package-revision-dist "dist" json->dist))             ;dist
+
+(define (versions->package-revisions versions)
+  (match versions
+    (((version . package-spec) ...)
+     (map json->package-revision package-spec))
+    (_ '())))
+
+(define (versions->package-versions versions)
+  (match versions
+    (((version . package-spec) ...)
+     (map string->semver versions))
+    (_ '())))
+
+(define-json-mapping <meta-package> make-meta-package meta-package?
+  json->meta-package
+  (name meta-package-name)                                       ;string
+  (description meta-package-description)                         ;string
+  (dist-tags meta-package-dist-tags "dist-tags" json->dist-tags) ;dist-tags
+  (revisions meta-package-revisions "versions" versions->package-revisions))
+
+;; TODO: Support other registries
+(define *registry* "https://registry.npmjs.org")
+(define *default-page* "https://www.npmjs.com/package")
+
+(define (lookup-meta-package name)
+  (let ((json (json-fetch (string-append *registry* "/" (uri-encode name)))))
+    (and=> json json->meta-package)))
+
+(define lookup-meta-package* (memoize lookup-meta-package))
+
+(define (http-error-code arglist)
+  (match arglist
+    (('http-error _ _ _ (code)) code)
+    (_ #f)))
+
+(define (meta-package-versions meta)
+  (map package-revision-version
+       (meta-package-revisions meta)))
+
+(define (meta-package-latest meta)
+  (and=> (meta-package-dist-tags meta) dist-tags-latest))
+
+(define* (meta-package-package meta #:optional
+                               (version (meta-package-latest meta)))
+  (match version
+    ((? semver?) (find (lambda (revision)
+                         (semver=? version (package-revision-version revision)))
+                       (meta-package-revisions meta)))
+    ((? string?) (meta-package-package meta (string->semver version)))
+    (_ #f)))
+
+(define* (semver-latest svs #:optional (svr *semver-range-any*))
+  (find (cut semver-range-contains? svr <>)
+        (sort svs semver>?)))
+
+(define* (resolve-package name #:optional (svr *semver-range-any*))
+  (let ((meta (lookup-meta-package* name)))
+    (and meta
+         (let* ((version (semver-latest (or (meta-package-versions meta) '()) svr))
+                (pkg (meta-package-package meta version)))
+           pkg))))
+
+\f
+;;;
+;;; Converting packages
+;;;
+
+(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)))))
+
+(define (npm-name->name npm-name)
+  "Return a Guix package name for the npm package with name NPM-NAME."
+  (define (clean name)
+    (string-map (lambda (chr) (if (char=? chr #\/) #\- chr))
+                (string-filter (negate (cut char=? <> #\@)) name)))
+  (guix-name "node-" (clean npm-name)))
+
+(define (name+version->symbol name version)
+  (string->symbol (string-append name "-" version)))
+
+(define (package-revision->symbol package)
+  (let* ((npm-name (package-revision-name package))
+         (version (semver->string (package-revision-version package)))
+         (name (npm-name->name npm-name)))
+    (name+version->symbol name version)))
+
+(define (package-revision->input package)
+  "Return the `inputs' entry for PACKAGE."
+  (let* ((npm-name (package-revision-name package))
+         (name (npm-name->name npm-name)))
+    `(,name
+      (,'unquote ,(package-revision->symbol package)))))
+
+(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)
+     (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)))
+            (peer-names (map versioned-package-name peer-dependencies))
+            ;; lset-difference for treating peer-dependencies as dependencies, which leads to dependency cycles.
+            ;; lset-union for treating them as (ignored) dev-dependencies, which leads to broken packages.
+            (dev-names (lset-union string= (map versioned-package-name dev-dependencies) peer-names))
+            (extra-phases (match dev-names
+                            (() '())
+                            ((dev-names ...)
+                             `((add-after 'patch-dependencies 'delete-dev-dependencies
+                                 (lambda _
+                                   (delete-dependencies '(,@(reverse dev-names))))))))))
+       (values
+        `(package
+           (name ,name)
+           (version ,(semver->string (package-revision-version npm-package)))
+           (source (origin
+                     (method url-fetch)
+                     (uri ,url)
+                     (sha256 (base32 ,(hash-url url)))))
+           (build-system node-build-system)
+           (arguments
+            '(#:tests? #f
+              #:phases (modify-phases %standard-phases
+                         (delete 'build)
+                         ,@extra-phases)))
+           ,@(match dependencies
+               (() '())
+               ((dependencies ...)
+                `((inputs
+                   (,'quasiquote ,(map package-revision->input resolved-deps))))))
+           (home-page ,home-page)
+           (synopsis ,synopsis)
+           (description ,description)
+           (license ,license))
+        (map (match-lambda (($ <package-revision> name version)
+                            (list name (semver->string version))))
+             resolved-deps))))
+    (_ #f)))
+
+\f
+;;;
+;;; Interface
+;;;
+
+(define npm-binary->guix-package
+  (lambda* (name #:key (version *semver-range-any*) #:allow-other-keys)
+    (let* ((svr (match version
+                  ((? string?) (string->semver-range version))
+                  (_ version)))
+           (pkg (resolve-package name svr)))
+      (and=> pkg npm-package->package-sexp))))
+
+(define* (npm-binary-recursive-import package-name #:key version)
+  (recursive-import package-name
+                    #:repo->guix-package (memoize npm-binary->guix-package)
+                    #:version version
+                    #:guix-name npm-name->name))
diff --git a/guix/scripts/import.scm b/guix/scripts/import.scm
index f84a964a53..dccf6488b2 100644
--- a/guix/scripts/import.scm
+++ b/guix/scripts/import.scm
@@ -47,7 +47,7 @@ (define %standard-import-options '())
 
 (define importers '("gnu" "pypi" "cpan" "hackage" "stackage" "egg" "elpa"
                     "gem" "go" "cran" "crate" "texlive" "json" "opam"
-                    "minetest" "elm" "hexpm"))
+                    "minetest" "elm" "hexpm" "npm-binary"))
 
 (define (resolve-importer name)
   (let ((module (resolve-interface
diff --git a/guix/scripts/import/npm-binary.scm b/guix/scripts/import/npm-binary.scm
new file mode 100644
index 0000000000..825c43bbc3
--- /dev/null
+++ b/guix/scripts/import/npm-binary.scm
@@ -0,0 +1,113 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2014 David Thompson <davet@gnu.org>
+;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (guix scripts import npm-binary)
+  #:use-module (guix ui)
+  #:use-module (guix utils)
+  #:use-module (guix scripts)
+  #:use-module (guix import npm-binary)
+  #:use-module (guix scripts import)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
+  #:use-module (srfi srfi-37)
+  #:use-module (srfi srfi-41)
+  #:use-module (ice-9 match)
+  #:use-module (ice-9 format)
+  #:export (guix-import-npm-binary))
+
+\f
+;;;
+;;; Command-line options.
+;;;
+
+(define %default-options
+  '())
+
+(define (show-help)
+  (display (G_ "Usage: guix import npm-binary PACKAGE-NAME [VERSION]
+Import and convert the NPM package PACKAGE-NAME using the
+`npm-build-system' (but without building the package from source)."))
+  (display (G_ "
+  -h, --help             display this help and exit"))
+  (display (G_ "
+  -r, --recursive        import packages recursively"))
+  (display (G_ "
+  -V, --version          display version information and exit"))
+  (newline)
+  (show-bug-report-information))
+
+(define %options
+  ;; Specification of the command-line options.
+  (cons* (option '(#\h "help") #f #f
+                 (lambda args
+                   (show-help)
+                   (exit 0)))
+         (option '(#\V "version") #f #f
+                 (lambda args
+                   (show-version-and-exit "guix import npm-binary")))
+         (option '(#\r "recursive") #f #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'recursive #t result)))
+         %standard-import-options))
+
+\f
+;;;
+;;; Entry point.
+;;;
+
+(define (guix-import-npm-binary . args)
+  (define (parse-options)
+    ;; Return the alist of option values.
+    (args-fold* args %options
+                (lambda (opt name arg result)
+                  (leave (G_ "~A: unrecognized option~%") name))
+                (lambda (arg result)
+                  (alist-cons 'argument arg result))
+                %default-options))
+
+  (let* ((opts (parse-options))
+         (args (filter-map (match-lambda
+                             (('argument . value)
+                              value)
+                             (_ #f))
+                           (reverse opts))))
+    (let loop ((args args))
+      (match args
+        ((package-name version)
+         (if (assoc-ref opts 'recursive)
+             ;; Recursive import
+             (map (match-lambda
+                    ((and ('package ('name name) ('version version) . rest) pkg)
+                     `(define-public ,(name+version->symbol name version)
+                        ,pkg))
+                    (_ #f))
+                  (npm-binary-recursive-import package-name #:version version))
+             ;; Single import
+             (let ((sexp (npm-binary->guix-package package-name #:version version)))
+               (unless sexp
+                 (leave (G_ "failed to download meta-data for package '~a@~a'~%")
+                        package-name version))
+               sexp)))
+        ((package-name)
+         (loop (list package-name "*")))
+        (()
+         (leave (G_ "too few arguments~%")))
+        ((many ...)
+         (leave (G_ "too many arguments~%")))))))
-- 
2.39.2





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH] import: Add binary npm importer.
  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
  0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-03-28 15:49 UTC (permalink / raw)
  To: jlicht
  Cc: Timothy Sample, Josselin Poiret, Tobias Geerinckx-Rice,
	Simon Tournier, Mathieu Othacehe, Christopher Baines,
	Lars-Dominik Braun, 62375, Ricardo Wurmus

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.  :-)

> +     (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?

Thanks,
Ludo’.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH] import: Add binary npm importer.
  2023-03-28 15:49   ` Ludovic Courtès
@ 2023-04-08 18:29     ` Jelle Licht
  2023-04-17 21:14       ` [bug#62375] [PATCH 0/1] npm binary importer Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Jelle Licht @ 2023-04-08 18:29 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Timothy Sample, Josselin Poiret, Tobias Geerinckx-Rice,
	Simon Tournier, Mathieu Othacehe, Christopher Baines,
	Lars-Dominik Braun, 62375, Ricardo Wurmus

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH 0/1] npm binary importer
  2023-04-08 18:29     ` Jelle Licht
@ 2023-04-17 21:14       ` Ludovic Courtès
  2023-06-18 21:03         ` Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-04-17 21:14 UTC (permalink / raw)
  To: Jelle Licht
  Cc: Timothy Sample, Josselin Poiret, Christopher Baines,
	Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, 62375, Ricardo Wurmus

Hi,

Jelle Licht <jlicht@fsfe.org> skribis:

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

[...]

>>> +  (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'.

Oh right, my bad.

The other option is to call record accessors; it would avoid the risk
associated with index-based matches but may be more verbose.  Your call!

Ludo’.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH 0/1] npm binary importer
  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
  0 siblings, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2023-06-18 21:03 UTC (permalink / raw)
  To: Jelle Licht
  Cc: Timothy Sample, Josselin Poiret, Christopher Baines,
	Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, 62375, Ricardo Wurmus

Hey Jelle,

How far are we from merging?  :-)

TIA,
Ludo’.

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

> Hi,
>
> Jelle Licht <jlicht@fsfe.org> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>>> +  (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'.
>
> Oh right, my bad.
>
> The other option is to call record accessors; it would avoid the risk
> associated with index-based matches but may be more verbose.  Your call!
>
> Ludo’.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH 0/1] npm binary importer
  2023-06-18 21:03         ` Ludovic Courtès
@ 2023-06-22  9:39           ` Jelle Licht
  0 siblings, 0 replies; 9+ messages in thread
From: Jelle Licht @ 2023-06-22  9:39 UTC (permalink / raw)
  To: Ludovic Courtès
  Cc: Timothy Sample, Josselin Poiret, Christopher Baines,
	Simon Tournier, Mathieu Othacehe, Tobias Geerinckx-Rice,
	Lars-Dominik Braun, 62375, Ricardo Wurmus


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

> Hey Jelle,
>
> How far are we from merging?  :-)

Hey Ludo,

Thanks for checking in! I haven't had a whole lot of time recently due
to a bunch of (positive) personal things, but I'll reach out soon~ish
with the next iteration.

Cheers,
- Jelle




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] [PATCH 0/1] npm binary importer
  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
@ 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
  2 siblings, 0 replies; 9+ messages in thread
From: Nicolas Graves via Guix-patches via @ 2024-02-08  0:59 UTC (permalink / raw)
  To: jlicht, 62375
  Cc: dev, mail, zimon.toutoune, othacehe, ludo, me, rekado,
	Jelle Licht

On 2023-03-22 12:25, jlicht@fsfe.org wrote:

> From: Jelle Licht <jlicht@fsfe.org>
>
> Folks,
>
> Here a revised patch to add the npm binary importer. To give some context,
> 'binary' here refers to the fact that this downloads archives straight from
> the npm registry at https://registry.npmjs.org. Some of these downloaded
> archives may not contain the original sources, so unless properly vetted the
> output of this importer does not allow one to easily generate package
> expressions for inclusion in guix's collection of packages.
>
> It should work as-is for most simple NPM packages. As noted in an inline
> comment somewhere, the way both npm and our very own node-build-system treats
> peer dependencies may require some manual intervention from time to time to
> hook in the right dependency at the right spot. The upside here is that when
> this happens, it's either trivial for a human to spot and fix, or wholly
> incompatible with our current approach so not fixable without writing a custom
> build system to deal with dependency cycles :-).
>
> Please test and review, let us get this merged as it might be useful to many
> people for building out their personal channels and/or package expressions.
>
> Special thanks to Timothy Sample and Lars-Dominik Braun for involvement in
> realising this, way too long ago.
>
>
> Jelle Licht (1):
>   import: Add binary npm importer.
>
>  Makefile.am                        |   2 +
>  guix/import/npm-binary.scm         | 269 +++++++++++++++++++++++++++++
>  guix/scripts/import.scm            |   2 +-
>  guix/scripts/import/npm-binary.scm | 113 ++++++++++++
>  4 files changed, 385 insertions(+), 1 deletion(-)
>  create mode 100644 guix/import/npm-binary.scm
>  create mode 100644 guix/scripts/import/npm-binary.scm

Hey Jelle,

Just to understand, is there a reason why you didn't went further with
the sucrase approach? 

-- 
Best regards,
Nicolas Graves




^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#62375] Continue the npm-binary importer
  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
  2024-02-08  0:59 ` Nicolas Graves via Guix-patches via
@ 2024-03-24 14:54 ` Pablo Zamora
  2 siblings, 0 replies; 9+ messages in thread
From: Pablo Zamora @ 2024-03-24 14:54 UTC (permalink / raw)
  To: 62375@debbugs.gnu.org

[-- Attachment #1: Type: text/plain, Size: 512 bytes --]

Dear Guix,

I am stuck while trying to define the 'bitwarden' package definition on my personal channel here https://github.com/Pablo12345678901/guix-custom-channel/blob/master/dev/my-bitwarden.scm . I am facing issues due to the npm dependencies.

One of the potential solution would be this importer. I would like to finish writing it and perform the tests.

On which branch should I send git pull request to continue what has been done so far ? This would be my first commit with guix.

Pablo Zamora

[-- Attachment #2: Type: text/html, Size: 2125 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-03-24 20:21 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
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

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).