Skip to content

build: add GN build files - #47637

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:patch-gn
Nov 11, 2023
Merged

build: add GN build files#47637
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
zcbenz:patch-gn

Conversation

@zcbenz

Copy link
Copy Markdown
Contributor

As per discussion in nodejs/TSC#1353, this PR adds GN build files into Node's repo.

I have provided 2 ways to test the build:

  1. Build with a standalone version of GN.
  2. Build with depot_tools from Chromium.

Both have GitHub actions successfully building for win/mac/linux on arm/arm64/x64/x86.

This PR only adds configurations for building the node target, it does not include configurations for building tests, which I plan to add in a later PR, to make review easier.

After this is merged to Node, I'll upstream my changes in https://github.com/photoionization/node-ci to https://chromium.googlesource.com/v8/node-ci/, which is used by V8 team to test Node.js with latest V8. I'll also setup a nightly build of Node with gn in https://github.com/photoionization/node_with_gn, so we can find out early when the gn build is broken.

/cc @codebytere@victorgomes@joyeecheung@targos

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/http2
  • @nodejs/net
  • @nodejs/single-executable
  • @nodejs/url
  • @nodejs/wasi

@nodejs-github-botnodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 20, 2023
Comment threadtools/search_files.py Outdated
Comment threaddeps/ada/BUILD.gn Outdated
@gengjiawen

Copy link
Copy Markdown
Member

Maybe add gn setup to https://github.com/nodejs/node/blob/main/.gitpod.yml

With GN it is possible to reuse Chromium's sanitizers in Node, a full list of supported sanitizers can be found at: source.chromium.org/chromium/chromium/src/+/main:build/config/sanitizers/sanitizers.gni. They are very helpful tools for debugging various leaks and crashes.

Would be good since Node.js only have ASAN for now.

@targos

Copy link
Copy Markdown
Member

Does this attempt to build node with the same default config as with ./configure --ninja && make ?

@targos

Copy link
Copy Markdown
Member

I have depot_tools from Chromium on my PATH. How can I run the GN build?

@zcbenz

Copy link
Copy Markdown
ContributorAuthor

Maybe add gn setup to https://github.com/nodejs/node/blob/main/.gitpod.yml

I didn't know gitpod, will give it a try.

@zcbenz

Copy link
Copy Markdown
ContributorAuthor

Does this attempt to build node with the same default config as with ./configure --ninja && make ?

No, the GN config has some significant differences, for example it links with a custom libc++ statically. Eventually I would like to be able to build node with the same default config of gyp, but currently it is not my focus since neither V8 nor Electron needs it.

I have depot_tools from Chromium on my PATH. How can I run the GN build?

It uses the same workflow with checking out V8, the github action file has some instructions in it.

First checkout the code:

mkdir node-ci
cd node-ci
git clone https://github.com/photoionization/node-ci
gclient config https://github.com/photoionization/node-ci --unmanaged
gclient sync

Then you run gn and ninja:

cd node-ci
./tools/gn-gen.py out/Release
autoninja -C out/Release

Note that the https://github.com/photoionization/node-ci repo is a fork of https://chromium.googlesource.com/v8/node-ci/, the latter is used by V8 team to build Node with GN. I'll upstream the changes in my fork once GN configurations are merged into Node.

Comment threadBUILD.gn Outdated
@zcbenzzcbenz changed the title build: Add GN build filesbuild: add GN build filesApr 24, 2023
@zcbenz
zcbenzforce-pushed the patch-gn branch 3 times, most recently from ff7e849 to b0a0174CompareApril 26, 2023 10:56

@joyeecheungjoyeecheung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

How is the GN files supposed to be used? Just gn args outdir and then run the build with ninja?

Comment threadBUILD.gn Outdated
Comment threaddeps/ada/BUILD.gn Outdated
Comment threadunofficial.gni Outdated
@gengjiawen

Copy link
Copy Markdown
Member

Maybe add gn setup to main/.gitpod.yml

I didn't know gitpod, will give it a try.

create a .gitpod.Dockerfile in project root, and add deps to it

FROM gitpod/workspace-full
ENV PATH=${PATH}:/home/gitpod/depot_tools
RUN cd ~ && git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git --depth=1
// other setup

Then in .gitpod.yml

image:
file: .gitpod.Dockerfile

@zcbenz

Copy link
Copy Markdown
ContributorAuthor

create a .gitpod.Dockerfile in project root, and add deps to it

Thanks for the guidance! I'll try to support it in a separate PR.

Comment threadunofficial.gni Outdated
Comment threaddeps/ada/unofficial.gni Outdated
@zcbenz

Copy link
Copy Markdown
ContributorAuthor

This PR has been rebased on main branch, currently it depends on #49178 and #49279.

@targos

Copy link
Copy Markdown
Member

#49279 just landed.

@joyeecheung

Copy link
Copy Markdown
Member

Is this ready for review now?

targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #47637
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@zcbenz

zcbenz commented Nov 12, 2023

Copy link
Copy Markdown
ContributorAuthor

The "BSD-style license" refers to Chromium's license.

The tools/generate_config_gypi.py, tools/search_files.py, and the GN build files are written from scratch up by Electron team and we can just delete their copyright headers to make Node's LICENSE file apply to them, like the other tools and gyps files in Node.

The tools/tools/gypi_to_gn.py is derived from Chromium so we must keep its copyright header, for which I'll need to add an entry in Node's LICENSE file.

I'll start a new PR correcting them.

@targostargos mentioned this pull request Nov 12, 2023
targos pushed a commit that referenced this pull request Nov 14, 2023
PR-URL: #47637
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@zcbenz
zcbenz deleted the patch-gn branch November 28, 2023 01:57
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #47637
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
@UlisesGasconUlisesGascon mentioned this pull request Dec 12, 2023
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 10, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 21, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 21, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 22, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 28, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 29, 2024
codebytere added a commit to electron/electron that referenced this pull request Oct 31, 2024
jkleinsc pushed a commit to electron/electron that referenced this pull request Nov 4, 2024
* chore: bump Node.js to v22.9.0
* build: drop base64 dep in GN build
nodejs/node#52856
* build,tools: make addons tests work with GN
nodejs/node#50737
* fs: add fast api for InternalModuleStat
nodejs/node#51344
* src: move package_json_reader cache to c++
nodejs/node#50322
* crypto: disable PKCS#1 padding for privateDecrypt
nodejs-private/node-private#525
* src: move more crypto code to ncrypto
nodejs/node#54320
* crypto: ensure valid point on elliptic curve in SubtleCrypto.importKey
nodejs/node#50234
* src: shift more crypto impl details to ncrypto
nodejs/node#54028
* src: switch crypto APIs to use Maybe<void>
nodejs/node#54775
* crypto: remove DEFAULT_ENCODING
nodejs/node#47182
* deps: update libuv to 1.47.0
nodejs/node#50650
* build: fix conflict gyp configs
nodejs/node#53605
* lib,src: drop --experimental-network-imports
nodejs/node#53822
* esm: align sync and async load implementations
nodejs/node#49152
* esm: remove unnecessary toNamespacedPath calls
nodejs/node#53656
* module: detect ESM syntax by trying to recompile as SourceTextModule
nodejs/node#52413
* test: adapt debugger tests to V8 11.4
nodejs/node#49639
* lib: update usage of always on Atomics API
nodejs/node#49639
* test: adapt test-fs-write to V8 internal changes
nodejs/node#49639
* test: adapt to new V8 trusted memory spaces
nodejs/node#50115
* deps: update libuv to 1.47.0
nodejs/node#50650
* src: use non-deprecated v8::Uint8Array::kMaxLength
nodejs/node#50115
* src: update default V8 platform to override functions with location
nodejs/node#51362
* src: add missing TryCatch
nodejs/node#51362
* lib,test: handle new Iterator global
nodejs/node#51362
* src: use non-deprecated version of CreateSyntheticModule
nodejs/node#50115
* src: remove calls to recently deprecated V8 APIs
nodejs/node#52996
* src: use new V8 API to define stream accessor
nodejs/node#53084
* src: do not use deprecated V8 API
nodejs/node#53084
* src: do not use soon-to-be-deprecated V8 API
nodejs/node#53174
* src: migrate to new V8 interceptors API
nodejs/node#52745
* src: use supported API to get stalled TLA messages
nodejs/node#51362
* module: print location of unsettled top-level await in entry points
nodejs/node#51999
* test: make snapshot comparison more flexible
nodejs/node#54375
* test: do not set concurrency on parallelized runs
nodejs/node#52177
* src: move FromNamespacedPath to path.cc
nodejs/node#53540
* test: adapt to new V8 trusted memory spaces
nodejs/node#50115
* build: add option to enable clang-cl on Windows
nodejs/node#52870
* chore: fixup patch indices
* chore: add/remove changed files
* esm: drop support for import assertions
nodejs/node#54890
* build: compile with C++20 support
nodejs/node#52838
* deps: update nghttp2 to 1.62.1
nodejs/node#52966
* src: parse inspector profiles with simdjson
nodejs/node#51783
* build: add GN build files
nodejs/node#47637
* deps,lib,src: add experimental web storage
nodejs/node#52435
* build: add missing BoringSSL dep
* src: rewrite task runner in c++
nodejs/node#52609
* fixup! build: add GN build files
* src: stop using deprecated fields of v8::FastApiCallbackOptions
nodejs/node#54077
* fix: shadow variable
* build: add back incorrectly removed SetAccessor patch
* fixup! fixup! build: add GN build files
* crypto: fix integer comparison in crypto for BoringSSL
* src,lib: reducing C++ calls of esm legacy main resolvenodejs/node#48325
* src: move more crypto_dh.cc code to ncrypto
nodejs/node#54459
* chore: fixup GN files for previous commit
* src: move more crypto code to ncrypto
nodejs/node#54320
* Fixup Perfetto ifdef guards
* fix: missing electron_natives dep
* fix: node_use_node_platform = false
* fix: include src/node_snapshot_stub.cc in libnode
* 5507047: [import-attributes] Remove support for import assertions
https://chromium-review.googlesource.com/c/v8/v8/+/5507047
* fix: restore v8-sandbox.h in filenames.json
* fix: re-add original-fs generation logic
* fix: ngtcp2 openssl dep
* test: try removing NAPI_VERSION undef
* chore(deps): bump @types/node
* src: move more crypto_dh.cc code to ncrypto
nodejs/node#54459
* esm: remove unnecessary toNamespacedPath calls
nodejs/node#53656
* buffer: fix out of range for toString
nodejs/node#54553
* lib: rewrite AsyncLocalStorage without async_hooks
nodejs/node#48528
* module: print amount of load time of a cjs module
nodejs/node#52213
* test: skip reproducible snapshot test on 32-bit
nodejs/node#53592
* fixup! src: move more crypto_dh.cc code to ncrypto
* test: adjust emittedUntil return type
* chore: remove redundant wpt streams patch
* fixup! chore(deps): bump @types/node
* fix: gn executable name on Windows
* fix: build on Windows
* fix: rename conflicting win32 symbols in //third_party/sqlite
On Windows otherwise we get:
lld-link: error: duplicate symbol: sqlite3_win32_write_debug
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:47987
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_sleep
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48042
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_is_nt
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48113
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_utf8_to_unicode
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48470
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_unicode_to_utf8
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48486
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_mbcs_to_utf8
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48502
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_mbcs_to_utf8_v2
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48518
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_utf8_to_mbcs
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48534
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
lld-link: error: duplicate symbol: sqlite3_win32_utf8_to_mbcs_v2
>>> defined at .\..\..\third_party\electron_node\deps\sqlite\sqlite3.c:48550
>>> obj/third_party/electron_node/deps/sqlite/sqlite/sqlite3.obj
>>> defined at obj/third_party/sqlite\chromium_sqlite3/sqlite3_shim.obj
* docs: remove unnecessary ts-expect-error after types bump
* src: move package resolver to c++
nodejs/node#50322
* build: set ASAN detect_container_overflow=0
nodejs/node#55584
* chore: fixup rebase
* test: disable failing ASAN test
* win: almost fix race detecting ESRCH in uv_kill
libuv/libuv#4341
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.lib / srcIssues and PRs related to general changes in the lib or src directory.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@zcbenz@nodejs-github-bot@gengjiawen@targos@joyeecheung@Qard@anonrig@jasnell@richardlau