Build node with GN - Githubissues

nornagon commented 6 years ago

Electron is starting to move its build system from GYP to GN to align with Chromium, which will make rolling Chromium substantially easier. However, we still have to shell out to GYP to build node.js, which is a lot of bridging code to maintain. I see there have been prior conversations about porting node's build system to GN from GYP, but they seem to have petered out without conclusion.

Would node.js be willing to accept patches to switch from GYP to GN?

NB, not suggesting that node does such a thing for 3rd-party native modules, just for node's own build.

bnoordhuis commented 6 years ago

That's already possible with ./configure --build-v8-with-gn.

edit: unless you mean porting all of the build system over. The answer is 'no' in that case.

nornagon commented 6 years ago

Sorry, I'm not referring to building v8 with GN, I'm referring to building node.js itself with GN.

bnoordhuis commented 6 years ago

Don't know if you saw my ninja edit but I think it's safe to say a switch to GN is not on the table.

nornagon commented 6 years ago

Ah, no, I missed it (would be awesome if GH would refresh edits live). Could you expand on why that's the case? (Is there perhaps a document describing blockers/reasoning that you could refer me to?)

bnoordhuis commented 6 years ago

See https://github.com/nodejs/TSC/issues/464 which touches on that.

Speaking for myself, getting burned by a made-by-Google-for-Google build system once is enough; the next one should be one with broad community support.

mhdawson commented 6 years ago

Based on earlier discussion Google said they don't support other projects using GN and recommended against it.

joyeecheung commented 6 years ago

We are basically left with 3 choices after gyp:

Blaze: Google uses that (Blaze) internally so they can support it, see https://github.com/nodejs/TSC/issues/464 for a breakdown. It's a Java tool although it has distributions with JDK bundled. Would be the most difficult to transition the addon tooling into.
GN: Currently used by V8 but people from the V8/Chromium/GN side all recommended against it in https://github.com/nodejs/TSC/issues/464 due to support issues, it's also coupled in depot_tools and platform support is limited. There is already ./configure --build-v8-with-gn in our code base though. Future of addon tooling is bleak.
CMake: We'll have to port the build files over and maintain them by ourselves (I guess we would need a revamp of https://github.com/nodejs/build/issues/1003) No additional dependencies other than cmake itself. If we want to transition the addon tooling in the future as well there is already cmake.js

See https://docs.google.com/document/d/101BP4BpZoP4tsMGo4j_MhoyLv169-2Oq_HeyWykCNGc/edit (OP of that TSC issue) for a breakdown

bnoordhuis commented 6 years ago

Also apropos cmake, I'm reasonably confident I can teach node-gyp to convert a binding.gyp to CMakeLists.txt on the fly and effectively phase out GYP.

nornagon commented 6 years ago

It's totally possible to use GN without depot_tools, see e.g. https://github.com/yue/build-gn. GN works fine on win/mac/linux, not sure about arm64 support but chromium itself builds against arm64: e.g.. @joyeecheung can you expand on what you mean by "platform support is limited"? Also not sure what you mean by "addon tooling".

joyeecheung commented 6 years ago

Regarding OP's request:

Would node.js be willing to accept patches to switch from GYP to GN?

I think patches for GN support can be accepted as long as they are not the default flow of building Node.js and are more in a situation similar to our Android builds (self-serve). If Electron needs GN to build Chromium then building Node.js with GN would be the most viable option for Electron, but it is not the case for Node.js itself and we have been explicitly advised against using it from the V8/Chromium/GN team.

joyeecheung commented 6 years ago

It's totally possible to use GN without depot_tools, see e.g. https://github.com/yue/build-gn.

Didn't know that, good to hear, thanks

GN works fine on win/mac/linux, not sure about arm64 support but chromium itself builds against arm64: e.g.. @joyeecheung can you expand on what you mean by "platform support is limited"?

https://docs.google.com/document/d/101BP4BpZoP4tsMGo4j_MhoyLv169-2Oq_HeyWykCNGc/edit touched on that a little bit (regarding FreeBSD and AIX). See supported platforms for the full list of platforms and tiers that we support.

