Skip to content

Commit

Permalink
Auto merge of #14205 - gmorenz:links_overrides_in_unit, r=weihanglo
Browse files Browse the repository at this point in the history
Fix passing of links-overrides with target-applies-to-host and an implicit target

### What does this PR try to resolve?

This fixes the link-overrides half of #14195, both the panic, and the fact that the field is being discarded, the latter of which caused the former as discussed in [the issue](#14195 (comment)).

It does so following the blueprint laid out in #13900 - which is also in my opinion the current best summary of the broader context.

### How should we test and review this PR?

For reviewing, comparing to the changes in #13900 might be useful.

### Additional information

I'm pushing a PR for the other half of #14195 simultaneously. I thought it better to keep the PRs small since they're independent, though if merged simultaneously there will be a conflict over the ordering of fields in `Unit`.
  • Loading branch information
bors committed Jul 18, 2024
2 parents 913d4b8 + 9098072 commit 61424d6
Show file tree
Hide file tree
Showing 9 changed files with 68 additions and 26 deletions.
12 changes: 1 addition & 11 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
//! * [`TargetInfo::rustc_outputs`] to get a list of supported file types.

use crate::core::compiler::apply_env_config;
use crate::core::compiler::{
BuildOutput, BuildRunner, CompileKind, CompileMode, CompileTarget, CrateType,
};
use crate::core::compiler::{BuildRunner, CompileKind, CompileMode, CompileTarget, CrateType};
use crate::core::{Dependency, Package, Target, TargetKind, Workspace};
use crate::util::context::{GlobalContext, StringList, TargetConfig};
use crate::util::interning::InternedString;
Expand Down Expand Up @@ -1038,14 +1036,6 @@ impl<'gctx> RustcTargetData<'gctx> {
CompileKind::Target(s) => &self.target_config[&s],
}
}

/// If a build script is overridden, this returns the `BuildOutput` to use.
///
/// `lib_name` is the `links` library name and `kind` is whether it is for
/// Host or Target.
pub fn script_override(&self, lib_name: &str, kind: CompileKind) -> Option<&BuildOutput> {
self.target_config(kind).links_overrides.get(lib_name)
}
}

