-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Update to semver ≥ 7 #2005
Comments
Also affects Arch linux. |
Can you send a PR for this ? Thanks. |
Done in #2006 😉 |
so, we have |
Without the patch above people encounter this error: https://gist.github.com/goddessfreya/90babde3d51401c51ef9e3d717bd5f05#file-gistfile1-txt-L181 |
This seems to be Arch Linux issue. They for some reason decided not to respect the npm version defined in package.json as @rvagg pointed out but use a system-wide installed semver package, see https://git.archlinux.org/svntogit/community.git/tree/trunk/PKGBUILD?h=packages/node-gyp#n28:
So if you download the package archive by clicking https://www.archlinux.org/packages/community/any/node-gyp/download/ the |
@vladimiry That explanation does not explain why it is also broken on Debian. I checked the Debian experimental package and apparently they apply this patch to get node-gyp to work: Description: fix for semver 7
Author: Xavier Guimard <yadd@debian.org>
Forwarded: <no|not-needed|url proving that it has been forwarded>
Last-Update: 2019-12-26
--- a/lib/find-python.js
+++ b/lib/find-python.js
@@ -226,7 +226,7 @@
}
this.addLog(`- version is "${version}"`)
- const range = semver.Range(this.semverRange)
+ const range = new semver.Range(this.semverRange)
var valid = false
try {
valid = range.test(version) This bug obviously effects more than one distribution. |
It's because instead of respecting npm versioning they prefer to maintain a single/system-wide set of npm modules that they put to respository. This is, in my opinion, a vicious way when it comes to installing/using the npm modules. |
Is there are way to do this conditionally so that |
By the way, here is the workaround that should be working on any Linux system where the
And well, it's better not to install/use any npm module from the at least Arch/Debian repositories since they don't respect npm versioning. |
Hi @rvagg, I'm Debian Developer. For security reason, our policy does not accept embedded code. That's why there is only one semver in our distribution. I posted here my patch, published in our package. Cheers, |
Linux package managers 🤦♂ @guimard I know all about these policies, they're outdated and conflict with Node.js' packaging style and these kinds of problems have been popping up for years. This is no surprise and why I always recommend people (a) never install Node.js from their distro, but use https://github.com/nodesource/distributions instead and (b) never ever use packaged Node.js packages from their distro, because these kinds of breakages are inevitable. OK, so In post-7 it uses an ES But this means that we should be able to just stick a Even with this fix it's extremely probable that using node-gyp (and any other bundled Node.js package) from your Linux distro will make more problems show up that we have zero test coverage for and could be extremely risky--and yes @guimard even "security" concerns because they create these unknown edges where we have zero understanding of the interactions between packages outside of the specific ranges that we test for. So anyone reading this -- ditch your distro version of node-gyp and anything else you've installed from their and use |
Thanks for the info. That must be a pain maintaining npm versioning through the manual patches. I personally think it's very hard to impossible to do not being a core developer of all the involved npm modules. Asking as a curiosity matter, do you use some sort of script that scans all the npm packages in the dist repo and then lists all the cases of npm versioning breaking (automated dependencies versioning breaking detection)? |
Hi @vladimiry, for node.js modules packaged in Debian, we never use |
Otherwise system versions are run and trigger nodejs/node-gyp#2005 bug which is already fixed in node-gyp 6.1.0.
Otherwise system versions are run and trigger nodejs/node-gyp#2005 bug which is already fixed in node-gyp 6.1.0.
node-gyp is not compatible with semver ≥ 7. I found this little fix that seems to fix the problem:
The text was updated successfully, but these errors were encountered: