From ead0452672434a8ff7993602f82820e5c7ed7cd1 Mon Sep 17 00:00:00 2001 From: Kaiyi Li Date: Sun, 12 May 2024 12:54:17 -0700 Subject: [PATCH] Refactor that prepares to add rustc libdir to `BuildPlatform` * Rename `BuildPlatforms` to `BuildPlatform` as we only plan to store a single target in that struct. * Add a `BuildPlatformBuilder` to create `BuildPlatform`, because it will become complicated to construct `BuildPlatform`: * we may or may not add a target triple * we may or may not add a target libdir * we may or may not add a host libdir * we detect the host platform in the ctor * Create a serialized form of `BuildPlatform`: `BuildPlatformSummary` * Change `discover_build_platforms` to `discover_target_triple` which only discovers the `TargetTriple` --- cargo-nextest/src/dispatch.rs | 35 +-- nextest-metadata/src/test_list.rs | 17 +- .../src/cargo_config/target_triple.rs | 12 + nextest-runner/src/config/config_impl.rs | 4 +- nextest-runner/src/config/overrides.rs | 10 +- nextest-runner/src/config/scripts.rs | 8 +- nextest-runner/src/config/test_helpers.rs | 20 +- nextest-runner/src/list/binary_list.rs | 19 +- nextest-runner/src/list/rust_build_meta.rs | 170 +++++++++++-- nextest-runner/src/list/test_list.rs | 14 +- nextest-runner/src/platform.rs | 223 +++++++++++++++--- nextest-runner/src/reporter.rs | 5 +- nextest-runner/src/runner.rs | 4 +- nextest-runner/src/target_runner.rs | 6 +- nextest-runner/tests/integration/basic.rs | 12 +- nextest-runner/tests/integration/fixtures.rs | 4 +- .../tests/integration/target_runner.rs | 11 +- 17 files changed, 465 insertions(+), 109 deletions(-) diff --git a/cargo-nextest/src/dispatch.rs b/cargo-nextest/src/dispatch.rs index e47207b66d1..0c7d8e5e009 100644 --- a/cargo-nextest/src/dispatch.rs +++ b/cargo-nextest/src/dispatch.rs @@ -21,13 +21,13 @@ use nextest_runner::{ VersionOnlyConfig, }, double_spawn::DoubleSpawnInfo, - errors::{UnknownHostPlatform, WriteTestListError}, + errors::WriteTestListError, list::{ BinaryList, OutputFormat, RustTestArtifact, SerializableFormat, TestExecuteContext, TestList, }, partition::PartitionerBuilder, - platform::BuildPlatforms, + platform::{self, BuildPlatformBuilder}, redact::Redactor, reporter::{structured, FinalStatusLevel, StatusLevel, TestOutputDisplay, TestReporterBuilder}, reuse_build::{archive_to_file, ArchiveReporter, PathMapper, ReuseBuildInfo}, @@ -687,7 +687,7 @@ impl CargoOptions { graph: &PackageGraph, manifest_path: Option<&Utf8Path>, output: OutputContext, - build_platforms: BuildPlatforms, + build_platforms: platform::BuildPlatform, ) -> Result { // Don't use the manifest path from the graph to ensure that if the user cd's into a // particular crate and runs cargo nextest, then it behaves identically to cargo test. @@ -971,7 +971,8 @@ impl From for FinalStatusLevel { #[derive(Debug)] struct BaseApp { output: OutputContext, - build_platforms: BuildPlatforms, + /// TODO: support multiple --target options + build_platforms: platform::BuildPlatform, cargo_metadata_json: Arc, package_graph: Arc, // Potentially remapped workspace root (might not be the same as the graph). @@ -1006,7 +1007,15 @@ impl BaseApp { // Next, read the build platforms. let build_platforms = match reuse_build.binaries_metadata() { Some(kind) => kind.binary_list.rust_build_meta.build_platforms.clone(), - None => discover_build_platforms(&cargo_configs, cargo_opts.target.as_deref())?, + None => { + let mut build_platform_builder = BuildPlatformBuilder::default(); + if let Some(target_triple) = + discover_target_triple(&cargo_configs, cargo_opts.target.as_deref()) + { + build_platform_builder.set_target(target_triple).add(); + } + build_platform_builder.build()? + } }; // Read the Cargo metadata. @@ -1248,7 +1257,7 @@ impl BaseApp { }) } - fn load_runner(&self, build_platforms: &BuildPlatforms) -> &TargetRunner { + fn load_runner(&self, build_platforms: &platform::BuildPlatform) -> &TargetRunner { self.target_runner .get_or_init(|| runner_for_target(&self.cargo_configs, build_platforms)) } @@ -1890,7 +1899,7 @@ fn acquire_graph_data( manifest_path: Option<&Utf8Path>, target_dir: Option<&Utf8Path>, cargo_opts: &CargoOptions, - build_platforms: &BuildPlatforms, + build_platforms: &platform::BuildPlatform, output: OutputContext, ) -> Result { let cargo_target_arg = build_platforms.to_cargo_target_arg()?; @@ -1927,11 +1936,11 @@ fn acquire_graph_data( Ok(json) } -fn discover_build_platforms( +fn discover_target_triple( cargo_configs: &CargoConfigs, target_cli_option: Option<&str>, -) -> Result { - let target_triple = match TargetTriple::find(cargo_configs, target_cli_option) { +) -> Option { + match TargetTriple::find(cargo_configs, target_cli_option) { Ok(Some(triple)) => { log::debug!( "using target triple `{}` defined by `{}`; {}", @@ -1950,14 +1959,12 @@ fn discover_build_platforms( warn_on_err("target triple", &err); None } - }; - - BuildPlatforms::new(target_triple) + } } fn runner_for_target( cargo_configs: &CargoConfigs, - build_platforms: &BuildPlatforms, + build_platforms: &platform::BuildPlatform, ) -> TargetRunner { match TargetRunner::new(cargo_configs, build_platforms) { Ok(runner) => { diff --git a/nextest-metadata/src/test_list.rs b/nextest-metadata/src/test_list.rs index 909a9fd3696..ca073bd4193 100644 --- a/nextest-metadata/src/test_list.rs +++ b/nextest-metadata/src/test_list.rs @@ -461,7 +461,7 @@ pub enum RustBinaryIdNameAndKind<'a> { } /// Rust metadata used for builds and test runs. -#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize, Default)] #[serde(rename_all = "kebab-case")] pub struct RustBuildMetaSummary { /// The target directory for Rust artifacts. @@ -485,6 +485,11 @@ pub struct RustBuildMetaSummary { /// The target platforms used while compiling the Rust artifacts. #[serde(default)] + pub target_platforms_v2: Vec, + + /// Deprecated in favor of target_platforms_v2. The target platforms used while compiling the + /// Rust artifacts. + #[serde(default)] pub target_platforms: Vec, /// A deprecated form of the target platform used for cross-compilation, if any. @@ -510,6 +515,14 @@ pub struct RustNonTestBinarySummary { pub path: Utf8PathBuf, } +/// Serialized representation of the host and the target platform. +#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[serde(rename_all = "kebab-case")] +pub struct BuildPlatformSummary { + /// The target platform, if specified. + pub target: PlatformSummary, +} + /// Information about the kind of a Rust non-test binary. /// /// This is part of [`RustNonTestBinarySummary`], and is used to determine runtime environment @@ -705,6 +718,7 @@ mod tests { linked_paths: BTreeSet::new(), target_platform: None, target_platforms: vec![], + target_platforms_v2: vec![], }; "no target platform")] #[test_case(r#"{ "target-directory": "/foo", @@ -720,6 +734,7 @@ mod tests { linked_paths: BTreeSet::new(), target_platform: Some("x86_64-unknown-linux-gnu".to_owned()), target_platforms: vec![], + target_platforms_v2: vec![], }; "single target platform specified")] fn test_deserialize_old_rust_build_meta(input: &str, expected: RustBuildMetaSummary) { let build_meta: RustBuildMetaSummary = diff --git a/nextest-runner/src/cargo_config/target_triple.rs b/nextest-runner/src/cargo_config/target_triple.rs index 06bb33ebfce..f0ec855ab5d 100644 --- a/nextest-runner/src/cargo_config/target_triple.rs +++ b/nextest-runner/src/cargo_config/target_triple.rs @@ -24,6 +24,18 @@ pub struct TargetTriple { } impl TargetTriple { + /// Create an x86_64-unknown-linux-gnu [`TargetTriple`]. Useful for testing. + /// + /// # Panics + /// + /// Panics if the underlying implementation fail to parse the `"x86_64-unknown-linux-gnu"` + /// triple string. + pub fn x86_64_unknown_linux_gnu() -> Self { + TargetTriple::deserialize_str(Some("x86_64-unknown-linux-gnu".to_owned())) + .expect("creating TargetTriple from linux gnu triple string shoould succeed") + .expect("the output of deserialize_str shouldn't be None") + } + /// Converts a `PlatformSummary` that was output by `TargetTriple::serialize` back to a target triple. /// This target triple is assumed to originate from a build-metadata config. pub fn deserialize( diff --git a/nextest-runner/src/config/config_impl.rs b/nextest-runner/src/config/config_impl.rs index dd4b2cee46b..d1c7fa0fc4f 100644 --- a/nextest-runner/src/config/config_impl.rs +++ b/nextest-runner/src/config/config_impl.rs @@ -13,7 +13,7 @@ use crate::{ UnknownConfigScriptError, UnknownTestGroupError, }, list::TestList, - platform::BuildPlatforms, + platform::BuildPlatform, reporter::{FinalStatusLevel, StatusLevel, TestOutputDisplay}, }; use camino::{Utf8Path, Utf8PathBuf}; @@ -616,7 +616,7 @@ impl<'cfg> NextestProfile<'cfg, PreBuildPlatform> { /// /// This is a separate step from parsing the config and reading a profile so that cargo-nextest /// can tell users about configuration parsing errors before building the binary list. - pub fn apply_build_platforms(self, build_platforms: &BuildPlatforms) -> NextestProfile<'cfg> { + pub fn apply_build_platforms(self, build_platforms: &BuildPlatform) -> NextestProfile<'cfg> { let compiled_data = self.compiled_data.apply_build_platforms(build_platforms); NextestProfile { name: self.name, diff --git a/nextest-runner/src/config/overrides.rs b/nextest-runner/src/config/overrides.rs index bb24a6680c9..131da2fabc9 100644 --- a/nextest-runner/src/config/overrides.rs +++ b/nextest-runner/src/config/overrides.rs @@ -7,7 +7,7 @@ use super::{ use crate::{ config::{FinalConfig, PreBuildPlatform, RetryPolicy, SlowTimeout, TestGroup, ThreadsRequired}, errors::{ConfigParseCompiledDataError, ConfigParseErrorKind}, - platform::BuildPlatforms, + platform::BuildPlatform as NextestBuildPlatform, reporter::TestOutputDisplay, }; use guppy::graph::{cargo::BuildPlatform, PackageGraph}; @@ -354,7 +354,7 @@ impl CompiledData { pub(super) fn apply_build_platforms( self, - build_platforms: &BuildPlatforms, + build_platforms: &NextestBuildPlatform, ) -> CompiledData { let overrides = self .overrides @@ -475,15 +475,15 @@ impl CompiledOverride { pub(super) fn apply_build_platforms( self, - build_platforms: &BuildPlatforms, + build_platforms: &NextestBuildPlatform, ) -> CompiledOverride { let host_eval = self.data.host_spec.eval(&build_platforms.host); let host_test_eval = self.data.target_spec.eval(&build_platforms.host); let target_eval = build_platforms .target .as_ref() - .map_or(host_test_eval, |triple| { - self.data.target_spec.eval(&triple.platform) + .map_or(host_test_eval, |target| { + self.data.target_spec.eval(&target.triple.platform) }); CompiledOverride { diff --git a/nextest-runner/src/config/scripts.rs b/nextest-runner/src/config/scripts.rs index 6869b981a28..08cb28cc3c0 100644 --- a/nextest-runner/src/config/scripts.rs +++ b/nextest-runner/src/config/scripts.rs @@ -11,7 +11,7 @@ use crate::{ double_spawn::{DoubleSpawnContext, DoubleSpawnInfo}, errors::{ConfigParseCompiledDataError, InvalidConfigScriptName, SetupScriptError}, list::TestList, - platform::BuildPlatforms, + platform::BuildPlatform as NextestBuildPlatform, test_command::{apply_ld_dyld_env, create_command, LocalExecuteContext}, }; use camino::Utf8Path; @@ -381,15 +381,15 @@ impl CompiledProfileScripts { pub(super) fn apply_build_platforms( self, - build_platforms: &BuildPlatforms, + build_platforms: &NextestBuildPlatform, ) -> CompiledProfileScripts { let host_eval = self.data.host_spec.eval(&build_platforms.host); let host_test_eval = self.data.target_spec.eval(&build_platforms.host); let target_eval = build_platforms .target .as_ref() - .map_or(host_test_eval, |triple| { - self.data.target_spec.eval(&triple.platform) + .map_or(host_test_eval, |target| { + self.data.target_spec.eval(&target.triple.platform) }); CompiledProfileScripts { diff --git a/nextest-runner/src/config/test_helpers.rs b/nextest-runner/src/config/test_helpers.rs index 6c86ccd6e6f..4e57c1926ff 100644 --- a/nextest-runner/src/config/test_helpers.rs +++ b/nextest-runner/src/config/test_helpers.rs @@ -4,7 +4,7 @@ use crate::{ cargo_config::{TargetDefinitionLocation, TargetTriple, TargetTripleSource}, config::{CustomTestGroup, TestGroup}, - platform::BuildPlatforms, + platform::{BuildPlatform as NextestBuildPlatform, BuildPlatformTarget}, }; use camino::{Utf8Path, Utf8PathBuf}; use guppy::{ @@ -83,15 +83,17 @@ pub(super) fn binary_query<'a>( } } -pub(super) fn build_platforms() -> BuildPlatforms { - BuildPlatforms::new_with_host( - Platform::new("x86_64-unknown-linux-gnu", TargetFeatures::Unknown).unwrap(), - Some(TargetTriple { - platform: Platform::new("aarch64-apple-darwin", TargetFeatures::Unknown).unwrap(), - source: TargetTripleSource::Env, - location: TargetDefinitionLocation::Builtin, +pub(super) fn build_platforms() -> NextestBuildPlatform { + NextestBuildPlatform { + host: Platform::new("x86_64-unknown-linux-gnu", TargetFeatures::Unknown).unwrap(), + target: Some(BuildPlatformTarget { + triple: TargetTriple { + platform: Platform::new("aarch64-apple-darwin", TargetFeatures::Unknown).unwrap(), + source: TargetTripleSource::Env, + location: TargetDefinitionLocation::Builtin, + }, }), - ) + } } pub(super) fn test_group(name: &str) -> TestGroup { diff --git a/nextest-runner/src/list/binary_list.rs b/nextest-runner/src/list/binary_list.rs index 4190f89d11f..647f233fd67 100644 --- a/nextest-runner/src/list/binary_list.rs +++ b/nextest-runner/src/list/binary_list.rs @@ -5,7 +5,7 @@ use crate::{ errors::{FromMessagesError, RustBuildMetaParseError, WriteTestListError}, helpers::convert_rel_path_to_forward_slash, list::{BinaryListState, OutputFormat, RustBuildMeta, Styles}, - platform::BuildPlatforms, + platform::BuildPlatform as NextestBuildPlatform, write_str::WriteStr, }; use camino::{Utf8Path, Utf8PathBuf}; @@ -51,7 +51,7 @@ impl BinaryList { pub fn from_messages( reader: impl io::BufRead, graph: &PackageGraph, - build_platforms: BuildPlatforms, + build_platforms: NextestBuildPlatform, ) -> Result { let mut state = BinaryListBuildState::new(graph, build_platforms); @@ -164,7 +164,7 @@ struct BinaryListBuildState<'g> { } impl<'g> BinaryListBuildState<'g> { - fn new(graph: &'g PackageGraph, build_platforms: BuildPlatforms) -> Self { + fn new(graph: &'g PackageGraph, build_platforms: NextestBuildPlatform) -> Self { let rust_target_dir = graph.workspace().target_directory().to_path_buf(); Self { @@ -394,6 +394,7 @@ mod tests { use crate::{ cargo_config::{TargetDefinitionLocation, TargetTriple, TargetTripleSource}, list::SerializableFormat, + platform::BuildPlatformBuilder, }; use indoc::indoc; use maplit::btreeset; @@ -426,7 +427,9 @@ mod tests { source: TargetTripleSource::CliOption, location: TargetDefinitionLocation::Builtin, }; - let build_platforms = BuildPlatforms::new(Some(fake_triple)).unwrap(); + let mut build_platform_builder = BuildPlatformBuilder::default(); + build_platform_builder.set_target(fake_triple).add(); + let build_platforms = build_platform_builder.build().unwrap(); let mut rust_build_meta = RustBuildMeta::new("/fake/target", build_platforms); rust_build_meta @@ -499,6 +502,14 @@ mod tests { }, "build-script-out-dirs": {}, "linked-paths": [], + "target-platforms-v2": [ + { + "target": { + "triple": "x86_64-unknown-linux-gnu", + "target-features": "unknown" + } + } + ], "target-platforms": [ { "triple": "x86_64-unknown-linux-gnu", diff --git a/nextest-runner/src/list/rust_build_meta.rs b/nextest-runner/src/list/rust_build_meta.rs index ca5bf75e543..590e0d2f628 100644 --- a/nextest-runner/src/list/rust_build_meta.rs +++ b/nextest-runner/src/list/rust_build_meta.rs @@ -2,11 +2,10 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 use crate::{ - cargo_config::TargetTriple, errors::RustBuildMetaParseError, helpers::convert_rel_path_to_main_sep, list::{BinaryListState, TestListState}, - platform::BuildPlatforms, + platform::BuildPlatform, reuse_build::PathMapper, }; use camino::Utf8PathBuf; @@ -43,14 +42,14 @@ pub struct RustBuildMeta { pub linked_paths: BTreeMap>, /// The build platforms: host and target triple - pub build_platforms: BuildPlatforms, + pub build_platforms: BuildPlatform, state: PhantomData, } impl RustBuildMeta { /// Creates a new [`RustBuildMeta`]. - pub fn new(target_directory: impl Into, build_platforms: BuildPlatforms) -> Self { + pub fn new(target_directory: impl Into, build_platforms: BuildPlatform) -> Self { Self { target_directory: target_directory.into(), base_output_directories: BTreeSet::new(), @@ -93,7 +92,7 @@ impl RustBuildMeta { build_script_out_dirs: BTreeMap::new(), linked_paths: BTreeMap::new(), state: PhantomData, - build_platforms: BuildPlatforms { + build_platforms: BuildPlatform { host: Platform::current().unwrap(), target: None, }, @@ -135,15 +134,15 @@ impl RustBuildMeta { impl RustBuildMeta { /// Creates a `RustBuildMeta` from a serializable summary. pub fn from_summary(summary: RustBuildMetaSummary) -> Result { - let target_triple = if !summary.target_platforms.is_empty() { - // TODO: support multiple --target options - TargetTriple::deserialize(summary.target_platforms.first().cloned())? + let build_platforms = if let Some(summary) = summary.target_platforms_v2.first() { + summary.clone().try_into()? + } else if let Some(summary) = summary.target_platforms.first() { + // Compatibility with metadata generated by older versions of nextest. + summary.clone().try_into()? } else { // Compatibility with metadata generated by older versions of nextest. - TargetTriple::deserialize_str(summary.target_platform)? + BuildPlatform::from_summary_str(summary.target_platform.clone())? }; - let build_platforms = BuildPlatforms::new(target_triple) - .map_err(|error| RustBuildMetaParseError::UnknownHostPlatform(error.error))?; Ok(Self { target_directory: summary.target_directory, @@ -168,13 +167,150 @@ impl RustBuildMeta { non_test_binaries: self.non_test_binaries.clone(), build_script_out_dirs: self.build_script_out_dirs.clone(), linked_paths: self.linked_paths.keys().cloned().collect(), - target_platform: self - .build_platforms - .target - .as_ref() - .map(|triple| triple.platform.triple_str().to_owned()), + target_platform: self.build_platforms.to_summary_str(), + target_platforms: vec![self.build_platforms.clone().into()], // TODO: support multiple --target options - target_platforms: vec![self.build_platforms.to_summary()], + target_platforms_v2: vec![self.build_platforms.clone().into()], + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + cargo_config::TargetTriple, + platform::{BuildPlatformBuilder, BuildPlatformTarget}, + }; + use nextest_metadata::BuildPlatformSummary; + use target_spec::{summaries::PlatformSummary, Platform}; + use test_case::test_case; + + impl Default for RustBuildMeta { + fn default() -> Self { + RustBuildMeta::::new( + Utf8PathBuf::default(), + BuildPlatformBuilder::default() + .build() + .expect("creating BuildPlatform without target triple should succeed"), + ) + } + } + + fn x86_64_pc_windows_msvc_triple() -> TargetTriple { + TargetTriple::deserialize_str(Some("x86_64-pc-windows-msvc".to_owned())) + .expect("creating TargetTriple from windows msvc triple string shoould succeed") + .expect("the output of deserialize_str shouldn't be None") + } + + fn x86_64_apple_darwin_triple() -> TargetTriple { + TargetTriple::deserialize_str(Some("x86_64-apple-darwin".to_owned())) + .expect("creating TargetTriple from apple triple string shoould succeed") + .expect("the output of deserialize_str shouldn't be None") + } + + fn host_platform() -> Platform { + Platform::current().expect("should detect the host platform successfully") + } + + #[test_case(RustBuildMetaSummary { + ..Default::default() + }, RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: None, + }, + ..Default::default() + }; "no target platforms")] + #[test_case(RustBuildMetaSummary { + target_platform: Some("x86_64-unknown-linux-gnu".to_owned()), + ..Default::default() + }, RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: Some(BuildPlatformTarget{ + triple: TargetTriple::x86_64_unknown_linux_gnu(), + }), + }, + ..Default::default() + }; "only target platform field")] + #[test_case(RustBuildMetaSummary { + target_platform: Some("x86_64-unknown-linux-gnu".to_owned()), + target_platforms: vec![PlatformSummary::new("x86_64-pc-windows-msvc")], + ..Default::default() + }, RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: Some(BuildPlatformTarget{ + triple: x86_64_pc_windows_msvc_triple() + }), + }, + ..Default::default() + }; "target platform and target platforms field")] + #[test_case(RustBuildMetaSummary { + target_platform: Some("x86_64-unknown-linux-gnu".to_owned()), + target_platforms: vec![PlatformSummary::new("x86_64-pc-windows-msvc")], + target_platforms_v2: vec![BuildPlatformSummary { + target: PlatformSummary::new("x86_64-apple-darwin"), + }], + ..Default::default() + }, RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: Some(BuildPlatformTarget{ + triple: x86_64_apple_darwin_triple() + }), + }, + ..Default::default() + }; "target platform and target platforms and target platforms v2 field")] + fn test_from_summary(summary: RustBuildMetaSummary, expected: RustBuildMeta) { + let actual = RustBuildMeta::::from_summary(summary) + .expect("RustBuildMeta should deserialize from summary with success."); + assert_eq!(actual, expected); + } + + fn not_host_platform_triple() -> TargetTriple { + cfg_if::cfg_if! { + if #[cfg(windows)] { + TargetTriple::x86_64_unknown_linux_gnu() + } else { + x86_64_pc_windows_msvc_triple() + } } } + + #[test_case(RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: None, + }, + ..Default::default() + }, RustBuildMetaSummary { + target_platform: None, + target_platforms: vec![host_platform().to_summary()], + target_platforms_v2: vec![BuildPlatformSummary{ + target: host_platform().to_summary() + }], + ..Default::default() + }; "build platforms without target")] + #[test_case(RustBuildMeta:: { + build_platforms: BuildPlatform { + host: host_platform(), + target: Some(BuildPlatformTarget { + triple: not_host_platform_triple() + }), + }, + ..Default::default() + }, RustBuildMetaSummary { + target_platform: Some(not_host_platform_triple().platform.triple_str().to_owned()), + target_platforms: vec![not_host_platform_triple().platform.to_summary()], + target_platforms_v2: vec![BuildPlatformSummary{ + target: not_host_platform_triple().platform.to_summary() + }], + ..Default::default() + }; "build platforms with target")] + fn test_to_summary(meta: RustBuildMeta, expected: RustBuildMetaSummary) { + let actual = meta.to_summary(); + assert_eq!(actual, expected); + } } diff --git a/nextest-runner/src/list/test_list.rs b/nextest-runner/src/list/test_list.rs index 8ae64c76d6d..b55911c5834 100644 --- a/nextest-runner/src/list/test_list.rs +++ b/nextest-runner/src/list/test_list.rs @@ -959,7 +959,7 @@ mod tests { use crate::{ cargo_config::{TargetDefinitionLocation, TargetTriple, TargetTripleSource}, list::SerializableFormat, - platform::BuildPlatforms, + platform::BuildPlatformBuilder, test_filter::RunIgnored, }; use guppy::CargoMetadata; @@ -1033,7 +1033,9 @@ mod tests { source: TargetTripleSource::CliOption, location: TargetDefinitionLocation::Builtin, }; - let build_platforms = BuildPlatforms::new(Some(fake_triple)).unwrap(); + let mut build_platform_builder = BuildPlatformBuilder::default(); + build_platform_builder.set_target(fake_triple).add(); + let build_platforms = build_platform_builder.build().unwrap(); let fake_env = EnvironmentMap::empty(); let rust_build_meta = RustBuildMeta::new("/fake", build_platforms).map_paths(&PathMapper::noop()); @@ -1139,6 +1141,14 @@ mod tests { "non-test-binaries": {}, "build-script-out-dirs": {}, "linked-paths": [], + "target-platforms-v2": [ + { + "target": { + "triple": "aarch64-unknown-linux-gnu", + "target-features": "unknown" + } + } + ], "target-platforms": [ { "triple": "aarch64-unknown-linux-gnu", diff --git a/nextest-runner/src/platform.rs b/nextest-runner/src/platform.rs index 5ecb187131a..d8a66f64925 100644 --- a/nextest-runner/src/platform.rs +++ b/nextest-runner/src/platform.rs @@ -5,44 +5,78 @@ use crate::{ cargo_config::{CargoTargetArg, TargetTriple}, - errors::{TargetTripleError, UnknownHostPlatform}, + errors::{RustBuildMetaParseError, TargetTripleError, UnknownHostPlatform}, }; +use nextest_metadata::BuildPlatformSummary; use target_spec::summaries::PlatformSummary; pub use target_spec::Platform; -/// A representation of host and target platforms. -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct BuildPlatforms { - /// The host platform. - pub host: Platform, - - /// The target platform, if specified. - /// - /// In the future, this will become a list of target triples once multiple `--target` arguments - /// are supported. - pub target: Option, +/// Creates a new [`BuildPlatform`]. +#[derive(Default)] +pub struct BuildPlatformBuilder { + target: Option, } -impl BuildPlatforms { - /// Creates a new `BuildPlatform`. +impl BuildPlatformBuilder { + /// Creates a new [`BuildPlatformTargetBuilder`] used to populate a [`BuildPlatformTarget`] in + /// the final built [`BuildPlatform`]. /// /// Returns an error if the host platform could not be determined. - pub fn new(target: Option) -> Result { + pub fn set_target(&mut self, triple: TargetTriple) -> BuildPlatformTargetBuilder<'_> { + BuildPlatformTargetBuilder { + parent: self, + triple, + } + } + + /// Creates a new [`BuildPlatform`]. + pub fn build(self) -> Result { let host = Platform::current().map_err(|error| UnknownHostPlatform { error })?; - Ok(Self { host, target }) + Ok(BuildPlatform { + host, + target: self.target, + }) } +} - /// Creates a new `BuildPlatform` where the host is specified. - /// - /// This is intended for testing situations. Most users should call [`Self::new`] instead. - pub fn new_with_host(host: Platform, target: Option) -> Self { - Self { host, target } +/// Populate a [`BuildPlatformTarget`] in the [`BuildPlatform`] built by +/// [`BuildPlatformBuilder`]. +pub struct BuildPlatformTargetBuilder<'a> { + parent: &'a mut BuildPlatformBuilder, + triple: TargetTriple, +} + +impl<'a> BuildPlatformTargetBuilder<'a> { + /// Build a [`BuildPlatform`] and add it to the [`BuildPlatformBuilder`]. + pub fn add(self) { + self.parent.target = Some(BuildPlatformTarget { + triple: self.triple, + }); } +} +/// The target platform. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct BuildPlatformTarget { + /// The target triplet, which consists of machine, vendor and OS. + pub triple: TargetTriple, +} + +/// A representation of host and target platform. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct BuildPlatform { + /// The host platform. + pub host: Platform, + + /// The target platform, if specified. + pub target: Option, +} + +impl BuildPlatform { /// Returns the argument to pass into `cargo metadata --filter-platform `. pub fn to_cargo_target_arg(&self) -> Result { match &self.target { - Some(target) => target.to_cargo_target_arg(), + Some(target) => target.triple.to_cargo_target_arg(), None => { // If there's no target, use the host platform. Ok(CargoTargetArg::Builtin(self.host.triple_str().to_owned())) @@ -50,19 +84,144 @@ impl BuildPlatforms { } } - /// Converts a target triple to a `String` that can be stored in the build-metadata. + /// Converts a target triple to a [`String`] that can be stored in the build-metadata. /// - /// cargo-nextest represents the host triple with `None` during runtime. However the - /// build-metadata might be used on a system with a different host triple. Therefore, the host - /// triple is detected if `target_triple` is `None`. + /// Only for backward compatibility. + pub fn to_summary_str(&self) -> Option { + self.target + .as_ref() + .map(|triple| triple.triple.platform.triple_str().to_owned()) + } + + /// Creates a [`BuildPlatform`] from a serializable summary. /// - /// XXX: This isn't quite correct -- instead, we should serialize this into our own - /// `BuildPlatformsSummary`. - pub fn to_summary(&self) -> PlatformSummary { - if let Some(target) = &self.target { - target.platform.to_summary() + /// Only for backward compatibility. + pub fn from_summary_str(summary: Option) -> Result { + let mut builder = BuildPlatformBuilder::default(); + if let Some(target_triple) = TargetTriple::deserialize_str(summary)? { + let target_builder = builder.set_target(target_triple); + target_builder.add(); + } + builder + .build() + .map_err(|error| RustBuildMetaParseError::UnknownHostPlatform(error.error)) + } +} + +/// Converts a target triple to a [`PlatformSummary`] that can be stored in the build-metadata. +/// +/// Deprecated in favor of [`BuildPlatform`]. +impl From for PlatformSummary { + fn from(value: BuildPlatform) -> Self { + if let Some(target) = &value.target { + target.triple.platform.to_summary() } else { - self.host.to_summary() + value.host.to_summary() + } + } +} + +/// Converts a target triple to a [`BuildPlatformSummary`] that can be stored in the build-metadata. +/// +/// cargo-nextest represents the host triple with `None` during runtime. However the +/// build-metadata might be used on a system with a different host triple. Therefore, the host +/// triple is detected if `target_triple` is `None`. +impl From for BuildPlatformSummary { + fn from(value: BuildPlatform) -> Self { + let target = { + if let Some(target) = &value.target { + target.triple.platform.to_summary() + } else { + value.host.to_summary() + } + }; + Self { target } + } +} + +/// Creates a [`BuildPlatform`] from a serializable summary for backwards compatibility. +impl TryFrom for BuildPlatform { + type Error = RustBuildMetaParseError; + + fn try_from(summary: PlatformSummary) -> Result { + let mut builder = BuildPlatformBuilder::default(); + if let Some(target_triple) = TargetTriple::deserialize(Some(summary))? { + let target_builder = builder.set_target(target_triple); + target_builder.add(); } + builder + .build() + .map_err(|error| RustBuildMetaParseError::UnknownHostPlatform(error.error)) + } +} + +/// Creates a [`BuildPlatform`] from a serializable summary. +impl TryFrom for BuildPlatform { + type Error = RustBuildMetaParseError; + + fn try_from(summary: BuildPlatformSummary) -> Result { + let mut builder = BuildPlatformBuilder::default(); + if let Some(target_triple) = TargetTriple::deserialize(Some(summary.target))? { + let target_builder = builder.set_target(target_triple); + target_builder.add(); + } + builder + .build() + .map_err(|error| RustBuildMetaParseError::UnknownHostPlatform(error.error)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_build_platform_builder_default() { + let build_platform = BuildPlatformBuilder::default() + .build() + .expect("default builder should build successfully"); + assert_eq!( + build_platform, + BuildPlatform { + host: Platform::current().expect("should detect the current platform successfully"), + target: None, + } + ); + } + + #[test] + fn test_build_platform_builder_with_target() { + let mut builder = BuildPlatformBuilder::default(); + builder + .set_target(TargetTriple::x86_64_unknown_linux_gnu()) + .add(); + let build_platform = builder + .build() + .expect("should build with target successfully"); + assert_eq!( + build_platform, + BuildPlatform { + host: Platform::current().expect("should detect the current platform successfully"), + target: Some(BuildPlatformTarget { + triple: TargetTriple::x86_64_unknown_linux_gnu(), + }) + } + ); + } + + #[test] + fn test_build_platform_builder_with_target_builder_not_added() { + let mut builder = BuildPlatformBuilder::default(); + builder.set_target(TargetTriple::x86_64_unknown_linux_gnu()); + let build_platform = builder + .build() + .expect("should build with target successfully"); + assert_eq!( + build_platform, + BuildPlatform { + host: Platform::current().expect("should detect the current platform successfully"), + target: None, + } + ); } } diff --git a/nextest-runner/src/reporter.rs b/nextest-runner/src/reporter.rs index 9afb258ea64..65fa5916fa4 100644 --- a/nextest-runner/src/reporter.rs +++ b/nextest-runner/src/reporter.rs @@ -1944,7 +1944,8 @@ impl Styles { mod tests { use super::*; use crate::{ - config::NextestConfig, platform::BuildPlatforms, reporter::structured::StructuredReporter, + config::NextestConfig, platform::BuildPlatformBuilder, + reporter::structured::StructuredReporter, }; use test_strategy::proptest; @@ -2216,7 +2217,7 @@ mod tests { let test_list = TestList::empty(); let config = NextestConfig::default_config("/fake/dir"); let profile = config.profile(NextestConfig::DEFAULT_PROFILE).unwrap(); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let mut buf: Vec = Vec::new(); let output = ReporterStderr::Buffer(&mut buf); diff --git a/nextest-runner/src/runner.rs b/nextest-runner/src/runner.rs index 62c0786602e..498e7d8e835 100644 --- a/nextest-runner/src/runner.rs +++ b/nextest-runner/src/runner.rs @@ -2370,7 +2370,7 @@ enum TerminateMode { #[cfg(test)] mod tests { use super::*; - use crate::{config::NextestConfig, platform::BuildPlatforms}; + use crate::{config::NextestConfig, platform::BuildPlatformBuilder}; #[test] fn no_capture_settings() { @@ -2382,7 +2382,7 @@ mod tests { let test_list = TestList::empty(); let config = NextestConfig::default_config("/fake/dir"); let profile = config.profile(NextestConfig::DEFAULT_PROFILE).unwrap(); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let handler_kind = SignalHandlerKind::Noop; let profile = profile.apply_build_platforms(&build_platforms); let runner = builder diff --git a/nextest-runner/src/target_runner.rs b/nextest-runner/src/target_runner.rs index dcb2fd105f0..12cb60065f2 100644 --- a/nextest-runner/src/target_runner.rs +++ b/nextest-runner/src/target_runner.rs @@ -6,7 +6,7 @@ use crate::{ cargo_config::{CargoConfig, CargoConfigSource, CargoConfigs, DiscoveredConfig, Runner}, errors::TargetRunnerError, - platform::BuildPlatforms, + platform, }; use camino::{Utf8Path, Utf8PathBuf}; use nextest_metadata::BuildPlatform; @@ -27,11 +27,11 @@ impl TargetRunner { /// or via a `CARGO_TARGET_{TRIPLE}_RUNNER` environment variable pub fn new( configs: &CargoConfigs, - build_platforms: &BuildPlatforms, + build_platforms: &platform::BuildPlatform, ) -> Result { let host = PlatformRunner::by_precedence(configs, &build_platforms.host)?; let target = match &build_platforms.target { - Some(target) => PlatformRunner::by_precedence(configs, &target.platform)?, + Some(target) => PlatformRunner::by_precedence(configs, &target.triple.platform)?, None => host.clone(), }; diff --git a/nextest-runner/tests/integration/basic.rs b/nextest-runner/tests/integration/basic.rs index 09d0d027a46..92b3bc0dcd9 100644 --- a/nextest-runner/tests/integration/basic.rs +++ b/nextest-runner/tests/integration/basic.rs @@ -10,7 +10,7 @@ use nextest_runner::{ config::{NextestConfig, RetryPolicy}, double_spawn::DoubleSpawnInfo, list::BinaryList, - platform::BuildPlatforms, + platform::BuildPlatformBuilder, reporter::heuristic_extract_description, runner::{ExecutionDescription, ExecutionResult, TestRunnerBuilder}, signal::SignalHandlerKind, @@ -27,7 +27,7 @@ fn test_list_binaries() -> Result<()> { set_env_vars(); let graph = &*PACKAGE_GRAPH; - let build_platforms = BuildPlatforms::new(None)?; + let build_platforms = BuildPlatformBuilder::default().build()?; let binary_list = BinaryList::from_messages( Cursor::new(&*FIXTURE_RAW_CARGO_TEST_OUTPUT), graph, @@ -104,7 +104,7 @@ fn test_run() -> Result<()> { let profile = config .profile(NextestConfig::DEFAULT_PROFILE) .expect("default config is valid"); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let profile = profile.apply_build_platforms(&build_platforms); let runner = TestRunnerBuilder::default() @@ -213,7 +213,7 @@ fn test_run_ignored() -> Result<()> { let profile = config .profile(NextestConfig::DEFAULT_PROFILE) .expect("default config is valid"); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let profile = profile.apply_build_platforms(&build_platforms); let runner = TestRunnerBuilder::default() @@ -411,7 +411,7 @@ fn test_retries(retries: Option) -> Result<()> { let profile = config .profile("with-retries") .expect("with-retries config is valid"); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let profile = profile.apply_build_platforms(&build_platforms); let profile_retries = profile.retries(); @@ -561,7 +561,7 @@ fn test_termination() -> Result<()> { let profile = config .profile("with-termination") .expect("with-termination config is valid"); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let profile = profile.apply_build_platforms(&build_platforms); let runner = TestRunnerBuilder::default() diff --git a/nextest-runner/tests/integration/fixtures.rs b/nextest-runner/tests/integration/fixtures.rs index ff480855c03..b7fef7585b0 100644 --- a/nextest-runner/tests/integration/fixtures.rs +++ b/nextest-runner/tests/integration/fixtures.rs @@ -14,7 +14,7 @@ use nextest_runner::{ list::{ BinaryList, RustBuildMeta, RustTestArtifact, TestExecuteContext, TestList, TestListState, }, - platform::BuildPlatforms, + platform::BuildPlatformBuilder, reporter::TestEventKind, reuse_build::PathMapper, runner::{ @@ -332,7 +332,7 @@ impl FixtureTargets { ) .unwrap(); let env = EnvironmentMap::new(&cargo_configs); - let build_platforms = BuildPlatforms::new(None).unwrap(); + let build_platforms = BuildPlatformBuilder::default().build().unwrap(); let binary_list = Arc::new( BinaryList::from_messages( Cursor::new(&*FIXTURE_RAW_CARGO_TEST_OUTPUT), diff --git a/nextest-runner/tests/integration/target_runner.rs b/nextest-runner/tests/integration/target_runner.rs index 11e7e3ca1d1..a634d24b2ae 100644 --- a/nextest-runner/tests/integration/target_runner.rs +++ b/nextest-runner/tests/integration/target_runner.rs @@ -8,7 +8,7 @@ use nextest_runner::{ cargo_config::{CargoConfigs, TargetTriple}, config::NextestConfig, double_spawn::DoubleSpawnInfo, - platform::BuildPlatforms, + platform::{BuildPlatform, BuildPlatformBuilder}, runner::TestRunnerBuilder, signal::SignalHandlerKind, target_runner::{PlatformRunner, TargetRunner}, @@ -17,7 +17,7 @@ use nextest_runner::{ use std::env; use target_spec::Platform; -fn runner_for_target(triple: Option<&str>) -> Result<(BuildPlatforms, TargetRunner)> { +fn runner_for_target(triple: Option<&str>) -> Result<(BuildPlatform, TargetRunner)> { let configs = CargoConfigs::new_with_isolation( Vec::::new(), &workspace_root(), @@ -25,8 +25,11 @@ fn runner_for_target(triple: Option<&str>) -> Result<(BuildPlatforms, TargetRunn Vec::new(), ) .unwrap(); - let triple = TargetTriple::find(&configs, triple)?; - let build_platforms = BuildPlatforms::new(triple)?; + let mut build_platform_builder = BuildPlatformBuilder::default(); + if let Some(triple) = TargetTriple::find(&configs, triple)? { + build_platform_builder.set_target(triple).add(); + } + let build_platforms = build_platform_builder.build()?; let target_runner = TargetRunner::new(&configs, &build_platforms)?; Ok((build_platforms, target_runner)) }