Win-version-info prebuildify seems incompatible with cross-platform prebuilds

[pimterry]

I build a Windows package on Linux, by setting npm_config_platform and npm_config_target_platform both to win32.

Until recently this worked fine, but I've just updated win-version-info to v5.0.1 from v3 (i.e. upgrading to the prebuildify release) and this no longer works correctly.

When npm_config_platform is set to win32, skip.js exits with 1, so that the node-gyp-build script runs. That command then builds for Linux though, not Windows, ignoring the npm_config_platform setting, and resulting in an install including a Linux binary that can't be loaded on Windows.

To reproduce, on Linux:

export npm_config_platform=win32

npm install win-version-info

file node_modules/win-version-info/build/Release/VersionInfo.node
# Prints:
# node_modules/win-version-info/build/Release/VersionInfo.node: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=9ae95691ae49675006d9df3d36477aa4fede57be, not stripped

Despite building for Windows, this is an Linux binary. That file command should print something like "PE32+ executable (DLL) (GUI) x86-64, for MS Windows".

I'm currently investigating setting PREBUILDS_ONLY at runtime, which would work around this by shipping a Linux build on Windows and then ignoring it, but it seems like a bit of a hack - it would be preferably to skip the unnecessary build to avoid shipping an unusable binary, and if I ever add another module where I do want to allow non-built binaries to be used then I'll be completely stuck.

Any ideas?

[pimterry]

I think I've found a workaround, though it's undocumented and feels weird. Looking closely at https://github.com/prebuild/node-gyp-build/blob/28b562f593ed2d0f3a1dcca4ee781f36d4b20e21/build-test.js, it seems I can make that succeed (and so skip builds) by setting an env var with the package name - i.e. export WIN_VERSION_INFO=anything skips the build.

The node-gyp-build docs don't mention anything about this though, and I'm surprised that env var doesn't have a name that implies this behaviour at all...

Is this a reasonable solution? In the above repro, this skips the build so that node_modules/win-version-info/build/Release/VersionInfo.node doesn't exist at all, and installs must use the prebuilds (which is totally fine by me).

[vweevers]

To make sure I understand, you previously relied on prebuild-install to download a prebuilt binary for Windows. So the question is not about cross-compiling, correct?

If so, the thing getting in your way is that node-gyp-build tests the win32 prebuilt binary at install time, by loading it with require(). That only works if the platform you're installing for is the platform you're running on. Possible solutions:

1. In node-gyp-build, skip that require() if a new environment variable like PREBUILD_SKIP_COMPAT_CHECK is set (would need a better name)
2. Use npm install --ignore-scripts. I reckon you've considered this, but need scripts for other packages?
3. Your WIN_VERSION_INFO=anything workaround - which it indeed is. In node-gyp-build we can't guarantee it will continue to work because it's outside the intended use case (although we'd likely bump major if and when we make changes to this logic).

[pimterry]

To make sure I understand, you previously relied on prebuild-install to download a prebuilt binary for Windows. So the question is not about cross-compiling, correct?

Yes exactly - I'm cross-building my application overall, but the expectation is that every dependency will be prebuilt, there shouldn't generally be any native compilation. If cross-compiling worked then this build would have succeeded (because on Linux it would've built a Windows-compatible binary, which would then work at runtime) but using the existing correct prebuild is even better.

Use npm install --ignore-scripts. I reckon you've considered this, but need scripts for other packages?

Yes, exactly, this is one dependency of many in a complex codebase, I'm aiming for a focused fix with without other side-effects like that.

In node-gyp-build, skip that require() if a new environment variable like PREBUILD_SKIP_COMPAT_CHECK is set (would need a better name)

I think this would be a good result. A per-package solution is useful here - maybe the env var value could be a list of packages where it should be skipped, comma-separated? Like PREBUILD_SKIP_COMPAT_CHECK=win-version-info,other-package.

An even better result could be changing the check, instead of skipping it completely: instead of testing a full require(), fall back to just checking if a matching prebuilt binary for the arch+platform exists (i.e. just assume it works), and maybe print a warning to make that clear. That way, if you set this variable for a target where there are no prebuilds available at all, and so you know everything will fail even without testing it, then the install fails explicitly.

Maybe that should actually be the default if npm_config_platform !== process.platform? And for arch too. That env var is used by prebuild-install and others, and seems like a common clear signal that you're trying to install packages for a different platform, and so a require() test is never going be accurate.

[vweevers]

It makes sense to skip the require() test if platform or arch doesn't match! Wanna send a PR to node-gyp-build? Also so that we can discuss it with more people. I can't think of any downsides.