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

Fork of node-gyp-build breaks other packages (npm hangs silently) #584

Closed
kristophjunge opened this issue Aug 27, 2023 · 6 comments · Fixed by #589
Closed

Fork of node-gyp-build breaks other packages (npm hangs silently) #584

kristophjunge opened this issue Aug 27, 2023 · 6 comments · Fixed by #589
Labels

Comments

@kristophjunge
Copy link

With commit be4b5c68092fd5cf629ea83ab07a80f88e561e3b first included in version 6.0.0-beta.13 a custom fork of node-gyp-build was introduced.

The problem is that other packages independent of zeromq.js use node-gyp-build too.
The original node-gyp-build and the fork @aminya/node-gyp-build both register a bin with the same name node-gyp-build.

See in package.json of @aminya/node-gyp-build:

  "bin": {
    "node-gyp-build": "./bin.js",
    "node-gyp-build-optional": "./optional.js",
    "node-gyp-build-test": "./build-test.js"
  },

Symlinks created in .bin after installation.

$ ls -al node_modules/.bin
node-gyp-build-test -> ../@aminya/node-gyp-build/build-test.js
node-gyp-build-optional -> ../@aminya/node-gyp-build/optional.js
node-gyp-build -> ../@aminya/node-gyp-build/bin.js

Now other packages (here examples from my project) cannot install anymore since their install fails with the modified @aminya/node-gyp-build:

├─┬ bufferutil@4.0.7
│ └── node-gyp-build@4.6.1
└─┬ utf-8-validate@6.0.3
  └── node-gyp-build@4.6.1 deduped

The problems manifests as npm install hanging endlessly while showing the log message of the previous installation step (even with log level silly). So its kind of hard to find/debug. If only zeromq.js is installed it works fine, if only the other dependecy is installed it works fine. It also depends on the order in which npm installs the packages which one fails.

Suggested solutions:

  1. Longterm: Please consider removing the fork and using the original again as it is in general not good practice. I guess this is obvious but i understand it was done for reasons.

  2. Quick: A rename of the bin, e.g. node-gyp-build -> aminya-node-gyp-build in package.json should resolve the conflict.
    The install command in zeromq.js could then be changed to

"install": "(shx test -f ./script/build.js || run-s build.js) && cross-env npm_config_build_from_source=true aminya-node-gyp-build",

For now i stick with version <= 6.0.0-beta.11 of zeromq.js.

Thank you for your work btw!

@sangaman
Copy link

Thanks for this summary! I'm pretty sure I've been running into this issue and was confounded, but after reading this it makes sense.

I strongly agree that there should not be a dependency on a personal fork of a widely used package like node-gyp-build. I don't know the reasons of why a node-gyp-fork was used, the PR and commit that started using the fork don't explain why #536, but can the required fixes/changes be merged upstream?

@aminya
Copy link
Member

aminya commented Sep 13, 2023

There is a reason for this fork.
prebuild/node-gyp-build#55

@sangaman
Copy link

There is a reason for this fork. prebuild/node-gyp-build#55

Is it just about logging output for npm commands? That doesn't seem like a good hill to die on. I know I wasted hours dealing with this bug due to the conflict of the package name breaking the install command when other packages rely on node-gyp. And I can't imagine it's worth maintaining a separate fork of node-gyp over log output.

@seydx
Copy link

seydx commented Nov 19, 2023

any updates on this? having the same issue

@elysianmichael
Copy link

continues to be an issue for us...and it's painfully unclear what the issue before finding this

@aminya
Copy link
Member

aminya commented Nov 21, 2023

What's your environment setup?

The fork was created because there were no logs when the prebuild was not used, resulting in many complaints and confusion. There were also critical bugs in node-gyp-build that were fixed in the fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants