Simplify logic around abi crosswalk

[springmeyer]

Context

When node-pre-gyp started node was at 0.8.x and process.versions.modules had recently been added to node core to allow dynamically knowing the ABI version for a given node version.

node-pre-gyp needs to know this crosswalk in order to allow you to install a module for a given node version different than your host (important for packaging like https://github.com/mapbox/mapbox-studio-classic/blob/89bce13040c0ae3ff7c6c64371a8c428962c8b24/scripts/build-atom.sh#L81-L85). Instead of making you provide the ABI it makes sense to provide your exact node version you are targeting (MAJOR.MINOR.PATCH) and then node-pre-gyp is smart enough to grab the right ABI.

Two constraints existed:

How to generate the crosswalk between node version and ABI and keep it updated?

In an effort to avoid extra network that might result in a build/install failure for node modules using node-pre-
gyp, I committed to storing the generated crosswalk inside a json file in node-pre-gyp.

It was unclear at the time how that crosswalk would evolve. Would each node minor version bump the ABI or would each major version change it? Or would major versions get released that did not change the ABI?

Over time what happened is that:

• During the 0.x series the minor version bumped the ABI during the even series but during the odd series the ABI got bumped even in patch versions. So, a cross walk of exact major.minor.patch was needed to tame the chaos.
• During the iojs phase of 1.x releases the ABI version was bumped during a minor release.
• After the nodejs series started releasing 2.x and above we've seen the ABI only get bumped with new major versions, predictably.

Problem

This crosswalk is hard to maintain. Every time a node release happens (even a point release) it needs to be updated. And modules depending on node-pre-gyp also need to be updated if they use the --target flag and specific a release node version that node-pre-gyp does not know about. Tests in node-pre-gyp assert that it is updated, but this means the node-pre-gyp tests start failing after new releases of node, which is annoying and creates busywork.

Solution

Here is a potential solution that keeps this functionality working but reduces maintenance burden.

• node-pre-gyp is modified to require an extra flag that points to an json file on the on the filesystem.
• That file would be in the format of https://nodejs.org/download/release/index.json
• it would be expected that users needing the --target flag would download https://nodejs.org/download/release/index.json and supply the path to it to node-pre-gyp
• On travis this would be tested such that every travis build would download https://nodejs.org/download/release/index.json and ensure it works to use.

[bruce-one]

Hmm...

My initial reaction to this problem (which I see is in line with what was done previously for the 0.x node builds -- but may not be a good solution): if this version isn't known keep going back through versions until the most recent one (with the same patch (and minor?)) number is found.

Are there any issues with that technique? I'm assuming there's a potential issue around if the api version changes during a patch/minor release, but is that an issue any more? And/or if it is an issue, I'd assume it's only an issue with non-LTS releases, in which case is the user opting in to instability?

Because, wrt to the specific issue that just happened, then just a simple patch that tweaks the 0.10 abi lookup code could be sufficient?

Eg (only looking at patch version - could always cycle through minor as well if that aligns with upstream)

diff --git a/lib/util/versioning.js b/lib/util/versioning.js
index 8ebbe3a..01207f1 100644
--- a/lib/util/versioning.js
+++ b/lib/util/versioning.js
@@ -136,18 +136,15 @@ function get_runtime_abi(runtime, target_version) {
                             break;
                         }
                     }