/// Structure used to deal with Rustdoc fingerprinting
Expand Down
10 changes: 3 additions & 7 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const OLD_CARGO_WARNING_SYNTAX: &str = "cargo:warning=";
/// [the doc]: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#cargo-warning
const NEW_CARGO_WARNING_SYNTAX: &str = "cargo::warning=";
/// Contains the parsed output of a custom build script.
#[derive(Clone, Debug, Hash, Default)]
#[derive(Clone, Debug, Hash, Default, PartialEq, Eq, PartialOrd, Ord)]
pub struct BuildOutput {
/// Paths to pass to rustc with the `-L` flag.
pub library_paths: Vec<PathBuf>,
Expand Down Expand Up @@ -160,7 +160,7 @@ pub struct BuildDeps {
/// See the [build script documentation][1] for more.
///
/// [1]: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html#cargorustc-link-argflag
#[derive(Clone, Hash, Debug, PartialEq, Eq)]
#[derive(Clone, Hash, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub enum LinkArgTarget {
/// Represents `cargo::rustc-link-arg=FLAG`.
All,
Expand Down Expand Up @@ -1168,11 +1168,7 @@ pub fn build_map(build_runner: &mut BuildRunner<'_, '_>) -> CargoResult<()> {
// If there is a build script override, pre-fill the build output.
if unit.mode.is_run_custom_build() {
if let Some(links) = unit.pkg.manifest().links() {
if let Some(output) = build_runner
.bcx
.target_data
.script_override(links, unit.kind)
{
if let Some(output) = unit.links_overrides.get(links) {
let metadata = build_runner.get_run_build_script_metadata(unit);
build_runner.build_script_outputs.lock().unwrap().insert(
unit.pkg.package_id(),
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ pub fn generate_std_roots(
features.clone(),
target_data.info(*kind).rustflags.clone(),
target_data.info(*kind).rustdocflags.clone(),
target_data.target_config(*kind).links_overrides.clone(),
/*is_std*/ true,
/*dep_hash*/ 0,
IsArtifact::No,
Expand Down
13 changes: 12 additions & 1 deletion src/cargo/core/compiler/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ use crate::util::hex::short_hash;
use crate::util::interning::InternedString;
use crate::util::GlobalContext;
use std::cell::RefCell;
use std::collections::HashSet;
use std::collections::{BTreeMap, HashSet};
use std::fmt;
use std::hash::{Hash, Hasher};
use std::ops::Deref;
use std::rc::Rc;
use std::sync::Arc;

use super::BuildOutput;

/// All information needed to define a unit.
///
/// A unit is an object that has enough information so that cargo knows how to build it.
Expand Down Expand Up @@ -82,6 +84,12 @@ pub struct UnitInner {
/// [`BuildContext::extra_args_for`]: crate::core::compiler::build_context::BuildContext::extra_args_for
/// [`TargetInfo.rustdocflags`]: crate::core::compiler::build_context::TargetInfo::rustdocflags
pub rustdocflags: Arc<[String]>,
/// Build script override for the given library name.
///
/// Any package with a `links` value for the given library name will skip
/// running its build script and instead use the given output from the
/// config file.
pub links_overrides: Rc<BTreeMap<String, BuildOutput>>,
// if `true`, the dependency is an artifact dependency, requiring special handling when
// calculating output directories, linkage and environment variables provided to builds.
pub artifact: IsArtifact,
Expand Down Expand Up @@ -176,6 +184,7 @@ impl fmt::Debug for Unit {
.field("features", &self.features)
.field("rustflags", &self.rustflags)
.field("rustdocflags", &self.rustdocflags)
.field("links_overrides", &self.links_overrides)
.field("artifact", &self.artifact.is_true())
.field(
"artifact_target_for_features",
Expand Down Expand Up @@ -225,6 +234,7 @@ impl UnitInterner {
features: Vec<InternedString>,
rustflags: Arc<[String]>,
rustdocflags: Arc<[String]>,
links_overrides: Rc<BTreeMap<String, BuildOutput>>,
is_std: bool,
dep_hash: u64,
artifact: IsArtifact,
Expand Down Expand Up @@ -260,6 +270,7 @@ impl UnitInterner {
features,
rustflags,
rustdocflags,
links_overrides,
is_std,
dep_hash,
artifact,
Expand Down
11 changes: 6 additions & 5 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,11 +457,7 @@ fn compute_deps_custom_build(
state: &State<'_, '_>,
) -> CargoResult<Vec<UnitDep>> {
if let Some(links) = unit.pkg.manifest().links() {
if state
.target_data
.script_override(links, unit.kind)
.is_some()
{
if unit.links_overrides.get(links).is_some() {
// Overridden build scripts don't have any dependencies.
return Ok(Vec::new());
}
Expand Down Expand Up @@ -861,6 +857,11 @@ fn new_unit_dep_with_profile(
features,
state.target_data.info(kind).rustflags.clone(),
state.target_data.info(kind).rustdocflags.clone(),
state
.target_data
.target_config(kind)
.links_overrides
.clone(),
state.is_std,
/*dep_hash*/ 0,
artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes),
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,7 @@ fn traverse_and_share(
unit.features.clone(),
unit.rustflags.clone(),
unit.rustdocflags.clone(),
unit.links_overrides.clone(),
unit.is_std,
unit.dep_hash,
unit.artifact,
Expand Down Expand Up @@ -725,6 +726,7 @@ fn traverse_and_share(
unit.features.clone(),
unit.rustflags.clone(),
unit.rustdocflags.clone(),
unit.links_overrides.clone(),
unit.is_std,
new_dep_hash,
unit.artifact,
Expand Down Expand Up @@ -888,6 +890,7 @@ fn override_rustc_crate_types(
unit.features.clone(),
unit.rustflags.clone(),
unit.rustdocflags.clone(),
unit.links_overrides.clone(),
unit.is_std,
unit.dep_hash,
unit.artifact,
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_compile/unit_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ impl<'a> UnitGenerator<'a, '_> {
features.clone(),
self.target_data.info(kind).rustflags.clone(),
self.target_data.info(kind).rustdocflags.clone(),
self.target_data.target_config(kind).links_overrides.clone(),
/*is_std*/ false,
/*dep_hash*/ 0,
IsArtifact::No,
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/util/context/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::util::CargoResult;
use serde::Deserialize;
use std::collections::{BTreeMap, HashMap};
use std::path::PathBuf;
use std::rc::Rc;

/// Config definition of a `[target.'cfg(…)']` table.
///
Expand Down Expand Up @@ -36,7 +37,7 @@ pub struct TargetConfig {
/// Any package with a `links` value for the given library name will skip
/// running its build script and instead use the given output from the
/// config file.
pub links_overrides: BTreeMap<String, BuildOutput>,
pub links_overrides: Rc<BTreeMap<String, BuildOutput>>,
}

/// Loads all of the `target.'cfg()'` tables.
Expand Down Expand Up @@ -128,7 +129,7 @@ fn load_config_table(gctx: &GlobalContext, prefix: &str) -> CargoResult<TargetCo
rustflags,
rustdocflags,
linker,
links_overrides,
links_overrides: Rc::new(links_overrides),
})
}

Expand Down
38 changes: 38 additions & 0 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5777,3 +5777,41 @@ hello
"#]])
.run();
}

#[cargo_test]
fn links_overrides_with_target_applies_to_host() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "mylib-sys"
edition = "2021"
version = "0.0.1"
authors = []
links = "mylib"
"#,
)
.file("src/lib.rs", "")
.file("build.rs", "bad file")
.build();

p.cargo("build -v")
.masquerade_as_nightly_cargo(&["target-applies-to-host"])
.args(&[
"-Ztarget-applies-to-host",
"--config",
"target-applies-to-host=false",
])
.args(&[
"--config",
&format!(r#"target.{}.mylib.rustc-link-search=["foo"]"#, rustc_host()),
])
.with_stderr_data(str![[r#"
[COMPILING] mylib-sys v0.0.1 ([ROOT]/foo)
[RUNNING] `rustc --crate-name mylib_sys [..] -L foo`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.run();
}

0 comments on commit 61424d6

Please sign in to comment.