Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 1490bba

Browse files
jyn514onur-ozkan
authored andcommitted
add and document a new is_system_llvm abstraction
1 parent 1a47f5b commit 1490bba

File tree

4 files changed

+52
-26
lines changed

4 files changed

+52
-26
lines changed

‎src/bootstrap/src/core/build_steps/dist.rs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,23 +2032,24 @@ fn install_llvm_file(builder: &Builder<'_>, source: &Path, destination: &Path) {
20322032
///
20332033
/// Returns whether the files were actually copied.
20342034
fn maybe_install_llvm(builder: &Builder<'_>, target: TargetSelection, dst_libdir: &Path) -> bool {
2035-
if let Some(config) = builder.config.target_config.get(&target) {
2036-
if config.llvm_config.is_some() && !builder.config.llvm_from_ci {
2037-
// If the LLVM was externally provided, then we don't currently copy
2038-
// artifacts into the sysroot. This is not necessarily the right
2039-
// choice (in particular, it will require the LLVM dylib to be in
2040-
// the linker's load path at runtime), but the common use case for
2041-
// external LLVMs is distribution provided LLVMs, and in that case
2042-
// they're usually in the standard search path (e.g., /usr/lib) and
2043-
// copying them here is going to cause problems as we may end up
2044-
// with the wrong files and isn't what distributions want.
2045-
//
2046-
// This behavior may be revisited in the future though.
2047-
//
2048-
// If the LLVM is coming from ourselves (just from CI) though, we
2049-
// still want to install it, as it otherwise won't be available.
2050-
return false;
2051-
}
2035+
// If the LLVM was externally provided, then we don't currently copy
2036+
// artifacts into the sysroot. This is not necessarily the right
2037+
// choice (in particular, it will require the LLVM dylib to be in
2038+
// the linker's load path at runtime), but the common use case for
2039+
// external LLVMs is distribution provided LLVMs, and in that case
2040+
// they're usually in the standard search path (e.g., /usr/lib) and
2041+
// copying them here is going to cause problems as we may end up
2042+
// with the wrong files and isn't what distributions want.
2043+
//
2044+
// This behavior may be revisited in the future though.
2045+
//
2046+
// NOTE: this intentionally doesn't use `is_rust_llvm`; whether this is patched or not doesn't matter,
2047+
// we only care if the shared object itself is managed by bootstrap.
2048+
//
2049+
// If the LLVM is coming from ourselves (just from CI) though, we
2050+
// still want to install it, as it otherwise won't be available.
2051+
if builder.is_system_llvm(target) {
2052+
return false;
20522053
}
20532054

20542055
// On macOS, rustc (and LLVM tools) link to an unversioned libLLVM.dylib

‎src/bootstrap/src/core/build_steps/test.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,6 +1845,8 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the
18451845
llvm_components_passed = true;
18461846
}
18471847
if !builder.is_rust_llvm(target) {
1848+
// FIXME: missing Rust patches is not the same as being system llvm; we should rename the flag at some point.
1849+
// Inspecting the tests with `// no-system-llvm` in src/test *looks* like this is doing the right thing, though.
18481850
cmd.arg("--system-llvm");
18491851
}
18501852

‎src/bootstrap/src/core/config/config.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1810,7 +1810,14 @@ impl Config {
18101810
}
18111811
target.llvm_config = Some(config.src.join(s));
18121812
}
1813-
target.llvm_has_rust_patches = cfg.llvm_has_rust_patches;
1813+
if let Some(patches) = cfg.llvm_has_rust_patches {
1814+
assert_eq!(
1815+
config.submodules,
1816+
Some(false),
1817+
"cannot set `llvm-has-rust-patches` for a managed submodule (set `build.submodules = false` if you want to apply patches)"
1818+
);
1819+
target.llvm_has_rust_patches = Some(patches);
1820+
}
18141821
if let Some(ref s) = cfg.llvm_filecheck {
18151822
target.llvm_filecheck = Some(config.src.join(s));
18161823
}

‎src/bootstrap/src/lib.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -823,18 +823,34 @@ impl Build {
823823
INTERNER.intern_path(self.out.join(&*target.triple).join("md-doc"))
824824
}
825825

826-
/// Returns `true` if no custom `llvm-config` is set for the specified target.
826+
/// Returns `true` if this is an external version of LLVM not managed by bootstrap.
827+
/// In particular, we expect llvm sources to be available when this is false.
827828
///
828-
/// If no custom `llvm-config` was specified then Rust's llvm will be used.
829+
/// NOTE: this is not the same as `!is_rust_llvm` when `llvm_has_patches` is set.
830+
fn is_system_llvm(&self, target: TargetSelection) -> bool {
831+
match self.config.target_config.get(&target) {
832+
Some(Target { llvm_config: Some(_), .. }) => {
833+
let ci_llvm = self.config.llvm_from_ci && target == self.config.build;
834+
!ci_llvm
835+
}
836+
// We're building from the in-tree src/llvm-project sources.
837+
Some(Target { llvm_config: None, .. }) => false,
838+
None => false,
839+
}
840+
}
841+
842+
/// Returns `true` if this is our custom, patched, version of LLVM.
843+
///
844+
/// This does not necessarily imply that we're managing the `llvm-project` submodule.
829845
fn is_rust_llvm(&self, target: TargetSelection) -> bool {
830846
match self.config.target_config.get(&target) {
847+
// We're using a user-controlled version of LLVM. The user has explicitly told us whether the version has our patches.
848+
// (They might be wrong, but that's not a supported use-case.)
849+
// In particular, this tries to support `submodules = false` and `patches = false`, for using a newer version of LLVM that's not through `rust-lang/llvm-project`.
831850
Some(Target { llvm_has_rust_patches: Some(patched), .. }) => *patched,
832-
Some(Target { llvm_config, .. }) => {
833-
// If the user set llvm-config we assume Rust is not patched,
834-
// but first check to see if it was configured by llvm-from-ci.
835-
(self.config.llvm_from_ci && target == self.config.build) || llvm_config.is_none()
836-
}
837-
None => true,
851+
// The user hasn't promised the patches match.
852+
// This only has our patches if it's downloaded from CI or built from source.
853+
_ => !self.is_system_llvm(target),
838854
}
839855
}
840856

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /