From 0a4de8b56d83af204e7d6e25a5a839bdc66ff8cb Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 26 Jan 2022 13:41:12 +1100 Subject: [PATCH 01/10] Abstract `explicit_kinds` from `generate_targets` This makes the specific kinds to use for build-std available. --- src/cargo/core/compiler/standard_lib.rs | 53 ++++++++++++++++--------- src/cargo/core/package.rs | 18 +++++++++ src/cargo/ops/cargo_compile.rs | 3 +- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index e0baebd516b..13da16ae2f3 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -10,6 +10,7 @@ use crate::core::{Dependency, PackageId, PackageSet, Resolve, SourceId, Workspac use crate::ops::{self, Packages}; use crate::util::errors::CargoResult; use crate::Config; +use std::collections::hash_map::Entry; use std::collections::{HashMap, HashSet}; use std::env; use std::path::PathBuf; @@ -169,7 +170,8 @@ pub fn generate_std_roots( crates: &[String], std_resolve: &Resolve, std_features: &ResolvedFeatures, - kinds: &[CompileKind], + requested_kinds: &[CompileKind], + explicit_host_kind: CompileKind, package_set: &PackageSet<'_>, interner: &UnitInterner, profiles: &Profiles, @@ -183,43 +185,56 @@ pub fn generate_std_roots( let std_pkgs = package_set.get_many(std_ids)?; // Generate a map of Units for each kind requested. let mut ret = HashMap::new(); - for pkg in std_pkgs { + + let std_pkg_infos: Vec<_> = std_pkgs.iter().map(|pkg| { let lib = pkg .targets() .iter() .find(|t| t.is_lib()) .expect("std has a lib"); - // I don't think we need to bother with Check here, the difference - // in time is minimal, and the difference in caching is - // significant. - let mode = CompileMode::Build; - let features = std_features.activated_features( - pkg.package_id(), - FeaturesFor::NormalOrDevOrArtifactTarget(None), - ); - for kind in kinds { - let list = ret.entry(*kind).or_insert_with(Vec::new); - let unit_for = UnitFor::new_normal(*kind); + // std does not have artifact dependencies at the moment + let unit_for = UnitFor::new_normal(explicit_host_kind); + let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDevOrArtifactTarget(None)); + (pkg, lib, unit_for, features) + }).collect(); + + + for kind in package_set + .packages() + .flat_map(|pkg| pkg.explicit_kinds(requested_kinds, explicit_host_kind)) + { + let e = match ret.entry(kind) { + Entry::Vacant(e) => e, + Entry::Occupied(_) => continue, + }; + let units = std_pkg_infos.iter().map(|(pkg, lib, unit_for, features)| { + // I don't think we need to bother with Check here, the difference + // in time is minimal, and the difference in caching is + // significant. + let mode = CompileMode::Build; let profile = profiles.get_profile( pkg.package_id(), /*is_member*/ false, /*is_local*/ false, - unit_for, - *kind, + *unit_for, + kind, ); - list.push(interner.intern( + interner.intern( pkg, lib, profile, - *kind, + kind, mode, features.clone(), /*is_std*/ true, /*dep_hash*/ 0, IsArtifact::No, - )); - } + ) + }); + + e.insert(units.collect()); } + Ok(ret) } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index f93c249e5da..996e6f51336 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -186,6 +186,24 @@ impl Package { self.targets().iter().any(|t| t.is_custom_build()) } + /// Returns explicit kinds either forced by `forced-target` in `Cargo.toml`, + /// fallback to `default-target`, or specified in cli parameters. + pub fn explicit_kinds(&self, requested_kinds: &[CompileKind], explicit_host_kind: CompileKind) -> Vec { + if let Some(k) = self.manifest().forced_kind() { + vec![k] + } else { + requested_kinds + .iter() + .map(|kind| match kind { + CompileKind::Host => { + self.manifest().default_kind().unwrap_or(explicit_host_kind) + } + CompileKind::Target(t) => CompileKind::Target(*t), + }) + .collect() + } + } + pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Package { Package { inner: Rc::new(PackageInner { diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 07dabce6d08..5ac7f415139 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -549,7 +549,8 @@ pub fn create_bcx<'a, 'cfg>( &crates, std_resolve, std_features, - &explicit_host_kinds, + &build_config.requested_kinds, + explicit_host_kind, &pkg_set, interner, &profiles, From 3330273a20c1bdaa42ba9a15d6389ebca58c6c8f Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 26 Jan 2022 14:44:23 +1100 Subject: [PATCH 02/10] Remove obsolete error and unused var `build-std` now works with no `--target` specified to either use defaulted target or host target. --- src/cargo/ops/cargo_compile.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 5ac7f415139..0e13283ad49 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -401,6 +401,12 @@ pub fn create_bcx<'a, 'cfg>( } = resolve; let std_resolve_features = if let Some(crates) = &config.cli_unstable().build_std { + if build_config.build_plan { + config + .shell() + .warn("-Zbuild-std does not currently fully support --build-plan")?; + } + let (std_package_set, std_resolve, std_features) = standard_lib::resolve_std(ws, &target_data, &build_config, crates)?; pkg_set.add_set(std_package_set); @@ -467,14 +473,6 @@ pub fn create_bcx<'a, 'cfg>( // assuming `--target $HOST` was specified. See // `rebuild_unit_graph_shared` for more on why this is done. let explicit_host_kind = CompileKind::Target(CompileTarget::new(&target_data.rustc.host)?); - let explicit_host_kinds: Vec<_> = build_config - .requested_kinds - .iter() - .map(|kind| match kind { - CompileKind::Host => explicit_host_kind, - CompileKind::Target(t) => CompileKind::Target(*t), - }) - .collect(); // Passing `build_config.requested_kinds` instead of // `explicit_host_kinds` here so that `generate_targets` can do From f0a3b8e59f30298bb1b484832c1e6cbba1268faa Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 26 Jan 2022 14:44:42 +1100 Subject: [PATCH 03/10] Add per-package-target test --- tests/build-std/main.rs | 46 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/build-std/main.rs b/tests/build-std/main.rs index c1355b317ce..a765946c70e 100644 --- a/tests/build-std/main.rs +++ b/tests/build-std/main.rs @@ -170,6 +170,52 @@ fn cross_custom() { .run(); } +/// like cross-custom but uses per-package-target instead +#[cargo_test(build_std)] +fn per_package_target() { + let p = project() + .file( + "Cargo.toml", + r#" + cargo-features = ["per-package-target"] + [package] + name = "foo" + version = "0.1.0" + edition = "2018" + default-target = "custom-target.json" + + [target.custom-target.dependencies] + dep = { path = "dep" } + "#, + ) + .file( + "src/lib.rs", + "#![no_std] pub fn f() -> u32 { dep::answer() }", + ) + .file("dep/Cargo.toml", &basic_manifest("dep", "0.1.0")) + .file("dep/src/lib.rs", "#![no_std] pub fn answer() -> u32 { 42 }") + .file( + "custom-target.json", + r#" + { + "llvm-target": "x86_64-unknown-none-gnu", + "data-layout": "e-m:e-i64:64-f80:128-n8:16:32:64-S128", + "arch": "x86_64", + "target-endian": "little", + "target-pointer-width": "64", + "target-c-int-width": "32", + "os": "none", + "linker-flavor": "ld.lld" + } + "#, + ) + .build(); + + p.cargo("build -v") + .build_std_arg("core") + .run(); +} + #[cargo_test(build_std)] fn custom_test_framework() { let p = project() From 5ec609c6259a77bc520e307c971ea6c71f02b8a7 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 26 Jan 2022 18:34:43 +1100 Subject: [PATCH 04/10] Run rustfmt --- src/cargo/core/compiler/standard_lib.rs | 27 ++++++++++++++----------- src/cargo/core/package.rs | 6 +++++- tests/build-std/main.rs | 4 +--- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 13da16ae2f3..3da5dded1c1 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -186,18 +186,21 @@ pub fn generate_std_roots( // Generate a map of Units for each kind requested. let mut ret = HashMap::new(); - let std_pkg_infos: Vec<_> = std_pkgs.iter().map(|pkg| { - let lib = pkg - .targets() - .iter() - .find(|t| t.is_lib()) - .expect("std has a lib"); - // std does not have artifact dependencies at the moment - let unit_for = UnitFor::new_normal(explicit_host_kind); - let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDevOrArtifactTarget(None)); - (pkg, lib, unit_for, features) - }).collect(); - + let std_pkg_infos: Vec<_> = std_pkgs + .iter() + .map(|pkg| { + let lib = pkg + .targets() + .iter() + .find(|t| t.is_lib()) + .expect("std has a lib"); + // std does not have artifact dependencies at the moment + let unit_for = UnitFor::new_normal(explicit_host_kind); + let features = + std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev); + (pkg, lib, unit_for, features) + }) + .collect(); for kind in package_set .packages() diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 996e6f51336..8b1027526d2 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -188,7 +188,11 @@ impl Package { /// Returns explicit kinds either forced by `forced-target` in `Cargo.toml`, /// fallback to `default-target`, or specified in cli parameters. - pub fn explicit_kinds(&self, requested_kinds: &[CompileKind], explicit_host_kind: CompileKind) -> Vec { + pub fn explicit_kinds( + &self, + requested_kinds: &[CompileKind], + explicit_host_kind: CompileKind, + ) -> Vec { if let Some(k) = self.manifest().forced_kind() { vec![k] } else { diff --git a/tests/build-std/main.rs b/tests/build-std/main.rs index a765946c70e..c682f42201b 100644 --- a/tests/build-std/main.rs +++ b/tests/build-std/main.rs @@ -211,9 +211,7 @@ fn per_package_target() { ) .build(); - p.cargo("build -v") - .build_std_arg("core") - .run(); + p.cargo("build -v").build_std_arg("core").run(); } #[cargo_test(build_std)] From 3056fdd39666c1ccb3cecc9e9d937537e562be5b Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Sat, 29 Jan 2022 20:27:25 +1100 Subject: [PATCH 05/10] Move comments --- src/cargo/core/package.rs | 6 ++++++ src/cargo/ops/cargo_compile.rs | 14 +------------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 8b1027526d2..6a5a78218b9 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -188,6 +188,12 @@ impl Package { /// Returns explicit kinds either forced by `forced-target` in `Cargo.toml`, /// fallback to `default-target`, or specified in cli parameters. + /// + /// If `--target` has not been specified, then the return value + /// is the same as if `--target $HOST` was specified. See + /// `rebuild_unit_graph_shared` for why callers want to do this. + /// However, if the package has its own `package.default-target` + /// key, then that gets used instead of `$HOST` pub fn explicit_kinds( &self, requested_kinds: &[CompileKind], diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 0e13283ad49..f2941cd971e 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1010,19 +1010,7 @@ fn generate_targets( // why this is done. However, if the package has its own // `package.target` key, then this gets used instead of // `$HOST` - let explicit_kinds = if let Some(k) = pkg.manifest().forced_kind() { - vec![k] - } else { - requested_kinds - .iter() - .map(|kind| match kind { - CompileKind::Host => { - pkg.manifest().default_kind().unwrap_or(explicit_host_kind) - } - CompileKind::Target(t) => CompileKind::Target(*t), - }) - .collect() - }; + let explicit_kinds = pkg.explicit_kinds(requested_kinds, explicit_host_kind); for kind in explicit_kinds.iter() { let unit_for = if initial_target_mode.is_any_test() { From b6980bf243810b2cc8893daff3c1691c05b4c5a3 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Wed, 30 Mar 2022 21:11:26 +1100 Subject: [PATCH 06/10] Resolve conflicts --- src/cargo/core/compiler/standard_lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index 3da5dded1c1..1aea2625b85 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -197,7 +197,7 @@ pub fn generate_std_roots( // std does not have artifact dependencies at the moment let unit_for = UnitFor::new_normal(explicit_host_kind); let features = - std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev); + std_features.activated_features(pkg.package_id(), FeaturesFor::default()); (pkg, lib, unit_for, features) }) .collect(); From a022179dce1c625a5ee3b46633d6c1c8cb7b701c Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 24 May 2022 21:27:06 +1000 Subject: [PATCH 07/10] Removed re-export of sysroot types from mock-proc-macro --- tests/testsuite/mock-std/library/proc_macro/src/lib.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/testsuite/mock-std/library/proc_macro/src/lib.rs b/tests/testsuite/mock-std/library/proc_macro/src/lib.rs index 82a76840615..6f13220f587 100644 --- a/tests/testsuite/mock-std/library/proc_macro/src/lib.rs +++ b/tests/testsuite/mock-std/library/proc_macro/src/lib.rs @@ -1,11 +1,6 @@ #![feature(staged_api)] #![stable(since = "1.0.0", feature = "dummy")] -extern crate proc_macro; - -#[stable(since = "1.0.0", feature = "dummy")] -pub use proc_macro::*; - #[stable(since = "1.0.0", feature = "dummy")] pub fn custom_api() { } From 14e87947259933c70add1c436f8587eb5b9c771e Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 24 May 2022 21:27:19 +1000 Subject: [PATCH 08/10] Added more tests to ensure proc-macro crates use sysroot --- tests/testsuite/standard_lib.rs | 107 +++++++++++++++++++++++++++++++- 1 file changed, 106 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/standard_lib.rs b/tests/testsuite/standard_lib.rs index 07b038b4a5d..e280bb55be0 100644 --- a/tests/testsuite/standard_lib.rs +++ b/tests/testsuite/standard_lib.rs @@ -396,16 +396,121 @@ fn target_proc_macro() { "src/lib.rs", r#" extern crate proc_macro; - pub fn f() { + fn f() { let _ts = proc_macro::TokenStream::new(); } "#, ) + .file( + "Cargo.toml", + r#" + [package] + name = "pm" + version = "0.1.0" + + [lib] + proc-macro = true + "#, + ) .build(); p.cargo("build -v").build_std(&setup).target_host().run(); } +// We already have `basic` which uses `proc_macro::custom_api()`. This case attempts to use +// `TokenStream` which would error because we are using the sysroot version. +#[cargo_test] +fn non_proc_macro_crate_uses_non_sysroot_proc_macro() { + let setup = match setup() { + Some(s) => s, + None => return, + }; + let p = project() + .file( + "src/lib.rs", + r#" + extern crate proc_macro; + fn f() { + let _ts = proc_macro::TokenStream::new(); + } + "#, + ) + .file( + "Cargo.toml", + r#" + [package] + name = "pm" + version = "0.1.0" + "#, + ) + .build(); + p.cargo("build -v") + .build_std(&setup) + .target_host() + .run_expect_error(); +} + +#[cargo_test] +fn intergrated_proc_macro() { + let setup = match setup() { + Some(s) => s, + None => return, + }; + let p = project() + .file( + "src/main.rs", + r#" + fn main() { + println!("The answer is {}", pm::m!()); + } + "#, + ) + .file( + "pm/src/lib.rs", + r#" + extern crate proc_macro; + use proc_macro::TokenStream; + + #[proc_macro] + pub fn m(_item: TokenStream) -> TokenStream { + "42".parse().unwrap() + } + "#, + ) + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [workspace] + members = ["pm"] + + [dependencies] + pm = { path = "./pm" } + "#, + ) + .file( + "pm/Cargo.toml", + r#" + [package] + name = "pm" + version = "0.1.0" + + [lib] + proc-macro = true + "#, + ) + .build(); + + p.cargo("run -v") + .build_std(&setup) + .target_host() + .with_stdout_contains("The answer is 42") + .run(); +} + #[cargo_test] fn bench() { let setup = match setup() { From f416ef01835646022fa60ff4b024f0085fd7c917 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 24 May 2022 21:47:36 +1000 Subject: [PATCH 09/10] Remove --target bail --- src/cargo/core/compiler/build_config.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/cargo/core/compiler/build_config.rs b/src/cargo/core/compiler/build_config.rs index 51e6512b161..d9c555c8088 100644 --- a/src/cargo/core/compiler/build_config.rs +++ b/src/cargo/core/compiler/build_config.rs @@ -84,11 +84,6 @@ impl BuildConfig { anyhow::bail!("jobs may not be 0"); } - if config.cli_unstable().build_std.is_some() && requested_kinds[0].is_host() { - // TODO: This should eventually be fixed. - anyhow::bail!("-Zbuild-std requires --target"); - } - Ok(BuildConfig { requested_kinds, jobs, From 369a5dff41dfb55a708f0adb219654c868b65ea7 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Tue, 24 May 2022 22:37:45 +1000 Subject: [PATCH 10/10] Rm --target from test cases --- tests/testsuite/standard_lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/testsuite/standard_lib.rs b/tests/testsuite/standard_lib.rs index e280bb55be0..133bada7a65 100644 --- a/tests/testsuite/standard_lib.rs +++ b/tests/testsuite/standard_lib.rs @@ -246,6 +246,10 @@ fn basic() { p.cargo("build").build_std(&setup).target_host().run(); p.cargo("run").build_std(&setup).target_host().run(); p.cargo("test").build_std(&setup).target_host().run(); + p.cargo("check -v").build_std(&setup).run(); + p.cargo("build").build_std(&setup).run(); + p.cargo("run").build_std(&setup).run(); + p.cargo("test").build_std(&setup).run(); } #[cargo_test] @@ -414,7 +418,7 @@ fn target_proc_macro() { ) .build(); - p.cargo("build -v").build_std(&setup).target_host().run(); + p.cargo("build -v").build_std(&setup).run(); } // We already have `basic` which uses `proc_macro::custom_api()`. This case attempts to use @@ -444,10 +448,7 @@ fn non_proc_macro_crate_uses_non_sysroot_proc_macro() { "#, ) .build(); - p.cargo("build -v") - .build_std(&setup) - .target_host() - .run_expect_error(); + p.cargo("build -v").build_std(&setup).run_expect_error(); } #[cargo_test] @@ -506,7 +507,6 @@ fn intergrated_proc_macro() { p.cargo("run -v") .build_std(&setup) - .target_host() .with_stdout_contains("The answer is 42") .run(); }