From cf1b7514d3f56e8f775e21e8923c805a0816507b Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 13 Aug 2023 15:35:28 -0700 Subject: [PATCH 1/2] Read default annotation values from package's Cargo.toml metadata --- crate_universe/src/config.rs | 87 ++++++++++++++++++- .../src/metadata/metadata_annotation.rs | 14 +-- examples/crate_universe/WORKSPACE.bazel | 29 +------ .../crate_universe/using_cxx/Cargo.Bazel.lock | 12 +-- .../using_cxx/cargo-bazel-lock.json | 38 ++++---- .../using_cxx/cxxbridge-cmd.Cargo.Bazel.lock | 10 +-- .../using_cxx/cxxbridge-cmd.Cargo.lock | 2 +- .../using_cxx/include/blobstore.h | 2 +- 8 files changed, 129 insertions(+), 65 deletions(-) diff --git a/crate_universe/src/config.rs b/crate_universe/src/config.rs index 493bbcb2ef..7e4d838495 100644 --- a/crate_universe/src/config.rs +++ b/crate_universe/src/config.rs @@ -161,7 +161,7 @@ pub enum Checksumish { }, } -#[derive(Debug, Default, Deserialize, Serialize, Clone)] +#[derive(Debug, Default, Deserialize, Serialize, Clone, PartialEq)] pub struct CrateAnnotations { /// Which subset of the crate's bins should get produced as `rust_binary` targets. pub gen_binaries: Option, @@ -335,6 +335,89 @@ impl Sum for CrateAnnotations { } } +/// A subset of `crate.annotation` that we allow packages to define in their +/// free-form Cargo.toml metadata. +/// +/// ```toml +/// [package.metadata.bazel] +/// additive_build_file_contents = """ +/// ... +/// """ +/// data = ["font.woff2"] +/// extra_aliased_targets = { ... } +/// gen_build_script = false +/// ``` +/// +/// These are considered default values which apply if the Bazel workspace does +/// not specify a different value for the same annotation in their +/// crates_repository attributes. +#[derive(Debug, Deserialize)] +pub struct AnnotationsProvidedByPackage { + pub gen_build_script: Option, + pub data: Option>, + pub data_glob: Option>, + pub compile_data: Option>, + pub compile_data_glob: Option>, + pub rustc_env: Option>, + pub rustc_env_files: Option>, + pub rustc_flags: Option>, + pub build_script_env: Option>, + pub build_script_rustc_env: Option>, + pub additive_build_file_content: Option, + pub extra_aliased_targets: Option>, +} + +impl CrateAnnotations { + pub fn apply_defaults_from_package_metadata(&mut self, pkg_metadata: &serde_json::Value) { + #[deny(unused_variables)] + let AnnotationsProvidedByPackage { + gen_build_script, + data, + data_glob, + compile_data, + compile_data_glob, + rustc_env, + rustc_env_files, + rustc_flags, + build_script_env, + build_script_rustc_env, + additive_build_file_content, + extra_aliased_targets, + } = match AnnotationsProvidedByPackage::deserialize(&pkg_metadata["bazel"]) { + Ok(annotations) => annotations, + // Ignore bad annotations. The set of supported annotations evolves + // over time across different versions of crate_universe, and we + // don't want a library to be impossible to import into Bazel for + // having old or broken annotations. The Bazel workspace can specify + // its own correct annotations. + Err(_) => return, + }; + + fn default(workspace_value: &mut Option, default_value: Option) { + if workspace_value.is_none() { + *workspace_value = default_value; + } + } + + default(&mut self.gen_build_script, gen_build_script); + default(&mut self.gen_build_script, gen_build_script); + default(&mut self.data, data); + default(&mut self.data_glob, data_glob); + default(&mut self.compile_data, compile_data); + default(&mut self.compile_data_glob, compile_data_glob); + default(&mut self.rustc_env, rustc_env); + default(&mut self.rustc_env_files, rustc_env_files); + default(&mut self.rustc_flags, rustc_flags); + default(&mut self.build_script_env, build_script_env); + default(&mut self.build_script_rustc_env, build_script_rustc_env); + default( + &mut self.additive_build_file_content, + additive_build_file_content, + ); + default(&mut self.extra_aliased_targets, extra_aliased_targets); + } +} + /// A unique identifier for Crates #[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)] pub struct CrateId { @@ -444,7 +527,7 @@ impl std::fmt::Display for CrateId { } } -#[derive(Debug, Hash, Clone)] +#[derive(Debug, Hash, Clone, PartialEq)] pub enum GenBinaries { All, Some(BTreeSet), diff --git a/crate_universe/src/metadata/metadata_annotation.rs b/crate_universe/src/metadata/metadata_annotation.rs index b19d696d80..089322f11a 100644 --- a/crate_universe/src/metadata/metadata_annotation.rs +++ b/crate_universe/src/metadata/metadata_annotation.rs @@ -383,7 +383,7 @@ impl Annotations { .packages .iter() .filter_map(|(pkg_id, pkg)| { - let extras: Vec = config + let mut crate_extra: CrateAnnotations = config .annotations .iter() .filter(|(id, _)| id.matches(pkg)) @@ -395,18 +395,20 @@ impl Annotations { extra }) .cloned() - .collect(); + .sum(); - if !extras.is_empty() { + crate_extra.apply_defaults_from_package_metadata(&pkg.metadata); + + if crate_extra == CrateAnnotations::default() { + None + } else { Some(( CrateId::new(pkg.name.clone(), pkg.version.to_string()), PairredExtras { package_id: pkg_id.clone(), - crate_extra: extras.into_iter().sum(), + crate_extra, }, )) - } else { - None } }) .collect(); diff --git a/examples/crate_universe/WORKSPACE.bazel b/examples/crate_universe/WORKSPACE.bazel index 405f9558ff..1635a96e1d 100644 --- a/examples/crate_universe/WORKSPACE.bazel +++ b/examples/crate_universe/WORKSPACE.bazel @@ -277,27 +277,6 @@ no_cargo_crate_repositories() crates_repository( name = "using_cxx", - annotations = { - # CXX provides a header file that should be used in C++ sources that depend on cxx. - "cxx": [ - crate.annotation( - additive_build_file_content = """ - # This file is included in the BUILD for the cxx crate, to export its header - # file for C++ code to depend on. - cc_library( - name = "cxx_cc", - visibility = ["//visibility:public"], - hdrs = ["include/cxx.h"], - srcs = ["src/cxx.cc"], - includes = ["include"], - linkstatic = True, - ) - """, - extra_aliased_targets = {"cxx_cc": "cxx_cc"}, - gen_build_script = False, - ), - ], - }, cargo_lockfile = "//using_cxx:Cargo.Bazel.lock", # `generator` is not necessary in official releases. # See load satement for `cargo_bazel_bootstrap`. @@ -305,7 +284,7 @@ crates_repository( lockfile = "//using_cxx:cargo-bazel-lock.json", packages = { "cxx": crate.spec( - version = "1.0.0", + version = "1.0.105", ), }, splicing_config = splicing_config( @@ -341,10 +320,10 @@ rust_binary( ), ) """, - sha256 = "df13eece12ed9e7bd4fb071a6af4c44421bb9024d339d029f5333bcdaca00000", - strip_prefix = "cxxbridge-cmd-1.0.100", + sha256 = "0b3eea393dbcbc1e875302846de4e4f9a31bf2e57ad3657bc83d61d00293b0fe", + strip_prefix = "cxxbridge-cmd-1.0.105", type = "tar.gz", - urls = ["https://crates.io/api/v1/crates/cxxbridge-cmd/1.0.100/download"], + urls = ["https://crates.io/api/v1/crates/cxxbridge-cmd/1.0.105/download"], ) crates_repository( diff --git a/examples/crate_universe/using_cxx/Cargo.Bazel.lock b/examples/crate_universe/using_cxx/Cargo.Bazel.lock index c046eec23f..fc79b417e4 100644 --- a/examples/crate_universe/using_cxx/Cargo.Bazel.lock +++ b/examples/crate_universe/using_cxx/Cargo.Bazel.lock @@ -13,9 +13,9 @@ dependencies = [ [[package]] name = "cxx" -version = "1.0.104" +version = "1.0.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba1ba0a82363c553ecb7b4cd58ba6e1c017baef8e3cca4e7d66ca17879201144" +checksum = "666a3ec767f4bbaf0dcfcc3b4ea048b90520b254fdf88813e763f4c762636c14" dependencies = [ "cc", "cxxbridge-flags", @@ -25,15 +25,15 @@ dependencies = [ [[package]] name = "cxxbridge-flags" -version = "1.0.104" +version = "1.0.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "409667bbb937bae87f7cfa91ca29e1415bb92d415371e3344b5269c10d90d595" +checksum = "d6e8c238aadc4b9f2c00269d04c87abb23f96dd240803872536eed1a304bb40e" [[package]] name = "cxxbridge-macro" -version = "1.0.104" +version = "1.0.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5fb2a9757fb085d6d97856b28d4f049141ca4a61a64c697f4426433b5f6caa1f" +checksum = "59d9ffb4193dd22180b8d5747b1e095c3d9c9c665ce39b0483a488948f437e06" dependencies = [ "proc-macro2", "quote", diff --git a/examples/crate_universe/using_cxx/cargo-bazel-lock.json b/examples/crate_universe/using_cxx/cargo-bazel-lock.json index 0b9064d9cf..300867e584 100644 --- a/examples/crate_universe/using_cxx/cargo-bazel-lock.json +++ b/examples/crate_universe/using_cxx/cargo-bazel-lock.json @@ -1,5 +1,5 @@ { - "checksum": "c1f1a6f4e385c34b24fba8b5f9cbf6ff89dc26aa85f4054d46b06e4a6efff068", + "checksum": "8ce2f458f525297ce6ecbd1afb3d304637c90f010c01cc7831bc997d78d42b2c", "crates": { "cc 1.0.82": { "name": "cc", @@ -42,13 +42,13 @@ }, "license": "MIT OR Apache-2.0" }, - "cxx 1.0.104": { + "cxx 1.0.105": { "name": "cxx", - "version": "1.0.104", + "version": "1.0.105", "repository": { "Http": { - "url": "https://crates.io/api/v1/crates/cxx/1.0.104/download", - "sha256": "ba1ba0a82363c553ecb7b4cd58ba6e1c017baef8e3cca4e7d66ca17879201144" + "url": "https://crates.io/api/v1/crates/cxx/1.0.105/download", + "sha256": "666a3ec767f4bbaf0dcfcc3b4ea048b90520b254fdf88813e763f4c762636c14" } }, "targets": [ @@ -88,27 +88,27 @@ "proc_macro_deps": { "common": [ { - "id": "cxxbridge-macro 1.0.104", + "id": "cxxbridge-macro 1.0.105", "target": "cxxbridge_macro" } ], "selects": {} }, - "version": "1.0.104" + "version": "1.0.105" }, "license": "MIT OR Apache-2.0", - "additive_build_file_content": "\n# This file is included in the BUILD for the cxx crate, to export its header\n# file for C++ code to depend on.\ncc_library(\n name = \"cxx_cc\",\n visibility = [\"//visibility:public\"],\n hdrs = [\"include/cxx.h\"],\n srcs = [\"src/cxx.cc\"],\n includes = [\"include\"],\n linkstatic = True,\n)\n", + "additive_build_file_content": "cc_library(\n name = \"cxx_cc\",\n srcs = [\"src/cxx.cc\"],\n hdrs = [\"include/cxx.h\"],\n include_prefix = \"rust\",\n includes = [\"include\"],\n linkstatic = True,\n strip_include_prefix = \"include\",\n visibility = [\"//visibility:public\"],\n)\n", "extra_aliased_targets": { "cxx_cc": "cxx_cc" } }, - "cxxbridge-flags 1.0.104": { + "cxxbridge-flags 1.0.105": { "name": "cxxbridge-flags", - "version": "1.0.104", + "version": "1.0.105", "repository": { "Http": { - "url": "https://crates.io/api/v1/crates/cxxbridge-flags/1.0.104/download", - "sha256": "409667bbb937bae87f7cfa91ca29e1415bb92d415371e3344b5269c10d90d595" + "url": "https://crates.io/api/v1/crates/cxxbridge-flags/1.0.105/download", + "sha256": "d6e8c238aadc4b9f2c00269d04c87abb23f96dd240803872536eed1a304bb40e" } }, "targets": [ @@ -134,17 +134,17 @@ "selects": {} }, "edition": "2021", - "version": "1.0.104" + "version": "1.0.105" }, "license": "MIT OR Apache-2.0" }, - "cxxbridge-macro 1.0.104": { + "cxxbridge-macro 1.0.105": { "name": "cxxbridge-macro", - "version": "1.0.104", + "version": "1.0.105", "repository": { "Http": { - "url": "https://crates.io/api/v1/crates/cxxbridge-macro/1.0.104/download", - "sha256": "5fb2a9757fb085d6d97856b28d4f049141ca4a61a64c697f4426433b5f6caa1f" + "url": "https://crates.io/api/v1/crates/cxxbridge-macro/1.0.105/download", + "sha256": "59d9ffb4193dd22180b8d5747b1e095c3d9c9c665ce39b0483a488948f437e06" } }, "targets": [ @@ -181,7 +181,7 @@ "selects": {} }, "edition": "2021", - "version": "1.0.104" + "version": "1.0.105" }, "license": "MIT OR Apache-2.0" }, @@ -208,7 +208,7 @@ "deps": { "common": [ { - "id": "cxx 1.0.104", + "id": "cxx 1.0.105", "target": "cxx" } ], diff --git a/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.Bazel.lock b/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.Bazel.lock index fc4f212305..ed18814c73 100644 --- a/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.Bazel.lock +++ b/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.Bazel.lock @@ -1,5 +1,5 @@ { - "checksum": "7f4d3942ea59e714a8f596d2517046e89e7cc0918c78bfe4924fc37b8f7b1d4e", + "checksum": "92513866045681ab1c1489220c4043a7dca51fe4a96f58d9b78e09b13b5b0825", "crates": { "anstyle 1.0.1": { "name": "anstyle", @@ -217,9 +217,9 @@ }, "license": "Apache-2.0" }, - "cxxbridge-cmd 1.0.100": { + "cxxbridge-cmd 1.0.105": { "name": "cxxbridge-cmd", - "version": "1.0.100", + "version": "1.0.105", "repository": null, "targets": [], "library_target_name": null, @@ -253,7 +253,7 @@ "selects": {} }, "edition": "2021", - "version": "1.0.100" + "version": "1.0.105" }, "license": "MIT OR Apache-2.0" }, @@ -784,7 +784,7 @@ }, "binary_crates": [], "workspace_members": { - "cxxbridge-cmd 1.0.100": "" + "cxxbridge-cmd 1.0.105": "" }, "conditions": { "cfg(windows)": [ diff --git a/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.lock b/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.lock index b4dd6d372b..2df94a7dc4 100644 --- a/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.lock +++ b/examples/crate_universe/using_cxx/cxxbridge-cmd.Cargo.lock @@ -46,7 +46,7 @@ dependencies = [ [[package]] name = "cxxbridge-cmd" -version = "1.0.100" +version = "1.0.105" dependencies = [ "clap", "codespan-reporting", diff --git a/examples/crate_universe/using_cxx/include/blobstore.h b/examples/crate_universe/using_cxx/include/blobstore.h index 2dec552fb9..c1521e770c 100644 --- a/examples/crate_universe/using_cxx/include/blobstore.h +++ b/examples/crate_universe/using_cxx/include/blobstore.h @@ -1,7 +1,7 @@ #pragma once #include -#include "cxx.h" +#include "rust/cxx.h" namespace org { namespace blobstore { From 082c8605c367c87fca1773f3eb852daa25d6a8e9 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Thu, 24 Aug 2023 13:43:03 -0700 Subject: [PATCH 2/2] Add test of package-provided annotations --- .../src/metadata/metadata_annotation.rs | 38 ++++++++++ crate_universe/src/test.rs | 16 ++++ .../metadata/has_package_metadata/Cargo.lock | 7 ++ .../metadata/has_package_metadata/Cargo.toml | 15 ++++ .../has_package_metadata/metadata.json | 73 +++++++++++++++++++ 5 files changed, 149 insertions(+) create mode 100644 crate_universe/test_data/metadata/has_package_metadata/Cargo.lock create mode 100644 crate_universe/test_data/metadata/has_package_metadata/Cargo.toml create mode 100644 crate_universe/test_data/metadata/has_package_metadata/metadata.json diff --git a/crate_universe/src/metadata/metadata_annotation.rs b/crate_universe/src/metadata/metadata_annotation.rs index 089322f11a..57bc68f888 100644 --- a/crate_universe/src/metadata/metadata_annotation.rs +++ b/crate_universe/src/metadata/metadata_annotation.rs @@ -562,4 +562,42 @@ mod test { assert!(result_str.contains("Unused annotations were provided. Please remove them")); assert!(result_str.contains("mock-crate")); } + + #[test] + fn defaults_from_package_metadata() { + let crate_id = CrateId::new("has_package_metadata".to_owned(), "0.0.0".to_owned()); + let annotations = CrateAnnotations { + rustc_env: Some({ + let mut rustc_env = BTreeMap::new(); + rustc_env.insert("BAR".to_owned(), "bar is set".to_owned()); + rustc_env + }), + ..CrateAnnotations::default() + }; + + let mut config = Config::default(); + config + .annotations + .insert(crate_id.clone(), annotations.clone()); + + // Combine the above annotations with default values provided by the + // crate author in package metadata. + let combined_annotations = Annotations::new( + test::metadata::has_package_metadata(), + test::lockfile::has_package_metadata(), + config, + ) + .unwrap(); + + let extras = &combined_annotations.pairred_extras[&crate_id].crate_extra; + let expected = CrateAnnotations { + // This comes from has_package_metadata's [package.metadata.bazel]. + additive_build_file_content: Some("genrule(**kwargs)\n".to_owned()), + // The package metadata defines a default rustc_env containing FOO, + // but it is superseded by a rustc_env annotation containing only + // BAR. These dictionaries are intentionally not merged together. + ..annotations + }; + assert_eq!(*extras, expected); + } } diff --git a/crate_universe/src/test.rs b/crate_universe/src/test.rs index 3717ec4ea1..e97e20b654 100644 --- a/crate_universe/src/test.rs +++ b/crate_universe/src/test.rs @@ -114,6 +114,14 @@ pub mod metadata { ))) .unwrap() } + + pub fn has_package_metadata() -> cargo_metadata::Metadata { + serde_json::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_data/metadata/has_package_metadata/metadata.json" + ))) + .unwrap() + } } pub mod lockfile { @@ -174,4 +182,12 @@ pub mod lockfile { ))) .unwrap() } + + pub fn has_package_metadata() -> cargo_lock::Lockfile { + cargo_lock::Lockfile::from_str(include_str!(concat!( + env!("CARGO_MANIFEST_DIR"), + "/test_data/metadata/has_package_metadata/Cargo.lock" + ))) + .unwrap() + } } diff --git a/crate_universe/test_data/metadata/has_package_metadata/Cargo.lock b/crate_universe/test_data/metadata/has_package_metadata/Cargo.lock new file mode 100644 index 0000000000..688070729e --- /dev/null +++ b/crate_universe/test_data/metadata/has_package_metadata/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "has_package_metadata" +version = "0.0.0" diff --git a/crate_universe/test_data/metadata/has_package_metadata/Cargo.toml b/crate_universe/test_data/metadata/has_package_metadata/Cargo.toml new file mode 100644 index 0000000000..be3ce50ae2 --- /dev/null +++ b/crate_universe/test_data/metadata/has_package_metadata/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "has_package_metadata" +version = "0.0.0" +edition = "2021" + +# Required to satisfy cargo but no `lib.rs` is expected to +# exist within test data. +[lib] +path = "lib.rs" + +[package.metadata.bazel] +additive_build_file_content = """ +genrule(**kwargs) +""" +rustc_env = { "FOO" = "foo is set" } diff --git a/crate_universe/test_data/metadata/has_package_metadata/metadata.json b/crate_universe/test_data/metadata/has_package_metadata/metadata.json new file mode 100644 index 0000000000..8893de8565 --- /dev/null +++ b/crate_universe/test_data/metadata/has_package_metadata/metadata.json @@ -0,0 +1,73 @@ +{ + "packages": [ + { + "name": "has_package_metadata", + "version": "0.0.0", + "id": "has_package_metadata 0.0.0 (path+file://{TEMP_DIR}/has_package_metadata)", + "license": null, + "license_file": null, + "description": null, + "source": null, + "dependencies": [], + "targets": [ + { + "kind": [ + "lib" + ], + "crate_types": [ + "lib" + ], + "name": "has_package_metadata", + "src_path": "{TEMP_DIR}/has_package_metadata/lib.rs", + "edition": "2021", + "doc": true, + "doctest": true, + "test": true + } + ], + "features": {}, + "manifest_path": "{TEMP_DIR}/has_package_metadata/Cargo.toml", + "metadata": { + "bazel": { + "additive_build_file_content": "genrule(**kwargs)\n", + "rustc_env": { + "FOO": "foo is set" + } + } + }, + "publish": null, + "authors": [], + "categories": [], + "keywords": [], + "readme": null, + "repository": null, + "homepage": null, + "documentation": null, + "edition": "2021", + "links": null, + "default_run": null, + "rust_version": null + } + ], + "workspace_members": [ + "has_package_metadata 0.0.0 (path+file://{TEMP_DIR}/has_package_metadata)" + ], + "workspace_default_members": [ + "has_package_metadata 0.0.0 (path+file://{TEMP_DIR}/has_package_metadata)" + ], + "resolve": { + "nodes": [ + { + "id": "has_package_metadata 0.0.0 (path+file://{TEMP_DIR}/has_package_metadata)", + "dependencies": [], + "deps": [], + "features": [] + } + ], + "root": "has_package_metadata 0.0.0 (path+file://{TEMP_DIR}/has_package_metadata)" + }, + "target_directory": "{TEMP_DIR}/has_package_metadata/target", + "version": 1, + "workspace_root": "{TEMP_DIR}/has_package_metadata", + "metadata": null +}