Also not sure what you mean by "addon tooling".

I meant providing support for building 3rd-party native modules (as mentioned in the OP) with one of those tools and transitioning the ecosystem into it.

bnoordhuis commented 6 years ago

platform support is limited

Little to no freebsd/openbsd/etc. support. You're basically restricted to what Google is willing to support.

I think patches for GN support can be accepted

Supporting multiple build systems is already a hassle for libuv, and that's a fairly simple project to build.

I don't see it happening for Node.js. Without major ongoing maintenance it bitrots in no time.

joyeecheung commented 6 years ago

Supporting multiple build systems is already a hassle for libuv, and that's a fairly simple project to build. I don't see it happening for Node.js. Without major ongoing maintenance it bitrots in no time.

I agree, but Idon't see any harm if we do not support GN, just allowing the build files to exist in the code base and get maintained by people who need them (e.g. Electron) though.

bnoordhuis commented 6 years ago

I'm skeptical that works. If it's there, people will use it and complain when it's broken.

As well, someone needs to review those updates. And thus us maintainers' workloads expand ever further towards infinity.

joyeecheung commented 6 years ago

I'm skeptical that works. If it's there, people will use it and complain when it's broken. As well, someone needs to review those updates. And thus us maintainers' workloads expand ever further towards infinity.

Would it help if we make the configure option prints stuff (maybe in red) about this being not supported by Node.js core and do not document it/document about the lack of support?

(Just saying...or am I?)

bnoordhuis commented 6 years ago

You could cat an .au to /dev/dsp where we scream at the top of our lungs "DO NOT USE!" and still people will file bugs.

We already print warnings. People even include them in their bug reports, then later admit they didn't see them. It's hopeless.

joyeecheung commented 6 years ago

We already print warnings. People even include them in their bug reports, then later admit they didn't see them. It's hopeless.

Yeah, but for someone who even knows what GN is, and wants to use it to build Node, and goes through the docs to find the configure option that does it, they are probably very unlikely to ignore the warnings. Also there probably would not be that many people who want to do it (I guess building Electron could lead you to that but how many people actually builds Electron along with Chromium and Node.js?)

bnoordhuis commented 6 years ago

I expect the answer is "many" because of how many downstream users electron has. My cmake branch hasn't even been merged yet and there are already at least two people using it.

nornagon commented 6 years ago

btw, it seems like cmake doesn't support smartos, freebsd or AIX officially either: https://open.cdash.org/index.php?project=CMake

aduh95 commented 6 years ago

