Skip to content

build: allow Linux distros to build against system shared libraries - #34841

Merged
jkleinsc merged 3 commits into
electron:mainfrom
brjsp:main
Aug 17, 2022
Merged

build: allow Linux distros to build against system shared libraries#34841
jkleinsc merged 3 commits into
electron:mainfrom
brjsp:main

Conversation

@brjsp

@brjsp brjsp commented Jul 7, 2022

Copy link
Copy Markdown
Contributor

Description of Change

Add GN flags that make it easier for downstream packagers to build the embedded nodejs copy in Electron against system libraries instead of vendored copies in third_party/electron_node/deps:

  • cares
  • llhttp
  • nghttp2
  • hdr_histogram

See also chromium's build/linux/unbundle/README for more Electron-relevant information about vendored dependencies.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes

Release Notes

Notes: none

@brjsp
brjsp requested review from a team as code owners July 7, 2022 17:18
@welcome

welcome Bot commented Jul 7, 2022

Copy link
Copy Markdown

💖 Thanks for opening this pull request! 💖

We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix.

Examples of commit messages with semantic prefixes:

  • fix: don't overwrite prevent_default if default wasn't prevented
  • feat: add app.isPackaged() method
  • docs: app.isDefaultProtocolClient is now available on Linux

Things that will help get your PR across the finish line:

  • Follow the JavaScript, C++, and Python coding style.
  • Run npm run lint locally to catch formatting errors earlier.
  • Document any user-facing changes you've made following the documentation styleguide.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

Comment thread patches/node/build_add_gn_build_files.patch Outdated
Comment thread patches/node/build_add_gn_build_files.patch
@brjsp

brjsp commented Jul 11, 2022

Copy link
Copy Markdown
Contributor Author

Do you know why the build fails?

@zcbenz

zcbenz commented Jul 12, 2022

Copy link
Copy Markdown
Contributor

CI is complaining about patches need to be updated, you can apply this patch to fix it:

From dab9ef2c53d220b7442ee2ef5173a7f49207c383 Mon Sep 17 00:00:00 2001
From: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Date: Mon, 11 Jul 2022 10:18:57 +0000
Subject: chore: update patches


diff --git a/patches/node/build_add_gn_build_files.patch b/patches/node/build_add_gn_build_files.patch
index 40fb27d80b71feedae515e88b74d1906b1c4f127..aeed3833e2f0d2e1dc6c9abb9a2371edc5a078b6 100644
--- a/patches/node/build_add_gn_build_files.patch
+++ b/patches/node/build_add_gn_build_files.patch
@@ -7,7 +7,7 @@ This adds GN build files for Node, so we don't have to build with GYP.
 
 diff --git a/BUILD.gn b/BUILD.gn
 new file mode 100644
-index 0000000000000000000000000000000000000000..4afca42d22ee702af50da92aa08c1de897891424
+index 0000000000000000000000000000000000000000..9ab5cee8dd9580ada72652f32b44243dbeae89aa
 --- /dev/null
 +++ b/BUILD.gn
 @@ -0,0 +1,440 @@
-- 
2.34.1

@brjsp

brjsp commented Jul 12, 2022

Copy link
Copy Markdown
Contributor Author

@zcbenz

@VerteDinde

Copy link
Copy Markdown
Member

@brjsp So sorry, would you mind rebasing this branch with main? It looks like our CircleCI run got stuck, and is blocking us from merging this - once that runs, we should be good to go 🙂

Add GN flags to build the embedded nodejs copy against system libraries
instead of vendored copies in `third_party/electron_node/deps`:

* cares
* llhttp
* nghttp2
* hdr_histogram

See also chromium's build/linux/unbundle/README for more
Electron-relevant information about vendored dependencies.
@brjsp

brjsp commented Jul 14, 2022

Copy link
Copy Markdown
Contributor Author

@VerteDinde

@brjsp

brjsp commented Aug 10, 2022

Copy link
Copy Markdown
Contributor Author

@VerteDinde @zcbenz

Comment thread patches/node/build_add_gn_build_files.patch Outdated
@jkleinsc
jkleinsc merged commit 8e4a168 into electron:main Aug 17, 2022
@welcome

welcome Bot commented Aug 17, 2022

Copy link
Copy Markdown

Congrats on merging your first pull request! 🎉🎉🎉

@release-clerk

release-clerk Bot commented Aug 17, 2022

Copy link
Copy Markdown

No Release Notes

schetle pushed a commit to schetle/electron that referenced this pull request Nov 3, 2022
…lectron#34841)

* build: allow Linux distros to build against system shared libraries

Add GN flags to build the embedded nodejs copy against system libraries
instead of vendored copies in `third_party/electron_node/deps`:

* cares
* llhttp
* nghttp2
* hdr_histogram

See also chromium's build/linux/unbundle/README for more
Electron-relevant information about vendored dependencies.

* Update patches/node/build_add_gn_build_files.patch

Co-authored-by: Darshan Sen <[email protected]>

Co-authored-by: Darshan Sen <[email protected]>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
…lectron#34841)

* build: allow Linux distros to build against system shared libraries

Add GN flags to build the embedded nodejs copy against system libraries
instead of vendored copies in `third_party/electron_node/deps`:

* cares
* llhttp
* nghttp2
* hdr_histogram

See also chromium's build/linux/unbundle/README for more
Electron-relevant information about vendored dependencies.

* Update patches/node/build_add_gn_build_files.patch

Co-authored-by: Darshan Sen <[email protected]>

Co-authored-by: Darshan Sen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants