Codecov Report

Merging #751 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #751   +/-   ##
=======================================
  Coverage   96.22%   96.22%           
=======================================
  Files          27       27           
  Lines         874      874           
=======================================
  Hits          841      841           
  Misses         33       33

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27e267f...4871b28. Read the comment docs.

/ping @nodejs/citgm @nodejs/build

Any idea why it didn't fail at https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/nodes=debian8-64? Unfortunately, I can't find a way to confirm that it tested level in that run, but it really should have, so.... ¯\(ツ)/¯

🤷‍♂ Either I'm wrong about glibc being the issue, or there's a difference between the rackspace-debian8-x64-1 and -2 machines, or that run indeed didn't test level.

Any idea why it didn't fail at https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/nodes=debian8-64? Unfortunately, I can't find a way to confirm that it tested level in that run, but it really should have, so.... ¯_(ツ)_/¯

Seems to have passed on that run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/nodes=debian8-64/testReport/(root)/citgm/level_v5_0_1/

equally ¯\_(ツ)_/¯

🤷‍♂ Either I'm wrong about glibc being the issue, or there's a difference between the rackspace-debian8-x64-1 and -2 machines, or that run indeed didn't test level.

Could very well be a difference in machines if pass/fail is consistent depending on where it was run.

Seems to have passed on that run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/nodes=debian8-64/testReport/(root)/citgm/level_v5_0_1/

That one passed because leveldown got compiled from source, didn't use a prebuild. Uhm.

So of the available history it looks like two builds passed and they both ran on test-rackspace-debian8-x64-2 while the failures are on test-rackspace-debian8-x64-1:

Seems to have passed on that run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2019/nodes=debian8-64/testReport/(root)/citgm/level_v5_0_1/

That one passed because leveldown got compiled from source, didn't use a prebuild. Uhm.

So I guess the question is why doesn't the prebuild get used on test-rackspace-debian8-x64-2?

So I guess the question is why doesn't the prebuild get used on test-rackspace-debian8-x64-2?

On npm install, node-gyp-build checks if the prebuild can be loaded. If not it falls back to compilation. It might be that test-rackspace-debian8-x64-1 passes that loading check for some reason and only hits the problematic code on npm test.

Who has more details on these machines?

Short of that, would it be possible to run on both machines again, just to rule out e.g. race issues?

I've done some updates and minor cleanups on both of those machines. The difference in installed packages is here: https://gist.github.com/rvagg/bd20b289a1f3f9f281ece67a622fc065 - it's a diff of dpkg -l from both machines, the -1 machine is first and -2 is second in this diff.

I've had to remove jessie-backports from the apt sources from both. Some of the differences may be explained from backports that existed at the last time each of the machines were updated. What's most interesting is a number of ubuntu packages listed on the -1. I wonder if at some stage we tried to use some ubuntu sources to get upgrades for something, maybe gcc. In which case, we may have a bit of a mixed up state on that machine that's not truly Debian 8. I have no idea why this might cause a rebuild rather than download of binaries. @vweevers do you know what mechanism it's using to determine system compatibility with binaries? What's it inspecting for on the system to determine libc or whatever else it needs?

I'm going to rebuild this server from the original image, I think I can still do that in Rackspace even though it's an old image, and see how I go.

reprovisioned that machine, don't know if it'll have any impact or not.

do you know what mechanism it's using to determine system compatibility with binaries? What's it inspecting for on the system to determine libc or whatever else it needs?

@rvagg It matches these system properties against a list of prebuilt binaries (included in the npm package; there's no separate download step):

$ find prebuilds -type f
prebuilds/android-arm/node.napi.armv7.node
prebuilds/android-arm64/node.napi.armv8.node
prebuilds/darwin-x64/node.napi.node
prebuilds/linux-arm/node.napi.armv7.node
prebuilds/linux-arm64/node.napi.armv8.node
prebuilds/linux-x64/node.napi.musl.node
prebuilds/linux-x64/node.napi.node
prebuilds/win32-x64/node.napi.node

If a match is found (should be linux-x64/node.napi.node) it's tested by essentially doing require('path/to/linux-x64/node.napi.node'). If that throws, we fall back to compilation.

well, that list doesn't include anything I could imagine varying between those two machines, it's all pretty basic - node version, glibc or musl, uv version, that's all that really matters here and it's only node version that's going to vary. When citgm is run against Node master it's going to likely get a NODE_MODULE_VERSION that you don't have prebuilds for so maybe the discrepancy described here is as simple as comparing citgm runs on master vs an existing release line branch?

The NODE_MODULE_VERSION shouldn't matter as leveldown uses N-API.

@vweevers shouldn't matter, but var abi = process.versions.modules is in there, that's NODE_MODULE_VERSION, are you able to bypass it somehow?

shouldn't matter, but var abi = process.versions.modules is in there, that's NODE_MODULE_VERSION, are you able to bypass it somehow?

Yes. node-gyp-build only matches against ABI if a prebuilt binary has e.g. .abi54 in its filename.

IMO we can let go of the question "why doesn't it use the prebuild". There might be a bug somewhere in the tooling, but it's likely not relevant to node core - so we don't have to discuss that here. I'm fairly certain about Debian 8 not being compatible with leveldown prebuilds, and I'm fine with that.

Dare I suggest we just remove debian8 from citgm entirely? It's EOL in less than 12 months.

Dare I suggest we just remove debian8 from citgm entirely? It's EOL in less than 12 months.

SGTM

Dare I suggest we just remove debian8 from citgm entirely? It's EOL in less than 12 months.

SGTM

Done! (We do still test on debian 9, https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2026/.)

This can be closed, then.