@nornagon Actually it seems that smartos is the only one not officially supported by CMake (https://gitlab.kitware.com/cmake/cmake):

Supported Platforms

Microsoft Windows

Apple macOS

Linux

FreeBSD

OpenBSD

Solaris

AIX

Other UNIX-like operating systems may work too out of the box, if not it should not be a major problem to port CMake to this platform. Subscribe and post to the CMake Users List to ask if others have had experience with the platform.

tojocky commented 6 years ago

Fuchsia build system uses GN: https://github.com/fuchsia-mirror/docs/blob/master/development/build/overview.md Zircon is built with GNU Make.

Also GN team did an effort to isolate GN from chromium code: https://gn.googlesource.com/gn/

codebytere commented 6 years ago

@hashseed we've done work locally on this and have all the necessary build files in place for building Node with GN as part of Electron here. There are a few more fixup commits after that but that's the bulk!

refack commented 6 years ago

we've done work locally on this and have all the necessary build files in place for building Node with GN as part of Electron here. There are a few more fixup commits after that but that's the bulk!

It was discussed several times before, and IMHO GN is not the best way forward.

We still use GYP for our native addons. Switching to GN will essentially abandon them. The status there is not good as is.
From my experience GN works where Google has got it to work. It's a much less mature system then GYP (or CMake)
We have a lot of institutional knowledge both in GYP and in the .gyp files.
Using GYP does not require anything other then python, which we are already dependant on.

ofrobots commented 6 years ago

The choice of build system in core doesn't necessarily have anything to do with how the add-ons are built. We don't have to conflate these two issues.

On Sat, Oct 13, 2018, 6:22 PM Refael Ackermann notifications@github.com wrote:

we've done work locally on this and have all the necessary build files in place for building Node with GN as part of Electron here https://github.com/electron/node/commit/88b494191c2a5b50b01dab80cd61ba3c0e0fbeb9. There are a few more fixup commits after that but that's the bulk!

It was discussed several times before, and IMHO GN is not the best way forward.

We still use GYP for our native addons. Switching to GN will essentially abandon them. The status there is not good as is.

From my experience GN works where Google has got it to work. It's a much less mature system then GYP (or CMake)

We have a lot of institutional knowledge both in GYP and in the .gyp files.

Using GYP does not require anything other then python, which we are already dependant on.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/21410#issuecomment-429588476, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE0qasNs52-Gp5kbijo7wiG-qGIn2G9ks5ukpHigaJpZM4UuOep .

joyeecheung commented 6 years ago

We still use GYP for our native addons. Switching to GN will essentially abandon them. The status there is not good as is.

I believe the request has been switched to "adding GN files in core, in parallel with GYP files", not "replacing GYP files with GN files".

Using GYP does not require anything other then python, which we are already dependant on.

https://gn.googlesource.com/gn/+/master seems to only depends on python, ninja and the usual compiler toolchains (judging from the output of build/gen.py, it doesn't seem to be terribly difficult to remove the ninja dependency either), but again if we are not replacing GYP with GN, just adding GN files so that people can build with GN, that probably doesn't matter, whoever uses them will find a way to put GN on their PATH somehow.

On the maintenance issue, we don't support building ninja in core either, but you can still build node with ninja with build files generated from GYP files anyway, and we even have a document on how to do that. If Electron has decided to build Node.js with GN, when the GN files are broken, there will be people making sure they are fixed anyway - there is a company (and another giant company very soon) behind that project, after all. Worst case scenario, they probably won't fix the GN files for all the platforms we support and won't port all of our flags to GN, and we might potentially get bug reports from people trying to use those flags with GN files or on unsupported platforms, we can either leave the issues open, or remind them that we do not maintain those files and close the issue.

hashseed commented 6 years ago

I believe that there is a lot of value in adding GN as alternative, but non-default build system. Reasons:

GN configs already exists and is maintained by the Electron project. I think it would be a big sign of goodwill to accommodate the Electron project by making integration with them a smoother experience.
Not requiring gyp configs could decouple the canary builds from manually updating gyp configs. This currently is done mostly by @targos and @refack.
Any other build system is not going to solve the add-on problem either. We should not conflate building core with building add-ons imo.
It should be a lot easier to build with sanitizer instrumentation with GN. From experience, sanitizer builds provide a lot of value in finding memory leaks and race conditions.

mhdawson commented 6 years ago

I have no opposition with GN as an alternative but do want to ask on question. I think last time we talked about GN as an option the message from the v8 team was that GN was not really supported for use other than with Chrome. Has that changed?

hashseed commented 6 years ago

This has not changed in that we cannot expect the GN team to provide any help and support for the Node.js use case, if it differs from what it offers to Chromium.

refack commented 6 years ago

Having GN side by side IMHO is worth discussing...

I'm not convinced that checking in what will essentially be "dead code" (WRT to node core and our CI) is good practice. It might be a good will gesture towards Electron, but it is not free for us. (And I totally understand Electrons motivation, since AFAIK they need to GN build Chromium as well)

The choice of build system in core doesn't necessarily have anything to do with how the add-ons are built. We don't have to conflate these two issues.

@ofrobots, @hashseed The conflation is intentional. node-gyp gets almost 0 maintenance, and it is a real pain point. Reducing coverage on GYP will IMHO indirectly hurt node-gyp.

On the maintenance issue... If Electron has decided to build Node.js with GN, when the GN files are broken, there will be people making sure they are fixed anyway - there is a company (and another giant company very soon) behind that project, after all. Worst case scenario, they probably won't fix the GN files for all the platforms we support and won't port all of our flags to GN, and we might potentially get bug reports from people trying to use those flags with GN files or on unsupported platforms, we can either leave the issues open, or remind them that we do not maintain those files and close the issue.

@joyeecheung The maintenance issue is the crux of this. IIUC what you are describing is essentially us tracking dead code. Code that we don't use nor maintain nor CI... In that case I don't see the upside to any party.

What I'm missing here is the plan for the day after we check it in. If by having a GN scaffolding we can eliminate the need for https://github.com/v8/node and https://github.com/electron/node/ and get some more resources and contributors then I'm sold. But as long as those two repos exist I don't see the benefit to node core. Who is going to maintain the GN scaffold? Who is going to port changes between the two? Who is going to CI this? For example i'm working on significantly improving the dev workflow on Windows in https://github.com/nodejs/node/pull/23439. Will I need to port this to GN? That's a very inhibitory requirement.

tl;dr I wasn't at collab summit, and wasn't a part of the discussion. So please catch me up...

refack commented 6 years ago

On the maintenance issue, we don't support building ninja in core either, but you can still build node with ninja with build files generated from GYP files anyway, and we even have a document on how to do that.

P.S. that's a good example, but it does not strengthen your point. building with ninja doesn't work. And ATM I'm the only one maintain it for my personal use. So it's a very good example of how quickly dead code rots.

joyeecheung commented 6 years ago

P.S. that's a good example, but it does not strengthen your point. building with ninja doesn't work. And ATM I'm the only one maintain it for my personal use. So it's a very good example of how quickly dead code rots.

Yeah, ninja is probably not a good example, but then I don't think ninja in core is depended by any project with significant velocity?

If by having a GN scaffolding we can eliminate the need for https://github.com/v8/node and https://github.com/electron/node/ and get some more resources and contributors then I'm sold. But as long as those two repos exist I don't see the benefit to node core. Who is going to maintain the GN scaffold? Who is going to port changes between the two? Who is going to CI this?

Judging from my observation those two won't be eliminated even with the GN scaffold because the flaky test policies or branching stories there are just different from this repo. There may not be significant benefit for node core, but is there significant harm in not maintaining them? Overhead of closing issues, maybe?

For example i'm working on significantly improving the dev workflow on Windows in #23439. Will I need to port this to GN? That's a very inhibitory requirement.

AFAICT, not porting the changes in #23439 doesn't really break anything. If the main consumers of GN don't really care, there is no reason to require anybody to port changes to GN, and whoever need those can port the changes themselves in subsequent PRs.

refack commented 6 years ago

Yeah, ninja is probably not a good example, but then I don't think ninja in core is depended by any project with significant velocity?

😄 ninja is the best part of the chromium build toolchain. It's make without the cruft. The only builder that I found that can do reproducible-builds (IIUC it actually hashes the inputs, not just stat them). And it's cross platform. We should build node with ninja by default 🎉

joyeecheung commented 6 years ago

@refack No I mean, ninja support (i.e. configs for ninja, not ninja itself) in core is broken because it's not depended by any project, only by human(s?) who want to use ninja, so it's not exactly in the same situation as GN support in core. If only a few humans use the config, then only the human who uses it will want to fix it, but if there is a project using the config, the project maintainers should be inclined to fix it - potentially, that's a larger number of humans.

refack commented 6 years ago

Ahhh. Yes that sounds like the right analysis.

hashseed commented 6 years ago

I'm not convinced that checking in what will essentially be "dead code" (WRT to node core and our CI) is good practice. It might be a good will gesture towards Electron, but it is not free for us. (And I totally understand Electrons motivation, since AFAIK they need to GN build Chromium as well)

By the same argument V8 should never have added tools/gen-postmortem-metadata.py or platform support for FreeBSD, AIX, Cygwin, QNX, Solaris, and OpenBSD either. These are dead code to us too, and only maintained by whoever needs them downstream. More often than not these files get out of date, but that never caused any trouble for us.

I would consider it a success if the GN config files are kept up-to-date with only the bare minimum set of features, and not necessarily include options not used by downstream users. How about we land these files and see how things work out?

refack commented 6 years ago

How about we land these files and see how things work out?

Wasn't it your team that removed the gyp scaffolding from V8 since it was deprecated 😉

I'm not totally opposed, I'm just not sold yet. I'm worried about maintenance, and I still don't see the upside, (if all the maintenance will be done down stream anyway) but I'm willing to listen.

Let see the PR and figure out how this is going to work.

hashseed commented 6 years ago

Wasn't it your team that removed the gyp scaffolding from V8 since it was deprecated

touché :)

I was actually also hoping that GN would benefit Node.js as well, from being integrated, e.g. providing a wider window when upgrading V8, e.g. node canary would not be blocked on gyp configs. Also, sanitizers.

joyeecheung commented 6 years ago

Would it make sense if we use GN to build the canary? It should be easier to port our necessary GYP changes to GN to build that (I guess the necessary changes are probably mostly file list changes, maybe a build flag or two once in a while) than to port V8 GN changes to GYP, so that when the bot bumps V8 in the canary it'll be easier to fix and we will not have all the canary releases missing until the GYP files are fixed with V8 lkgr. One downside is that we will not be notified about GYP breakage sooner, but it seems to be a lot of overhead to keep the build working with lkgr when we only land stable on master.

targos commented 6 years ago

@joyeecheung I would be very happy if we could do it. It would probably spare me ~30 minutes per week.

refack commented 6 years ago

Would it make sense if we use GN to build the canary?

Yes and no. For one we could do it today with ./configure --build-v8-with-gn. But the v8-canary is built and tested in our CI cluster which doesn't have GN...

One downside is that we will not be notified about GYP breakage sooner, but it seems to be a lot of overhead to keep the build working with lkgr when we only land stable on master.

AFAIK we don't test with LKGR, we test with the head of the new dev branches, and porting to gyp is work that needs to be done anyway. From my experience there where few instances when that work had to be undone because of V8 dev branch changes.

