Skip to content

Commit 1c5ec2b

Browse files
authored
Don't use $CARGO_BUILD_TARGET in cargo metadata (#15271)
This commit fixes a (five year old!) regression in `cargo metadata` where if `--filter-platform` isn't explicitly specified it will accidentally read `$CARGO_BUILD_TARGET` (or `build.target` configuration) and use that as the default `--filter-platform`. The reason for this is that the calculation for targets changed in #8167 and while the shared function makes sense for other commands such as `cargo build` the targets have a different meaning in `cargo metadata` so a slightly different set of functionality is desired. This commit fixes the issue by introducing a new constructor for the list of `CompileKind` variants where the fallback of "if nothing is specified" is explicitly chosen. Closes #15270 <!-- Thanks for submitting a pull request 🎉! Here are some tips for you: * If this is your first contribution, read "Cargo Contribution Guide" first: https://doc.crates.io/contrib/ * Run `cargo fmt --all` to format your code changes. * Small commits and pull requests are always preferable and easy to review. * If your idea is large and needs feedback from the community, read how: https://doc.crates.io/contrib/process/#working-on-large-features * Cargo takes care of compatibility. Read our design principles: https://doc.crates.io/contrib/design.html * When changing help text of cargo commands, follow the steps to generate docs: https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages * If your PR is not finished, set it as "draft" PR or add "WIP" in its title. * It's ok to use the CI resources to test your PR, but please don't abuse them. ### What does this PR try to resolve? Explain the motivation behind this change. A clear overview along with an in-depth explanation are helpful. You can use `Fixes #<issue number>` to associate this PR to an existing issue. ### How should we test and review this PR? Demonstrate how you test this change and guide reviewers through your PR. With a smooth review process, a pull request usually gets reviewed quicker. If you don't know how to write and run your tests, please read the guide: https://doc.crates.io/contrib/tests ### Additional information Other information you want to mention in this PR, such as prior arts, future extensions, an unresolved problem, or a TODO list. -->
2 parents bf43053 + 70dc4d8 commit 1c5ec2b

File tree

4 files changed

+84
-14
lines changed

4 files changed

+84
-14
lines changed

src/cargo/core/compiler/compile_kind.rs

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,20 @@ pub enum CompileKind {
2929
Target(CompileTarget),
3030
}
3131

32+
/// Fallback behavior in the
33+
/// [`CompileKind::from_requested_targets_with_fallback`] function when
34+
/// no targets are specified.
35+
pub enum CompileKindFallback {
36+
/// The build configuration is consulted to find the default target, such as
37+
/// `$CARGO_BUILD_TARGET` or reading `build.target`.
38+
BuildConfig,
39+
40+
/// Only the host should be returned when targets aren't explicitly
41+
/// specified. This is used by `cargo metadata` for example where "only
42+
/// host" has a special meaning in terms of the returned metadata.
43+
JustHost,
44+
}
45+
3246
impl CompileKind {
3347
pub fn is_host(&self) -> bool {
3448
matches!(self, CompileKind::Host)
@@ -53,6 +67,21 @@ impl CompileKind {
5367
pub fn from_requested_targets(
5468
gctx: &GlobalContext,
5569
targets: &[String],
70+
) -> CargoResult<Vec<CompileKind>> {
71+
CompileKind::from_requested_targets_with_fallback(
72+
gctx,
73+
targets,
74+
CompileKindFallback::BuildConfig,
75+
)
76+
}
77+
78+
/// Same as [`CompileKind::from_requested_targets`] except that if `targets`
79+
/// doesn't explicitly mention anything the behavior of what to return is
80+
/// controlled by the `fallback` argument.
81+
pub fn from_requested_targets_with_fallback(
82+
gctx: &GlobalContext,
83+
targets: &[String],
84+
fallback: CompileKindFallback,
5685
) -> CargoResult<Vec<CompileKind>> {
5786
let dedup = |targets: &[String]| {
5887
Ok(targets
@@ -70,9 +99,11 @@ impl CompileKind {
7099
return dedup(targets);
71100
}
72101

73-
let kinds = match &gctx.build_config()?.target {
74-
None => Ok(vec![CompileKind::Host]),
75-
Some(build_target_config) => dedup(&build_target_config.values(gctx)?),
102+
let kinds = match (fallback, &gctx.build_config()?.target) {
103+
(_, None) | (CompileKindFallback::JustHost, _) => Ok(vec![CompileKind::Host]),
104+
(CompileKindFallback::BuildConfig, Some(build_target_config)) => {
105+
dedup(&build_target_config.values(gctx)?)
106+
}
76107
};
77108

78109
kinds

src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ pub use self::build_context::{
7676
use self::build_plan::BuildPlan;
7777
pub use self::build_runner::{BuildRunner, Metadata, UnitHash};
7878
pub use self::compilation::{Compilation, Doctest, UnitOutput};
79-
pub use self::compile_kind::{CompileKind, CompileTarget};
79+
pub use self::compile_kind::{CompileKind, CompileKindFallback, CompileTarget};
8080
pub use self::crate_type::CrateType;
8181
pub use self::custom_build::LinkArgTarget;
8282
pub use self::custom_build::{BuildOutput, BuildScriptOutputs, BuildScripts};

src/cargo/ops/cargo_output_metadata.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::core::compiler::artifact::match_artifacts_kind_with_targets;
2-
use crate::core::compiler::{CompileKind, RustcTargetData};
2+
use crate::core::compiler::{CompileKind, CompileKindFallback, RustcTargetData};
33
use crate::core::dependency::DepKind;
44
use crate::core::package::SerializedPackage;
55
use crate::core::resolver::{features::CliFeatures, HasDevUnits, Resolve};
@@ -132,8 +132,16 @@ fn build_resolve_graph(
132132
) -> CargoResult<(Vec<SerializedPackage>, MetadataResolve)> {
133133
// TODO: Without --filter-platform, features are being resolved for `host` only.
134134
// How should this work?
135-
let requested_kinds =
136-
CompileKind::from_requested_targets(ws.gctx(), &metadata_opts.filter_platforms)?;
135+
//
136+
// Otherwise note that "just host" is used as the fallback here if
137+
// `filter_platforms` is empty to intentionally avoid reading
138+
// `$CARGO_BUILD_TARGET` (or `build.target`) which makes sense for other
139+
// subcommands like `cargo build` but does not fit with this command.
140+
let requested_kinds = CompileKind::from_requested_targets_with_fallback(
141+
ws.gctx(),
142+
&metadata_opts.filter_platforms,
143+
CompileKindFallback::JustHost,
144+
)?;
137145
let mut target_data = RustcTargetData::new(ws, &requested_kinds)?;
138146
// Resolve entire workspace.
139147
let specs = Packages::All(Vec::new()).to_package_id_specs(ws)?;

tests/testsuite/metadata.rs

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,7 +1224,7 @@ fn workspace_metadata_with_dependencies_no_deps() {
12241224
name = "bar"
12251225
version = "0.5.0"
12261226
authors = ["wycats@example.com"]
1227-
1227+
12281228
[dependencies]
12291229
baz = { path = "../baz/" }
12301230
artifact = { path = "../artifact/", artifact = "bin" }
@@ -1474,13 +1474,13 @@ fn workspace_metadata_with_dependencies_and_resolve() {
14741474
name = "artifact"
14751475
version = "0.5.0"
14761476
authors = []
1477-
1477+
14781478
[lib]
14791479
crate-type = ["staticlib", "cdylib", "rlib"]
1480-
1480+
14811481
[[bin]]
14821482
name = "bar-name"
1483-
1483+
14841484
[[bin]]
14851485
name = "baz-name"
14861486
"#,
@@ -1494,10 +1494,10 @@ fn workspace_metadata_with_dependencies_and_resolve() {
14941494
name = "bin-only-artifact"
14951495
version = "0.5.0"
14961496
authors = []
1497-
1497+
14981498
[[bin]]
14991499
name = "a-name"
1500-
1500+
15011501
[[bin]]
15021502
name = "b-name"
15031503
"#,
@@ -4343,7 +4343,7 @@ fn workspace_metadata_with_dependencies_no_deps_artifact() {
43434343
name = "bar"
43444344
version = "0.5.0"
43454345
authors = ["wycats@example.com"]
4346-
4346+
43474347
[dependencies]
43484348
baz = { path = "../baz/" }
43494349
baz-renamed = { path = "../baz/" }
@@ -4954,3 +4954,34 @@ local-time = 1979-05-27
49544954
)
49554955
.run();
49564956
}
4957+
4958+
#[cargo_test]
4959+
fn metadata_ignores_build_target_configuration() -> anyhow::Result<()> {
4960+
let p = project()
4961+
.file(
4962+
"Cargo.toml",
4963+
r#"
4964+
[package]
4965+
name = "foo"
4966+
4967+
[target.'cfg(something)'.dependencies]
4968+
foobar = "0.0.1"
4969+
"#,
4970+
)
4971+
.file("src/lib.rs", "")
4972+
.build();
4973+
Package::new("foobar", "0.0.1").publish();
4974+
4975+
let output1 = p
4976+
.cargo("metadata -q --format-version 1")
4977+
.exec_with_output()?;
4978+
let output2 = p
4979+
.cargo("metadata -q --format-version 1")
4980+
.env("CARGO_BUILD_TARGET", rustc_host())
4981+
.exec_with_output()?;
4982+
assert!(
4983+
output1.stdout == output2.stdout,
4984+
"metadata should not change when `CARGO_BUILD_TARGET` is set",
4985+
);
4986+
Ok(())
4987+
}

0 commit comments

Comments
 (0)