Skip to content

Introduced TargetTriple newtype. #2315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Introduced TargetTriple newtype. #2315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crate_universe/private/srcs.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,5 @@ CARGO_BAZEL_SRCS = [
Label("//crate_universe:src/utils/starlark/select.rs"),
Label("//crate_universe:src/utils/starlark/serialize.rs"),
Label("//crate_universe:src/utils/starlark/target_compatible_with.rs"),
Label("//crate_universe:src/utils/target_triple.rs"),
]
4 changes: 3 additions & 1 deletion crate_universe/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use serde::de::value::SeqAccessDeserializer;
use serde::de::{Deserializer, SeqAccess, Visitor};
use serde::{Deserialize, Serialize, Serializer};

use crate::utils::target_triple::TargetTriple;

/// Representations of different kinds of crate vendoring into workspaces.
#[derive(Debug, Serialize, Deserialize, Clone)]
#[serde(rename_all = "lowercase")]
Expand Down Expand Up @@ -632,7 +634,7 @@ pub struct Config {

/// A set of platform triples to use in generated select statements
#[serde(default, skip_serializing_if = "BTreeSet::is_empty")]
pub supported_platform_triples: BTreeSet<String>,
pub supported_platform_triples: BTreeSet<TargetTriple>,
}

impl Config {
Expand Down
3 changes: 2 additions & 1 deletion crate_universe/src/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::context::platforms::resolve_cfg_platforms;
use crate::lockfile::Digest;
use crate::metadata::{Annotations, Dependency};
use crate::utils::starlark::{Select, SelectList};
use crate::utils::target_triple::TargetTriple;

pub use self::crate_context::*;

Expand All @@ -37,7 +38,7 @@ pub struct Context {
pub workspace_members: BTreeMap<CrateId, String>,

/// A mapping of `cfg` flags to platform triples supporting the configuration
pub conditions: BTreeMap<String, BTreeSet<String>>,
pub conditions: BTreeMap<String, BTreeSet<TargetTriple>>,

/// A list of crates visible to any bazel module.
pub direct_deps: BTreeSet<CrateId>,
Expand Down
95 changes: 57 additions & 38 deletions crate_universe/src/context/platforms.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ use cfg_expr::targets::{get_builtin_target_by_triple, TargetInfo};
use cfg_expr::{Expression, Predicate};

use crate::context::CrateContext;
use crate::utils::cargo_target;
use crate::utils::starlark::Select;
use crate::utils::target_triple::TargetTriple;

/// Walk through all dependencies in a [CrateContext] list for all configuration specific
/// dependencies to produce a mapping of configuration to compatible platform triples.
/// Also adds mappings for all known platform triples.
/// dependencies to produce a mapping of configurations/Cargo target_triples to compatible
/// Bazel target_triples. Also adds mappings for all known target_triples.
pub fn resolve_cfg_platforms(
crates: Vec<&CrateContext>,
supported_platform_triples: &BTreeSet<String>,
) -> Result<BTreeMap<String, BTreeSet<String>>> {
supported_platform_triples: &BTreeSet<TargetTriple>,
) -> Result<BTreeMap<String, BTreeSet<TargetTriple>>> {
// Collect all unique configurations from all dependencies into a single set
let configurations: BTreeSet<String> = crates
.iter()
Expand Down Expand Up @@ -46,33 +46,33 @@ pub fn resolve_cfg_platforms(
let target_infos = supported_platform_triples
.iter()
.map(
|target| match get_builtin_target_by_triple(&cargo_target(target)) {
Some(info) => Ok((target, info)),
|target_triple| match get_builtin_target_by_triple(&target_triple.to_cargo()) {
Some(info) => Ok((target_triple, info)),
None => Err(anyhow!(
"Invalid platform triple in supported platforms: {}",
target
target_triple
)),
},
)
.collect::<Result<BTreeMap<&String, &'static TargetInfo>>>()?;
.collect::<Result<BTreeMap<&TargetTriple, &'static TargetInfo>>>()?;

// `cfg-expr` does not understand configurations that are simply platform triples
// (`x86_64-unknown-linux-gnu` vs `cfg(target = "x86_64-unkonwn-linux-gnu")`). So
// in order to parse configurations, the text is renamed for the check but the
// original is retained for comaptibility with the manifest.
let rename = |cfg: &str| -> String { format!("cfg(target = \"{}\")", cargo_target(cfg)) };
let rename = |cfg: &str| -> String { format!("cfg(target = \"{cfg}\")") };
let original_cfgs: BTreeMap<String, String> = configurations
.iter()
.filter(|cfg| !cfg.starts_with("cfg("))
.map(|cfg| (rename(cfg), cfg.clone()))
.collect();

configurations
let mut conditions = configurations
.into_iter()
// `cfg-expr` requires that the expressions be actual `cfg` expressions. Any time
// there's a target triple (which is a valid constraint), convert it to a cfg expression.
.map(|cfg| match cfg.starts_with("cfg(") {
true => cfg.to_string(),
true => cfg,
false => rename(&cfg),
})
// Check the current configuration with against each supported triple
Expand All @@ -92,7 +92,7 @@ pub fn resolve_cfg_platforms(
_ => false,
})
})
.map(|(triple, _)| String::from(*triple))
.map(|(triple, _)| (*triple).clone())
.collect();

// Map any renamed configurations back to their original IDs
Expand All @@ -103,11 +103,16 @@ pub fn resolve_cfg_platforms(

Ok((cfg, triples))
})
.chain(supported_platform_triples.iter().filter_map(|triple| {
let target = get_builtin_target_by_triple(&cargo_target(triple));
target.map(|target| Ok((triple.clone(), [target.triple.to_string()].into())))
}))
.collect()
.collect::<Result<BTreeMap<String, BTreeSet<TargetTriple>>>>()?;
for target_triple in supported_platform_triples.iter() {
let target = get_builtin_target_by_triple(&target_triple.to_cargo())
.expect("TargetTriple has already been validated by `cargo tree` invocation");
conditions
.entry(target.triple.to_string())
.or_default()
.insert(target_triple.clone());
}
Ok(conditions)
}

