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] Regression in 10.4.0 results in failed install #7364
Comments
I can't reproduce this with the latest npm or with ~/D/n/s/gyp $ rm -rf package-lock.json node_modules/;npx npm@10.5.0 install
> gyp@1.0.0 npx
> npm install
added 42 packages, and audited 43 packages in 6s
found 0 vulnerabilities
~/D/n/s/gyp $ npm pkg get dependencies
{
"@azure/msal-node-extensions": "^1.0.15"
} I believe the dependency in question published a new version |
@wraithgar: You can see the regression by attempting to install
|
@mikeharder what about with npm 10 latest? older versions of npm 10 aren't relevant |
@ljharb: Regression was introduced in
|
@mikeharder right, but #7364 (comment) suggests it's a bug in 1.0.14. Try with 1.0.15 of that package. |
@ljharb: Yes, 1.0.15 of that package includes a workaround to this breaking change / regression in npm. But it's still a breaking change / regression you might want to know about. |
It's not a regression, it was a bugfix that exposed broken behavior in some packages. npm has always ran that script if there is no You can see a two year old issue about it where it was happening in npm 7 and 8 at #5234. |
I think the part that is unexpected here is that there isn't a binding file in the published package. It wasn't immediately clear to me that it's making the binding / no binding determination based on the contents of the folder at publish time rather than the contents of the package actually published. Whether this is a bug or a regression ultimately doesn't matter to me but the issue is that it worked in one version and then didn't work in the next with no clear indication of what was wrong. I'm suggesting npm do one of the following:
I don't know all the intricate details of how npm works under the hood but the fact of the matter is that npm first allowed us to publish a package which worked for several years and then overnight broke installation of all versions of that package. And yes, we added a no-op install script in 1.0.15. It feels like a hack tbh but if that's the recommended pattern to address this we'll keep it, I just suggest that it be officially documented. |
This is part of our long-term plan in making npm to stop modifying the manifest at publish time. |
I'd argue that that should have been a prerequisite before making this change but I'm glad to hear it's on the roadmap. Is including a blank install script in the package the recommended best practice to deal with this or is there a better way to ship a package with precompiled binaries? |
Yeah for now having an existing |
Is there an existing issue for this?
This issue exists in the latest npm version
Current Behavior
Starting in 10.4.0 npm is failing to install a native addon package I own because it's attempting to run
node-gyp rebuild
and is not finding thebinding.gyp
file. We ship precompiled binaries and thus have no need to recompile or include the bindings. From searching through previous issues I gather that npm is automatically adding the default install script to some hidden metadata(?) during publish when a binding file exists. I'm not sure if this is expected behavior but if so it probably should have been a breaking change.A couple additional notes:
Expected Behavior
Don't attempt to recompile binaries unless I've explicitly configured my package to do so on install.
Steps To Reproduce
@azure/msal-node-extensions
Environment
; copy and paste output from `npm config ls` here
The text was updated successfully, but these errors were encountered: