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

Add GN configurations to Node #1353

Closed
zcbenz opened this issue Mar 10, 2023 · 22 comments
Closed

Add GN configurations to Node #1353

zcbenz opened this issue Mar 10, 2023 · 22 comments

Comments

@zcbenz
Copy link

zcbenz commented Mar 10, 2023

What I am asking for

I'm asking for including GN configurations in Node's repo, so users can build Node with GN without forking.

Precisely, about 1100 lines of BUILD.gn files and 3 python scripts.

It is also simple to setup a CI job building Node with GN, and even integrating GN into Node's current building process, please check the example: https://github.com/photoionization/node_with_gn.

What I am NOT asking for

I'm not asking for a complete replacement of GYP with GN, it is currently not possible since Node supports building for some platforms not supported by GN yet (adding support for those platforms is not hard).

This issue is not exactly the same with nodejs/node#21410, I'm not talking about possibilities, this is finished work which I would like to include in upstream, and you can immediately see why you would want to use it (read the benefits below).

Is GYP not enough?

Why bother with another build system when GYP is working fine? Many people may ask. My answer is, GYP is not fine, not at all.

First of all, GYP is abandoned by its creators, it has been forked by Node but only in maintenance mode. And since Node is the only project using it, you don't have outsider building experts improving this tool, you only have people not interested in build tools forced to fix bugs when there is no other choice.

And GYP is buggy and fragile, you can see it by simply trying with ninja generator (configure.py --ninja). Node's build files have been built around bugs and weird behaviors of GYP.

The alternatives

Since no one is willing to translate V8's build configurations to a foreign build system, this really leaves us with only 2 options for choosing a build system for Node: GN and Bazel.

I personally have no experience with Bazel so I can't do a features compare. But the fact is, there are already people building Node with GN, including Electron team and V8 team. The latter may be surprising to you, but V8 is maintaining an active GN configuration for Node (https://chromium.googlesource.com/v8/node-ci/), and it is used in Chromium project.

This means no matter which build system Node uses, Electron and V8 still have to maintain a GN configuration anyway. But if Node can support building with GN, all those resources will come here to support the upstream.

The benefits of GN