IMHO we could think about creating auto-porting tools. The feature set and metaphors used of GN and GYP are not that dissimilar. See @nornagon's https://github.com/electron/node/pull/63 Actually I think that building a limited GYP-GN bridge transpiler that covers our use cases is not unreasonable. (RE @hashseed's mention of other GN benefits)

refack commented 6 years ago

It would probably spare me ~30 minutes per week.

BTW: this is exactly what I was asking during the V8 gyp deprecation process. Who is going to do the porting (https://github.com/nodejs/TSC/issues/411), and is part of my apprehension regarding this change. I could make the argument that the V8 team could help with gyp porting, since AFAIK they have more resources than us...

sam-github commented 6 years ago

I use ninja, the gyp builds on my machine occaisonally don't build the the js2c files, for reasons I haven't been able to track down through the muck that is gyp. The ninja builds don't have this problem. So, thanks for working on them!

@refack Here's the consensus among the people sitting around the table as I understood it, I hope I don't misstate anyone.

A number of us (including me) share your concern about dead code drops. After a fair bit of discussion and airing of the problems that could occur, everyone present seemed to agree that:

delivering gn via our own node repo would have advantages to multiple downstream embedders (not just electron)
It could be of benefit in our own ci to run memory and other error checking builds, apparently there are some sanitization capabilities built into gn we could use even if we only did the build on linux.

Very importantly, electron committed to maintaining the gn files. Its something they do already, just out of tree. I assume if that doesn't happen and they are left to rot we would rip the gn files out again.

Electron is floating some specific to them patches they don't think they can upstream, but some more concerted effort has kicked off to get what they can upstream, I think I saw some folks clustered around laptops. There was some conversation in particular about upstreaming changes that are potentially useful for any node embedder.

hashseed commented 6 years ago

Building a GN to GYP transpiler or vice versa for a single use case sounds like more work than worthwhile tbh.

--build-v8-with-gn works with caveats. Specifically, V8 has ICU as dependency, and Node does too. That means using the hybrid build, ICU is built twice, one shipped with Node and one shipped with gclient with V8. If the versions mismatch, we run into crashes at runtime. That has forced us to sync ICU updates between Node and Chromium. With a full GN build this would not be necessary.

mmarchini commented 6 years ago

--build-v8-with-gn works with caveats. Specifically, V8 has ICU as dependency, and Node does too. That means using the hybrid build, ICU is built twice, one shipped with Node and one shipped with gclient with V8. If the versions mismatch, we run into crashes at runtime. That has forced us to sync ICU updates between Node and Chromium. With a full GN build this would not be necessary.

Does that mean we would need to use gclient sync to build node, as well as have the entire depot_tools installed?

It could be of benefit in our own ci to run memory and other error checking builds, apparently there are some sanitization capabilities built into gn we could use even if we only did the build on linux.

Sanitizers come from GN or clang? If they come from clang we could probably use them with gyp. Also, I think I saw some PRs here to add support to build node with sanitizers.

To be clear, I'm not opposed to add GN to the project, I'm just trying to understand the implications.

refack commented 6 years ago

Here's the consensus among the people sitting around the table as I understood it, I hope I don't misstate anyone.

Thank you very much @sam-github. I have voiced concern in the past that collab summit can turn into an exclusive decision making forum (IIUC the mandate is for high bandwidth discussion that is brought back to GitHub for consensus seeking). For the sake of inclusivity, I really appreciate you bringing the context back to the public!

Very importantly, electron committed to maintaining the gn files. Its something they do already, just out of tree. I assume if that doesn't happen and they are left to rot we would rip the gn files out again.

👍 👍 👍

That means using the hybrid build, ICU is built twice

@hashseed (It is actually built 3 times: host&target for V8 and once more for node) Anyway that's a bug, I'm sure we can fix anyway

Does that mean we would need to use gclient sync to build node, as well as have the entire depot_tools installed?

IIUC no. If I'm wrong, and we do need depot_tools that is a problem.

hashseed commented 6 years ago

Depot tools come with GN and ninja binaries, so yeah. Gclient sync is only necessary to fetch dependencies, which for Node we probably don't need, as the dependencies live in the same repository. Unless of course you want to ensure a particular version of clang to be fetched, like done for Chrome and V8.

hashseed commented 6 years ago

I thought we only build separately for host and target for cross compiled builds, e.g. arm?

nornagon commented 6 years ago

FYI, I've been working on a prototype of this here: https://github.com/electron/node/tree/gn-upstreaming

It's not ready to be a PR yet, but it does successfully build node (at least, on my machine 😉).

One tricky piece is that v8's GN files depend on Chromium's build/ infrastructure, which is fairly large and which I don't think would make sense to vendor into the node repo. I've solved that hackily in my current branch with a little setup script that fetches the repository, but there's probably a more sensible way to do it.

joyeecheung commented 6 years ago

I have voiced concern in the past that collab summit can turn into an exclusive decision making forum (IIUC the mandate is for high bandwidth discussion that is brought back to GitHub for consensus seeking). For the sake of inclusivity, I really appreciate you bringing the context back to the public!

I think by around the table @sam-github was talking about this thread? There wasn't a physical table where we actually sat down and seeked consensus about GN v.s. GYP AFAIR (and most sessions were like brainstorming and demoing instead of consensus seeking). I didn't go to every session but I couldn't find any session specific to build in the agenda either.

(At least all the tables that I could remember sharing with people in this thread had a lot of food on them and we weren't talking about code that much :S The sudden wave of replies probably stemmed from chats but AFAIK no decisions were attempted to be made there)

sam-github commented 6 years ago

@joyeecheung there was a physical table, it was the LTS and Release group discussion. @addaleax was present and probably has a better memory than me who else was there.

nodejs / node

Build node with GN #21410