Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: generate valid config.gypi #31404

Merged
merged 1 commit into from Oct 14, 2021
Merged

fix: generate valid config.gypi #31404

merged 1 commit into from Oct 14, 2021

Conversation

zcbenz
Copy link
Member

@zcbenz zcbenz commented Oct 13, 2021

Description of Change

Currently Electron generates a malformed config.gypi file in Node headers, which is fine since it is actually ignored by node-gyp when building modules.

However with nodejs/node-gyp#2497, the config.gypi file in Node headers is going to be respected, and with it we will finally be able to get rid of our patches on Node's common.gypi file. And to make Electron's Node headers work correctly with node-gyp, we must generated a valid config.gypi file.

Release Notes

Notes: Generate valid config.gypi file in Node.js headers.

@zcbenz zcbenz added semver/patch backwards-compatible bug fixes target/14-x-y labels Oct 13, 2021
@zcbenz zcbenz requested a review from a team as a code owner October 13, 2021 02:20
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Oct 13, 2021
@ckerr
Copy link
Member

ckerr commented Oct 13, 2021

Currently Electron generates a malformed config.gypi file in Node headers, which is fine since it is actually ignored by node-gyp when building modules.

🥇

Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me.

@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Oct 14, 2021
@jkleinsc
Copy link
Contributor

Merging as CI failure is known flake unrelated to PR change.

@jkleinsc jkleinsc merged commit d1e0b63 into main Oct 14, 2021
@jkleinsc jkleinsc deleted the valid-config-gypi branch October 14, 2021 14:07
@release-clerk
Copy link

release-clerk bot commented Oct 14, 2021

Release Notes Persisted

Generate valid config.gypi file in Node.js headers.

@trop
Copy link
Contributor

trop bot commented Oct 14, 2021

I was unable to backport this PR to "14-x-y" cleanly;
you will need to perform this backport manually.

@trop trop bot removed the target/14-x-y label Oct 14, 2021
@trop
Copy link
Contributor

trop bot commented Oct 14, 2021

I was unable to backport this PR to "15-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 14, 2021

I was unable to backport this PR to "16-x-y" cleanly;
you will need to perform this backport manually.

@trop
Copy link
Contributor

trop bot commented Oct 15, 2021

@zcbenz has manually backported this PR to "16-x-y", please check out #31441

@trop
Copy link
Contributor

trop bot commented Oct 15, 2021

@zcbenz has manually backported this PR to "15-x-y", please check out #31442

@trop
Copy link
Contributor

trop bot commented Oct 15, 2021

@zcbenz has manually backported this PR to "14-x-y", please check out #31443

t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 27, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
t57ser pushed a commit to t57ser/electron that referenced this pull request Oct 29, 2021
@trop trop bot mentioned this pull request Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants