fix: ensure native modules use the correct config #35159
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This works by patching node.h to check that two defines are set using the equivilant of an XNOR operation. One define "ELECTRON_ENSURE_CONFIG_GYPI" is set via common.gypi which is _already_ used to build native modules and has been since the dawn of time. Therefore this define will be set for all native module compilations targetting the Electron runtime. The second define "USING_ELECTRON_CONFIG_GYPI" is only defined when the gypi argument "using_electron_config_gypi" is set to 1 which is only done so via config.gypi. Only new enough versions of node-gyp correctly use the config.gypi file thus resulting in a compilation error on version of node-gyp that are too old.
VerteDinde
added
semver/patch
codebytere
approved these changes
Aug 1, 2022
|
Release Notes Persisted
|
|
I have automatically backported this PR to "20-x-y", please check out #35160 |
schetle
pushed a commit
to schetle/electron
that referenced
this pull request
Nov 3, 2022
* fix: ensure native modules are built with config.gypi This works by patching node.h to check that two defines are set using the equivilant of an XNOR operation. One define "ELECTRON_ENSURE_CONFIG_GYPI" is set via common.gypi which is _already_ used to build native modules and has been since the dawn of time. Therefore this define will be set for all native module compilations targetting the Electron runtime. The second define "USING_ELECTRON_CONFIG_GYPI" is only defined when the gypi argument "using_electron_config_gypi" is set to 1 which is only done so via config.gypi. Only new enough versions of node-gyp correctly use the config.gypi file thus resulting in a compilation error on version of node-gyp that are too old. * chore: fix lint Co-authored-by: Samuel Attard <sattard@salesforce.com>
khalwa
pushed a commit
to solarwindscloud/electron
that referenced
this pull request
Feb 22, 2023
* fix: ensure native modules are built with config.gypi This works by patching node.h to check that two defines are set using the equivilant of an XNOR operation. One define "ELECTRON_ENSURE_CONFIG_GYPI" is set via common.gypi which is _already_ used to build native modules and has been since the dawn of time. Therefore this define will be set for all native module compilations targetting the Electron runtime. The second define "USING_ELECTRON_CONFIG_GYPI" is only defined when the gypi argument "using_electron_config_gypi" is set to 1 which is only done so via config.gypi. Only new enough versions of node-gyp correctly use the config.gypi file thus resulting in a compilation error on version of node-gyp that are too old. * chore: fix lint Co-authored-by: Samuel Attard <sattard@salesforce.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
Addresses #34885
In Electron 20, we modified how the config.gypi file was generated (it is now generated dynamically through a script). As part of that change, we need to ensure that any downstream users building native modules are using a current enough downstream version of node-gyp and electron-rebuild (a version containing at least this PR, e.g. >=8.4.0 for node-gyp and >=3.2.8 for electron-rebuild, particularly if they're using an alpha or beta).
This PR modifies the config.gypi and node files to add a warning to downstream module builders, ensuring they're on a high enough version. If the version is lower, native modules may successfully build, but will crash on unexpected ways to due missing includes.
Checklist
npm testpassesRelease Notes
Notes: Adds a warning for building native modules with node-gyp and electron-rebuild, encouraging users to upgrade to >=8.4.0 for node-gyp and >=3.2.8 for electron-rebuild if using Electron 20 or higher..