Skip to content

Commit

Permalink
Refactor that prepares to add rustc libdir to BuildPlatform
Browse files Browse the repository at this point in the history
* 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`
  • Loading branch information
06393993 committed May 18, 2024
1 parent c7536ff commit ead0452
Show file tree
Hide file tree
Showing 17 changed files with 465 additions and 109 deletions.
35 changes: 21 additions & 14 deletions cargo-nextest/src/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -687,7 +687,7 @@ impl CargoOptions {
graph: &PackageGraph,
manifest_path: Option<&Utf8Path>,
output: OutputContext,
build_platforms: BuildPlatforms,
build_platforms: platform::BuildPlatform,
) -> Result<BinaryList> {
// 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.
Expand Down Expand Up @@ -971,7 +971,8 @@ impl From<FinalStatusLevelOpt> for FinalStatusLevel {
#[derive(Debug)]
struct BaseApp {
output: OutputContext,
build_platforms: BuildPlatforms,
/// TODO: support multiple --target options
build_platforms: platform::BuildPlatform,
cargo_metadata_json: Arc<String>,
package_graph: Arc<PackageGraph>,
// Potentially remapped workspace root (might not be the same as the graph).
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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<String> {
let cargo_target_arg = build_platforms.to_cargo_target_arg()?;
Expand Down Expand Up @@ -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<BuildPlatforms, UnknownHostPlatform> {
let target_triple = match TargetTriple::find(cargo_configs, target_cli_option) {
) -> Option<TargetTriple> {
match TargetTriple::find(cargo_configs, target_cli_option) {
Ok(Some(triple)) => {
log::debug!(
"using target triple `{}` defined by `{}`; {}",
Expand All @@ -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) => {
Expand Down
17 changes: 16 additions & 1 deletion nextest-metadata/src/test_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -485,6 +485,11 @@ pub struct RustBuildMetaSummary {

/// The target platforms used while compiling the Rust artifacts.
#[serde(default)]
pub target_platforms_v2: Vec<BuildPlatformSummary>,

/// Deprecated in favor of target_platforms_v2. The target platforms used while compiling the
/// Rust artifacts.
#[serde(default)]
pub target_platforms: Vec<PlatformSummary>,

/// A deprecated form of the target platform used for cross-compilation, if any.
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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 =
Expand Down
12 changes: 12 additions & 0 deletions nextest-runner/src/cargo_config/target_triple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions nextest-runner/src/config/config_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
UnknownConfigScriptError, UnknownTestGroupError,
},
list::TestList,
platform::BuildPlatforms,
platform::BuildPlatform,
reporter::{FinalStatusLevel, StatusLevel, TestOutputDisplay},
};
use camino::{Utf8Path, Utf8PathBuf};
Expand Down Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions nextest-runner/src/config/overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -354,7 +354,7 @@ impl CompiledData<PreBuildPlatform> {

pub(super) fn apply_build_platforms(
self,
build_platforms: &BuildPlatforms,
build_platforms: &NextestBuildPlatform,
) -> CompiledData<FinalConfig> {
let overrides = self
.overrides
Expand Down Expand Up @@ -475,15 +475,15 @@ impl CompiledOverride<PreBuildPlatform> {

pub(super) fn apply_build_platforms(
self,
build_platforms: &BuildPlatforms,
build_platforms: &NextestBuildPlatform,
) -> CompiledOverride<FinalConfig> {
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 {
Expand Down
8 changes: 4 additions & 4 deletions nextest-runner/src/config/scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -381,15 +381,15 @@ impl CompiledProfileScripts<PreBuildPlatform> {

pub(super) fn apply_build_platforms(
self,
build_platforms: &BuildPlatforms,
build_platforms: &NextestBuildPlatform,
) -> CompiledProfileScripts<FinalConfig> {
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 {
Expand Down
20 changes: 11 additions & 9 deletions nextest-runner/src/config/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 15 additions & 4 deletions nextest-runner/src/list/binary_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -51,7 +51,7 @@ impl BinaryList {
pub fn from_messages(
reader: impl io::BufRead,
graph: &PackageGraph,
build_platforms: BuildPlatforms,
build_platforms: NextestBuildPlatform,
) -> Result<Self, FromMessagesError> {
let mut state = BinaryListBuildState::new(graph, build_platforms);

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -394,6 +394,7 @@ mod tests {
use crate::{
cargo_config::{TargetDefinitionLocation, TargetTriple, TargetTripleSource},
list::SerializableFormat,
platform::BuildPlatformBuilder,
};
use indoc::indoc;
use maplit::btreeset;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit ead0452

Please sign in to comment.