I can't easily test this since only some of our bots see this and I don't have a Linux box at hand, but it looks like it should work and I can verify on the bots once this is in :)

Thanks!

Nice!!

I think it might make sense to pretty soon add build/{android,...}/BUILD.gn that defines a build/{android,...}_compiler config that's added to the config list soon, instead of expanding this duplicate way to define compiler flags.

address comment

Nice! There are enough comments below that I won't hit "accept" just yet, but none of the comments are blocking and you can talk me out of all of them if you disagree with the suggestions.

Ah right, this from before I generated llvm-config.h long long ago. Well, that was easy :-)

Out of interest, what's the motivation for this? It seems to add way more code than it removes, so there must be some other advantage, but the patch description doesn't say.

move aarch64, arm to production quality

Super cool!

lgtm!

lgtm assuming that explicitly mentioning the default toolchain in a label is a no-op.

lgtm. What about the rest of git grep root_out_dir llvm/utils/?

(landed my change; thanks for the review!)

There are many many references to root_out_dir, cmd-f root_out_dir on the lhs in D56487 for example. Does it make sense to change them all at once? Or should we land D56487 fist and at least change all the write_cmake_config ones at once?

Thanks for the patch! Please run git ls-files '*.gn' '*.gni' | xargs -n 1 gn format after making GN changes. (I forgot that for the PowerPC target too, so I'm running it for this file here too, so nothing to do for you, just pointing it out for next time.)

Ok, I've abandoned this revision. Thanks for the feedback, everyone!

…can you land this? :-)

(This would also help with https://crbug.com/917351)

I'd be interested in feedback from more people if anyone has any (e.g. "don't care"). Given the current feedback, I'll put this on hold for now.

I'd prefer to land this as a txt file:

• rst requires a newline after toplevel list entries to accept nested lists, which makes the txt file less readable
• md doesn't, but it insists on `` around every string that contains an underscore, else parts of the word become italic

In D56419#1349087, @kristina wrote:

I'm neutral on the issue but I would urge Nico to really think this through, lashback from some major downstream Linux distro vendors last time was pretty nasty, including flat out dropping Clang/LLVM support if CMake became a second class citizen. Is rehashing the same thing again worth it? Consider that a lot of people, no matter what you say will see it as GN slowly creeping in to replace CMake, not many are familiar with it outside Google or Chromium/Skia contributors.

I don't see the downside of adding a hidden file at the toplevel yet.

The change promotes an unsupported, parallel, build system to the top of LLVM. It's precisely the situation I and others who spoke up on llvm-dev were trying to avoid.

In D56419#1349014, @brzycki wrote:

I am strongly against this commit. @thakis when this was discussed on llvm-dev the original proposal was the compromise of placing .gn in utils to signify that it was not the default, recommended, method to build LLVM. With this change LLVM will truly have two, parallel build systems once again.

Lgtm

In D53945#1343312, @thakis wrote:

Out of interest, why aren't most of the tests for this lit tests? The usual llvm way of testing stuff is to write a small llvm tool using your library (which will be useful outside of a testing context too) and then use that to write lit tests.

Thanks!

way better

remove needless static_library target

no template

It looks like I need to add knowledge about the tests going in target_out_dir in two other places, so I think I'll remove the new template() here and put this directly in the build file of the test, and just add a comment in the gni file along the lines of If you change output_dir, look through git grep target_out_dir '*/unittests/*'`.

In D56195#1346133, @serge-sans-paille wrote:

LGTM, but *only* with respect to the Python 2/3 compatibility of the modification, which is just a drop in the ocean of this patch.

pulled some stuff out into dependent patches to make this smaller

phosek: ping :-) Almost done!

Looks like dblaikie is away for a bit. rnk, would you mind looking at this one too?

remove require

forgot to add test