Skip to content

Commit

Permalink
Collect targets in a deterministic order (#1736)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dtolnay
dtolnay committed Jan 1, 2023
1 parent 52e02c2 commit 532e60f
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 97 deletions.
144 changes: 73 additions & 71 deletions crate_universe/src/context/crate_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ pub struct CrateDependency {
#[serde(default)]
pub struct TargetAttributes {
/// The module name of the crate (notably, not the package name).
//
// This must be the first field of `TargetAttributes` to make it the
// lexicographically first thing the derived `Ord` implementation will sort
// by. The `Ord` impl controls the order of multiple rules of the same type
// in the same BUILD file. In particular, this makes packages with multiple
// bin crates generate those `rust_binary` targets in alphanumeric order.
pub crate_name: String,

/// The path to the crate's root source file, relative to the manifest.
Expand All @@ -39,17 +45,17 @@ pub struct TargetAttributes {

#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone)]
pub enum Rule {
/// `cargo_build_script`
BuildScript(TargetAttributes),
/// `rust_library`
Library(TargetAttributes),

/// `rust_proc_macro`
ProcMacro(TargetAttributes),

/// `rust_library`
Library(TargetAttributes),

/// `rust_binary`
Binary(TargetAttributes),

/// `cargo_build_script`
BuildScript(TargetAttributes),
}

/// A set of attributes common to most `rust_library`, `rust_proc_macro`, and other
Expand Down Expand Up @@ -221,7 +227,7 @@ pub struct CrateContext {
pub repository: Option<SourceAnnotation>,

/// A list of all targets (lib, proc-macro, bin) associated with this package
pub targets: Vec<Rule>,
pub targets: BTreeSet<Rule>,

/// The name of the crate's root library target. This is the target that a dependent
/// would get if they were to depend on `{crate_name}`.
Expand Down Expand Up @@ -554,7 +560,7 @@ impl CrateContext {
node: &Node,
packages: &BTreeMap<PackageId, Package>,
include_build_scripts: bool,
) -> Vec<Rule> {
) -> BTreeSet<Rule> {
let package = &packages[&node.id];

let package_root = package
Expand All @@ -567,60 +573,56 @@ impl CrateContext {
.targets
.iter()
.flat_map(|target| {
target
.kind
.iter()
.filter_map(|kind| {
// Unfortunately, The package graph and resolve graph of cargo metadata have different representations
// for the crate names (resolve graph sanitizes names to match module names) so to get the rest of this
// content to align when rendering, the package target names are always sanitized.
let crate_name = sanitize_module_name(&target.name);

// Locate the crate's root source file relative to the package root normalized for unix
let crate_root = pathdiff::diff_paths(&target.src_path, package_root).map(
// Normalize the path so that it always renders the same regardless of platform
|root| root.to_string_lossy().replace('\\', "/"),
);

// Conditionally check to see if the dependencies is a build-script target
if include_build_scripts && kind == "custom-build" {
return Some(Rule::BuildScript(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a proc-macro target
if kind == "proc-macro" {
return Some(Rule::ProcMacro(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a library target
if ["lib", "rlib"].contains(&kind.as_str()) {
return Some(Rule::Library(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a library target
if kind == "bin" {
return Some(Rule::Binary(TargetAttributes {
crate_name: target.name.clone(),
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

None
})
.collect::<Vec<Rule>>()
target.kind.iter().filter_map(move |kind| {
// Unfortunately, The package graph and resolve graph of cargo metadata have different representations
// for the crate names (resolve graph sanitizes names to match module names) so to get the rest of this
// content to align when rendering, the package target names are always sanitized.
let crate_name = sanitize_module_name(&target.name);

// Locate the crate's root source file relative to the package root normalized for unix
let crate_root = pathdiff::diff_paths(&target.src_path, package_root).map(
// Normalize the path so that it always renders the same regardless of platform
|root| root.to_string_lossy().replace('\\', "/"),
);

// Conditionally check to see if the dependencies is a build-script target
if include_build_scripts && kind == "custom-build" {
return Some(Rule::BuildScript(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a proc-macro target
if kind == "proc-macro" {
return Some(Rule::ProcMacro(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a library target
if ["lib", "rlib"].contains(&kind.as_str()) {
return Some(Rule::Library(TargetAttributes {
crate_name,
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

// Check to see if the dependencies is a library target
if kind == "bin" {
return Some(Rule::Binary(TargetAttributes {
crate_name: target.name.clone(),
crate_root,
srcs: Glob::new_rust_srcs(),
}));
}

None
})
})
.collect()
}
Expand Down Expand Up @@ -661,7 +663,7 @@ mod test {
assert_eq!(context.name, "common");
assert_eq!(
context.targets,
vec![
BTreeSet::from([
Rule::Library(TargetAttributes {
crate_name: "common".to_owned(),
crate_root: Some("lib.rs".to_owned()),
Expand All @@ -672,7 +674,7 @@ mod test {
crate_root: Some("main.rs".to_owned()),
srcs: Glob::new_rust_srcs(),
}),
]
]),
);
}

Expand Down Expand Up @@ -709,7 +711,7 @@ mod test {
assert_eq!(context.name, "common");
assert_eq!(
context.targets,
vec![
BTreeSet::from([
Rule::Library(TargetAttributes {
crate_name: "common".to_owned(),
crate_root: Some("lib.rs".to_owned()),
Expand All @@ -720,7 +722,7 @@ mod test {
crate_root: Some("main.rs".to_owned()),
srcs: Glob::new_rust_srcs(),
}),
]
]),
);
assert_eq!(
context.common_attrs.data_glob,
Expand Down Expand Up @@ -769,7 +771,7 @@ mod test {
assert!(context.build_script_attrs.is_some());
assert_eq!(
context.targets,
vec![
BTreeSet::from([
Rule::Library(TargetAttributes {
crate_name: "openssl_sys".to_owned(),
crate_root: Some("src/lib.rs".to_owned()),
Expand All @@ -780,7 +782,7 @@ mod test {
crate_root: Some("build/main.rs".to_owned()),
srcs: Glob::new_rust_srcs(),
})
]
]),
);

// Cargo build scripts should include all sources
Expand Down Expand Up @@ -810,11 +812,11 @@ mod test {
assert!(context.build_script_attrs.is_none());
assert_eq!(
context.targets,
vec![Rule::Library(TargetAttributes {
BTreeSet::from([Rule::Library(TargetAttributes {
crate_name: "openssl_sys".to_owned(),
crate_root: Some("src/lib.rs".to_owned()),
srcs: Glob::new_rust_srcs(),
})],
})]),
);
}

Expand All @@ -841,11 +843,11 @@ mod test {
assert!(context.build_script_attrs.is_none());
assert_eq!(
context.targets,
vec![Rule::Library(TargetAttributes {
BTreeSet::from([Rule::Library(TargetAttributes {
crate_name: "sysinfo".to_owned(),
crate_root: Some("src/lib.rs".to_owned()),
srcs: Glob::new_rust_srcs(),
})],
})]),
);
}
}
20 changes: 10 additions & 10 deletions crate_universe/src/rendering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Library(mock_target_attributes())],
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand All @@ -363,11 +363,11 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::BuildScript(TargetAttributes {
targets: BTreeSet::from([Rule::BuildScript(TargetAttributes {
crate_name: "build_script_build".to_owned(),
crate_root: Some("build.rs".to_owned()),
..TargetAttributes::default()
})],
})]),
// Build script attributes are required.
build_script_attrs: Some(BuildScriptAttributes::default()),
..CrateContext::default()
Expand Down Expand Up @@ -397,7 +397,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::ProcMacro(mock_target_attributes())],
targets: BTreeSet::from([Rule::ProcMacro(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand All @@ -422,7 +422,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Binary(mock_target_attributes())],
targets: BTreeSet::from([Rule::Binary(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand All @@ -447,7 +447,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Binary(mock_target_attributes())],
targets: BTreeSet::from([Rule::Binary(mock_target_attributes())]),
additive_build_file_content: Some(
"# Hello World from additive section!".to_owned(),
),
Expand Down Expand Up @@ -493,7 +493,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Library(mock_target_attributes())],
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand All @@ -515,7 +515,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Library(mock_target_attributes())],
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand Down Expand Up @@ -545,7 +545,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Library(mock_target_attributes())],
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
..CrateContext::default()
},
);
Expand Down Expand Up @@ -584,7 +584,7 @@ mod test {
CrateContext {
name: crate_id.name,
version: crate_id.version,
targets: vec![Rule::Library(mock_target_attributes())],
targets: BTreeSet::from([Rule::Library(mock_target_attributes())]),
common_attrs: CommonAttributes {
rustc_flags: rustc_flags.clone(),
..CommonAttributes::default()
Expand Down
8 changes: 4 additions & 4 deletions proto/3rdparty/crates/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ alias(
)

alias(
name = "protobuf-codegen__protoc-gen-rust",
actual = "@rules_rust_proto__protobuf-codegen-2.8.2//:protoc-gen-rust__bin",
name = "protobuf-codegen__protobuf-bin-gen-rust-do-not-use",
actual = "@rules_rust_proto__protobuf-codegen-2.8.2//:protobuf-bin-gen-rust-do-not-use__bin",
tags = ["manual"],
)

alias(
name = "protobuf-codegen__protobuf-bin-gen-rust-do-not-use",
actual = "@rules_rust_proto__protobuf-codegen-2.8.2//:protobuf-bin-gen-rust-do-not-use__bin",
name = "protobuf-codegen__protoc-gen-rust",
actual = "@rules_rust_proto__protobuf-codegen-2.8.2//:protoc-gen-rust__bin",
tags = ["manual"],
)
8 changes: 4 additions & 4 deletions proto/3rdparty/crates/BUILD.protobuf-codegen-2.8.2.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ rust_library(
)

rust_binary(
name = "protoc-gen-rust__bin",
name = "protobuf-bin-gen-rust-do-not-use__bin",
srcs = glob(
include = [
"**/*.rs",
Expand All @@ -115,7 +115,7 @@ rust_binary(
}),
crate_features = [
],
crate_root = "src/bin/protoc-gen-rust.rs",
crate_root = "src/bin/protobuf-bin-gen-rust-do-not-use.rs",
data = select_with_or({
"//conditions:default": [
],
Expand Down Expand Up @@ -156,7 +156,7 @@ rust_binary(
)

rust_binary(
name = "protobuf-bin-gen-rust-do-not-use__bin",
name = "protoc-gen-rust__bin",
srcs = glob(
include = [
"**/*.rs",
Expand All @@ -183,7 +183,7 @@ rust_binary(
}),
crate_features = [
],
crate_root = "src/bin/protobuf-bin-gen-rust-do-not-use.rs",
crate_root = "src/bin/protoc-gen-rust.rs",
data = select_with_or({
"//conditions:default": [
],
Expand Down
Loading

0 comments on commit 532e60f

Please sign in to comment.