By supporting GN, developers of Node can immediately get following goodies:
(You can try it NOW with https://github.com/photoionization/node_with_gn)

Unified compiler and libc++ on all platforms

GN supports using the exactly the same version of clang for building on Linux/macOS/Windows, with binaries maintained by Chromium project. And it also supports building with the exactly the same version of libc++. Just behind a flag.

This means you don't have to struggle with MSVC and GCC when trying to use bleeding edge C++17/20 features. It also means you no longer needs to figure out how to setup new clang/libc++ when building on a not-so-new Linux distribution.

(Building with system compiler and C++ standard library is also supported.)

Linux sysroot images

When doing cross compilation on Linux, it is extremely difficult to install the correct libraries for the target arch. With GN this is solved with sysroot images, the script downloads the headers and libraries of the target arch to disk for the build system to link.

This also solves problem of providing prebuilt binaries that run on most Linux distributions. When building Node.js on a newer linux distribution, it may be linked to a new version of glibc which is not available in old linux distributions, resulting in prebuilt binaries unable to run on those platforms (nodejs/node#43246). There are also problems that different distributions may have libraries put under different places, so the prebuilt binaries may not be able to find the correct lib on a foreign distribution.

The sysroot images provided by Chromium are designed to solve this problem, and it is free for Node to use, behind a build flag.

Compile Windows binaries on Linux and macOS

With GN it is possible to compile Windows binaries on Linux and macOS, the GitHub Actions of node_with_gn builds the Windows binaries on a free linux runner.

Distributed compiler service

GN supports building with goma, which is like ccache but does all the compilation and caching in a central server.

Compiling V8 is very slow but with goma build you can build Node from scratch within a minute and half (tested with Electron's goma service). You still have to setup a very powerful goma server but it is very worthy investment, not only much shorter local build time, but also much shorter time spent on CI.

Sanitizers

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

Rust support

GN supports building rust code, and linking with C++ code. Chromium has rewritten a few parts like JSON parser in rust, and if Node wants to do the same all the tools are already here in GN.

And it is not only about rewriting existing code. For components like http parser and url parser, there are a few good rust implementation already being used in browsers, with GN support in Node it will be practical to replace those components in Node with safe rust equivalents.

Downsides with GN

The biggest downside with GN is, the vanilla GN does not have any preset build configurations with it, each project using GN must also have its own build configurations instructing things like how to invoke clang. I solved this problem by forking GN to support preset build configurations, and then ship Chromium's build configurations with it. The forked GN can be found at https://github.com/yue/build-gn.

So to build Node with GN we must either ship a large stock of build configurations inside Node (Like what V8 does), or use a forked GN with a forked Chromium build configurations. But either way Node will receive help from Electron team and I believe it is still much better than sticking to GYP.

@nschonni
Copy link
Member

Previous discussions from last year #901

@zcbenz
Copy link
Author

zcbenz commented Mar 10, 2023

Thanks for the link, this time I have a finished example showing how things work, with a simple and clear target (add a few GN files to upstream). I hope we can finally get started.

@GeoffreyBooth
Copy link
Member

cc @nodejs/build @nodejs/cpp-reviewers

If we do this, it might be best to ship this as part of 20.0.0.

@zcbenz Since it appears the work has already been done, do you want to open a PR?

@zcbenz
Copy link
Author

zcbenz commented Mar 12, 2023

There are a few prerequisite PRs stills waiting for merge: https://github.com/nodejs/node/pulls/zcbenz.
(They are not specific to GN so the discussions here should be affect them though.)

@bnoordhuis
Copy link
Member

Node can't move away from gyp until nodejs/node-gyp#1118 is fixed. (ETA: never)

That means GN will at best be a second build system and I don't think it's reasonable to ask an upstream project to maintain an alternative build system for the benefit of a (single!) downstream project.

Tier 2/3 platform support is another reason why a move to GN is not realistic.

Apropos unified libc++: useless to node, breaks native add-ons.

@zcbenz
Copy link
Author

zcbenz commented Mar 14, 2023

I don't think nodejs/node-gyp#1118 is a blocker.

For a node executable (official one and custom ones like electron) to load a node native module, there are only 2 requirements:

  1. Both binaries should link to the same C++ standard library because V8 uses C++ STL containers in its API;
  2. Both binaries have same feature defines (like V8_COMPRESS_POINTERS) when compiling code.

Compiler and linker flags do not matter, you can surely load a debug build of native module in a node executable compiled with -O3, and vice versa.

These 2 requirements are easy to meet, that's why official node.js can load binaries produced by cmake.js, and electron can load binaries produced by node-gyp. And with correct flags node executable built with GN can surely load binaries produced by node-gyp.

So for tools like node-gyp and cmake.js, currently only 3 pieces of information are needed:

  1. Build flags like v8_enable_pointer_compression=true, currently provided by config.gypi;
  2. Build rules instructing when to define things like V8_COMPRESS_POINTERS, currently provided by common.gypi;
  3. How to link with C++ standard library (Node is just using system default), currently defined in common.gypi.

(If we even simplify the problem further, only a list of defines is actually required.)


With above information, how to fix nodejs/node-gyp#1118 is clear (it is essentially a variant of nodejs/node-gyp#2497).

An ideal fix could be:

  1. First in node.js we split common.gypi into a few parts: one node_features.gypi contains when to define build flags like V8_COMPRESS_POINTERS, one libcxx.gypi contains how to link with C++ standard library (in node it is just linking to system default, in electron we will instruct it to link with a custom libc++), and one common.gypi that contains everything else.
  2. Then in node-gyp we change it to load only node_features.gypi and libcxx.gypi, and then perhaps add some build flags to its addon.gypi.

In electron and node_with_gn, it is easy to maintain 2 additional node_features.gypi and libcxx.gypi files, since they only include very simple information. And in cmake.js, it can now parse those files to generate correct build settings for node with different configurations (currently cmake.js is unable to build modules for node executables compiled with different flags from the official one).

And we can even go further by storing the informations in language-independent JSON files like node_features.json and libcxx.json, and write a tool to generate gypi files from them during the configuration step.

I can start working on this issue soon, since fixing it will also solve some headaches in electron.


So nodejs/node-gyp#1118 can be fixed and I'll fix it soon, and the possibility of completely removing GYP in Node is still there.

The path to integrate GN in Node could look like:

  1. Add GN configurations to Node.
  2. Start fixing issues like Don't include common.gypi from node source tree node-gyp#1118 that rely on GYP.
  3. Make GN as a secondary build system in Node.
  4. Remove GYP.

On tier 2/3 platform support, it is possible to implement in GN and it does not take too much work. FreeBSD and OpenBSD ports have already done it. Basically for a local build, you only need to add a toolchain file for each platform since those platforms usually use same build tools:

http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/patch-build_toolchain_freebsd_BUILD_gn?rev=1.1&content-type=text/x-cvsweb-markup
http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/patch-build_toolchain_openbsd_BUILD_gn?rev=1.4&content-type=text/x-cvsweb-markup
http://cvsweb.openbsd.org/cgi-bin/cvsweb/ports/www/chromium/patches/patch-build_config_v8_target_cpu_gni?rev=1.2&content-type=text/x-cvsweb-markup

Cross compiling BSD on Linux/macOS is also possible, by adding a cross compilation toolchain with a --target= flag, and again this will be another advantage using GN.


On unified libc++, it is optional and Node can totally link with system C++ standard library just like with GYP, but I think using unified libc++ is beneficial for Node ecosystem and probably something you will eventually do.

Chromium project now is quite aggressive on adopting new C++ standards, it is already using C++20 in most of its codebase and so does V8, and Node has to follow whether you want it or not. This means you have to build Node with a recent version of C++ standard library, it is not a problem on Windows/macOS, but on Linux, many old distributions don't ship new versions of libstdc++, and as a result you can not build or use Node on those distributions (issues like nodesource/distributions#1392, they were talking about glibc but libstdc++ is the real problem).

The solution that ends all the mess is statically compiling libc++ with Node, and it is possible to support native addons with this approach (electron is already doing it). All we need to do is to export the libc++ symbols in Node (like how uv and V8 symbols are exported) and ship libc++ headers in the headers tarball, and instruct node-gyp to use the shipped libc++ instead of using the system default one.

@victorgomes
Copy link

Regarding building V8 with bazel: we currently do not recommend this, it is mostly for internal use at Google. It does not support all V8 features.

Google does not officially encourage using GN outside Chromium. However, a GN build for Node could be extremely beneficial for both V8 and Node. As it was already mentioned, we currently maintain an ad-hoc GN configuration for Node (https://chromium.googlesource.com/v8/node-ci/) to run Node with V8 ToT. Since this configuration is maintained on a best effort basis, it is deviating a bit from Node each release and we run only a small set of tests in our CI. Running V8 ToT for Node can also be beneficial to catch API issues early on.

@bnoordhuis
Copy link
Member

I've worked on node-gyp for years and my gut tells me there's going to be an infinite tail end of bugs and problems. For better or worse, many of node's build details leak out in ways that affect add-ons. Sometimes add-ons exploit them deliberately.

Apropos statically linking libc++: I don't see that happening. Many add-ons are wrappers around third-party C++ libraries that in turn link to the system's libstdc++.

Given how conservative node is w.r.t. breaking changes, an ecosystem apocalypse like that is simply unthinkable.

@richardlau
Copy link
Member

Thoughts:

  • Who is going to be maintaining these parallel build files? (As mentioned, we unfortunately cannot get rid of gyp anytime soon.) I don't think Node.js has much experience among the collaborator base with gn. (If a group of collaborators want to put themselves forward to commit to keeping these up-to-date, that's fine.)
  • Many of the "benefits" of GN seem to imply either becoming reliant on Google assets (precompiled binaries, sysroot, etc) or a non-zero amount of effort to replicate that on our side. As has been stated, Google do not support any of this for use outside of Chromium.
  • While I think sanitizers are not a terrible idea, I'm not convinced people are looking much at the existing sanitizer GitHub workflow we have.
  • Forked gn sounds like a very bad idea, in the same way that we're currently stuck on a forked gyp.

My own opinion is that I would not block the idea if other collaborators wanted this and committed to maintaining it. (FTR I would not be among them.)

@GeoffreyBooth
Copy link
Member

It feels to me though that aren’t we stuck with two less than ideal options? Yes GN has some knotty issues to work out especially around maintenance and support, but gyp is abandoned and has been a big wart for us for years. Maybe gyp is the less bad of two bad options, but I don’t think we should just default to the status quo because something about the alternative isn’t perfect. The status quo when it comes to gyp is also pretty bad.

With regard to GN, if Electron is already supporting it, then we wouldn’t be on our own here, would we? Perhaps some of the members of Electron who are currently supporting building Node with GN for Electron might prefer to support building Node with GN within Node core rather than in some fork elsewhere.

@zcbenz
Copy link
Author

zcbenz commented Mar 14, 2023

Regarding the maintenance of the GN files, Electron team is very willing to maintain those files in upstream Node than in our own fork. I can't speak for V8 or Chromium team, but I have been adding some changes to make Node's GN adoption easier, and they have been very helpful (for example inspector_protocol, zlib, v8.

My point is, no matter Node adopts GN or not, Electron and V8 team still have to maintain GN configurations anyway. But if Node can support building with GN, all those resources will come here to support the upstream.

@zcbenz
Copy link
Author

zcbenz commented Mar 14, 2023

Forked gn sounds like a very bad idea, in the same way that we're currently stuck on a forked gyp.

Using forked GN is optional, the official way doing a GN build outside Chromium is to put build configurations in a DEPS file and then use depot_tools to checkout them.

Note that while Google does not recommend using GN outside Chromium, it is actually a supported use case and lots of Chromium's sub projects use it (because not everyone wants to work on the huge Chromium codebase), including V8, node-ci (V8's node GN build), webrtc, skia, and lots of others.

A safe move for Node to do is, first we add GN configuration files to Node, so Electron and V8 can start to use them and fix bugs, in the meanwhile I will write separate tools to build Node with GN (with forked GN or upstream GN, or very likely both), so Node developers can make use of it to test GN configurations, and add CI for GN.

I'm hoping that Node developers will love the workflow of GN and use it for daily development, so the transit from GYP to GN will be on Node team's initiative.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 15, 2023

As someone who uses both GYP and GN on a daily basis I think the difference only shows up when one updates the build files. For most developers there isn't any special advantage that a GN workflow has over GYP, as most of the time Node.js developers don't have to update the build files or only need to do trivial updates, like adding new files, where the difference of GN v.s. GYP doesn't really show. Only those who are doing non-trivial changes to the build files - e.g. for updating V8 or adding third-party dependencies that need special configuration - could really tell the difference. Other advantages that the OP mentioned are a bit too far-fetched for our build resources - I doubt if we'd have resources to support any of them even if we were already GN-only. Even the "buggy and fragile" aspect of GYP that the OP mentions doesn't really show for me that often personally - I only feel it when I have to do non-trivial updates to the GYP configs, but changes like that are like only once-a-month in the core repo. I also use configure --ninja daily on different platforms with combinations of other build options (I have stopped using the make generator for years) and I rarely encounter any issues with that.

The most practical advantage of GN is that we get the V8 build configs out of the box, though that advantage is less significant if we still have to maintain GYP configs in parallel instead of completely dropping support for GYP. I think it would be up to @nodejs/v8-update (and probably especially @targos) to tell how sustainable the maintenance of the GYP configs is and if we are at a point where a path forward must be considered.

That said I don't think it's a bad idea to have GN build files in Node.js if that helps the workflow of Electron and V8, because leaving important downstream/upstream projects having to fork Node.js in order to build/test is certainly not ideal and ultimately gets in the way of the project's success. But I would be concerned if we start to require CI with GN builds to pass for regular PRs while we have to maintain both builds - that would double the work that contributors have to do when they do have to update the build files. Requiring contributors to update both is definitely much worse than only having to update GYP configs i.e. the status quo, and the far-fetched goal of only having to update GN configs isn't enough motivation for having to update both for a long time (and I suspect that would be very long). So if we add GN configs to core we should make it as clear as possible that it is only there for the sake of downstream projects e.g. explicitly saying in the GN configs that this isn't the right place to update build configs if they are not porting GYP changes over to support a downstream project, in case a new contributor spends too much time on updating GN configs and only realize that they have to do it again in GYP, even though the deal is that Node.js contributors only have to maintain GYP configs while the downstream maintain the GN configs.

@targos
Copy link
Member

targos commented Mar 15, 2023

tell how sustainable the maintenance of the GYP configs is and if we are at a point where a path forward must be considered.

I wouldn't say that the current maintenance of the GYP configs is sustainable. It is reasonably possible to keep them working for the current way things work, but I am terrified every time V8 does a significant refactor of its BUILD.gn file. I also gave up porting new build options (except simple flags) to GYP a long time ago (perfetto for example).

@gengjiawen
Copy link
Member

I am -1 on this one too. GN and gyp is not sustainable in the long run.

@nornagon
Copy link

(see also previous discussion here: nodejs/node#21410)

Regarding building 3rd-party native modules, it seems like that could remain as GYP even if Node switches to building with GN. In Electron we have a (somewhat hacky) script which generates common.gypi by running configure.py, reading config.gypi and combining it with some information from our GN build. So it seems plausible that 3rd-party native modules could continue to be built with GYP, thus keeping the ecosystem intact, while still changing Node's internal build system away from GYP. There would be some effort involved in maintaining that script, but I think it might be less effort than maintaining the entirety of GYP as well as GYP build files for V8.

@mhdawson
Copy link
Member

My thoughts are similar to those expressed by @joyeecheung.

I don't think moving to GN makes sense for the project but at the same time I don't think it's a bad idea to have GN build files in Node.js if that helps the workflow of Electron and V8, so long as they are not blocking for PRs to land/releases to go out.

A nightly job which build/tested with them could also make sense to help identify when they have been broken/need updates. The assumption is that the collaborators (assuming those from Electron and V8 to start) would watch that nightly job and fix any issues reasonably quickly.

@joyeecheung
Copy link
Member

joyeecheung commented Mar 15, 2023

At the TSC meeting today (with 10 members attending out of 22) we discussed about adding the GN configs to core repo (in parallel to GYP configs, which is separate from the proposal of replacing GYP with GN) as an unofficial build system that's maintained by downstream projects, and at the meeting there was no objection to this idea.

An actionable next step may be that the Electron team can start to upstream the prerequisite PRs for making GN work in Node.js, and then submit PR(s) for adding the GN configs. Having a nightly build for other projects to keep an eye on (as proposed in #1353 (comment)) would also be a good follow-up. I am removing the tsc-agenda label for now until this needs to be discussed at the TSC meeting again.

@zcbenz
Copy link
Author

zcbenz commented Mar 15, 2023

Thanks! I'll start a PR adding GN files after the following prerequisite PRs get merged:

@mhdawson
Copy link
Member

mhdawson commented Apr 5, 2023

@zcbenz I'm going to close this as I think the question was discussed/answered and follow up discussion is likely to be in PRs. Please let me know if you think that was not the right thing to do.

@gengjiawen
Copy link
Member

Sanitizers
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.

@zcbenz Can you share the command to do this in current Node.js repo. I assume all Prs are merged.
I want to do this check in the CI system.

@zcbenz
Copy link
Author

zcbenz commented Jan 5, 2024

Currently the GN build is cumbersome and you have to make use of either https://github.com/photoionization/node_with_gn or https://github.com/photoionization/node_ci, and the GN build of main branch is currently broken and waiting for these PRs to be merged:

I'm going to work on better toolings after nodejs/node#50737 gets merged, which is required for V8's CI system to adopt the GN build files in Node, and once V8's toolings recognize upstream Node I can make GN build easier to work with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests