heartsucker / node-deb

Debian packaging for Node.js projects
https://www.npmjs.com/package/node-deb
MIT License
206 stars 54 forks source link

Add dependency to npm when needed #78

Closed ghost closed 7 years ago

ghost commented 7 years ago

When option --install-strategy is set to "npm-install", the install process will use npm command, so a dependency to package "npm" is needed is this case

gigafied commented 7 years ago

npm is included with the nodejs package.

ghost commented 7 years ago

Hi,

It's not included in deb file on raspbian repository : http://mirrordirector.raspbian.org/raspbian/pool/main/n/nodejs/

So when we want install .deb generated by node-deb with option "--install_strategy npm-install" on a raspbian lite environment, the installation fails due to this missing dependency.

heartsucker commented 7 years ago

Heya @smennetrier. Raspbian isn't one of the officially supported OS's. What you can do is add npm via the --package-dependencies CLI option or the .node_deb.dependencies object in package.json. If this is an issue on Debian or Ubuntu, I'll look into a way to incorporate this.

ghost commented 7 years ago

I just tested on clean ubuntu xenial and clean debian stretch through docker, if you install nodejs thanks to command apt-get install nodejs, npm seems not installed.

In your tests, I think that you install nodejs by downloading https://nodejs.org/dist/v6.10.1/node-v6.10.1-linux-x64.tar.xz thanks to a curl command and not from apt-get install.

ghost commented 7 years ago

Hi @vidstige

It's not because I need it, it's because postinst use command npm and when you install nodejs through apt-get install, npm is not installed. It's installed only if user use a command like curl -sL https://deb.nodesource.com/setup_6.x | sudo -E bash - to install nodejs.

If user try to install package created from node-deb (with dpkg for example), he must use command apt-get install to install the package sudo and the package nodejs. Why not install the package npm if it's used in the install process ? At the moment, the package created by node-deb forces the user to install nodejs through 'apt-get install'. So the tests that install nodejs through curl command (with npm) don't reproduce the correct scenario.

heartsucker commented 7 years ago

Looking into this, and it looks like this is definitely a problem in Jessie. Also, npm has a lot of dependencies, so whatever we do in building the package needs to try emulate the install script logic to only add npm when absolutely necessary.

screenshot_2017-09-28_11-01-38

Also, it seems that the standard nodejs package in Stretch only provides nodejs, but the special baked nodejs package from their repo provides npm too.

Looking at the contents of the Trusty package, we see these lines:

Conflicts: nodejs-dev, nodejs-legacy, npm
Replaces: nodejs-dev (<= 0.8.22), nodejs-legacy, npm (<= 1.2.14)

Which is to say the solution to this is non-trviial. Because I don't know where most people are installing their nodejs installations from, I can't make an authoritative decision on what the default should be. The current setup seems to not break much, and it provides the flexibility to work around it.

On these grounds, I'm closing this issue, but if you feel strongly about it, please re-open it and continue the discussion.

ghost commented 7 years ago

Hi @heartsucker

I agree with you, so following the same logic, maybe that node-deb should provide a way to not add dependency to nodejs in order to let user choose between "apt-get install nodejs" or installing from curl command. In this case, a warning message can appear if install process do not detect nodejs (or npm).

heartsucker commented 7 years ago

The debs installed from the apt repo contain npm, and as an occasional sys admin, I am writing this tool to cater towards people who use the normal Debian channels. If someone chooses to curl | bash it, then they can alter their control file to fit this use case.

a warning message can appear if install process do not detect nodejs (or npm).

The problem with this that would only work if you package it on the system you deploy to. If I package on machine A and deploy to B, there's no way for A to know whether or not the nodejs package is there. Or at least that's what I'm assuming you're asking.

ghost commented 7 years ago

OK, understood.

Just one last things to try to convince you :-)

You said :

I am writing this tool to cater towards people who use the normal Debian channels.

So node-deb should not install nodejs from curl | bash in the tests in order to reproduce this behavior. And probably to reproduce the issue with npm ;-)

heartsucker commented 7 years ago

I just tested this on Debian Stretch using the official repos from nodejs.org, and the nodejs package provides both node and npm, so while not exactly correct, the curl | base I use for the docker containers is sufficiently close to real work conditions for the integration testing.

ghost commented 7 years ago

The package available on nodejs.org contains maybe both (node and npm) but not the package on the official Debian Stretch repository https://packages.debian.org/stretch/amd64/nodejs/filelist. And it's this package that is installed by command "apt-get install nodejs", command needed because it's a dependency to install package created by node-deb ;-)

heartsucker commented 7 years ago

Yes, sorry for the ambiguity on the word official. There's Debian official and NodeJS official. Because the version in Debian official is so old, I assume most people are using the NodJS official one. I've only worked at one company that used Node, we didn't used Debian official because it didn't have the versions we needed. I assume other people have the same issue.