Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] npm install as root does not run prepare scripts for git dependencies #2062

Closed
gcampax opened this issue Oct 28, 2020 · 3 comments
Closed
Labels
Bug thing that needs fixing Release 6.x work is associated with a specific npm 6 release

Comments

@gcampax
Copy link

gcampax commented Oct 28, 2020

(This might be the same issue as #1287)

Current Behavior:

If you run "npm install --unsafe-perm" (no other parameters) as root, git dependencies do not have their prepare script called. Other dependencies will be installed as normally.
The same problem applies to "npm link --unsafe-perm".

On other hand, running "npm install --unsafe-perm" or "npm install" as regular user does install all dependencies correctly.

Expected Behavior:

Running "npm install --unsafe-perm" as root becomes equivalent to "npm install" as non root, and all dependencies are installed correctly.

Steps To Reproduce:

  1. Make a docker container
  2. Clone a NPM package that has a git dependency
  3. Run "npm install"
  4. Watch the build fail as the dependency is not installed correctly.

This is particularly easy to trigger if for example the package is in TypeScript, which requires a build step.

Environment:

npm 6.14.4 in a docker container (node v10.21.0)

@gcampax gcampax added Bug thing that needs fixing Needs Triage needs review for next steps Release 6.x work is associated with a specific npm 6 release labels Oct 28, 2020
@mefyl
Copy link

mefyl commented Dec 15, 2020

I can reproduce a similar problem, but have a slightly different diagnostic. I'm not sure it's related to --unsafe-perm since it always happens when running as root (eg. containers). More importantly, when looking at the install logs, I think npm might actually call the prepare script since we get:

npm verb prepareGitDep undefined: installing devDeps and running prepare script.

The dist/ directory is eventually missing however, so my hunch is that it is produced at some point, but NPM does not include it in the final result.

Reproduction Dockerfile

FROM alpine

RUN apk add --no-cache git npm nodejs-current
RUN npm install --global [email protected]
RUN npm install --verbose --unsafe-perm --save git+https://github.com/mefyl/v-hotkey.git
RUN cat package-lock.json
RUN ls node_modules/v-hotkey/dist

Yields:

$ docker build .
[...]
Step 6/6 : RUN ls node_modules/v-hotkey/dist
 ---> Running in 0c34dc783b22
ls: node_modules/v-hotkey/dist: No such file or directory

Working Dockerfile as non-root

Note that if running as non-root, it works :

FROM alpine

RUN apk add --no-cache git npm nodejs-current
RUN npm install --global [email protected]
RUN adduser -D mefyl
WORKDIR /home/mefyl
RUN su mefyl -c 'npm install --verbose --unsafe-perm --save git+https://github.com/mefyl/v-hotkey.git'
RUN cat package-lock.json
# We should see a dist/ directory there
RUN ls node_modules/v-hotkey

Yields:

$ docker build .
[...]
Step 8/8 : RUN ls node_modules/v-hotkey/dist
 ---> Running in e1c26fa59c7d
v-hotkey.common.js
v-hotkey.common.js.map
v-hotkey.umd.js
v-hotkey.umd.js.map
v-hotkey.umd.min.js
v-hotkey.umd.min.js.map

@darcyclarke darcyclarke removed the Needs Triage needs review for next steps label Feb 13, 2021
@darcyclarke
Copy link
Contributor

npm v6 is no longer in active development; We will continue to push security releases to v6 at our team's discretion as-per our Support Policy.

If your bug is preproducible on v7, please re-file this issue using our new issue template.

If your issue was a feature request, please consider opening a new RRFC or RFC. If your issue was a question or other idea that was not CLI-specific, consider opening a discussion on our feedback repo

@mefyl
Copy link

mefyl commented Jun 3, 2021

People went through the trouble of reporting the bug, even providing a deterministic reproduction test case. Six months later, it seems you're basically closing it "in case it was fixed in v7 by any chance", and asking those same people to do redo the work to check for you if the bug is still present. We don't use npm anymore, so I'm not going to; the way I see it, blindly closing this is a loss for the maintainers, not the reporters, a missed opportunity to make npm more robust. That's how I see it when I'm on the other end, at least.

My two cents. Happy maintaining and thanks for the work !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Release 6.x work is associated with a specific npm 6 release
Projects
None yet
Development

No branches or pull requests

3 participants