[libcxx-commits] [libcxxabi] [runtimes] Remove explicit -isysroot from the testing configurations on macOS (PR #66265)

[Petr Hosek via libcxx-commits]

petrhosek wrote:

> How do you build on Apple platforms then? Don't you need access to a SDK? I'm a bit confused about what you're doing -- do you need to use the native platform SDK when you're building Fuchsia? What SDK are you passing as `--sysroot` when testing libc++?

We use `-isysroot` with the Xcode SDK. That doesn't require selecting active developer directory. You only need to select active developer directory if you are going to run binaries from the Xcode SDK (or other binaries that assume so like `xcrun`) which we don't since all our tools come from LLVM.

In the future, we hope to switch to LLVM libc on all platforms to make our build truly hermetic. At that point, we won't be using Xcode SDK at all and at that point forcing the use of `xcrun` is going to be even more undesirable.

> I know that, but as I already explained over in https://reviews.llvm.org/D151056, making everyone's config more complicated isn't the path we want to take. We should never have been setting the sysroot, it was only a workaround for https://gitlab.kitware.com/cmake/cmake/-/issues/19180 and that should have been done another way initially.
>
> These config files were designed to be really simple and to be easily created for custom needs. This was an answer to the incredible amount of platform-specific complexity that the previous test configuration system had, and what I'm doing right now is push back to avoid going down that path again.

That's reasonable, but these configs still implicitly encode assumptions, such as the fact your host environment is usable for building binaries. For example, if you don't specify `--sysroot=`, the compiler will use `/`, but if your host sysroot doesn't have the necessary files (i.e. headers and libraries), building tests will fail.

In our case, the fact that the host environment isn't usable for building binaries is intentional, we always build against an explicit sysroot (using `--sysroot`, `-isysroot` or `/winsysroot` depending on a platform) to ensure our build is hermetic as was suggested in https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html, see the "Getting to universal determinism" which says:

> #### Getting to universal determinism
> You then need to modify your build files to use `--sysroot` (Linux), `-isysroot` (macOS), `-imsvc` (Windows) to use these hermetic SDKs for builds. They need to be somewhere below your source root to not regress build directory name invariance.
> You also want to make sure your build doesn’t depend on environment variables, as already mentioned in the “Getting to incremental determinism”, since environments between different machines can be very different and difficult to control.

> Is there a reason why you don't have a `fuchsia-*.cfg.in` configuration file?

We plan to introduce one, but this issue is not about building and running tests on Fuchsia, the issue is building and running tests for other platforms we build and distribute runtimes for like Linux, macOS and Windows.

> > We also need to restrict the `xcrun` change only to `apple-*.cfg.in` configurations.
> 
> No, it is needed whenever the build host is an Apple platform, which is more general than the `apple-*.cfg.in` configurations. The normal LLVM configuration also needs to run under xcrun when it is being built on macOS, cause that's how you get to the SDK.

You seem to be assuming there's a single way to build the binaries for any given platform, but that's generally not the case:

* On macOS, you can either (1) run the binary under `xcrun`, which sets the `SDKROOT` environment variable, or you can (2) use the `-isysroot` flag. These can be used interchangeably, see: https://github.com/llvm/llvm-project/blob/5d86176f4868771527e2b7469c1bc87f09c96f0a/clang/lib/Driver/ToolChains/Darwin.cpp#L2144-L2161
* On Windows, you can (1) set the environment variables by running the `vcvars*.bat` file that's provided by VS SDK, or you can (2) use the `/winsysroot` flag which was created to simplify using hermetic SDK and avoid relying on environment variables: https://reviews.llvm.org/D95534

I'm fine choosing picking defaults, like using `xcrun`, but I don't think we should be forcing those onto all users with no way to opt out as is currently the case in this patch.

> I will land this tomorrow unless there is additional discussion. It looks like #67201 would allow you to solve your problem without creating a custom `.cfg.in` file, which is great. But even regardless of #67201, the intended way to use the libc++ test suite is to create a `.cfg.in` file that suits your needs if the general ones don't, not to add more configuration options to the general ones.

#67201 is an improvement but is not an idiomatic CMake. The idiomatic way to set sysroot in CMake is through `CMAKE_SYSROOT=/path/to/sysroot`, not `CMAKE_CXX_FLAGS_INIT="--sysroot=/path/to/sysroot"`. In practice, it means we'll have to set both, because other runtimes like compiler-rt use `CMAKE_SYSROOT`. So these changes aren't really reducing the overall complexity, they're increasing it (by overall I mean the scenario where you're building all runtimes, not just one of the runtimes like libc++).

https://github.com/llvm/llvm-project/pull/66265