From a312ce7db2d3b5065676c1b74da6498c81b26240 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 31 Dec 2022 18:34:49 -0800 Subject: [PATCH] Collect targets in a deterministic order --- crate_universe/src/context/crate_context.rs | 144 +++++++++--------- crate_universe/src/rendering.rs | 20 +-- proto/3rdparty/crates/BUILD.bazel | 8 +- .../crates/BUILD.protobuf-codegen-2.8.2.bazel | 8 +- wasm_bindgen/3rdparty/crates/BUILD.bazel | 8 +- .../crates/BUILD.wait-timeout-0.2.0.bazel | 8 +- 6 files changed, 99 insertions(+), 97 deletions(-) diff --git a/crate_universe/src/context/crate_context.rs b/crate_universe/src/context/crate_context.rs index 2ba0c443a8..3faf687640 100644 --- a/crate_universe/src/context/crate_context.rs +++ b/crate_universe/src/context/crate_context.rs @@ -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. @@ -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 @@ -221,7 +227,7 @@ pub struct CrateContext { pub repository: Option, /// A list of all targets (lib, proc-macro, bin) associated with this package - pub targets: Vec, + pub targets: BTreeSet, /// 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}`. @@ -554,7 +560,7 @@ impl CrateContext { node: &Node, packages: &BTreeMap, include_build_scripts: bool, - ) -> Vec { + ) -> BTreeSet { let package = &packages[&node.id]; let package_root = package @@ -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::>() + 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() } @@ -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()), @@ -672,7 +674,7 @@ mod test { crate_root: Some("main.rs".to_owned()), srcs: Glob::new_rust_srcs(), }), - ] + ]), ); } @@ -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()), @@ -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, @@ -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()), @@ -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 @@ -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(), - })], + })]), ); } @@ -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(), - })], + })]), ); } } diff --git a/crate_universe/src/rendering.rs b/crate_universe/src/rendering.rs index 83e008f70e..6bc00eca7e 100644 --- a/crate_universe/src/rendering.rs +++ b/crate_universe/src/rendering.rs @@ -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() }, ); @@ -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() @@ -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() }, ); @@ -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() }, ); @@ -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(), ), @@ -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() }, ); @@ -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() }, ); @@ -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() }, ); @@ -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() diff --git a/proto/3rdparty/crates/BUILD.bazel b/proto/3rdparty/crates/BUILD.bazel index 075d512c85..771e2cc810 100644 --- a/proto/3rdparty/crates/BUILD.bazel +++ b/proto/3rdparty/crates/BUILD.bazel @@ -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"], ) diff --git a/proto/3rdparty/crates/BUILD.protobuf-codegen-2.8.2.bazel b/proto/3rdparty/crates/BUILD.protobuf-codegen-2.8.2.bazel index 95a96a3b96..f70b9ff14c 100644 --- a/proto/3rdparty/crates/BUILD.protobuf-codegen-2.8.2.bazel +++ b/proto/3rdparty/crates/BUILD.protobuf-codegen-2.8.2.bazel @@ -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", @@ -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": [ ], @@ -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", @@ -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": [ ], diff --git a/wasm_bindgen/3rdparty/crates/BUILD.bazel b/wasm_bindgen/3rdparty/crates/BUILD.bazel index 0aca0f2771..8f4ec89879 100644 --- a/wasm_bindgen/3rdparty/crates/BUILD.bazel +++ b/wasm_bindgen/3rdparty/crates/BUILD.bazel @@ -209,14 +209,14 @@ alias( ) alias( - name = "wait-timeout__sleep", - actual = "@rules_rust_wasm_bindgen__wait-timeout-0.2.0//:sleep__bin", + name = "wait-timeout__reader", + actual = "@rules_rust_wasm_bindgen__wait-timeout-0.2.0//:reader__bin", tags = ["manual"], ) alias( - name = "wait-timeout__reader", - actual = "@rules_rust_wasm_bindgen__wait-timeout-0.2.0//:reader__bin", + name = "wait-timeout__sleep", + actual = "@rules_rust_wasm_bindgen__wait-timeout-0.2.0//:sleep__bin", tags = ["manual"], ) diff --git a/wasm_bindgen/3rdparty/crates/BUILD.wait-timeout-0.2.0.bazel b/wasm_bindgen/3rdparty/crates/BUILD.wait-timeout-0.2.0.bazel index aa43b6551b..53821befe5 100644 --- a/wasm_bindgen/3rdparty/crates/BUILD.wait-timeout-0.2.0.bazel +++ b/wasm_bindgen/3rdparty/crates/BUILD.wait-timeout-0.2.0.bazel @@ -268,7 +268,7 @@ rust_binary( ) rust_binary( - name = "sleep__bin", + name = "reader__bin", srcs = glob( include = [ "**/*.rs", @@ -295,7 +295,7 @@ rust_binary( }), crate_features = [ ], - crate_root = "src/bin/sleep.rs", + crate_root = "src/bin/reader.rs", data = select_with_or({ "//conditions:default": [ ], @@ -392,7 +392,7 @@ rust_binary( ) rust_binary( - name = "reader__bin", + name = "sleep__bin", srcs = glob( include = [ "**/*.rs", @@ -419,7 +419,7 @@ rust_binary( }), crate_features = [ ], - crate_root = "src/bin/reader.rs", + crate_root = "src/bin/sleep.rs", data = select_with_or({ "//conditions:default": [ ],