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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to