From 11bf15f7e303ff401f59e4781392074642ec6d43 Mon Sep 17 00:00:00 2001 From: Deadbeef Date: Thu, 13 Apr 2023 05:40:21 +0000 Subject: [PATCH] Support `per-pkg-target` for `-Zbuild-std`, take two This is a simplified minimal change, which changes the build-std to iterate over the packages to collect a list of targets std should be built for. Does the iterating over packages part have issues with artifact dependencies? I am not very familiar with artifact deps to know so any help would be appreciated. But it should be fine for now since artifact dependencies don't work with build-std yet. #10444 --- src/cargo/core/compiler/standard_lib.rs | 65 +++++++++---- src/cargo/core/package.rs | 22 +++++ src/cargo/ops/cargo_compile/mod.rs | 11 +-- src/cargo/ops/cargo_compile/unit_generator.rs | 15 +-- tests/build-std/main.rs | 47 +++++++++ .../mock-std/library/proc_macro/src/lib.rs | 7 +- tests/testsuite/standard_lib.rs | 96 ++++++++++++++++++- 7 files changed, 217 insertions(+), 46 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index b5ac0c997d1a..83ac4d56244c 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::path::PathBuf; @@ -168,7 +169,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, @@ -182,41 +184,62 @@ 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 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::NormalOrDev); - for kind in kinds { - let list = ret.entry(*kind).or_insert_with(Vec::new); - let unit_for = UnitFor::new_normal(*kind); + + // Get information for the list of packages for standard library + 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"); + let features = + std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev); + let unit_for = UnitFor::new_normal(explicit_host_kind); + (pkg, lib, unit_for, features) + }) + .collect(); + + // Iterate over all packages over the package set to find all targets requested. + 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, None, - )); - } + ) + }); + + e.insert(units.collect()); } + Ok(ret) } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 40ba9cdf894a..f2e273cf6f84 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -185,6 +185,28 @@ 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/mod.rs b/src/cargo/ops/cargo_compile/mod.rs index 3b6043d4fdbb..ceadb4f82ae8 100644 --- a/src/cargo/ops/cargo_compile/mod.rs +++ b/src/cargo/ops/cargo_compile/mod.rs @@ -341,14 +341,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_root_units` can do @@ -393,7 +385,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, diff --git a/src/cargo/ops/cargo_compile/unit_generator.rs b/src/cargo/ops/cargo_compile/unit_generator.rs index ce5a825fe3a6..a184edba0968 100644 --- a/src/cargo/ops/cargo_compile/unit_generator.rs +++ b/src/cargo/ops/cargo_compile/unit_generator.rs @@ -111,20 +111,7 @@ impl<'a> UnitGenerator<'a, '_> { // 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 { - self.requested_kinds - .iter() - .map(|kind| match kind { - CompileKind::Host => pkg - .manifest() - .default_kind() - .unwrap_or(self.explicit_host_kind), - CompileKind::Target(t) => CompileKind::Target(*t), - }) - .collect() - }; + let explicit_kinds = pkg.explicit_kinds(self.requested_kinds, self.explicit_host_kind); explicit_kinds .into_iter() diff --git a/tests/build-std/main.rs b/tests/build-std/main.rs index 47a4bb671c05..f10a2f898581 100644 --- a/tests/build-std/main.rs +++ b/tests/build-std/main.rs @@ -227,3 +227,50 @@ fn custom_test_framework() { .build_std_arg("core") .run(); } + +/// like cross-custom but uses per-package-target instead +#[cargo_test(build_std_real)] +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] <-- not sure why this doesn't work + [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") + .target_host() + .run(); +} 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 82a768406155..a4c49384d601 100644 --- a/tests/testsuite/mock-std/library/proc_macro/src/lib.rs +++ b/tests/testsuite/mock-std/library/proc_macro/src/lib.rs @@ -1,10 +1,15 @@ #![feature(staged_api)] #![stable(since = "1.0.0", feature = "dummy")] + extern crate proc_macro; +// Don't re-export everything in the root so that the mock std can be distinguished from the real one. #[stable(since = "1.0.0", feature = "dummy")] -pub use proc_macro::*; +pub mod exported { + #[stable(since = "1.0.0", feature = "dummy")] + pub use proc_macro::*; +} #[stable(since = "1.0.0", feature = "dummy")] pub fn custom_api() { diff --git a/tests/testsuite/standard_lib.rs b/tests/testsuite/standard_lib.rs index d3be303ea8bb..1616e4e82176 100644 --- a/tests/testsuite/standard_lib.rs +++ b/tests/testsuite/standard_lib.rs @@ -366,16 +366,110 @@ 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(build_std_mock)] +fn non_proc_macro_crate_uses_non_sysroot_proc_macro() { + let setup = setup(); + + 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(build_std_mock)] +fn intergrated_proc_macro() { + let setup = setup(); + + 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) + .with_stdout_contains("The answer is 42") + .target_host() + .run(); +} + #[cargo_test(build_std_mock)] fn bench() { let setup = setup();