sbc100 added inline comments.
================ Comment at: clang/CMakeLists.txt:179-183 +if(DEFAULT_SYSROOT) + message(WARNING "DEFAULT_SYSROOT is deprecated and will be removed. Use " + "configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)" + "to specify the default --sysroot=") +endif() ---------------- MaskRay wrote: > bcain wrote: > > MaskRay wrote: > > > MaskRay wrote: > > > > bcain wrote: > > > > > MaskRay wrote: > > > > > > bcain wrote: > > > > > > > At one time I believe that the clang configuration files could > > > > > > > not specify paths relative to the clang executable. AFAICT > > > > > > > `DEFAULT_SYSROOT` does support this. > > > > > > > > > > > > > > But if I'm mistaken about that can we add an example to the docs > > > > > > > at > > > > > > > https://clang.llvm.org/docs/UsersManual.html#configuration-files > > > > > > > illustrating how to use a relative sysroot? > > > > > > Clang configuration files just complement user-specified command > > > > > > line options. As one can do `--sysroot=./sysroot`, one can add > > > > > > `--sysroot=./sysroot` to a configuration file, too. > > > > > > > > > > > > If you think having a sysroot example is useful, I can add > > > > > > > > > > > > ``` > > > > > > # Relative --sysroot > > > > > > --sysroot=./sysroot > > > > > > ``` > > > > > > before > > > > > > clang/docs/UsersManual.rst:1018 `-c > > > > > > --target=x86_64-unknown-linux-gnu` > > > > > IIUC: when clang takes a `--sysroot=./sysroot` argument, it will > > > > > interpret that path as a prefix to the files it wants to access. So > > > > > the system will treat it as relative to the environment's `cwd`, > > > > > correct? > > > > > > > > > > But when `DEFAULT_SYSROOT` is set to a relative path, that relative > > > > > path is considered to be relative to `clang`. Therefore a convenient > > > > > feature that we take advantage of is setting it to something like > > > > > `../target/hexagon-unknown-linux-musl` in order to have anyone who > > > > > invokes `hexagon-unknown-linux-clang` from any path be able to find > > > > > the includes and libraries distributed with the toolchain without > > > > > having to specify the sysroot. > > > > > > > > > > Maybe there's a better way to achieve this without the need for a > > > > > relative `DEFAULT_SYSROOT` but it's been very useful and the config > > > > > files do not seem suited to replace it. > > > > `--sysroot=` is used as a prefix to certain files, primarily libc and > > > > GCC installations. > > > > > > > > `-DDEFAULT_SYSROOT=...` just changes `clang/lib/Driver/Driver.cpp:203` > > > > `SysRoot(DEFAULT_SYSROOT)`. > > > > There is no magic related to the `clang` executable path. CMake doesn't > > > > do any magic, either. > > > Ah, sorry. There is magic: D76653 (@sbc100). > > > Ah, sorry. There is magic: D76653 (@sbc100). > > > > Okay, so should we abandon the plan to deprecate `DEFAULT_SYSROOT`? Or > > could we add a corresponding feature to config files if we want to get rid > > of it? > The way relative `DEFAULT_SYSROOT` looks strange to me as it is not exactly > equivalent to user-specified `--sysroot=`. I hope that there is a way to > achieve @sbc100's goal. > > I think a proper mechanism will take time, and we probably can just > deprecated `GCC_INSTALL_PREFIX` for now. > And I wish that RHEL/CentOS can migrate to a modern practice. We use `DEFAULT_SYSROOT` in wasi-sdk to add clang-relative default sysroot: https://github.com/WebAssembly/wasi-sdk/blob/f3b43c703f1a3b6a93fcbdac9cb2b8439adcf0c1/Makefile#L78-L82 You can see that before D76653 landed it was not possible to make a relocatable version of the SDK that was independent of its installation path. The clang-relative path is pretty useful for this purpose and avoids the need to modify a config file to include that installation path, or have the user specify `--sysroot` each time. If we could find a way to specify a clang-relative path in the config file I think we could remove `DEFAULT_SYSROOT` completely. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158218/new/ https://reviews.llvm.org/D158218 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
