[PATCH] D89013: [libcxx] Support per-target __config_site in per-target runtime build

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I agree the design is clean from the "what we ship" perspective. I really like the idea of shipping the whole library in a generic spot and only having one file embody all the target specific configuration of the library. I think it's a very elegant way to handle the configuration of libc++, and it will have positive effects beyond your intended use case like giving us a place and a way to push platform-specific hacks to.

Where this really stinks IMO is in the way we build the library. I don't want us to have a separate `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` setting and a different mode for building libc++. This is at the heart of many discussions we've been having lately. Basically, the way I do things:

1. Build libc++ for one target using the most straightforward possible CMake settings
2. Then, post-process that by moving headers around, creating universal binaries, etc, all of it from a separate script.

That's how I set up the libc++ build at Apple because I didn't want to add too many platform specific things in the CMake. Otherwise we'd have e.g. a mode for building for many architectures (`x86_64` and `arm64`) at once on Apple platforms and creating a universal dylib from that. That's not the way CMake is intended to be used unless I fundamentally missed something. Also note that it would be much better if CMake had proper support for multi-arch, but I'm not aware of anything like that.

On the contrary, your approach (please correct me if I'm mis-speaking) is to make the CMake work out of the box exactly as you want it, even if that means adding complexity. This has the nice benefit that everything is done in a single place, however it forces us to go against the way CMake was designed in a few places, for example when it comes to building the library for several platforms.

My only push back on this patch, and on several patches in the same vein, is about this fundamental difference in approach that we have.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89013/new/

https://reviews.llvm.org/D89013