Repository navigation
Guide on introducing new dependencies in the codebase #16640
Description
Activity
- addeddocIssues and PRs related to Node.js documentation.Issues and PRs related to Node.js documentation.
on Oct 31, 2017 👍 I was going to take a look at creating a general "Dependencies in Node" guide once #16541 lands, to have a section for introducing dependencies, and then also maintenance guides for each current dependency. There's already a maintenance guide for V8 in master.
Reacted by Daijiro WachiThis will be very helpful when we would like to add tools. Here are a couple of things what I cared when added the new deps.
- Separate the commits into the actual integration and install
node_modules. - Land the integration commit first to make the review a bit easier
- To add
node_modules, I usednpm install --ignore-scriptsto not include the native build modules and addnpm rebuildscript to Makefile.
- Separate the commits into the actual integration and install
In cases (?) where it's something that needs to work at runtime (e.g. because the code will be part of the distributed packages), there's also testing for both *nix and Windows. If the thing in question supports
npm install && npm test, it can usetools/test-npm-packageReacted by Gibson FahnestockTo add more stuff to
deps/(for something like #15566), just unpack the tarball, and add an entry totools/license-builder.sh.-
I would like to hear an expert opinion on licenses when adding tooling to
/tools/. AFAICT that does not necessitate an explicit attribution as it should not be part of the runtime (a.k.a. the final product). -
npmed modules need special attention as they- may need maintaining and updating
- may come with a substantial amount of files (1000s) which comes with a degraded
gitexperiance
-
But we don't just distribute the runtime. As soon as we commit to this repository we've made a copy and are distributing (via GitHub). We only have the one LICENSE (i.e. we don't have one for the runtime binaries and one for the source).
Whether the attribution needs to be explicit depends on the terms of the license of the thing being added.
Hmm..I am not a lawyer but are those license files recursive? I mean we do attribute our dependencies in our own license, so other projects embedding Node.js don't have to attribute all of them again right? So it's gonna like, if our direct dependencies are doing proper licensing, then we don't have to worry about that? Because technically all the npm modules distribute nested node_modules as well, just not via github but via npm.
It's possible I'm missing something here but we do include the LICENSE files in the repo. I don't see any evidence that we need to put their contents into our own LICENSE.
Most of them just state "shall be included" and it is... Even something like ISC just states
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.Reading Apache, GPL, etc. it seems like our main concern here would be mainly with GPL-3.0 which requires explicit mention within the distribution's own license file. So check if anything within node_modules is GPL and add it, I suppose. The rest should be fine as is.
List of currently included GPL, LGPL, etc. licensed modules:
- jschardet
@apapirovski I think the GPL licenses require more than a mention and often have a "viral" component. I agree its important to have a list of licences that would be acceptable or not for inclusion.
Reacted by Richard LauAwesome if someone wants to write this thing, but it does seem like this conversation has stalled so I'm going to close this. Feel free to re-open if you disagree and feel it should not be closed.
We have a guide on introducing new modules, and some guides(including WIP) on maintaining existing dependencies, but still need a guide on introducing one.
cc @jasnell @jkrems @watilde because IIRC you have introduced new dependencies to the code base in the past year, would love to get your help on this.
Refs: #16637 (comment)