[PATCH 0/4] Unbundle node dependencies patch series

[PATCH 0/4] Unbundle node dependencies patch series

[Jelle Licht]

These patches allow us to make use of the existing c-ares package, as well as
an unbundled version of http parser.

Jelle Licht (4):
  gnu: node: Add http-parser.
  gnu: node: Add search path specification for 'NODE_PATH'.
  gnu: node: Do not use bundled dependencies.
  gnu: node: Use compression: prefix.

 gnu/packages/node.scm | 53 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 49 insertions(+), 4 deletions(-)

-- 
2.9.3

[PATCH 1/4] gnu: node: Add http-parser.

[Jelle Licht]

* gnu/packages/node.scm (http-parser): New variable.
* gnu/packages/node.scm (define-module): Import gnu packages tls with
  tls: prefix
---
 gnu/packages/node.scm | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 2b27774..545aaaa 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -32,7 +32,42 @@
   #:use-module (gnu packages linux)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages python)
-  #:use-module (gnu packages tls))
+  #:use-module ((gnu packages tls) #:prefix tls:)
+  #:use-module (gnu packages valgrind))
+
+(define-public http-parser
+  (package
+    (name "http-parser")
+    (version "2.7.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://github.com/nodejs/http-parser/archive/v"
+                                  version ".tar.gz"))
+              (sha256 (base32 "1cw6nf8xy4jhib1w0jd2y0gpqjbdasg8b7pkl2k2vpp54k9rlh3h"))))
+    (build-system gnu-build-system)
+    (arguments
+     '(#:make-flags (list "CC=gcc" (string-append "DESTDIR=" (assoc-ref %outputs "out")))
+       #:test-target "test-valgrind"
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (add-before 'build 'patch-makefile
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+              (substitute* '("Makefile")
+                (("/usr/local") ""))))
+         (replace 'build
+           (lambda* (#:key make-flags #:allow-other-keys)
+             (zero? (apply system* "make" "library" make-flags))))
+         )))
+    (inputs '())
+    (native-inputs `(("valgrind" ,valgrind)))
+    (home-page "https://github.com/nodejs/http-parser")
+    (synopsis "HTTP request/response parser for C")
+    (description "HTTP parser is a parser for HTTP messages written in C.  It
+parses both requests and responses.  The parser is designed to be used in
+performance HTTP applications.  It does not make any syscalls nor allocations,
+it does not buffer data, it can be interrupted at anytime.")
+    (license expat)))
 
 (define-public node
   (package
@@ -118,7 +153,7 @@
        ("which" ,which)))
     (inputs
      `(("libuv" ,libuv)
-       ("openssl" ,openssl)
+       ("openssl" ,tls:openssl)
        ("zlib" ,zlib)))
     (synopsis "Evented I/O for V8 JavaScript")
     (description "Node.js is a platform built on Chrome's JavaScript runtime
-- 
2.9.3

Re: [PATCH 1/4] gnu: node: Add http-parser.

[Alex Kost]

Jelle Licht (2016-08-27 14:23 +0300) wrote:

> * gnu/packages/node.scm (http-parser): New variable.
> * gnu/packages/node.scm (define-module): Import gnu packages tls with
>   tls: prefix

These are 2 independent things, so I think it should be 2 separate
patches, but actually why is it needed to prefix tls?

> ---
>  gnu/packages/node.scm | 39 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 37 insertions(+), 2 deletions(-)
>
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 2b27774..545aaaa 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -32,7 +32,42 @@
>    #:use-module (gnu packages linux)
>    #:use-module (gnu packages perl)
>    #:use-module (gnu packages python)
> -  #:use-module (gnu packages tls))
> +  #:use-module ((gnu packages tls) #:prefix tls:)
> +  #:use-module (gnu packages valgrind))
> +
> +(define-public http-parser
> +  (package
> +    (name "http-parser")
> +    (version "2.7.1")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/nodejs/http-parser/archive/v"
> +                                  version ".tar.gz"))

Here the following line is needed:
                 (file-name (string-append name "-" version ".tar.gz"))

Without it the source tarball will have "...-v2.7.1.tar.gz" name, which
is not very understandable.

> +              (sha256 (base32 "1cw6nf8xy4jhib1w0jd2y0gpqjbdasg8b7pkl2k2vpp54k9rlh3h"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:make-flags (list "CC=gcc" (string-append "DESTDIR=" (assoc-ref %outputs "out")))

Does it work if it will be PREFIX instead of DESTDIR?  I forgot the
exact reason, but we try to avoid using DESTDIR directly, and prefer to
set PREFIX instead.

Also please don't make such long lines :-)

> +       #:test-target "test-valgrind"
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (add-before 'build 'patch-makefile
> +           (lambda* (#:key inputs outputs #:allow-other-keys)

Just (lambda _ ...)  Keywords are not needed here

> +              (substitute* '("Makefile")
> +                (("/usr/local") ""))))

Please add #t in the end of this phase: succeeded phases should return
non-false value, and the return value of 'substitute*' is not specified.

> +         (replace 'build
> +           (lambda* (#:key make-flags #:allow-other-keys)
> +             (zero? (apply system* "make" "library" make-flags))))
> +         )))

Please move these lonely parentheses to the previous line.

> +    (inputs '())

This line is not needed as the empty list is the default value.

> +    (native-inputs `(("valgrind" ,valgrind)))
> +    (home-page "https://github.com/nodejs/http-parser")
> +    (synopsis "HTTP request/response parser for C")
> +    (description "HTTP parser is a parser for HTTP messages written in C.  It
> +parses both requests and responses.  The parser is designed to be used in
> +performance HTTP applications.  It does not make any syscalls nor allocations,
> +it does not buffer data, it can be interrupted at anytime.")
> +    (license expat)))
>  
>  (define-public node
>    (package
> @@ -118,7 +153,7 @@
>         ("which" ,which)))
>      (inputs
>       `(("libuv" ,libuv)
> -       ("openssl" ,openssl)
> +       ("openssl" ,tls:openssl)
>         ("zlib" ,zlib)))
>      (synopsis "Evented I/O for V8 JavaScript")
>      (description "Node.js is a platform built on Chrome's JavaScript runtime

-- 
Alex

[PATCH 2/4] gnu: node: Add search path specification for 'NODE_PATH'.

[Jelle Licht]

* gnu/packages/node.scm (node)[native-search-paths]: New field.
---
 gnu/packages/node.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 545aaaa..d1c5e1b 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -151,6 +151,10 @@ it does not buffer data, it can be interrupted at anytime.")
        ("procps" ,procps)
        ("util-linux" ,util-linux)
        ("which" ,which)))
+    (native-search-paths
+     (list (search-path-specification
+            (variable "NODE_PATH")
+            (files '("lib/node_modules")))))
     (inputs
      `(("libuv" ,libuv)
        ("openssl" ,tls:openssl)
-- 
2.9.3

[PATCH 3/4] gnu: node: Do not use bundled dependencies.

[Jelle Licht]

The Node build system was previously building its own copies of
C-ares and http-parser.

* gnu/packages/node.scm (node)[inputs]: Add c-ares and http-parser.
[arguments]: Add configure flags for using system libraries.
---
 gnu/packages/node.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index d1c5e1b..7c020e6 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -25,6 +25,7 @@
   #:use-module (guix derivations)
   #:use-module (guix download)
   #:use-module (guix build-system gnu)
+  #:use-module (gnu packages adns)
   #:use-module (gnu packages base)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages gcc)
@@ -86,6 +87,8 @@ it does not buffer data, it can be interrupted at anytime.")
      '(#:configure-flags '("--shared-openssl"
                            "--shared-zlib"
                            "--shared-libuv"
+                           "--shared-cares"
+                           "--shared-http-parser"
                            "--without-snapshot")
        #:phases
        (modify-phases %standard-phases
@@ -158,7 +161,9 @@ it does not buffer data, it can be interrupted at anytime.")
     (inputs
      `(("libuv" ,libuv)
        ("openssl" ,tls:openssl)
-       ("zlib" ,zlib)))
+       ("zlib" ,compression:zlib)
+       ("http-parser" ,http-parser)
+       ("c-ares" ,c-ares)))
     (synopsis "Evented I/O for V8 JavaScript")
     (description "Node.js is a platform built on Chrome's JavaScript runtime
 for easily building fast, scalable network applications.  Node.js uses an
-- 
2.9.3

Re: [PATCH 3/4] gnu: node: Do not use bundled dependencies.

[Alex Kost]

Jelle Licht (2016-08-27 14:23 +0300) wrote:

> The Node build system was previously building its own copies of
> C-ares and http-parser.
>
> * gnu/packages/node.scm (node)[inputs]: Add c-ares and http-parser.
> [arguments]: Add configure flags for using system libraries.
> ---
>  gnu/packages/node.scm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index d1c5e1b..7c020e6 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -25,6 +25,7 @@
>    #:use-module (guix derivations)
>    #:use-module (guix download)
>    #:use-module (guix build-system gnu)
> +  #:use-module (gnu packages adns)
>    #:use-module (gnu packages base)
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages gcc)
> @@ -86,6 +87,8 @@ it does not buffer data, it can be interrupted at anytime.")
>       '(#:configure-flags '("--shared-openssl"
>                             "--shared-zlib"
>                             "--shared-libuv"
> +                           "--shared-cares"
> +                           "--shared-http-parser"
>                             "--without-snapshot")
>         #:phases
>         (modify-phases %standard-phases
> @@ -158,7 +161,9 @@ it does not buffer data, it can be interrupted at anytime.")
>      (inputs
>       `(("libuv" ,libuv)
>         ("openssl" ,tls:openssl)
> -       ("zlib" ,zlib)))
> +       ("zlib" ,compression:zlib)

This change shouldn't belong this patch: here you use 'compression'
prefix which is introduced by the next patch.  This would leave the git
repo in a broken state on this commit.

> +       ("http-parser" ,http-parser)
> +       ("c-ares" ,c-ares)))
>      (synopsis "Evented I/O for V8 JavaScript")
>      (description "Node.js is a platform built on Chrome's JavaScript runtime
>  for easily building fast, scalable network applications.  Node.js uses an

-- 
Alex

Re: [PATCH 3/4] gnu: node: Do not use bundled dependencies.

[Jelle Licht]

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

2016-08-27 22:38 GMT+02:00 Alex Kost <alezost@gmail.com>:

> Jelle Licht (2016-08-27 14:23 +0300) wrote:
>
> > The Node build system was previously building its own copies of
> > C-ares and http-parser.
> >
> > * gnu/packages/node.scm (node)[inputs]: Add c-ares and http-parser.
> > [arguments]: Add configure flags for using system libraries.
> > ---
> >  gnu/packages/node.scm | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> >
> > diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> > index d1c5e1b..7c020e6 100644
> > --- a/gnu/packages/node.scm
> > +++ b/gnu/packages/node.scm
> > @@ -25,6 +25,7 @@
> >    #:use-module (guix derivations)
> >    #:use-module (guix download)
> >    #:use-module (guix build-system gnu)
> > +  #:use-module (gnu packages adns)
> >    #:use-module (gnu packages base)
> >    #:use-module (gnu packages compression)
> >    #:use-module (gnu packages gcc)
> > @@ -86,6 +87,8 @@ it does not buffer data, it can be interrupted at
> anytime.")
> >       '(#:configure-flags '("--shared-openssl"
> >                             "--shared-zlib"
> >                             "--shared-libuv"
> > +                           "--shared-cares"
> > +                           "--shared-http-parser"
> >                             "--without-snapshot")
> >         #:phases
> >         (modify-phases %standard-phases
> > @@ -158,7 +161,9 @@ it does not buffer data, it can be interrupted at
> anytime.")
> >      (inputs
> >       `(("libuv" ,libuv)
> >         ("openssl" ,tls:openssl)
> > -       ("zlib" ,zlib)))
> > +       ("zlib" ,compression:zlib)
>
> This change shouldn't belong this patch: here you use 'compression'
> prefix which is introduced by the next patch.  This would leave the git
> repo in a broken state on this commit.
>
> > +       ("http-parser" ,http-parser)
> > +       ("c-ares" ,c-ares)))
> >      (synopsis "Evented I/O for V8 JavaScript")
> >      (description "Node.js is a platform built on Chrome's JavaScript
> runtime
> >  for easily building fast, scalable network applications.  Node.js uses
> an
>
> --
> Alex
>

I probably put the wrong things together when rebasing. Should I provide
updated
patches in these threads, or should I just send in a new patch series?

- Jelle

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

Re: [PATCH 3/4] gnu: node: Do not use bundled dependencies.

[Alex Kost]

Jelle Licht (2016-08-28 22:38 +0300) wrote:

> I probably put the wrong things together when rebasing. Should I
> provide updated
> patches in these threads, or should I just send in a new patch
> series?

I think a new patch series is better (especially since there may be
another number of patches).  Also please answer why prefixing
'compression' and 'tls' modules is needed, thanks.

-- 
Alex

[PATCH 4/4] gnu: node: Use compression: prefix.

[Jelle Licht]

* gnu/packages/node.scm (define-module): Import gnu packages compression
  with a prefix
(node): Likewise.
---
 gnu/packages/node.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 7c020e6..351e988 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -27,7 +27,8 @@
   #:use-module (guix build-system gnu)
   #:use-module (gnu packages adns)
   #:use-module (gnu packages base)
-  #:use-module (gnu packages compression)
+  #:use-module ((gnu packages compression) #:prefix compression:)
+  #:use-module (gnu packages curl)
   #:use-module (gnu packages gcc)
   #:use-module (gnu packages libevent)
   #:use-module (gnu packages linux)
-- 
2.9.3

Re: [PATCH 4/4] gnu: node: Use compression: prefix.

[Alex Kost]

Jelle Licht (2016-08-27 14:23 +0300) wrote:

> * gnu/packages/node.scm (define-module): Import gnu packages compression
>   with a prefix
> (node): Likewise.
> ---
>  gnu/packages/node.scm | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 7c020e6..351e988 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -27,7 +27,8 @@
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages adns)
>    #:use-module (gnu packages base)
> -  #:use-module (gnu packages compression)
> +  #:use-module ((gnu packages compression) #:prefix compression:)
> +  #:use-module (gnu packages curl)
>    #:use-module (gnu packages gcc)
>    #:use-module (gnu packages libevent)
>    #:use-module (gnu packages linux)

This patch is incomplete: you introduce 'compression' prefix, but you
use it in the previous patch.

Actually, why is this prefix needed?

Also why did you add curl module if it's not used at all?

-- 
Alex

Re: [PATCH 4/4] gnu: node: Use compression: prefix.

[Ludovic Courtès]

Howdy!

Jelle Licht <jlicht@fsfe.org> skribis:

> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -27,7 +27,8 @@
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages adns)
>    #:use-module (gnu packages base)
> -  #:use-module (gnu packages compression)
> +  #:use-module ((gnu packages compression) #:prefix compression:)
> +  #:use-module (gnu packages curl)

We must not use #:prefix or #:select for (gnu packages …) modules,
because of <http://bugs.gnu.org/15540> (the same applies to patch #1 in
this series.)

Ludo’.