-                } else if (major === 0) { // node.js
-                    if (target_parts[1] % 2 === 0) { // for stable/even node.js series
-                        // look for the last release that is the same minor release
-                        // e.g. we assume node 0.10.x is ABI compatible with >= 0.10.0
-                        while (--patch > 0) {
-                            var new_node_target = '' + major + '.' + minor + '.' + patch;
-                            if (abi_crosswalk[new_node_target]) {
-                                cross_obj = abi_crosswalk[new_node_target];
-                                console.log('Warning: node-pre-gyp could not find exact match for ' + target_version);
-                                console.log('Warning: but node-pre-gyp successfully choose ' + new_node_target + ' as ABI compatible target');
-                                break;
-                            }
+                }
+                if (!cross_obj) { // node.js
+                    while (--patch >= 0) {
+                        var new_node_target = '' + major + '.' + minor + '.' + patch;
+                        if (abi_crosswalk[new_node_target]) {
+                            cross_obj = abi_crosswalk[new_node_target];
+                            console.log('Warning: node-pre-gyp could not find exact match for ' + target_version);
+                            console.log('Warning: but node-pre-gyp successfully choose ' + new_node_target + ' as ABI compatible target');
+                            break;
                         }
                     }
                 }

Maybe this is just a short term solution whilst the solution you mention above is a more long term one? (Although, I have this nagging feeling I might be reading a little more complexity into your solution than there actually is.) My thought was that it seems in line with some of the existing handling, it's simple, and it's only an issue (leading to busywork) around minor (or major, if minor is abi stable now) jumps.

[springmeyer]

Thanks for the thoughts @bruce-one. Right I'd love to both 1) avoid the need to update the abi_crosswalk.json regularly, and 2) remove all this tedious code

node-pre-gyp/lib/util/versioning.js

Lines 122 to 154 in 737d9cf

	if (major === 1) {
	// look for last release that is the same major version
	// e.g. we assume io.js 1.x is ABI compatible with >= 1.0.0
	while (true) {
	if (minor > 0) --minor;
	if (patch > 0) --patch;
	var new_iojs_target = '' + major + '.' + minor + '.' + patch;
	if (abi_crosswalk[new_iojs_target]) {
	cross_obj = abi_crosswalk[new_iojs_target];
	console.log('Warning: node-pre-gyp could not find exact match for ' + target_version);
	console.log('Warning: but node-pre-gyp successfully choose ' + new_iojs_target + ' as ABI compatible target');
	break;
	}
	if (minor === 0 && patch === 0) {
	break;
	}
	}
	} else if (major === 0) { // node.js
	if (target_parts[1] % 2 === 0) { // for stable/even node.js series
	// look for the last release that is the same minor release
	// e.g. we assume node 0.10.x is ABI compatible with >= 0.10.0
	while (--patch > 0) {
	var new_node_target = '' + major + '.' + minor + '.' + patch;
	if (abi_crosswalk[new_node_target]) {
	cross_obj = abi_crosswalk[new_node_target];
	console.log('Warning: node-pre-gyp could not find exact match for ' + target_version);
	console.log('Warning: but node-pre-gyp successfully choose ' + new_node_target + ' as ABI compatible target');
	break;
	}
	}
	}
	}
	}

. Adding another branch that is customized to the node LTS releases would only address 1 and make 2 worse.

[bruce-one]

Okay, yep... So... Another quick thought.... (Just a bit of a (magic-ish) tweak on your solution - but writing it all out for my understanding :-p )

• Convert to https://nodejs.org/download/release/index.json format (and remove the abi_crosswalk format)
• Bundle that file, occasionally update it (eg (automatically?) on release?)
• Make versioning.js read that format instead of the current format
• When a lookup happens:
  • Check for ~/.node-pre-gyp/abi.json, if not found use the bundled one.
  • If a version is encountered that is unknown (eg 7.4.0), download a new version of that file and save it as ~/.node-pre-gyp/abi.json
  • If it's still not found, fail.
• Add a flag to allow putting abi.json somewhere else, and use that instead if specified.

It adds some async-ness around get_runtime_abi (if the version is unknown and a new abi.json is downloaded) which might be a bit clumsy, but hopefully it's survivable? (Happy to look at how clumsy...)

That would increase the likelihood of it "just working" for people using --target; but at the cost of added complexity. (I actually don't know how many people that would be, so I'm not sure whether it's actually necessary/useful?)

I think what you're suggesting makes sense and is a reasonable trade off, just thought I'd mention a possible alternative if the complexity seems "worth it" :-) (And it came to mind because of the potential of adding a ~/.node-pre-gyp directory for the cache.)

[springmeyer]

@bruce-one I really like your plan, especially moving to reading the https://nodejs.org/download/release/index.json format directly. And I agree that keeping the --target support "just working" has value. Also I think it being able to work without network is also important (solved by looking for ~/.node-pre-gyp/abi.json first).

Sounds like a potential way forward would be to merge your caching PR and then followup with this work once ~/.node-pre-gyp/ is in existence. I will try to find time to kick the tires on #272 more next week. Thanks so much for all the collaboration.