GNU bug report logs - #36602
[PATCH] Add node-build-system.

Previous Next

Full log

Message #8 received at 36602 <at> debbugs.gnu.org (full text, mbox):

From: Jelle Licht <jlicht <at> fsfe.org>
To: Julien Lepiller <julien <at> lepiller.eu>, 36602 <at> debbugs.gnu.org
Subject: Re: [bug#36602] [PATCH] Add node-build-system.
Date: Thu, 11 Jul 2019 20:25:06 +0200

Hello Julien,

Julien Lepiller <julien <at> lepiller.eu> writes:

> Hi Guix!
>
> This patch adds a node-build-system. I wasn't sure if it was ready yet,
> but I think, since I didn't change it in the last months, that it might
> actually be :)
>
> The patch was initially made by Jelle Licht, and I improved a bit on
> it. Note that packages built with this build system will embed symlinks
> to their dependencies, but not devDependencies (build-only
> dependencies) according to the information in the metadata. Each
> package is installed in lib/node_modules/package-name and symlinks are
> added to lib/node_modules/package-name/node_modules. This allows us to
> use only inputs instead of propagated inputs. Executables are installed
> in bin according to metadata, and they should work even if called
> directly from their store path.

I am probably a bit of a hypocrite for the following nitpicks, as I am
quite sure I was the one that introduced pretty much all of them them,
so I offer my apologies in advance :-).

> From 38158940be0ef4780cdbb553cfa039d21fcdda9b Mon Sep 17 00:00:00 2001
> From: Jelle Licht <jlicht <at> fsfe.org>
> Date: Tue, 23 Aug 2016 05:23:55 +0200
> Subject: [PATCH] build: Add node-build-system.
>
> * guix/build/node-build-system.scm: New file.
> * guix/build-system/node.scm: New file.
> * guix/build/json.scm: New file.
> * doc/guix.texi: Document it.
> * Makefile.am: Added new files.
>
> Co-Authored-By: Julien Lepiller <julien <at> lepiller.eu>
> ---
>  Makefile.am                      |   2 +
>  doc/guix.texi                    |  11 +
>  guix/build-system/node.scm       | 139 +++++++++++
>  guix/build/json.scm              | 387 +++++++++++++++++++++++++++++++
>  guix/build/node-build-system.scm | 159 +++++++++++++
>  5 files changed, 698 insertions(+)
>  create mode 100644 guix/build-system/node.scm
>  create mode 100644 guix/build/json.scm
>  create mode 100644 guix/build/node-build-system.scm
>
> diff --git a/Makefile.am b/Makefile.am
> index 82eda6042a..38f2d7e690 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -125,6 +125,7 @@ MODULES =					\
>    guix/build-system/guile.scm			\
>    guix/build-system/haskell.scm			\
>    guix/build-system/linux-module.scm		\
> +  guix/build-system/node.scm			\
>    guix/build-system/perl.scm			\
>    guix/build-system/python.scm			\
>    guix/build-system/ocaml.scm			\
> @@ -170,6 +171,7 @@ MODULES =					\
>    guix/build/gnu-build-system.scm		\
>    guix/build/gnu-dist.scm			\
>    guix/build/guile-build-system.scm		\
> +  guix/build/node-build-system.scm		\

We are missing the `json.scm' file in this listing.

> [snip]

> +(define* (node-build store name inputs
> +                     #:key
> +                     (npm-flags ''())
> +                     (global? #f)
I am not quite sure if this is needed. Put another way: would we not
want all package builds to have `global? #t' in Guix?

> +                     (test-target "test")
This one is no longer in use. 

> +                     (tests? #f)
I know that for most modules we will not even be able to run tests, but
it seems silly to disable them by default, as that would hide the issue.

> [snip]

> +(define* (install #:key outputs inputs global? #:allow-other-keys)
> +  "Install the node module to the output store item. MODULENAME defines
> +under which name the module will be installed, GLOBAL? determines whether this
> +is an npm global install."
> +  (let* ((out          (assoc-ref outputs "out"))
> +         (src-dir      (getcwd))
> +         (tgt-dir      (string-append out "/lib"))
> +         (bin-dir      (string-append out "/bin"))
> +         (modulename   (string-append  (assoc-ref (read-package-data) "name")))
> +         (data         (read-package-data))
> +         (bin-conf     (assoc-ref data "bin"))
> +         (dependencies (match (assoc-ref data "dependencies")
> +                         ((@ deps ...) deps)
> +                         (#f #f))))
It might be better to write out most of these
names. I think we could also move `modulename' one line lower, so it can
become `(modulename (assoc-ref data "name"))'.

If you want me to tidy up these things, let me know; I can do it first
thing after the weekend.

Thanks

Jelle

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.