Figure out which version to build prebuilts on

[ralphtheninja]

From https://nodejs.org/en/docs/guides/abi-stability/

A given version n of N-API will be available in the major version of Node.js in which it was published, and in all subsequent versions of Node.js, including subsequent major versions.

This tells me that if you build on the minimum possible node version (for leveldown this means v8.6.0) that will cover all future versions. Should we pick this strategy when making prebuilt binaries? At the moment it seems to work well to pick the current node version (v11.4.0 at time of writing) and make it work for earlier versions, but afaik n-api is not guaranteed to be backwards compatible, right?

Input appreciated.

EDIT:

Slightly below above mentioned quote it says:

That is, each version of N-API conveys the same meaning as a minor version in the semver system, meaning that all changes made to N-API will be backwards compatible.

So it seems we might as well pick the latest version of node.

[ralphtheninja]

Note: This hasn't been true from the start though, since n-api in node v8.0.0 has a different api compared to v8.6.0 (e.g. napi_create_error()).

[vweevers]

@gabrielschulhof @mcollina Could you advise us here?

[mcollina]

n-api has exited experimental status a little later in the Node 8 LTS cycle. I would build it with the latest version of older LTS line that it is supported. Currently, latest Node 8.

[vweevers]

Unfortunately, prebuildify --napi always targets latest node:

https://github.com/mafintosh/prebuildify/blob/5fe60fb80635f0945f7ea64b1ba4ab4c8a27d049/bin.js#L27-L34

[ralphtheninja]

@vweevers Maybe we can patch prebuildify to do something like:

if (argv.napi && !targets) {
  targets = [
    abi.supportedTargets.filter(onlyNode).pop(),
    abi.supportedTargets.filter(onlyElectron).pop(),
  ]

  if (targets[0].target === '9.0.0') targets[0].target = '9.6.1'
}

Then we should be able to do prebuildify --target whatever --napi

[vweevers]

I was thinking that too, but from the current code, it seems @mafintosh has different ideas about what the most suitable node version is?

BTW, should we also define an explicit NAPI_VERSION? Which:

restricts the N-API surface to just the functionality that was available in the specified (and earlier) versions

[ralphtheninja]

I was thinking that too, but from the current code, it seems @mafintosh has different ideas about what the most suitable node version is?

Yes.

BTW, should we also define an explicit NAPI_VERSION?

Yeah, we can do this. This should be set to 3 (which is the version used at v8.x LTS)

[vweevers]

if (argv.napi && !targets) {
  targets = [
    abi.supportedTargets.filter(onlyNode).pop(),
    abi.supportedTargets.filter(onlyElectron).pop(),
  ]

  if (targets[0].target === '9.0.0') targets[0].target = '9.6.1'
}

That doesn't cover electron, unless we call prebuildify twice.

[ralphtheninja]

We can do (with the patch)

prebuildify -t 8.14.0 -t electron@3.0.0 --napi

[vweevers]

Ah, of course. 👍

[ralphtheninja]

There's a TODO comment in there as well to use --lts. Maybe we can do something about that?

[ralphtheninja]

On second thought, I think I prefer using -t because then we can do prebuildify -t $(node -v) -t electron@3.0.0 on Travis.

[vweevers]

There's a TODO comment in there as well to use --lts. Maybe we can do something about that?

Question is what --lts means. Oldest LTS, latest LTS, latest LTS that has non-experimental N-API, ..

On second thought, I think I prefer using -t because then we can do prebuildify -t $(node -v) -t electron@3.0.0 on Travis.

Yeah.

[ralphtheninja]

Question is what --lts means. Oldest LTS, latest LTS, latest LTS that has non-experimental N-API, ..

This is why I changed my mind again :smile:

[vweevers]

So, action items:

• [x] Add #define NAPI_VERSION 3
• [x] Make PR to prebuildify
• [x] Patch node-gyp-build to support npm i --build-from-source https://github.com/Level/leveldown/issues/566
• [x] Update our version of prebuildify
• [x] Remove node 6 from Travis and AppVeyor, and make prebuilds on node 8, and add electron
• [x] Remove x86 from AppVeyor?

[ralphtheninja]

https://github.com/mafintosh/prebuildify/pull/17

[ralphtheninja]

@vweevers Should we still build on Current/node on appveyor/travis?

[vweevers]

@vweevers Should we still build on Current/node on appveyor/travis?

No opinion. We will have to update the matrix as node releases come out anyway, because we have to specify the version number on which to prebuild. I'm fine with either current/node or a hardcoded 11.

[ralphtheninja]

@vweevers Closing this.