Driver: Accept the --gcc-install-dir option multiple times #71446
+43
−17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The downside of the --gcc-install-dir is that you are required to specify a path that includes the gcc major version. This makes it hard to use for distributions in configuration files, because an upgrade of gcc would cause clang to stop working. We can fix this problem by accepting the --gcc-install-dir option multiple times. This way distributions can specify all possible versions of gcc that might exist on the system in a config and avoid problems when gcc is upgraded.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-driver Author: Tom Stellard (tstellar) ChangesThe downside of the --gcc-install-dir is that you are required to specify a path that includes the gcc major version. This makes it hard to use for distributions in configuration files, because an upgrade of gcc would cause clang to stop working. We can fix this problem by accepting the --gcc-install-dir option multiple times. This way distributions can specify all possible versions of gcc that might exist on the system in a config and avoid problems when gcc is upgraded. Full diff: https://github.com/llvm/llvm-project/pull/71446.diff 3 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 009ac1cfd8dc1b4..cb54bae9d322043 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -768,7 +768,11 @@ def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"<prefix>">,
HelpText<"Search $prefix$file for executables, libraries, and data files. "
"If $prefix is a directory, search $prefix/$file">;
def gcc_install_dir_EQ : Joined<["--"], "gcc-install-dir=">,
- HelpText<"Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+ HelpText<"Use GCC installation in the specified directory. The directory ends"
+ " with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+ " This option can be specified multiple times in which case clang will"
+ " start with last option and search until it finds a valid gcc install"
+ " among the given paths."
"Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation">;
def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
HelpText<"Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 3276590729e47ea..64645cd03b6f9fd 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2126,27 +2126,36 @@ void Generic_GCC::GCCInstallationDetector::init(
StringRef TripleNoVendorRef(TripleNoVendor);
// If --gcc-install-dir= is specified, skip filesystem detection.
- if (const Arg *A =
- Args.getLastArg(clang::driver::options::OPT_gcc_install_dir_EQ);
- A && A->getValue()[0]) {
- StringRef InstallDir = A->getValue();
+ std::vector<std::string> FailedGCCInstallDirs;
+ std::vector<std::string> GCCInstallDirs = Args.getAllArgValues(clang::driver::options::OPT_gcc_install_dir_EQ);
+ for (auto it = GCCInstallDirs.rbegin(); it != GCCInstallDirs.rend(); ++it) {
+ StringRef InstallDir = *it;
if (!ScanGCCForMultilibs(TargetTriple, Args, InstallDir, false)) {
- D.Diag(diag::err_drv_invalid_gcc_install_dir) << InstallDir;
- } else {
- (void)InstallDir.consume_back("/");
- StringRef VersionText = llvm::sys::path::filename(InstallDir);
- StringRef TripleText =
- llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
-
- Version = GCCVersion::Parse(VersionText);
- GCCTriple.setTriple(TripleText);
- GCCInstallPath = std::string(InstallDir);
- GCCParentLibPath = GCCInstallPath + "/../../..";
- IsValid = true;
+ FailedGCCInstallDirs.push_back(std::string(InstallDir));
+ continue;
}
+
+ (void)InstallDir.consume_back("/");
+ StringRef VersionText = llvm::sys::path::filename(InstallDir);
+ StringRef TripleText =
+ llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
+
+ Version = GCCVersion::Parse(VersionText);
+ GCCTriple.setTriple(TripleText);
+ GCCInstallPath = std::string(InstallDir);
+ GCCParentLibPath = GCCInstallPath + "/../../..";
+ IsValid = true;
return;
}
+ for (auto FailedDir : FailedGCCInstallDirs) {
+ D.Diag(diag::err_drv_invalid_gcc_install_dir) << FailedDir;
+ }
+
+ if (!FailedGCCInstallDirs.empty())
+ return;
+
+
// Compute the set of prefixes for our search.
SmallVector<std::string, 8> Prefixes;
StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
diff --git a/clang/test/Driver/gcc-install-dir.cpp b/clang/test/Driver/gcc-install-dir.cpp
index 955f162a2ce3a19..a7a91797811a124 100644
--- a/clang/test/Driver/gcc-install-dir.cpp
+++ b/clang/test/Driver/gcc-install-dir.cpp
@@ -16,6 +16,12 @@
// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1100 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/11/ --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
// DEBIAN_X86_64: "-internal-isystem"
// DEBIAN_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/x86_64-linux-gnu/c++/10"
@@ -41,3 +47,10 @@
// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu 2>&1 | FileCheck %s --check-prefix=INVALID
// INVALID: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu' does not contain a GCC installation
+
+// RUN: not %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1100 \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1200 2>&1 | FileCheck %s --check-prefix=INVALID-MULTIPLE
+// INVALID-MULTIPLE: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu/1200' does not contain a GCC installation
+// INVALID-MULTIPLE: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu/1100' does not contain a GCC installation
|
|
@llvm/pr-subscribers-clang Author: Tom Stellard (tstellar) ChangesThe downside of the --gcc-install-dir is that you are required to specify a path that includes the gcc major version. This makes it hard to use for distributions in configuration files, because an upgrade of gcc would cause clang to stop working. We can fix this problem by accepting the --gcc-install-dir option multiple times. This way distributions can specify all possible versions of gcc that might exist on the system in a config and avoid problems when gcc is upgraded. Full diff: https://github.com/llvm/llvm-project/pull/71446.diff 3 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 009ac1cfd8dc1b4..cb54bae9d322043 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -768,7 +768,11 @@ def B : JoinedOrSeparate<["-"], "B">, MetaVarName<"<prefix>">,
HelpText<"Search $prefix$file for executables, libraries, and data files. "
"If $prefix is a directory, search $prefix/$file">;
def gcc_install_dir_EQ : Joined<["--"], "gcc-install-dir=">,
- HelpText<"Use GCC installation in the specified directory. The directory ends with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+ HelpText<"Use GCC installation in the specified directory. The directory ends"
+ " with path components like 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
+ " This option can be specified multiple times in which case clang will"
+ " start with last option and search until it finds a valid gcc install"
+ " among the given paths."
"Note: executables (e.g. ld) used by the compiler are not overridden by the selected GCC installation">;
def gcc_toolchain : Joined<["--"], "gcc-toolchain=">, Flags<[NoXarchOption]>,
HelpText<"Specify a directory where Clang can find 'include' and 'lib{,32,64}/gcc{,-cross}/$triple/$version'. "
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 3276590729e47ea..64645cd03b6f9fd 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2126,27 +2126,36 @@ void Generic_GCC::GCCInstallationDetector::init(
StringRef TripleNoVendorRef(TripleNoVendor);
// If --gcc-install-dir= is specified, skip filesystem detection.
- if (const Arg *A =
- Args.getLastArg(clang::driver::options::OPT_gcc_install_dir_EQ);
- A && A->getValue()[0]) {
- StringRef InstallDir = A->getValue();
+ std::vector<std::string> FailedGCCInstallDirs;
+ std::vector<std::string> GCCInstallDirs = Args.getAllArgValues(clang::driver::options::OPT_gcc_install_dir_EQ);
+ for (auto it = GCCInstallDirs.rbegin(); it != GCCInstallDirs.rend(); ++it) {
+ StringRef InstallDir = *it;
if (!ScanGCCForMultilibs(TargetTriple, Args, InstallDir, false)) {
- D.Diag(diag::err_drv_invalid_gcc_install_dir) << InstallDir;
- } else {
- (void)InstallDir.consume_back("/");
- StringRef VersionText = llvm::sys::path::filename(InstallDir);
- StringRef TripleText =
- llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
-
- Version = GCCVersion::Parse(VersionText);
- GCCTriple.setTriple(TripleText);
- GCCInstallPath = std::string(InstallDir);
- GCCParentLibPath = GCCInstallPath + "/../../..";
- IsValid = true;
+ FailedGCCInstallDirs.push_back(std::string(InstallDir));
+ continue;
}
+
+ (void)InstallDir.consume_back("/");
+ StringRef VersionText = llvm::sys::path::filename(InstallDir);
+ StringRef TripleText =
+ llvm::sys::path::filename(llvm::sys::path::parent_path(InstallDir));
+
+ Version = GCCVersion::Parse(VersionText);
+ GCCTriple.setTriple(TripleText);
+ GCCInstallPath = std::string(InstallDir);
+ GCCParentLibPath = GCCInstallPath + "/../../..";
+ IsValid = true;
return;
}
+ for (auto FailedDir : FailedGCCInstallDirs) {
+ D.Diag(diag::err_drv_invalid_gcc_install_dir) << FailedDir;
+ }
+
+ if (!FailedGCCInstallDirs.empty())
+ return;
+
+
// Compute the set of prefixes for our search.
SmallVector<std::string, 8> Prefixes;
StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
diff --git a/clang/test/Driver/gcc-install-dir.cpp b/clang/test/Driver/gcc-install-dir.cpp
index 955f162a2ce3a19..a7a91797811a124 100644
--- a/clang/test/Driver/gcc-install-dir.cpp
+++ b/clang/test/Driver/gcc-install-dir.cpp
@@ -16,6 +16,12 @@
// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10/ --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1100 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
+// RUN: %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform --unwindlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/11/ --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/10 2>&1 | FileCheck %s --check-prefix=DEBIAN_X86_64
// DEBIAN_X86_64: "-internal-isystem"
// DEBIAN_X86_64-SAME: {{^}} "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10"
// DEBIAN_X86_64-SAME: {{^}} "-internal-isystem" "[[SYSROOT:[^"]+]]/usr/lib/gcc/x86_64-linux-gnu/10/../../../../include/x86_64-linux-gnu/c++/10"
@@ -41,3 +47,10 @@
// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu 2>&1 | FileCheck %s --check-prefix=INVALID
// INVALID: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu' does not contain a GCC installation
+
+// RUN: not %clangxx %s -### --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/debian_multiarch_tree \
+// RUN: -ccc-install-dir %S/Inputs/basic_linux_tree/usr/bin -resource-dir=%S/Inputs/resource_dir --stdlib=platform --rtlib=platform \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1100 \
+// RUN: --gcc-install-dir=%S/Inputs/debian_multiarch_tree/usr/lib/gcc/x86_64-linux-gnu/1200 2>&1 | FileCheck %s --check-prefix=INVALID-MULTIPLE
+// INVALID-MULTIPLE: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu/1200' does not contain a GCC installation
+// INVALID-MULTIPLE: error: '{{.*}}/usr/lib/gcc/x86_64-linux-gnu/1100' does not contain a GCC installation
|
You can test this locally with the following command:git-clang-format --diff bc7a3bd864be696217c4d11eddf16bed7646b60f 2474aad8585829936af66d8b1238a1876c0f8327 -- clang/lib/Driver/ToolChains/Gnu.cpp clang/test/Driver/gcc-install-dir.cppView the diff from clang-format here.diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index 64645cd03b6f..ba5a3b4f6b5d 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2127,7 +2127,8 @@ void Generic_GCC::GCCInstallationDetector::init(
// If --gcc-install-dir= is specified, skip filesystem detection.
std::vector<std::string> FailedGCCInstallDirs;
- std::vector<std::string> GCCInstallDirs = Args.getAllArgValues(clang::driver::options::OPT_gcc_install_dir_EQ);
+ std::vector<std::string> GCCInstallDirs =
+ Args.getAllArgValues(clang::driver::options::OPT_gcc_install_dir_EQ);
for (auto it = GCCInstallDirs.rbegin(); it != GCCInstallDirs.rend(); ++it) {
StringRef InstallDir = *it;
if (!ScanGCCForMultilibs(TargetTriple, Args, InstallDir, false)) {
@@ -2155,7 +2156,6 @@ void Generic_GCC::GCCInstallationDetector::init(
if (!FailedGCCInstallDirs.empty())
return;
-
// Compute the set of prefixes for our search.
SmallVector<std::string, 8> Prefixes;
StringRef GCCToolchainDir = getGCCToolchainDir(Args, D.SysRoot);
|
--gcc-install-dir specifying the version is by design, as otherwise there is no way to use an older GCC when the system has a newer GCC. This proposal makes the option's semantic special, and I haven't seen a precedent before. When your system GCC moves 12 to 13, do you remove Gentoo doesn't seem to have more driver option requirement. Perhaps @mgorny @thesamesam can talk about they upgrade GCC and update Clang configuration files. |
Yes.
Isn't this because the clang driver has custom logic for detecting gcc on Gentoo? |
|
I think I have expressed I wanted to say... The new probing semantics seems really unusual to me. I do not feel that how path-related options usually behave. Can the system be changed to keep both versions around before removing GCC 12? |
@MaskRay Ok, what do you think would be a good alternative? Another solution I considered was to have --gcc-install-dir accept paths like /usr/lib/gcc/x86_64-redhat-linux/ and then probe for the 'best' version in that directory, but I felt this complicated the logic too much. Probably the most simple solution that would satisfy our requirements would be to add a new option
No, it can't. |
|
Closing in favor of #73214 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The downside of the --gcc-install-dir is that you are required to specify a path that includes the gcc major version. This makes it hard to use for distributions in configuration files, because an upgrade of gcc would cause clang to stop working.
We can fix this problem by accepting the --gcc-install-dir option multiple times. This way distributions can specify all possible versions of gcc that might exist on the system in a config and avoid problems when gcc is upgraded.