serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Meta: Add an experimental GN build

Open ADKaster opened this issue 2 years ago • 7 comments

This PR adds the ability to build ladybird with the GN build system

https://gn.googlesource.com/gn/#gn

The build is incomplete in that it only knows how to build Ladybird, and not serenity itself. GN offers more ways to parallelize the build than CMake, while supporting cross-compilation and "host tools" in a more natural and native way than CMake.

On my MacBook Pro, building ladybird from 0 ccache and zero build directory with the CMake build takes this long:

[2780/2780] Linking CXX executable Ladybird/ladybird.app/Contents/MacOS/ladybird

real	3m7.292s
user	21m38.676s
sys	2m50.478s

While using gn it takes this long:

$ gn gen out && time ninja -C out
Done. Made 2603 targets from 130 files in 142ms
ninja: Entering directory `out'
[23/5930] ACTION //Userland/Libraries/LibTLS:ca_certificates_download(//Meta/gn/build/toolchain:unix)
Downloading version 2023-01-10 of cacert.pem...done
[46/5930] ACTION //Userland/Libraries/LibTimeZone:timezone_database_download(//Meta/gn/build/toolchain:unix)
Downloading version 2023b of gen/Cache/TZDB/tzdb.tar.gz...done
[3044/5930] ACTION //Userland/Libraries/LibUnicode:emoji_test_download(//Meta/gn/build/toolchain:unix)
Downloading version 15.0 of gen/Cache/EMOJI/emoji-test.txt...done
[3460/5930] ACTION //Userland/Libraries/LibUnicode:unicode_database_download(//Meta/gn/build/toolchain:unix)
Downloading version 15.0.0 of gen/Cache/UCD/UCD.zip...done
[3682/5930] ACTION //Userland/Libraries/LibLocale:locale_database_download(//Meta/gn/build/toolchain:unix)
Downloading version 42.0.0 of gen/Cache/CLDR/cldr.zip...done
[5930/5930] STAMP obj/default.stamp

real	1m53.155s
user	17m54.816s
sys	1m34.867s

Those numbers are kind of cheating since the CMake build does all its file downloads in the configure step. If we blast Build/lagom but keep Build/Caches and run again without ccache, the CMake build takes only

real	2m22.117s
user	20m29.872s
sys	2m34.071s

cc @nico @trflynn89

ADKaster avatar May 05 '23 20:05 ADKaster

A couple of flyby comments:

  • It'd be a lot more ergonomic if the BUILD.gn were in the the folder of the sources themselves, rather than having them in //Meta/gn/secondary/... (or is this just temporary while the GN build is experimental?)
  • Let's use 4 spaces for tabs instead of google's 2-space style? :) (at least for files not borrowed from LLVM)
  • Do we need to attribute LLVM or Google for those files we grabbed from them?

trflynn89 avatar May 06 '23 13:05 trflynn89

It'd be a lot more ergonomic if the BUILD.gn were in the the folder of the sources themselves, rather than having them in //Meta/gn/secondary/... (or is this just temporary while the GN build is experimental?)

Intended to just be an experimental thing. If we want to adopt it as a more official secondary build system then I can for sure get rid of the secondary build directory and put the files in the standard tree.

Let's use 4 spaces for tabs instead of google's 2-space style? :) (at least for files not borrowed from LLVM)

I'm not sure if we can convince gn format to give us four spaces, it's pretty opinionated on its style. If we want to fully manually format the files then we can go with four spaces though 🤷

Do we need to attribute LLVM or Google for those files we grabbed from them?

I'm not sure honestly. The LLVM files are quite similar to ones in Chromium (since Nico added them). They don't have a license identifier in LLVM, but their counterparts in chromium seem to. The ones I would think are copy-paste enough would be the BUILDCONFIG.gn and its friends (the gni files for that), and the write_cmake_config.py/gni, and compiled_action.gni The rest of the lifted files are either trivial or substantially modified imo.

ADKaster avatar May 06 '23 16:05 ADKaster

Updates since last reviewed:

  • formatted all files with gn format
  • default to symbol_level=1
  • removed library_name.gni and set liblagom- prefix in toolchain/BUILD.gn
  • documented args
  • removed unnecessary FIXMEs
  • couple other housekeeping things Nico noticed
  • use a temp file in download_file.py

ADKaster avatar May 12 '23 00:05 ADKaster

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why. Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

BuggieBot avatar May 28 '23 15:05 BuggieBot

Updates since last:

  • Fixed up build so that it works on Ubuntu 22.04 with gn args as below
host_cc = "clang"
host_cxx = "clang++"
is_clang = true
use_lld = true

ADKaster avatar May 28 '23 15:05 ADKaster

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

stale[bot] avatar Jun 18 '23 22:06 stale[bot]

This pull request has been closed because it has not had recent activity. Feel free to re-open if you wish to still contribute these changes. Thank you for your contributions!

stale[bot] avatar Jun 26 '23 08:06 stale[bot]