Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CMake] Deprecate GCC_INSTALL_PREFIX #77537

Merged
merged 1 commit into from
Jan 10, 2024
Merged

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jan 9, 2024

Part of https://reviews.llvm.org/D158218

GCC_INSTALL_PREFIX is a rarely-used legacy option inherited from
pre-CMake build system and has configuration file replacement nowadays.
Many clang/test/Driver tests specify --gcc-toolchain= to prevent
failures when GCC_INSTALL_PREFIX is specified: some contributors add
them to fix tests and some just do cargo culting. This is not healthy
for contributors adding cross compilation support for this rarely used
option.

DEFAULT_SYSROOT should in spirit be deprecated as well, but a relative
path doesn't have good replacement, so don't deprecate it for now.

Link: https://discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link: https://discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833


With GCC_INSTALL_PREFIX=/usr, clang a.c behaves like
clang --gcc-toolchain=/usr a.c.

Here is a simplified version of GCC installation detection code.

if (OPT_gcc_install_dir_EQ)
  return OPT_gcc_install_dir_EQ;

if (OPT_gcc_triple)
  candidate_gcc_triples = {OPT_gcc_triple};
else
  candidate_gcc_triples = collectCandidateTriples();
if (OPT_gcc_toolchain)
  prefixes = {OPT_gcc_toolchain};
else
  prefixes = {OPT_sysroot/usr, OPT_sysroot};
for (prefix : prefixes)
  if "$prefix/lib/gcc" exists // also tries $prefix/lib/gcc-cross
    for (triple : candidate_gcc_triples)
      if "$prefix/lib/gcc/$triple" exists
        return "$prefix/lib/gcc/$triple/$version"; // pick the largest version

--gcc-toolchain= specifies a directory where
lib/gcc{,-cross}/$triple/$version can be found. If you actually want
to use a specific version of GCC, specify something like
--gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11 in a configuration
file. You can also specify --gcc-triple=.

On Debian and its derivatives where the target triple omits the vendor part, the following ways are roughly equivalent, except that --gcc-install-dir= specifies a version as well:

clang --gcc-toolchain=/usr a.c
clang --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11 a.c
clang --gcc-triple=x86_64-linux-gnu a.c

@MaskRay MaskRay requested a review from zmodem January 9, 2024 22:53
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jan 9, 2024
@MaskRay MaskRay requested review from tstellar and petrhosek January 9, 2024 22:53
@llvmbot
Copy link
Member

llvmbot commented Jan 9, 2024

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

Changes

Part of https://reviews.llvm.org/D158218

GCC_INSTALL_PREFIX is a rarely-used legacy option inherited from
pre-CMake build system and has configuration file replacement nowadays.
Many clang/test/Driver tests specify --gcc-toolchain= or
--gcc-toolchain="" to prevent failures when GCC_INSTALL_PREFIX is
specified: some contributors add them to fix tests and some just do
cargo culting. This is not healthy for contributors adding cross
compilation support for this rarely used option.

DEFAULT_SYSROOT should in spirit be deprecated as well, but a relative
path doesn't have good replacement, so don't deprecate it for now.

Link: https://discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link: https://discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833


Full diff: https://github.com/llvm/llvm-project/pull/77537.diff

1 Files Affected:

  • (modified) clang/CMakeLists.txt (+6)
diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 9f814478c45503..7076907da2528a 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -193,6 +193,12 @@ set(C_INCLUDE_DIRS "" CACHE STRING
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
 set(DEFAULT_SYSROOT "" CACHE STRING
   "Default <path> to all compiler invocations for --sysroot=<path>." )
+if(GCC_INSTALL_PREFIX)
+  message(WARNING "GCC_INSTALL_PREFIX is deprecated and will be removed. Use "
+    "configuration files (https://clang.llvm.org/docs/UsersManual.html#configuration-files)"
+    "to specify the default --gcc-install-dir= or --gcc-triple=. --gcc-toolchain= is discouraged. "
+    "The option is often unneeded or better replaced with --gcc-install-dir=/--gcc-triple.")
+endif()
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
 

@MaskRay MaskRay requested a review from smeenai January 9, 2024 22:54
Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of removing this. It doesn't make a lot of sense now that we have better config file support.

@zmodem
Copy link
Collaborator

zmodem commented Jan 10, 2024

Seems okay to me, but it would be good to include an update to the release notes, which ideally shows an example of what replacing the cmake flag with a config file would look like.

Part of https://reviews.llvm.org/D158218

GCC_INSTALL_PREFIX is a rarely-used legacy option inherited from
pre-CMake build system and has configuration file replacement nowadays.
Many `clang/test/Driver` tests specify `--gcc-toolchain=` to prevent
failures when `GCC_INSTALL_PREFIX` is specified: some contributors add
them to fix tests and some just do cargo culting. This is not healthy
for contributors adding cross compilation support for this rarely used
option.

`DEFAULT_SYSROOT` should in spirit be deprecated as well, but a relative
path doesn't have good replacement, so don't deprecate it for now.

Link: https://discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link: https://discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833

---

With `GCC_INSTALL_PREFIX=/usr`, `clang a.c` behaves like
`clang --gcc-toolchain=/usr a.c`.

Here is a simplified version of GCC installation detection code.
```
if (OPT_gcc_install_dir_EQ)
  return OPT_gcc_install_dir_EQ;

if (OPT_gcc_triple)
  candidate_gcc_triples = {OPT_gcc_triple};
else
  candidate_gcc_triples = collectCandidateTriples();
if (OPT_gcc_toolchain)
  prefixes = {OPT_gcc_toolchain};
else
  prefixes = {OPT_sysroot/usr, OPT_sysroot};
for (prefix : prefixes)
  if "$prefix/lib/gcc" exists // also tries $prefix/lib/gcc-cross
    for (triple : candidate_gcc_triples)
      if "$prefix/lib/gcc/$triple" exists
        return "$prefix/lib/gcc/$triple/$version"; // pick the largest version
```

`--gcc-toolchain=` specifies a directory where
`lib/gcc{,-cross}/$triple/$version` can be found. If you actually want
to use a specific version of GCC, specify something like
`--gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11` in a configuration
file. You can also specify `--gcc-triple=`.

```
clang --gcc-toolchain=/usr a.c
clang --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11 a.c  # specific version
clang --gcc-triple=x86_64-linux-gnu a.c
```
@MaskRay MaskRay force-pushed the GCC_INSTALL_PREFIX branch from 36acf7d to 8bd31c3 Compare January 10, 2024 18:57
@MaskRay
Copy link
Member Author

MaskRay commented Jan 10, 2024

Seems okay to me, but it would be good to include an update to the release notes, which ideally shows an example of what replacing the cmake flag with a config file would look like.

Thanks for the feedback. I have added pseudocode how GCC installation detection works in the description and changed release note/CMake warning to mention this pull request. The Debian example is too long, so I decide not to add too much code in the release node:)

https://github.com/llvm/llvm-project/compare/36acf7d2bdcdcfae5a60259a9a6f4869419d721e..8bd31c3cdb6ce41194b063c9ac13e5e2fbfbcc02

@MaskRay MaskRay merged commit 3358c77 into llvm:main Jan 10, 2024
@MaskRay MaskRay deleted the GCC_INSTALL_PREFIX branch January 10, 2024 19:02
@trcrsired
Copy link

trcrsired commented Jan 23, 2024

Are you going to fix gcc search for clang? i build a gcc 14 manually and clang cannot find it on linux.

It actually works for x86_64-w64-mingw32-gcc cross compiler on linux.

it cannot find any installation i have if gcc is set up in the $PATH.

/usr/local/bin/gcc
$HOME/toolchains/native/bin/gcc

I mean, it should at least try to find gcc libs in $PATH automatically on linux.

justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
Part of https://reviews.llvm.org/D158218

GCC_INSTALL_PREFIX is a rarely-used legacy option inherited from
pre-CMake build system and has configuration file replacement nowadays.
Many `clang/test/Driver` tests specify `--gcc-toolchain=` to prevent
failures when `GCC_INSTALL_PREFIX` is specified: some contributors add
them to fix tests and some just do cargo culting. This is not healthy
for contributors adding cross compilation support for this rarely used
option.

`DEFAULT_SYSROOT` should in spirit be deprecated as well, but a relative
path doesn't have good replacement, so don't deprecate it for now.

