Update to semver ≥ 7

[guimard]

• Node Version: 10.17.0
• Platform: Linux/Debian
• Compiler: gcc 9
• Module:

node-gyp is not compatible with semver ≥ 7. I found this little fix that seems to fix the problem:

--- 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)

[goddessfreya]

Also affects Arch linux.

[gengjiawen]

Can you send a PR for this ? Thanks.

[guimard]

Done in #2006 😉

[rvagg]

so, we have "semver": "^5.7.1", how is >=7 getting involved and why is this a problem for folks?

[goddessfreya]

Without the patch above people encounter this error: https://gist.github.com/goddessfreya/90babde3d51401c51ef9e3d717bd5f05#file-gistfile1-txt-L181

[vladimiry]

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:

 # Experimental dedup
  rm -r "$pkgdir"/usr/lib/node_modules/$pkgname/node_modules/semver

So if you download the package archive by clicking https://www.archlinux.org/packages/community/any/node-gyp/download/ the /usr/lib/node_modules/node-gyp/node_modules/semver/ folder will be missed there as they seem to be relying on the system-wide version.

[goddessfreya]

@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.

[vladimiry]

apparently they apply this patch to get node-gyp to work:

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.

[cclauss]

Is there are way to do this conditionally so that
const range = semver.Range(this.semverRange) is executed if semver < 7 and const range = new semver.Range(this.semverRange) with new added, is executed if semver >= 7 so that our code continues to support both?

[vladimiry]

By the way, here is the workaround that should be working on any Linux system where the npm v5.2.0+ package is installed:

• Uninstalling the node-gyp package from the system.
• Running npx node-gyp instead of node-gyp. What is the "npx" tool?

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.

[guimard]

so, we have "semver": "^5.7.1", how is >=7 getting involved and why is this a problem for folks?

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,
Xavier

[rvagg]

what on earth ...

  # Experimental dedup
  rm -r "$pkgdir"/usr/lib/node_modules/$pkgname/node_modules/semver

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 semver.Range in pre-7 uses the instanceof trick to make new optional: https://github.com/npm/node-semver/blob/v5.7.1/semver.js#L856-L858

In post-7 it uses an ES class, so there's no way to pull off the instanceof trick: https://github.com/npm/node-semver/blob/master/classes/range.js

But this means that we should be able to just stick a new in and it'll be compatible with the current and future version(s) of semver, for this specific instance. I would normally put my foot down and tell Debian & Arch to suffer from the problems their policies create, but this one seems easy enough.

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 npm directly to install them, npm install node-gyp -g is easy.