From fb7a60a7754a519a94920fe0d23e23e78297215a Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 7 Dec 2020 06:56:44 -0800 Subject: [PATCH 1/6] Settings are now parsed from Cargo metadata, not parsing toml files --- impl/Cargo.toml | 2 +- impl/src/bin/cargo-raze.rs | 11 +- impl/src/metadata.rs | 12 +- impl/src/planning.rs | 2 +- impl/src/settings.rs | 430 +++++++++++++++++++++++++++++-------- 5 files changed, 357 insertions(+), 100 deletions(-) diff --git a/impl/Cargo.toml b/impl/Cargo.toml index 59096612b..1dc2e9642 100644 --- a/impl/Cargo.toml +++ b/impl/Cargo.toml @@ -40,6 +40,7 @@ rustc-serialize = "0.3.24" semver = { version = "0.11.0", features = ["serde"] } serde = "1.0.118" serde_derive = "1.0.118" +serde_json = "1.0.60" slug = "0.1.4" spdx = "0.3.4" tempfile = "3.1.0" @@ -53,5 +54,4 @@ hamcrest2 = "0.3.0" httpmock = "0.5.2" indoc = "1.0.3" lazy_static = "1.4.0" -serde_json = "1.0.60" tar = "0.4.30" diff --git a/impl/src/bin/cargo-raze.rs b/impl/src/bin/cargo-raze.rs index 6d2a0dd9a..0f6cebe1e 100644 --- a/impl/src/bin/cargo-raze.rs +++ b/impl/src/bin/cargo-raze.rs @@ -82,13 +82,10 @@ fn main() -> Result<()> { .unwrap_or_else(|e| e.exit()); // Load settings - let manifest_path = PathBuf::from( - options - .flag_manifest_path - .unwrap_or("./Cargo.toml".to_owned()), - ) - .canonicalize()?; - let settings = load_settings(&manifest_path)?; + let manifest_path = PathBuf::from(options + .flag_manifest_path + .unwrap_or("./Cargo.toml".to_owned())).canonicalize()?; + let settings = load_settings(&manifest_path, options.flag_cargo_bin_path.clone())?; if options.flag_verbose.unwrap_or(false) { println!("Loaded override settings: {:#?}", settings); } diff --git a/impl/src/metadata.rs b/impl/src/metadata.rs index deb2b4011..9a252606a 100644 --- a/impl/src/metadata.rs +++ b/impl/src/metadata.rs @@ -31,7 +31,7 @@ use url::Url; use crate::util::package_ident; -const SYSTEM_CARGO_BIN_PATH: &str = "cargo"; +pub(crate) const SYSTEM_CARGO_BIN_PATH: &str = "cargo"; pub(crate) const DEFAULT_CRATE_REGISTRY_URL: &str = "https://crates.io"; pub(crate) const DEFAULT_CRATE_INDEX_URL: &str = "https://github.com/rust-lang/crates.io-index"; @@ -42,7 +42,15 @@ pub trait MetadataFetcher { /** A lockfile generator which simply wraps the `cargo_metadata::MetadataCommand` command */ struct CargoMetadataFetcher { - cargo_bin_path: PathBuf, + pub cargo_bin_path: PathBuf, +} + +impl Default for CargoMetadataFetcher { + fn default() -> CargoMetadataFetcher { + CargoMetadataFetcher { + cargo_bin_path: SYSTEM_CARGO_BIN_PATH.into(), + } + } } impl MetadataFetcher for CargoMetadataFetcher { diff --git a/impl/src/planning.rs b/impl/src/planning.rs index af72f4013..d58785320 100644 --- a/impl/src/planning.rs +++ b/impl/src/planning.rs @@ -599,7 +599,7 @@ mod tests { let settings = { let (_temp_dir, files) = make_workspace(toml_file, None); - crate::settings::load_settings(&files.toml_path).unwrap() + crate::settings::load_settings(&files.toml_path, None).unwrap() }; let planner = BuildPlannerImpl::new( diff --git a/impl/src/settings.rs b/impl/src/settings.rs index 42cb6c83b..673153bb7 100644 --- a/impl/src/settings.rs +++ b/impl/src/settings.rs @@ -14,38 +14,23 @@ use crate::{ error::RazeError, - metadata::{DEFAULT_CRATE_INDEX_URL, DEFAULT_CRATE_REGISTRY_URL}, + metadata::{ + MetadataFetcher, DEFAULT_CRATE_INDEX_URL, DEFAULT_CRATE_REGISTRY_URL, SYSTEM_CARGO_BIN_PATH, + }, }; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, Context, Result}; +use cargo_metadata::{Metadata, MetadataCommand, Package}; use semver::VersionReq; use serde::{Deserialize, Serialize}; -use std::{collections::HashMap, fs::File, io::Read, path::Path, path::PathBuf}; +use std::{ + collections::HashMap, + hash::Hash, + path::{Path, PathBuf}, +}; pub type CrateSettingsPerVersion = HashMap; -/** - * A "deserializable struct" for the whole Cargo.toml - * - * Contains only `raze` settings, (we drop everything else in the toml on the floor). - */ -#[derive(Debug, Clone, Deserialize)] -pub struct CargoToml { - pub raze: Option, - pub package: Option, - pub workspace: Option, -} - -#[derive(Debug, Clone, Deserialize)] -pub struct PackageToml { - pub metadata: Option, -} - -#[derive(Debug, Clone, Deserialize)] -pub struct MetadataToml { - pub raze: Option, -} - -/** The configuration settings for `cargo-raze`, included in Cargo.toml. */ +/** The configuration settings for `cargo-raze`, included in a projects Cargo metadata */ #[derive(Debug, Clone, Deserialize)] pub struct RazeSettings { /** @@ -483,77 +468,299 @@ fn validate_settings( Ok(()) } -/** Parses raze settings from the contents of a `Cargo.toml` file */ -fn parse_raze_settings(toml_contents: CargoToml) -> Result { - // Workspace takes precedence - if let Some(raze) = toml_contents - .workspace - .and_then(|pkg| pkg.metadata.and_then(|data| data.raze)) - { - if toml_contents.raze.is_some() { - eprintln!( - "WARNING: Both [raze] and [workspace.metadata.raze] are set. Using \ - [workspace.metadata.raze] and ignoring [raze], which is deprecated." +/** The intermediate configuration settings for `cargo-raze`, included in a project's Cargo metadata + * + * Note that this struct should contain only `Option` and match all public fields of + * [`RazeSettings`](crate::settings::RazeSettings) + */ +#[derive(Debug, Clone, Deserialize)] +struct RawRazeSettings { + #[serde(default)] + pub workspace_path: Option, + #[serde(default)] + pub package_aliases_dir: Option, + #[serde(default)] + pub target: Option, + #[serde(default)] + pub targets: Option>, + #[serde(default)] + pub binary_deps: HashMap, + #[serde(default)] + pub crates: HashMap, + #[serde(default)] + pub gen_workspace_prefix: Option, + #[serde(default)] + pub genmode: Option, + #[serde(default)] + pub output_buildfile_suffix: Option, + #[serde(default)] + pub default_gen_buildrs: Option, + #[serde(default)] + pub registry: Option, + #[serde(default)] + pub index_url: Option, + #[serde(default)] + pub incompatible_relative_workspace_path: Option, + #[serde(default)] + pub rust_rules_workspace_name: Option, + #[serde(default)] + pub vendor_dir: Option, +} + +impl RawRazeSettings { + /** Checks whether or not the settings have non-package specific settings specified */ + fn contains_primary_options(&self) -> bool { + self.workspace_path.is_some() + || self.package_aliases_dir.is_some() + || self.target.is_some() + || self.targets.is_some() + || self.gen_workspace_prefix.is_some() + || self.genmode.is_some() + || self.output_buildfile_suffix.is_some() + || self.default_gen_buildrs.is_some() + || self.registry.is_some() + || self.index_url.is_some() + || self.incompatible_relative_workspace_path.is_some() + || self.rust_rules_workspace_name.is_some() + || self.vendor_dir.is_some() + } +} + +/** Grows a list with duplicate keys between two maps */ +fn extend_duplicates( + extended_list: &mut Vec, + main_map: &HashMap, + input_map: &HashMap, +) { + extended_list.extend( + input_map + .iter() + .filter_map(|(key, _value)| { + // Log the key if it exists in both the main and input maps + if main_map.contains_key(key) { + Some(key) + } else { + None + } + }) + .cloned() + .collect::>(), + ); +} + +/** Parse [`RazeSettings`](crate::settings::RazeSettings) from workspace metadata */ +fn parse_raze_settings_workspace( + metadata_value: &serde_json::value::Value, + metadata: &Metadata, +) -> Result { + let mut settings = RazeSettings::deserialize(metadata_value)?; + + let workspace_packages: Vec<&Package> = metadata + .packages + .iter() + .filter(|pkg| metadata.workspace_members.contains(&pkg.id)) + .collect(); + + let mut duplicate_binary_deps = Vec::new(); + let mut duplicate_crate_settings = Vec::new(); + + for package in workspace_packages.iter() { + if let Some(pkg_value) = package.metadata.get("raze") { + let pkg_settings = RawRazeSettings::deserialize(pkg_value)?; + if pkg_settings.contains_primary_options() { + return Err(anyhow!( + "The package '{}' contains Primary raze settings, please move these to the \ + `[workspace.metadata.raze]`", + package.name + )); + } + + // Log duplicate binary dependencies + extend_duplicates( + &mut duplicate_binary_deps, + &settings.binary_deps, + &pkg_settings.binary_deps, ); + + settings + .binary_deps + .extend(pkg_settings.binary_deps.into_iter()); + + // Log duplicate crate settings + extend_duplicates( + &mut duplicate_crate_settings, + &settings.crates, + &pkg_settings.crates, + ); + + settings.crates.extend(pkg_settings.crates.into_iter()); } - return Ok(raze); } - // Next is package - if let Some(raze) = toml_contents - .package - .and_then(|pkg| pkg.metadata.and_then(|data| data.raze)) - { - if toml_contents.raze.is_some() { - eprintln!( - "WARNING: Both [raze] and [package.metadata.raze] are set. Using [package.metadata.raze] \ - and ignoring [raze], which is deprecated." - ); + // Check for duplication errors + if duplicate_binary_deps.len() > 0 { + return Err(anyhow!( + "Duplicate `raze.binary_deps` values detected accross various crates: {:?}", + duplicate_binary_deps + )); + } + if duplicate_crate_settings.len() > 0 { + return Err(anyhow!( + "Duplicate `raze.crates.*` values detected accross various crates: {:?}", + duplicate_crate_settings + )); + } + + Ok(settings) +} + +/** Parse [`RazeSettings`](crate::settings::RazeSettings) from a project's root package's metadata */ +fn parse_raze_settings_root_package( + metadata_value: &serde_json::value::Value, + root_package: &Package, +) -> Result { + return RazeSettings::deserialize(metadata_value).with_context(|| { + format!( + "Failed to load raze settings from root package: {}", + root_package.name, + ) + }); +} + +/** Parse [`RazeSettings`](crate::settings::RazeSettings) from any workspace member's metadata */ +fn parse_raze_settings_any_package(metadata: &Metadata) -> Result { + let mut settings_packages = Vec::new(); + + for package in metadata.packages.iter() { + if let Some(pkg_value) = package.metadata.get("raze") { + let pkg_settings = RawRazeSettings::deserialize(pkg_value)?; + if pkg_settings.contains_primary_options() { + settings_packages.push(package); + } } - return Ok(raze); } - // Finally the direct raze settings - if let Some(raze) = toml_contents.raze { + // There should only be one package with raze + if settings_packages.len() > 1 { + return Err(anyhow!( + "Multiple packages contain primary raze settings: {:?}", + settings_packages + .iter() + .map(|pkg| &pkg.name) + .collect::>() + )); + } + + // UNWRAP: Safe due to checks above + let settings_value = settings_packages[0].metadata.get("raze").unwrap(); + RazeSettings::deserialize(settings_value) + .with_context(|| format!("Failed to deserialize raze settings: {:?}", settings_value)) +} + +/** A struct only to deserialize a Cargo.toml to in search of the legacy syntax for [`RazeSettings`](crate::settings::RazeSettings) */ +#[derive(Debug, Clone, Deserialize)] +pub struct LegacyCargoToml { + pub raze: RazeSettings, +} + +/** Parse [`RazeSettings`](crate::settings::RazeSettings) from a Cargo.toml file using the legacy syntax `[raze]` */ +fn parse_raze_settings_legacy(metadata: &Metadata) -> Result { + let root_toml = metadata.workspace_root.join("Cargo.toml"); + let toml_contents = std::fs::read_to_string(&root_toml)?; + let data = toml::from_str::(&toml_contents).with_context(|| { + format!( + "Failed to read `[raze]` settings from {}", + root_toml.display() + ) + })?; + Ok(data.raze) +} + +/** Parses raze settings from the contents of a `Cargo.toml` file */ +fn parse_raze_settings(metadata: Metadata) -> Result { + // Workspace takes precedence + let workspace_level_settigns = metadata.workspace_metadata.get("raze"); + if let Some(value) = workspace_level_settigns { + return parse_raze_settings_workspace(value, &metadata); + } + + // Root packages are the next priority + if let Some(root_package) = metadata.root_package() { + if let Some(value) = root_package.metadata.get("raze") { + return parse_raze_settings_root_package(value, root_package); + } + } + + // Attempt to load legacy settings but do not allow failures to propogate + if let Ok(settings) = parse_raze_settings_legacy(&metadata) { eprintln!( - "WARNING: The top-level [raze] key is deprecated. Please set [package.metadata.raze] \ - instead." + "WARNING: The top-level `[raze]` key is deprecated. Please set `[workspace.metadata.raze]` or \ + `[package.metadata.raze]` instead." ); - - return Ok(raze); + return Ok(settings); } - return Err(RazeError::Generic( - "Cargo.toml has no `raze`, `workspace.metadata.raze` or `package.metadata.raze` field".into(), - )); + // Finally check any package for settings + parse_raze_settings_any_package(&metadata) +} + +struct SettingsMetadataFetcher { + pub cargo_bin_path: PathBuf, } -pub fn load_settings>(cargo_toml_path: T) -> Result { - let path = cargo_toml_path.as_ref(); - let mut toml = match File::open(path) { - Ok(handle) => handle, - Err(err) => { - return Err(RazeError::Generic(err.to_string())); - }, +impl MetadataFetcher for SettingsMetadataFetcher { + fn fetch_metadata(&self, working_dir: &Path, _include_deps: bool) -> Result { + // This fetch should not require network access. + MetadataCommand::new() + .cargo_path(&self.cargo_bin_path) + .no_deps() + .current_dir(working_dir) + .other_options(vec!["--offline".to_owned()]) + .exec() + .with_context(|| { + format!( + "Failed to fetch Metadata with `{}` from `{}`", + &self.cargo_bin_path.display(), + working_dir.display() + ) + }) + } +} + +/** Load settings used to configure the functionality of Cargo Raze */ +pub fn load_settings>( + cargo_toml_path: T, + cargo_bin_path: Option, +) -> Result { + // Create a MetadataFetcher from either an optional Cargo binary path + // or a fallback expected to be found on the system. + let fetcher = SettingsMetadataFetcher { + cargo_bin_path: cargo_bin_path + .unwrap_or(SYSTEM_CARGO_BIN_PATH.to_string()) + .into(), }; - let toml_contents = { - let mut contents = String::new(); - let result = toml.read_to_string(&mut contents); - if let Some(err) = result.err() { - return Err(RazeError::Generic(err.to_string())); + // UNWRAP: safe due to earlier assert + let cargo_toml_dir = cargo_toml_path.as_ref().parent().unwrap(); + let metadata = { + let result = fetcher.fetch_metadata(cargo_toml_dir, false); + if result.is_err() { + return Err(RazeError::Generic(result.err().unwrap().to_string())); } + // UNWRAP: safe due to check above + result.unwrap() + }; - match toml::from_str::(&contents) { - Ok(toml_contents) => toml_contents, - Err(err) => return Err(RazeError::Generic(err.to_string())), + let mut settings = { + let result = parse_raze_settings(metadata); + if result.is_err() { + return Err(RazeError::Generic(result.err().unwrap().to_string())); } + // UNWRAP: safe due to check above + result.unwrap() }; - let mut settings = parse_raze_settings(toml_contents)?; - - // UNWRAP: Safe due to the fact that `path` was read as a file earlier - validate_settings(&mut settings, path.parent().unwrap())?; + validate_settings(&mut settings, cargo_toml_dir)?; Ok(settings) } @@ -563,8 +770,7 @@ pub mod tests { use crate::testing::{make_workspace, named_toml_contents}; use super::*; - use indoc::indoc; - use std::io::Write; + use indoc::{formatdoc, indoc}; use tempfile::TempDir; pub fn dummy_raze_settings() -> RazeSettings { @@ -588,7 +794,43 @@ pub mod tests { } #[test] - fn test_loading_settings() { + fn test_loading_package_settings() { + let toml_contents = indoc! { r#" + [package] + name = "load_settings_test" + version = "0.1.0" + + [lib] + path = "not_a_file.rs" + + [dependencies] + actix-web = "2.0.0" + actix-rt = "1.0.0" + + [target.x86_64-apple-ios.dependencies] + [target.x86_64-linux-android.dependencies] + bitflags = "1.2.1" + + [package.metadata.raze] + workspace_path = "//workspace_path/raze" + genmode = "Remote" + + [package.metadata.raze.binary_deps] + wasm-bindgen-cli = "0.2.68" + "# }; + + let temp_workspace_dir = TempDir::new() + .ok() + .expect("Failed to set up temporary directory"); + let cargo_toml_path = temp_workspace_dir.path().join("Cargo.toml"); + std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); + + let settings = load_settings(cargo_toml_path, None).unwrap(); + assert!(settings.binary_deps.len() > 0); + } + + #[test] + fn test_loading_settings_legacy() { let toml_contents = indoc! { r#" [package] name = "load_settings_test" @@ -608,7 +850,6 @@ pub mod tests { [raze] workspace_path = "//workspace_path/raze" genmode = "Remote" - incompatible_relative_workspace_path = true [raze.binary_deps] wasm-bindgen-cli = "0.2.68" @@ -618,10 +859,9 @@ pub mod tests { .ok() .expect("Failed to set up temporary directory"); let cargo_toml_path = temp_workspace_dir.path().join("Cargo.toml"); - let mut toml = File::create(&cargo_toml_path).unwrap(); - toml.write_all(toml_contents.as_bytes()).unwrap(); + std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); - let settings = load_settings(cargo_toml_path).unwrap(); + let settings = load_settings(cargo_toml_path, None).unwrap(); assert!(settings.binary_deps.len() > 0); } @@ -630,7 +870,8 @@ pub mod tests { let toml_contents = indoc! { r#" [workspace] members = [ - "test_crate", + "crate_a", + "crate_b", ] [workspace.metadata.raze] @@ -639,13 +880,24 @@ pub mod tests { "# }; let (dir, files) = make_workspace(toml_contents, None); - let test_crate_toml = dir.as_ref().join("test_crate").join("Cargo.toml"); - std::fs::create_dir_all(test_crate_toml.parent().unwrap()).unwrap(); - std::fs::write(test_crate_toml, named_toml_contents("test_crate", "0.0.1")).unwrap(); + for member in vec!["crate_a", "crate_b"].iter() { + let crate_toml = dir.as_ref().join(member).join("Cargo.toml"); + std::fs::create_dir_all(crate_toml.parent().unwrap()).unwrap(); + let toml_contents = formatdoc! { r#" + {named_contents} + + [package.metadata.raze.crates.settings-test-{name}.'*'] + additional_flags = [ + "--cfg={name}" + ] + "#, named_contents = named_toml_contents(member, "0.0.1"), name = member }; + std::fs::write(crate_toml, toml_contents).unwrap(); + } - let settings = load_settings(files.toml_path).unwrap(); + let settings = load_settings(files.toml_path, None).unwrap(); assert_eq!(&settings.workspace_path, "//workspace_path/raze"); assert_eq!(settings.genmode, GenMode::Remote); + assert_eq!(settings.crates.len(), 2); } #[test] From e5574edb1518def94019b6a471512c309653ae8a Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 7 Dec 2020 05:06:30 -0800 Subject: [PATCH 2/6] Updated documentation and rules_rust definitions --- .bazelversion | 2 +- README.md | 209 ++++++++++++++++++++++++++++----------------- examples/WORKSPACE | 12 +-- smoke-test.sh | 12 +-- 4 files changed, 138 insertions(+), 97 deletions(-) diff --git a/.bazelversion b/.bazelversion index 240bba906..5cdb444f3 100644 --- a/.bazelversion +++ b/.bazelversion @@ -1 +1 @@ -3.7.0 \ No newline at end of file +3.7.1 \ No newline at end of file diff --git a/README.md b/README.md index 17ba516b4..2b714c6fd 100644 --- a/README.md +++ b/README.md @@ -46,37 +46,25 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_rust", - sha256 = "b5d4d1c7609714dfef821355f40353c58aa1afb3803401b3442ed2355db9b0c7", - strip_prefix = "rules_rust-8d2b4eeeff9dce24f5cbb36018f2d60ecd676639", + sha256 = "0e2e633bf0f7f25392ffb477d677c88eb34fe70ffae05e3ad92fdd9f8d6579db", + strip_prefix = "rules_rust-bc0578798f50d018ca4278ad5610598c400992c9", urls = [ - # Master branch as of 2020-11-10 - "https://github.com/bazelbuild/rules_rust/archive/8d2b4eeeff9dce24f5cbb36018f2d60ecd676639.tar.gz", + # Master branch as of 2020-12-05 + "https://github.com/bazelbuild/rules_rust/archive/bc0578798f50d018ca4278ad5610598c400992c9.tar.gz", ], ) load("@io_bazel_rules_rust//rust:repositories.bzl", "rust_repositories") rust_repositories() - -load("@io_bazel_rules_rust//:workspace.bzl", "rust_workspace") - -rust_workspace() ``` -### Vendoring Mode - -In Vendoring mode, a root directly is selected that will house the vendored -dependencies and become the gateway to those build rules. "//cargo" is -conventional, but "//third_party/cargo" may be desirable to satisfy -organizational needs. Vendoring directly into root isn't well supported due to -implementation-specific idiosyncracies, but it may be supported in the future. -From here forward, "//cargo" will be the assumed directory. +### Generate a Cargo.toml -#### Generate a Cargo.toml - -First, generate a standard Cargo.toml with the dependencies of interest. Take -care to include a `[lib]` directive so that Cargo does not complain about -missing source files for this mock crate. Here is an example: +For Bazel only projects, users should first generate a standard Cargo.toml +with the dependencies of interest. Take care to include a `[lib]` directive +so that Cargo does not complain about missing source files for this mock +crate. Here is an example: ```toml [package] @@ -90,7 +78,7 @@ path = "fake_lib.rs" [dependencies] log = "=0.3.6" -[package.metadata.raze] +[workspace.metadata.raze] # The path relative path to the Bazel workspace root (location of # WORKSPACE.bazel/WORKSPACE file). If no workspace file is found, # the current working directory is used. @@ -100,48 +88,29 @@ workspace_path = "//cargo" # file located next to this `Cargo.toml` file. package_aliases_dir = "." -# The target to generate BUILD rules for. -target = "x86_64-unknown-linux-gnu" -``` - -#### Generate buildable targets - -First, install the required tools for vendoring and generating BUILDable -targets. - -```bash -$ cargo install cargo-raze -``` - -Following that, vendor your dependencies from within the cargo/ directory. This -will also update your `Cargo.lock` file. - -```bash -$ cargo vendor --versioned-dirs -``` - -Finally, generate your BUILD files, again from within the `cargo/` directory +# The set of targets to generate BUILD rules for. +targets = [ + "x86_64-apple-darwin", + "x86_64-pc-windows-msvc", + "x86_64-unknown-linux-gnu", +] -```bash -$ cargo raze +# The two acceptable options are "Remote" and "Vendored" which +# is used to idnicate whether the user is using a non-vendored or +# vendored set of dependencies. +genmode = "Remote" ``` -You can now depend on any _explicit_ dependencies in any Rust rule by depending on -`//cargo:your_dependency_name`. - -### Remote Dependency Mode - -In Remote mode, a directory similar to the vendoring mode is selected. In this -case, though, it contains only BUILD files, a vendoring instruction for the -WORKSPACE, and aliases to the explicit dependencies. Slightly different plumbing -is required. - -#### Generate a Cargo.toml +### Using existing Cargo.toml -Generate a Cargo.toml, similar to Vendoring mode but add a new `genmode` directive in the -`[package.metadata.raze]` section +Almost all canonical cargo setups should be able to function inplace with +`cargo-raze`. Assuming the Cargo workspace is now nested under a Bazel workspace, +Users can simply add [RazeSettings](./impl/src/settings.rs) to their Cargo.toml +files to be used for generating Bazel files ```toml +# Above this line should be the contents of your Cargo.toml file + [package.metadata.raze] # The path relative path to the Bazel workspace root (location of # WORKSPACE.bazel/WORKSPACE file). If no workspace file is found, @@ -152,12 +121,58 @@ workspace_path = "//cargo" # file located next to this `Cargo.toml` file. package_aliases_dir = "." -# The target to generate BUILD rules for. -target = "x86_64-unknown-linux-gnu" +# The set of targets to generate BUILD rules for. +targets = [ + "x86_64-apple-darwin", + "x86_64-pc-windows-msvc", + "x86_64-unknown-linux-gnu", +] +# The two acceptable options are "Remote" and "Vendored" which +# is used to idnicate whether the user is using a non-vendored or +# vendored set of dependencies. genmode = "Remote" ``` +#### Cargo workspace projects + +In projects that use [cargo workspaces](cargo_workspaces) uses should organize +all of their `raze` settings into the `[workspace.metadata.raze]` field in the +top level `Cargo.toml` file which contains the `[workspace]` definition. These +settings should be identical to the ones seen in `[package.metadata.raze]` in +[the previous section](#Using existing Cargo.toml). However, crate settings may still +be placed in the `Cargo.toml` files of the workspace memebers: + +```toml +# Above this line should be the contents of your package's Cargo.toml file + +# Note that `some-dependency` is the name of an example dependency and +# `<0.3.0` is a semver version for the dependency crate's version. This +# should always be compaitble in some way with the dependency version +# specified in the `[dependencies]` section of the package defined in +# this file +[package.metadata.raze.crates.some-dependency.'<0.3.0'] +additional_flags = [ + "--cfg=optional_feature_a", + "--cfg=optional_feature_b", +] + +# This demonstrates that multiple crate settings may be defined. +[package.metadata.raze.crates.some-other-dependency.'*'] +additional_flags = [ + "--cfg=special_feature", +] +``` + +[cargo_workspaces]: https://doc.rust-lang.org/book/ch14-03-cargo-workspaces.html) + +### Remote Dependency Mode + +In Remote mode, a directory similar to the vendoring mode is selected. In this +case, though, it contains only BUILD files, a vendoring instruction for the +WORKSPACE, and aliases to the explicit dependencies. Slightly different plumbing +is required. + This tells Raze not to expect the dependencies to be vendored and to generate different files. @@ -184,13 +199,47 @@ raze_fetch_remote_crates() ``` This tells Bazel where to get the dependencies from, and how to build them: -using the files generated into //cargo. +using the files generated into `//cargo`. _Note that this method's name depends on your `gen_workspace_prefix` setting_. You can depend on any _explicit_ dependencies in any Rust rule by depending on `//cargo:your_dependency_name`. +### Vendoring Mode + +In Vendoring mode, a root directly is selected that will house the vendored +dependencies and become the gateway to those build rules. `//cargo` is +conventional, but `//third_party/cargo` may be desirable to satisfy +organizational needs. Vendoring directly into root isn't well supported due to +implementation-specific idiosyncracies, but it may be supported in the future. +From here forward, `//cargo` will be the assumed directory. + +#### Generate buildable targets (vendored) + +First, install the required tools for vendoring and generating BUILDable +targets. + +```bash +$ cargo install cargo-raze +``` + +Following that, vendor your dependencies from within the cargo/ directory. This +will also update your `Cargo.lock` file. + +```bash +$ cargo vendor --versioned-dirs +``` + +Finally, generate your BUILD files, again from within the `cargo/` directory + +```bash +$ cargo raze +``` + +You can now depend on any _explicit_ dependencies in any Rust rule by depending on +`//cargo:your_dependency_name`. + ### Handling Unconventional Crates Some crates execute a "build script", which, while technically unrestricted in @@ -224,9 +273,9 @@ Cargo.toml, in the following manner ```toml [package.metadata.raze.crates.unicase.'2.1.0'] additional_flags = [ - # Rustc is 1.15, enable all optional settings - "--cfg=__unicase__iter_cmp", - "--cfg=__unicase__defauler_hasher", + # Rustc is 1.15, enable all optional settings + "--cfg=__unicase__iter_cmp", + "--cfg=__unicase__defauler_hasher", ] ``` @@ -245,21 +294,21 @@ openssl, this may in part look like: ```toml [package.metadata.raze.crates.openssl-sys.'0.9.24'] additional_flags = [ - # Vendored openssl is 1.0.2m - "--cfg=ossl102", - "--cfg=version=102", + # Vendored openssl is 1.0.2m + "--cfg=ossl102", + "--cfg=version=102", ] additional_deps = [ - "@//third_party/openssl:crypto", - "@//third_party/openssl:ssl", + "@//third_party/openssl:crypto", + "@//third_party/openssl:ssl", ] [package.metadata.raze.crates.openssl.'0.10.2'] additional_flags = [ - # Vendored openssl is 1.0.2m - "--cfg=ossl102", - "--cfg=version=102", - "--cfg=ossl10x", + # Vendored openssl is 1.0.2m + "--cfg=ossl102", + "--cfg=version=102", + "--cfg=ossl10x", ] ``` @@ -271,7 +320,7 @@ something like: ```python new_local_repository( name = "llvm", - build_file = "llvm.BUILD", + build_file = "BUILD.llvm.bazel", path = "/usr/lib/llvm-3.9", ) ``` @@ -283,10 +332,10 @@ pre-generation: ```toml [package.metadata.raze.crates.sdl2.'0.31.0'] skipped_deps = [ - "sdl2-sys-0.31.0" + "sdl2-sys-0.31.0" ] additional_deps = [ - "@//cargo/overrides/sdl2-sys:sdl2_sys" + "@//cargo/overrides/sdl2-sys:sdl2_sys" ] ``` @@ -301,7 +350,7 @@ to tell Bazel to expose such binaries for you: [package.metadata.raze.crates.bindgen.'0.32.2'] gen_buildrs = true # needed to build bindgen extra_aliased_targets = [ - "cargo_bin_bindgen" + "cargo_bin_bindgen" ] ``` @@ -331,7 +380,7 @@ specified by `workspace_path`. Setting default_gen_buildrs to true will cause cargo-raze to generate build scripts for all crates that require them: -``` +```toml [package.metadata.raze] workspace_path = "//cargo" genmode = "Remote" @@ -348,7 +397,7 @@ Even with this setting enabled, you may still need to provide extra settings for a few crates. For example, the ring crate needs access to the source tree at build time: -``` +```toml [package.metadata.raze.crates.ring.'*'] data_attr = "glob([\"src/**\"])" ``` @@ -356,7 +405,7 @@ data_attr = "glob([\"src/**\"])" If you wish to disable the build script on an individual crate, you can do so as follows: -``` +```toml [package.metadata.raze.crates.some_dependency.'*'] gen_buildrs = false ``` diff --git a/examples/WORKSPACE b/examples/WORKSPACE index 5ec2aebc1..b189f6479 100644 --- a/examples/WORKSPACE +++ b/examples/WORKSPACE @@ -4,11 +4,11 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_rust", - sha256 = "b5d4d1c7609714dfef821355f40353c58aa1afb3803401b3442ed2355db9b0c7", - strip_prefix = "rules_rust-8d2b4eeeff9dce24f5cbb36018f2d60ecd676639", + sha256 = "0e2e633bf0f7f25392ffb477d677c88eb34fe70ffae05e3ad92fdd9f8d6579db", + strip_prefix = "rules_rust-bc0578798f50d018ca4278ad5610598c400992c9", urls = [ - # Master branch as of 2020-11-10 - "https://github.com/bazelbuild/rules_rust/archive/8d2b4eeeff9dce24f5cbb36018f2d60ecd676639.tar.gz", + # Master branch as of 2020-12-05 + "https://github.com/bazelbuild/rules_rust/archive/bc0578798f50d018ca4278ad5610598c400992c9.tar.gz", ], ) @@ -16,10 +16,6 @@ load("@io_bazel_rules_rust//rust:repositories.bzl", "rust_repositories") rust_repositories() -load("@io_bazel_rules_rust//:workspace.bzl", "rust_workspace") - -rust_workspace() - load("//remote/binary_dependencies/cargo:crates.bzl", "remote_binary_dependencies_fetch_remote_crates") remote_binary_dependencies_fetch_remote_crates() diff --git a/smoke-test.sh b/smoke-test.sh index 2c3da57ef..629335b0d 100755 --- a/smoke-test.sh +++ b/smoke-test.sh @@ -40,21 +40,17 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_rust", - sha256 = "b5d4d1c7609714dfef821355f40353c58aa1afb3803401b3442ed2355db9b0c7", - strip_prefix = "rules_rust-8d2b4eeeff9dce24f5cbb36018f2d60ecd676639", + sha256 = "0e2e633bf0f7f25392ffb477d677c88eb34fe70ffae05e3ad92fdd9f8d6579db", + strip_prefix = "rules_rust-bc0578798f50d018ca4278ad5610598c400992c9", urls = [ - # Master branch as of 2020-11-10 - "https://github.com/bazelbuild/rules_rust/archive/8d2b4eeeff9dce24f5cbb36018f2d60ecd676639.tar.gz", + # Master branch as of 2020-12-05 + "https://github.com/bazelbuild/rules_rust/archive/bc0578798f50d018ca4278ad5610598c400992c9.tar.gz", ], ) load("@io_bazel_rules_rust//rust:repositories.bzl", "rust_repositories") rust_repositories() - -load("@io_bazel_rules_rust//:workspace.bzl", "rust_workspace") - -rust_workspace() EOF for ex in $(find $TEST_DIR/remote -maxdepth 1 -type d | tail -n+2 | sort); do From aece94784ba36c9a933b4af222f116c067705a9f Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 14 Dec 2020 13:18:34 -0800 Subject: [PATCH 3/6] Addressed PR feedback --- impl/src/settings.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/impl/src/settings.rs b/impl/src/settings.rs index 673153bb7..6dbbf79aa 100644 --- a/impl/src/settings.rs +++ b/impl/src/settings.rs @@ -39,11 +39,11 @@ pub struct RazeSettings { pub workspace_path: String, /** - * The relative path within each workspace member directory where aliases the member's dependencies should be rendered. - * + * The relative path within each workspace member directory where aliases the member's dependencies should be rendered. + * * By default, a new directory will be created next to the `Cargo.toml` file named `cargo` for users to refer to them - * as. For example, the toml file `//my/package:Cargo.toml` will have aliases rendered as something like - * `//my/package/cargo:dependency`. Note that setting this value to `"."` will cause the BUILD file in the same package + * as. For example, the toml file `//my/package:Cargo.toml` will have aliases rendered as something like + * `//my/package/cargo:dependency`. Note that setting this value to `"."` will cause the BUILD file in the same package * as the Cargo.toml file to be overwritten. */ #[serde(default = "default_package_aliases_dir")] @@ -694,8 +694,8 @@ fn parse_raze_settings(metadata: Metadata) -> Result { // Attempt to load legacy settings but do not allow failures to propogate if let Ok(settings) = parse_raze_settings_legacy(&metadata) { eprintln!( - "WARNING: The top-level `[raze]` key is deprecated. Please set `[workspace.metadata.raze]` or \ - `[package.metadata.raze]` instead." + "WARNING: The top-level `[raze]` key is deprecated. Please set `[workspace.metadata.raze]` \ + or `[package.metadata.raze]` instead." ); return Ok(settings); } @@ -740,8 +740,13 @@ pub fn load_settings>( .into(), }; - // UNWRAP: safe due to earlier assert - let cargo_toml_dir = cargo_toml_path.as_ref().parent().unwrap(); + let cargo_toml_dir = cargo_toml_path + .as_ref() + .parent() + .ok_or(RazeError::Generic(format!( + "Failed to find parent directory for cargo toml file: {:?}", + cargo_toml_path.as_ref().display(), + )))?; let metadata = { let result = fetcher.fetch_metadata(cargo_toml_dir, false); if result.is_err() { @@ -861,7 +866,7 @@ pub mod tests { let cargo_toml_path = temp_workspace_dir.path().join("Cargo.toml"); std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); - let settings = load_settings(cargo_toml_path, None).unwrap(); + let settings = load_settings(cargo_toml_path, /*cargo_bin_path=*/ None).unwrap(); assert!(settings.binary_deps.len() > 0); } From 997708e6e77632ccacd3b7577e3cebf4a4a0011b Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 14 Dec 2020 13:24:20 -0800 Subject: [PATCH 4/6] Addressed PR feedback --- impl/src/settings.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impl/src/settings.rs b/impl/src/settings.rs index 6dbbf79aa..c091c0135 100644 --- a/impl/src/settings.rs +++ b/impl/src/settings.rs @@ -710,7 +710,7 @@ struct SettingsMetadataFetcher { impl MetadataFetcher for SettingsMetadataFetcher { fn fetch_metadata(&self, working_dir: &Path, _include_deps: bool) -> Result { - // This fetch should not require network access. + // This fetch does not require network access. MetadataCommand::new() .cargo_path(&self.cargo_bin_path) .no_deps() From 8510e5b7b76e2d941f50dfaf583f31be6f22c492 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 14 Dec 2020 13:50:53 -0800 Subject: [PATCH 5/6] Updated documentation --- impl/src/settings.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/impl/src/settings.rs b/impl/src/settings.rs index c091c0135..f03f4f28f 100644 --- a/impl/src/settings.rs +++ b/impl/src/settings.rs @@ -704,6 +704,7 @@ fn parse_raze_settings(metadata: Metadata) -> Result { parse_raze_settings_any_package(&metadata) } +/** A cargo command wrapper for gathering cargo metadata used to parse [RazeSettings](crate::settings::RazeSettings) */ struct SettingsMetadataFetcher { pub cargo_bin_path: PathBuf, } From e8790c732595d9bf061af3d3e8c9794cba808446 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 14 Dec 2020 19:28:11 -0800 Subject: [PATCH 6/6] Updated documentation --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 2b714c6fd..8d8f7a6b3 100644 --- a/README.md +++ b/README.md @@ -375,6 +375,10 @@ included in the resulting output directory. Lockfiles for targets specified unde `[package.metadata.raze.binary_deps]` will be generated into a `lockfiles` directory inside the path specified by `workspace_path`. +Note that the `binary_deps` field can go in workspace _and_ package metadata, however, only one +definition of a binary dependency can exist at a time. If you have multiple packages that depend +on a single binary dependency, that definition needs to be be moved to the workspace metadata. + ### Build scripts by default Setting default_gen_buildrs to true will cause cargo-raze to generate build scripts