Link:
https://discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link:
https://discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833

---

With `GCC_INSTALL_PREFIX=/usr`, `clang a.c` behaves like
`clang --gcc-toolchain=/usr a.c`.

Here is a simplified version of GCC installation detection code.
```
if (OPT_gcc_install_dir_EQ)
  return OPT_gcc_install_dir_EQ;

if (OPT_gcc_triple)
  candidate_gcc_triples = {OPT_gcc_triple};
else
  candidate_gcc_triples = collectCandidateTriples();
if (OPT_gcc_toolchain)
  prefixes = {OPT_gcc_toolchain};
else
  prefixes = {OPT_sysroot/usr, OPT_sysroot};
for (prefix : prefixes)
  if "$prefix/lib/gcc" exists // also tries $prefix/lib/gcc-cross
    for (triple : candidate_gcc_triples)
      if "$prefix/lib/gcc/$triple" exists
        return "$prefix/lib/gcc/$triple/$version"; // pick the largest version
```

`--gcc-toolchain=` specifies a directory where
`lib/gcc{,-cross}/$triple/$version` can be found. If you actually want
to use a specific version of GCC, specify something like
`--gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11` in a configuration
file. You can also specify `--gcc-triple=`.

On Debian and its derivatives where the target triple omits the vendor
part, the following ways are roughly equivalent, except that
`--gcc-install-dir=` specifies a version as well:
```
clang --gcc-toolchain=/usr a.c
clang --gcc-install-dir=/usr/lib/gcc/x86_64-linux-gnu/11 a.c
clang --gcc-triple=x86_64-linux-gnu a.c
```
MaskRay added a commit to MaskRay/llvm-project that referenced this pull request Mar 20, 2024
unless USE_DEPRECATED_GCC_INSTALL_PREFIX (temporary escape hatch) is
set. Setting GCC_INSTALL_PREFIX leads to a warning for Clang 18.1
(llvm#77537) and will be completely removed for Clang 20.

Link: discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link: discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833
MaskRay added a commit that referenced this pull request Mar 21, 2024
unless USE_DEPRECATED_GCC_INSTALL_PREFIX (temporary escape hatch) is
set. Setting GCC_INSTALL_PREFIX leads to a warning for Clang 18.1
(#77537) and will be completely removed for Clang 20.

Link:
discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link:
discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833
chencha3 pushed a commit to chencha3/llvm-project that referenced this pull request Mar 23, 2024
…5891)

unless USE_DEPRECATED_GCC_INSTALL_PREFIX (temporary escape hatch) is
set. Setting GCC_INSTALL_PREFIX leads to a warning for Clang 18.1
(llvm#77537) and will be completely removed for Clang 20.

Link:
discourse.llvm.org/t/add-gcc-install-dir-deprecate-gcc-toolchain-and-remove-gcc-install-prefix/65091
Link:
discourse.llvm.org/t/correct-cmake-parameters-for-building-clang-and-lld-for-riscv/72833
carlocab added a commit to carlocab/wasi-sdk that referenced this pull request Dec 12, 2024
Upstream want to deprecate `DEFAULT_SYSROOT`[^1][^2][^3], but one
blocker is wasi-sdk's usage of it.

Let's try to help that along by switching to using config files[^4]
instead.

This should result in no user-facing changes in functionality. (If it
does, then that's an LLVM bug that should be fixed there.)

[^1]: https://reviews.llvm.org/D158218
[^2]: llvm/llvm-project#94284
[^3]: llvm/llvm-project#77537
[^4]: https://clang.llvm.org/docs/UsersManual.html#configuration-files
sunfishcode pushed a commit to WebAssembly/wasi-sdk that referenced this pull request Dec 12, 2024
Upstream want to deprecate `DEFAULT_SYSROOT`[^1][^2][^3], but one
blocker is wasi-sdk's usage of it.

Let's try to help that along by switching to using config files[^4]
instead.

This should result in no user-facing changes in functionality. (If it
does, then that's an LLVM bug that should be fixed there.)

[^1]: https://reviews.llvm.org/D158218
[^2]: llvm/llvm-project#94284
[^3]: llvm/llvm-project#77537
[^4]: https://clang.llvm.org/docs/UsersManual.html#configuration-files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants