Open pfrazee opened 5 years ago
Sorry @pfrazee for the lack of response here, I assume you've been able to move beyond this since December. For anyone else searching for this, @MarshallOfSound do you have a suggestion about this? I assume it's not uncommon.
Our docs have a recommendation to override the HOME var to redirect that folder to ~/.electron-gyp
--> https://electronjs.org/docs/tutorial/using-native-node-modules#manually-building-for-electron
If you use electron-rebuild
it will set that for you 👍
HOME=~/.electron-gyp node-gyp
is pretty nasty, perhaps we can fix that for you here and introduce an override argument or some way of detecting that it needs to go into ~/.electron-gyp? Surely there's something in process...
that could be used to switch that?
Using a hash of the distUrl or using the ~/${runtime}-gyp
value would probably work. node-gyp when building for electron still runs in the users locally node so we have to figure out which dir to use based solely on arguments
Reopened to see if we can come up with a solid solution for this to prevent the need for hacks like HOME
modification. (Prompted by #1807).
@rvagg How would you feel about changing devDir
from node-gyp
to node-gyp/${runtime}
? Not sure if that's considered breaking as it'll just redownload the headers 🤔
Well we just broke it with node-gyp@5 by switching to env-paths to do native "caches" dir, so /Users/user/Library/Caches/node-gyp on macOS, /home/user/.cache/node-gyp on Linux. So I guess we're not above breaking it! Doing a node-gyp@6 isn't out of the question to make this happen and maybe sooner than later would be a good idea.
And, to make it less breaking, we could skip a runtime/
subdirectory if it === node
, but I'm not even sure that's necessary. We've not had complaints yet about v5 breaking for anyone.
Looks like prebuild
is having this issue too: prebuild/prebuild#249.
While the Electron docs suggest using HOME=~/.electron-gyp node-gyp rebuild
to prevent collisions, it seems like the prebuild
team would like node-gyp
to handle this.
(aside: is doing HOME=<path> node-gyp rebuild ...
the same as specifying node-gyp rebuild --devdir=<path> ...
?)
it seems like the
prebuild
team would likenode-gyp
to handle this
To clarify: I expressed that preference, I don't speak for other prebuild
contributors.
Hi @vweevers, apologies for mis-speaking about prebuild
team.
This is happening now for electron 6 if you ever built something for node 6 also.
I pushed a workaround in my prebuildify module that looks like this if anyone is running into the same issues https://github.com/prebuild/prebuildify/commit/c5ab0eb20a0a3f4857ea712be1570d8c8ecdfc65
@rvagg helped multiple people this week with this since people are trying out electron 6, so would be great to fix this in node-gyp itself :)
Do we know if https://github.com/refack/GYP3 exhibits the same behavior?
@cclauss it shouldn't impact it, this logic comes from the JS side. We just need someone to work up a proposal. I'll try and get to it some time but would be appreciative if someone else had the time to throw together a basic proposal at least.
@nodejs/node-gyp Probably semver-major, but what if we cached based on the NODE_MODULE_VERSION instead of Node.js version? There should be no clashes for current releases based on https://github.com/nodejs/node/blob/master/doc/abi_version_registry.json (there may still be clashes for versions prior to the introduction of the ABI version registry).
@richardlau probably a good idea now that we've strongly pinned NODE_MODULE_VERSION to semver-major version; I don't think we really need to support < Node.js 10 at this stage so we might be safe now?
The only problem might be the --target
flag, how do we manage the ability to compile against different versions? This is something that addon authors rely on to do pre-build binaries for different versions of Node.js.
--target
is currently defined as "Node.js version to build for (default is process.version
)". That doesn't allow for Electron (putting the Electron version here is ambigious without some sort of qualifier) or any other runtime that ships a particular version of Node.js with an incompatible NODE_MODULE_VERSION
.
We could:
electron:6.0,0
--target-runtime electron
(default to node
). (I'd prefer this to changing the format of the --target
option)NODE_MODULE_VERSION
being targetted (not especially user friendly)
--target
to be the target NODE_MODULE_VERSION
--target-abi
?) to be the target NODE_MODULE_VERSION
Implementation wise we'd have to do a lookup in node-gyp
based on https://github.com/nodejs/node/blob/master/doc/abi_version_registry.json -- maybe try to fetch the current version but ship a version in node-gyp
as a backup for network restricted environments?
process.versions.modules
is the ABI version fwiw. There is also the node-abi
package that can be used for determining the ABI version of node / electron
yeah, having a recent version of the registry is going to be the biggest problem, and we're going to need special rules for figuring out what to do with certain numbers:
{ "modules": 79, "runtime": "node", "variant": "v8_7.8", "versions": "13" },
{ "modules": 78, "runtime": "node", "variant": "v8_7.7", "versions": "13.0.0-pre" },
{ "modules": 77, "runtime": "node", "variant": "v8_7.6", "versions": "13.0.0-pre" },
We probably exclude *-pre
versions, even though it'd be theoretically possible to find some headers that worked (index.json in nightlies could give us this).
I think I'm in favour of offloading some of the responsibility to users of --target
, we could deprecate --target
and introduce --target-abi
because it's more technically correct for what they're after. Having a central registry makes it (a) easier for users to grok and use and (b) much simpler for us to figure out what they're after.
@MarshallOfSound neither process.versions.modules
or the node-abi package is helpful here. We're shipping a tool that'll get bundled into npm and used for years in that single version without people updating it. We're still dealing with bug reports from versions 3.x and 4.x! So anything we do in this area needs to be at least a little future proof. "Sorry, I don't know what a modules version of 142 is, I only understand <= 80".
When rebuilding a module for Electron 4, the compiles began to fail by saying that
node_api.h
was not found.It appears that the
~/.node-gyp
folder had a4.0.0
subfolder for node v4. Electron v4 was colliding with that node-4 folder. The solution was to delete the ~/.node-gyp/4.0.0` folder and install again.Any ideas on how to handle the collision?