Add gn conversion aids #29
Conversation
…update a file if its contents haven't changed. However, we need to rerun torque when torque itself changes. In this case, the timestamp of torque is updated but all torque_outputs still have the old timestamp and are NOT updated since their contents hasn't changed. This leaves us in the unfortunate position of rerunning torque (which changes nothing) on each build. The solution is to make sure that all torque outputs get touched after torque is run. In the case where torque updates but no on-disk changes occur, we'll rebuild all torque outputs, but this is rarer.
…ncorporating tests) as v8 is updated. There are several parts to this: 1. Lists of files should be cut/pasted as simply as possible. Presently, there is cut/paste and then an editing pass to prepend directories, etc. It is even worse with generated variables. target_relative_sources and target_conditional_relative_sources are the solution 2. Configs from gn are broken out into separate calls to establish defines/flags/dependencies and must be reapplied individually on each target. config_defines/config_flags/config_include_dirs define these under an umbrella configuration name and target_config applies configurations to targets. This reduces the repetitive text in CMakeLists.txt. This will show up in the next commit.
Adds common configs Converts target d8 to use configs
Adds internal_config_base to v8-i18n-support
Removes unused parameter in config_include_dirs
Changes target_conditional_relative_sources to take pairs of conditional/srcfile
|
Would it be possible for you to break this up into smaller PRs? There are some changes in here that look good to me, others that can probably go in with some rework, and yet others that I probably won't merge, but it's kind of hard to pull apart right now. For the record, it's not my goal to reach feature parity with the GN build. I'm aiming for simple: simple build targets, not too many build-time knobs. If there's a particular direction you want to see the project go, can you open an issue (or issues) to discuss? Thanks. |
|
Pretty much each commit could be a separate PR. I wholly agree with your goals. Turning cmake into GN serves no one's purposes. My (nearly orthogonal) goals in this were:
Both of these involved a bunch of repetitive editing which you can see with the ${D} prefixes. Adding the tests will involve doing similar, but different editing. This is the motivation for target_relative_sources and target_conditional_relative_sources. Also, there is repetition (which can be a source of bugs) in the setting of include_dirs, definitions, and cflags. Much better to be able to name a group of settings and use that name to apply them to a variety of targets instead of individually invoking target_include_directories/target_compile_definitions on each one. This is the motivation for doing a lightweight implementation of configs. The file cmake/GNUtils.cmake has these functions. Almost all of the other changes have to do with changing the existing targets to use this. All this being said, I am interested in collaboration rather than running off in my own direction. I propose:
How does this sound? |
|
Just added the PR for cmake/GNUtils.cmake plus two targets. Starting point for discussion! |
|
Thanks for your comments.
That's a laudable goal, no objection. :-)
With patch releases it's already pretty easy: run With minor releases there is often minor build breakage to fix but I'm okay with that, it's inevitable. V8 sees a lot of churn, things move around a lot, etc. It probably takes me 30 minutes every 6 weeks. I'm willing to put up with that. :-)
I agree (the As a rule of thumb, I want to have as few layers as possible. If you have suggestions on how to combine those two objectives, I'd love to hear them. :-)
It exists because I wasn't sure yet what the directory layout would look like in the beginning but that's settled now. Can you take a look at #31? |
|
In bringing in the tests, I need to wander through a bunch of BUILD.gn files that all have paths (sources and includes) >relative< to the directory with BUILD.gn. Converting all these to be relative ${PROJECT_SOURCE_DIR} is just work. Work that is different for each place where BUILD.gn lives. All told, just to bring in tests/unittests is around 1k lines of CMake stuff. And then there are a bunch of other tests, too. My rule of thumb is the DRY principle (Don't Repeat Yourself) and it's is a fine line when to apply it. I agree that you can macro-ize/function-ize and hide essential meaning in layers and layers but it needs to be traded off against utility/simplicity/maintainability. We don't make people write the individual details of formatted output in their code. We give them printf. In CMakeLists.txt, you use file(GLOB...) instead of explicitly listing the files in use. With regards to #31 you've traded one sort of global edit (putting in ${D}) with another (putting in ${PROJECT_SOURCE_FILES/v8). As a quick example, here's the "simplified" version of a bit of the unittests: And this is what the expanded version would look like: |
Adding in a bunch of test targets from the BUILD.gn files involves a whole pile of repetitive editing. I've added cmake functions that will assist in this as well as bringing in the concept of configs from gn to remove repetitive settings of defines/include_dirs/etc. This should make the updating of CMakeLists.txt easier when new releases of v8 are processed.
Notable:
This is the conversion of CMakeLists.txt to use the new routines.