#[cfg(test)]
Expand All @@ -119,11 +124,11 @@ mod test {

use super::*;

fn supported_platform_triples() -> BTreeSet<String> {
fn supported_platform_triples() -> BTreeSet<TargetTriple> {
BTreeSet::from([
"aarch64-apple-darwin".to_owned(),
"i686-apple-darwin".to_owned(),
"x86_64-unknown-linux-gnu".to_owned(),
TargetTriple::from_bazel("aarch64-apple-darwin".to_owned()),
TargetTriple::from_bazel("i686-apple-darwin".to_owned()),
TargetTriple::from_bazel("x86_64-unknown-linux-gnu".to_owned()),
])
}

Expand Down Expand Up @@ -158,15 +163,17 @@ mod test {
// All known triples.
(
"aarch64-apple-darwin".to_owned(),
BTreeSet::from(["aarch64-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("aarch64-apple-darwin".to_owned())]),
),
(
"i686-apple-darwin".to_owned(),
BTreeSet::from(["i686-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("i686-apple-darwin".to_owned())]),
),
(
"x86_64-unknown-linux-gnu".to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned()
)]),
),
])
)
Expand Down Expand Up @@ -199,13 +206,15 @@ mod test {
let data = BTreeMap::from([
(
r#"cfg(target = "x86_64-unknown-linux-gnu")"#.to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned(),
)]),
),
(
r#"cfg(any(target_os = "macos", target_os = "ios"))"#.to_owned(),
BTreeSet::from([
"aarch64-apple-darwin".to_owned(),
"i686-apple-darwin".to_owned(),
TargetTriple::from_bazel("aarch64-apple-darwin".to_owned()),
TargetTriple::from_bazel("i686-apple-darwin".to_owned()),
]),
),
]);
Expand All @@ -223,15 +232,19 @@ mod test {
// All known triples.
(
"aarch64-apple-darwin".to_owned(),
BTreeSet::from(["aarch64-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"aarch64-apple-darwin".to_owned()
)]),
),
(
"i686-apple-darwin".to_owned(),
BTreeSet::from(["i686-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("i686-apple-darwin".to_owned())]),
),
(
"x86_64-unknown-linux-gnu".to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned()
)]),
),
])
);
Expand Down Expand Up @@ -269,20 +282,24 @@ mod test {
BTreeMap::from([
(
configuration,
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()])
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned()
)])
),
// All known triples.
(
"aarch64-apple-darwin".to_owned(),
BTreeSet::from(["aarch64-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("aarch64-apple-darwin".to_owned())]),
),
(
"i686-apple-darwin".to_owned(),
BTreeSet::from(["i686-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("i686-apple-darwin".to_owned())]),
),
(
"x86_64-unknown-linux-gnu".to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned()
)]),
),
])
);
Expand Down Expand Up @@ -321,15 +338,17 @@ mod test {
// All known triples.
(
"aarch64-apple-darwin".to_owned(),
BTreeSet::from(["aarch64-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("aarch64-apple-darwin".to_owned())]),
),
(
"i686-apple-darwin".to_owned(),
BTreeSet::from(["i686-apple-darwin".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel("i686-apple-darwin".to_owned())]),
),
(
"x86_64-unknown-linux-gnu".to_owned(),
BTreeSet::from(["x86_64-unknown-linux-gnu".to_owned()]),
BTreeSet::from([TargetTriple::from_bazel(
"x86_64-unknown-linux-gnu".to_owned()
)]),
),
])
);
Expand Down
19 changes: 10 additions & 9 deletions crate_universe/src/lockfile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ impl PartialEq<String> for Digest {
mod test {
use crate::config::{CrateAnnotations, CrateId};
use crate::splicing::cargo_config::{AdditionalRegistry, CargoConfig, Registry};
use crate::utils::target_triple::TargetTriple;

use super::*;

Expand Down Expand Up @@ -230,15 +231,15 @@ mod test {
)]),
cargo_config: None,
supported_platform_triples: BTreeSet::from([
"aarch64-apple-darwin".to_owned(),
"aarch64-unknown-linux-gnu".to_owned(),
"aarch64-pc-windows-msvc".to_owned(),
"wasm32-unknown-unknown".to_owned(),
"wasm32-wasi".to_owned(),
"x86_64-apple-darwin".to_owned(),
"x86_64-pc-windows-msvc".to_owned(),
"x86_64-unknown-freebsd".to_owned(),
"x86_64-unknown-linux-gnu".to_owned(),
TargetTriple::from_bazel("aarch64-apple-darwin".to_owned()),
TargetTriple::from_bazel("aarch64-unknown-linux-gnu".to_owned()),
TargetTriple::from_bazel("aarch64-pc-windows-msvc".to_owned()),
TargetTriple::from_bazel("wasm32-unknown-unknown".to_owned()),
TargetTriple::from_bazel("wasm32-wasi".to_owned()),
TargetTriple::from_bazel("x86_64-apple-darwin".to_owned()),
TargetTriple::from_bazel("x86_64-pc-windows-msvc".to_owned()),
TargetTriple::from_bazel("x86_64-unknown-freebsd".to_owned()),
TargetTriple::from_bazel("x86_64-unknown-linux-gnu".to_owned()),
]),
..Config::default()
};
Expand Down
31 changes: 16 additions & 15 deletions crate_universe/src/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use tracing::debug;

use crate::config::CrateId;
use crate::lockfile::Digest;
use crate::utils::cargo_target;
use crate::utils::starlark::SelectList;
use crate::utils::target_triple::TargetTriple;

pub use self::dependency::*;
pub use self::metadata_annotation::*;
Expand Down Expand Up @@ -458,17 +458,17 @@ impl FeatureGenerator {
pub fn generate(
&self,
manifest_path: &Path,
platform_triples: &BTreeSet<String>,
target_triples: &BTreeSet<TargetTriple>,
) -> Result<BTreeMap<CrateId, SelectList<String>>> {
debug!(
"Generating features for manifest {}",
manifest_path.display()
);

let manifest_dir = manifest_path.parent().unwrap();
let mut target_to_child = BTreeMap::new();
debug!("Spawning processes for {:?}", platform_triples);
for target in platform_triples {
let mut target_triple_to_child = BTreeMap::new();
debug!("Spawning processes for {:?}", target_triples);
for target_triple in target_triples {
// We use `cargo tree` here because `cargo metadata` doesn't report
// back target-specific features (enabled with `resolver = "2"`).
// This is unfortunately a bit of a hack. See:
Expand All @@ -488,28 +488,29 @@ impl FeatureGenerator {
.arg("--color=never")
.arg("--workspace")
.arg("--target")
.arg(cargo_target(target))
.arg(target_triple.to_cargo())
.env("RUSTC", &self.rustc_bin)
.stdout(std::process::Stdio::piped())
.stderr(std::process::Stdio::piped())
.spawn()
.with_context(|| {
format!(
"Error spawning cargo in child process to compute features for target '{}', manifest path '{}'",
target,
target_triple,
manifest_path.display()
)
})?;
target_to_child.insert(target, output);
target_triple_to_child.insert(target_triple, output);
}
let mut crate_features = BTreeMap::<CrateId, BTreeMap<String, BTreeSet<String>>>::new();
for (target, child) in target_to_child.into_iter() {
let mut crate_features =
BTreeMap::<CrateId, BTreeMap<TargetTriple, BTreeSet<String>>>::new();
for (target_triple, child) in target_triple_to_child.into_iter() {
let output = child
.wait_with_output()
.with_context(|| {
format!(
"Error running cargo in child process to compute features for target '{}', manifest path '{}'",
target,
target_triple,
manifest_path.display()
)
})?;
Expand All @@ -518,15 +519,15 @@ impl FeatureGenerator {
eprintln!("{}", String::from_utf8_lossy(&output.stderr));
bail!(format!("Failed to run cargo tree: {}", output.status))
}
debug!("Process complete for {}", target);
debug!("Process complete for {}", target_triple);
for (crate_id, features) in
parse_features_from_cargo_tree_output(output.stdout.lines())?
{
debug!("\tFor {} features were: {:?}", crate_id, features);
crate_features
.entry(crate_id)
.or_default()
.insert(target.to_owned(), features);
.insert(target_triple.clone(), features);
}
}
let mut result = BTreeMap::<CrateId, SelectList<String>>::new();
Expand All @@ -542,10 +543,10 @@ impl FeatureGenerator {
)
.unwrap_or_default();
let mut select_list = SelectList::default();
for (target, fs) in features {
for (target_triple, fs) in features {
if fs != common {
for f in fs {
select_list.insert(f, Some(target.clone()));
select_list.insert(f, Some(target_triple.to_bazel()));
}
}
}
Expand Down
Loading