From e655d34c00fc6cc2c177c9448a7a45881817db66 Mon Sep 17 00:00:00 2001 From: RS Date: Fri, 24 Jun 2022 05:55:26 -0700 Subject: [PATCH] Convert all paths to Camino (#511) Fixes #427. Fixes #510. --- impl/Cargo.lock | 5 +- impl/Cargo.toml | 1 + impl/src/bin/cargo-raze.rs | 41 +++-- impl/src/checks.rs | 21 +-- impl/src/context.rs | 16 +- impl/src/features.rs | 7 +- impl/src/metadata.rs | 148 ++++++++++-------- impl/src/planning.rs | 19 ++- impl/src/planning/subplanners.rs | 33 ++-- impl/src/rendering.rs | 10 +- impl/src/rendering/bazel.rs | 55 +++---- impl/src/settings.rs | 46 +++--- impl/src/testing.rs | 15 +- impl/src/util.rs | 30 ++-- third_party/cargo/crates.bzl | 11 +- ...o-1.0.4.bazel => BUILD.camino-1.0.9.bazel} | 8 +- .../remote/BUILD.cargo_metadata-0.14.0.bazel | 2 +- 17 files changed, 247 insertions(+), 221 deletions(-) rename third_party/cargo/remote/{BUILD.camino-1.0.4.bazel => BUILD.camino-1.0.9.bazel} (91%) diff --git a/impl/Cargo.lock b/impl/Cargo.lock index c2f66d51e..ce69793a1 100644 --- a/impl/Cargo.lock +++ b/impl/Cargo.lock @@ -351,9 +351,9 @@ checksum = "631ae5198c9be5e753e5cc215e1bd73c2b466a3565173db433f52bb9d3e66dba" [[package]] name = "camino" -version = "1.0.4" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d4648c6d00a709aa069a236adcaae4f605a6241c72bf5bee79331a4b625921a9" +checksum = "869119e97797867fd90f5e22af7d0bd274bd4635ebb9eb68c04f3f513ae6c412" dependencies = [ "serde", ] @@ -400,6 +400,7 @@ name = "cargo-raze" version = "0.16.0" dependencies = [ "anyhow", + "camino", "cargo-clone-crate", "cargo-lock", "cargo-platform", diff --git a/impl/Cargo.toml b/impl/Cargo.toml index a2bdb7f1c..e0ea3640e 100644 --- a/impl/Cargo.toml +++ b/impl/Cargo.toml @@ -24,6 +24,7 @@ path = "src/bin/cargo-raze.rs" [dependencies] anyhow = "1.0.38" +camino = "1.0.9" cargo_metadata = "0.14" cargo_toml = "0.8.1" cargo-clone-crate = { version = "0.1.6", default-features = false } diff --git a/impl/src/bin/cargo-raze.rs b/impl/src/bin/cargo-raze.rs index b37df968f..c0d693e58 100644 --- a/impl/src/bin/cargo-raze.rs +++ b/impl/src/bin/cargo-raze.rs @@ -16,10 +16,10 @@ use std::{ env, fs::{self, File}, io::Write, - path::{Path, PathBuf}, }; use anyhow::{anyhow, Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use cargo_metadata::Metadata; use docopt::Docopt; @@ -104,6 +104,11 @@ fn main() -> Result<()> { Ok(()) } +fn current_dir_utf8() -> Result { + Utf8PathBuf::from_path_buf(std::env::current_dir()?) + .map_err(|_e| anyhow!("std::env::current_dir is invalid UTF-8.")) +} + fn parse_options() -> Options { // When used as a cargo subcommand, the string "raze" will always be // passed as the second `argv` entry. We need to remove that to keep @@ -147,34 +152,26 @@ fn fetch_local_metadata(options: &Options) -> Result { // Gather basic, offline metadata to parse settings from let fetcher = if let Some(cargo_bin_path) = &options.flag_cargo_bin_path { SettingsMetadataFetcher { - cargo_bin_path: PathBuf::from(cargo_bin_path), + cargo_bin_path: Utf8PathBuf::from(cargo_bin_path), } } else { SettingsMetadataFetcher::default() }; let working_directory = if let Some(manifest_path) = &options.flag_manifest_path { - let manifest_path = PathBuf::from(manifest_path).canonicalize()?; + let manifest_path = Utf8PathBuf::from(manifest_path).canonicalize_utf8()?; if !manifest_path.is_file() { - return Err(anyhow!( - "manifest path `{}` is not a file.", - manifest_path.display() - )); + return Err(anyhow!("manifest path `{}` is not a file.", manifest_path)); } // UNWRAP: Unwrap safe due to check above. - PathBuf::from(manifest_path.parent().unwrap()) + Utf8PathBuf::from(manifest_path.parent().unwrap()) } else { - env::current_dir()? + current_dir_utf8()? }; fetcher .fetch_metadata(&working_directory, false) - .with_context(|| { - format!( - "Failed to fetch metadata for {}", - working_directory.display() - ) - }) + .with_context(|| format!("Failed to fetch metadata for {}", working_directory)) } fn fetch_raze_metadata( @@ -193,7 +190,7 @@ fn fetch_raze_metadata( }; let cargo_raze_working_dir = find_bazel_workspace_root(local_metadata.workspace_root.as_ref()) - .unwrap_or(env::current_dir()?); + .unwrap_or(current_dir_utf8()?); let binary_dep_info = if settings.genmode == GenMode::Remote { Some(&settings.binary_deps) @@ -238,12 +235,12 @@ fn render_files( local_metadata: &Metadata, ) -> Result<(RenderDetails, Vec)> { let cargo_raze_working_dir = find_bazel_workspace_root(local_metadata.workspace_root.as_ref()) - .unwrap_or(env::current_dir()?); + .unwrap_or(current_dir_utf8()?); let mut bazel_renderer = BazelRenderer::new(); let render_details = RenderDetails { cargo_root: metadata.cargo_workspace_root.clone(), - path_prefix: PathBuf::from(&settings.workspace_path.trim_start_matches('/')), + path_prefix: Utf8PathBuf::from(&settings.workspace_path.trim_start_matches('/')), package_aliases_dir: settings.package_aliases_dir.clone(), vendored_buildfile_name: settings.output_buildfile_suffix.clone(), bazel_root: cargo_raze_working_dir, @@ -276,7 +273,7 @@ fn write_files( .join("remote"); // Clean out the "remote" directory so users can easily see what build files are relevant if remote_dir.exists() { - let build_glob = format!("{}/BUILD*.bazel", remote_dir.display()); + let build_glob = format!("{}/BUILD*.bazel", remote_dir); for entry in glob::glob(&build_glob)? { fs::remove_file(entry?)?; } @@ -285,7 +282,7 @@ fn write_files( for output in bazel_file_outputs.iter() { if options.flag_dryrun.unwrap_or(false) { - println!("{}:\n{}", output.path.display(), output.contents); + println!("{}:\n{}", output.path, output.contents); continue; } // Ensure all parent directories exist @@ -303,10 +300,10 @@ fn write_files( } /// Writes rendered files to filesystem. -fn write_to_file(path: &Path, contents: &str, verbose: bool) -> Result<()> { +fn write_to_file(path: &Utf8Path, contents: &str, verbose: bool) -> Result<()> { File::create(&path).and_then(|mut f| f.write_all(contents.as_bytes()))?; if verbose { - println!("Generated {} successfully", path.display()); + println!("Generated {} successfully", path); } Ok(()) } diff --git a/impl/src/checks.rs b/impl/src/checks.rs index a66af9cf6..b09804534 100644 --- a/impl/src/checks.rs +++ b/impl/src/checks.rs @@ -15,11 +15,10 @@ use std::{ collections::{HashMap, HashSet}, fs, - path::Path, - path::PathBuf, }; use anyhow::{anyhow, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use crate::{ error::RazeError, @@ -39,7 +38,7 @@ const MAX_DISPLAYED_MISSING_RESOLVE_PACKAGES: usize = 5; pub fn check_metadata( raze_metadata: &RazeMetadata, settings: &RazeSettings, - bazel_workspace_root: &Path, + bazel_workspace_root: &Utf8Path, ) -> Result<()> { // Check for errors check_resolve_matches_packages(&raze_metadata.metadata)?; @@ -106,7 +105,7 @@ fn check_lockfile_for_missing_checksums( fn check_all_vendored( metadata: &Metadata, settings: &RazeSettings, - bazel_workspace_root: &Path, + bazel_workspace_root: &Utf8Path, ) -> Result<()> { let non_workspace_packages: Vec<&Package> = metadata .packages @@ -149,15 +148,18 @@ fn check_all_vendored( message: format!( "Failed to find expected vendored crates in {:?}: {:?}. Did you forget to run cargo \ vendor?", - expected_full_path.display(), - limited_missing_crates + expected_full_path, limited_missing_crates ), } .into(), ) } -fn vendor_path(bazel_workspace_root: &Path, workspace_path: &str, vendor_dir: &str) -> PathBuf { +fn vendor_path( + bazel_workspace_root: &Utf8Path, + workspace_path: &str, + vendor_dir: &str, +) -> Utf8PathBuf { bazel_workspace_root // Trim the absolute label identifier from the start of the workspace path .join(workspace_path.trim_start_matches('/')) @@ -167,13 +169,12 @@ fn vendor_path(bazel_workspace_root: &Path, workspace_path: &str, vendor_dir: &s /// Returns the packages expected path during current execution. fn expected_vendored_path( package: &Package, - bazel_workspace_root: &Path, + bazel_workspace_root: &Utf8Path, workspace_path: &str, vendor_dir: &str, ) -> String { vendor_path(bazel_workspace_root, workspace_path, vendor_dir) .join(package_ident(&package.name, &package.version.to_string())) - .display() .to_string() } @@ -289,7 +290,7 @@ mod tests { let result = check_all_vendored( &template_metadata(templates::DUMMY_MODIFIED_METADATA), &settings, - &PathBuf::from("/tmp/some/path"), + &Utf8PathBuf::from("/tmp/some/path"), ); // Vendored crates will not have been rendered at that path diff --git a/impl/src/context.rs b/impl/src/context.rs index 4f9bf3e7a..a0482a927 100644 --- a/impl/src/context.rs +++ b/impl/src/context.rs @@ -12,12 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{ - collections::{BTreeMap, BTreeSet}, - path::PathBuf, -}; +use std::collections::{BTreeMap, BTreeSet}; use crate::{features::Features, settings::CrateSettings}; +use camino::Utf8PathBuf; use semver::Version; use serde::{Deserialize, Serialize}; use url::Url; @@ -154,15 +152,15 @@ pub struct CrateContext { pub pkg_version: Version, pub edition: String, pub raze_settings: CrateSettings, - pub canonical_additional_build_file: Option, + pub canonical_additional_build_file: Option, pub default_deps: CrateDependencyContext, pub targeted_deps: Vec, pub license: LicenseData, pub features: Features, pub workspace_path_to_crate: String, - pub workspace_member_dependents: Vec, - pub workspace_member_dev_dependents: Vec, - pub workspace_member_build_dependents: Vec, + pub workspace_member_dependents: Vec, + pub workspace_member_dev_dependents: Vec, + pub workspace_member_build_dependents: Vec, pub is_workspace_member_dependency: bool, pub is_binary_dependency: bool, pub targets: Vec, @@ -203,5 +201,5 @@ pub struct WorkspaceContext { pub output_buildfile_suffix: String, // A list of relative paths from a Cargo workspace root to a Cargo package. - pub workspace_members: Vec, + pub workspace_members: Vec, } diff --git a/impl/src/features.rs b/impl/src/features.rs index d1ea5fae6..f674367ed 100644 --- a/impl/src/features.rs +++ b/impl/src/features.rs @@ -13,13 +13,14 @@ // limitations under the License. use std::collections::{BTreeMap, BTreeSet, HashMap}; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; use crate::error::RazeError; use crate::settings::RazeSettings; use crate::util::cargo_bin_path; use anyhow::{Error, Result}; +use camino::Utf8PathBuf; use cargo_metadata::{Package, PackageId, Version}; use serde::{Deserialize, Serialize}; @@ -133,7 +134,7 @@ fn clean_cargo_tree_output(cargo_tree_output: &str) -> Vec { // Runs `cargo-tree` with a very specific format argument that makes it easier // to extract per-platform targets. fn run_cargo_tree(cargo_dir: &Path, triple: &str) -> Result { - let cargo_bin: PathBuf = cargo_bin_path(); + let cargo_bin: Utf8PathBuf = cargo_bin_path(); let mut cargo_tree = Command::new(cargo_bin); cargo_tree.current_dir(cargo_dir); cargo_tree @@ -386,7 +387,7 @@ proc-macro2 v1.0.26|default,proc-macro| (*) } fn mock_cargo_tree_command(_cargo_dir: &Path, triple: &str) -> Result { - let cargo_tree_template_dir = PathBuf::from(std::file!()) + let cargo_tree_template_dir = Utf8PathBuf::from(std::file!()) .parent() .unwrap() .join("testing/cargo_tree") diff --git a/impl/src/metadata.rs b/impl/src/metadata.rs index e525b7db3..80fc9f256 100644 --- a/impl/src/metadata.rs +++ b/impl/src/metadata.rs @@ -16,11 +16,11 @@ use std::{ collections::{BTreeMap, HashMap}, env::consts, fs, - path::{Path, PathBuf}, string::String, }; use anyhow::{anyhow, Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use cargo_lock::Lockfile; use cargo_metadata::{Metadata, MetadataCommand, PackageId}; use glob::glob; @@ -41,12 +41,12 @@ pub(crate) const DEFAULT_CRATE_INDEX_URL: &str = "https://github.com/rust-lang/c /// An entity that can generate Cargo metadata within a Cargo workspace pub trait MetadataFetcher { - fn fetch_metadata(&self, working_dir: &Path, include_deps: bool) -> Result; + fn fetch_metadata(&self, working_dir: &Utf8Path, include_deps: bool) -> Result; } /// A lockfile generator which simply wraps the `cargo_metadata::MetadataCommand` command struct CargoMetadataFetcher { - pub cargo_bin_path: PathBuf, + pub cargo_bin_path: Utf8PathBuf, } impl Default for CargoMetadataFetcher { @@ -58,7 +58,7 @@ impl Default for CargoMetadataFetcher { } impl MetadataFetcher for CargoMetadataFetcher { - fn fetch_metadata(&self, working_dir: &Path, include_deps: bool) -> Result { + fn fetch_metadata(&self, working_dir: &Utf8Path, include_deps: bool) -> Result { let mut command = MetadataCommand::new(); if !include_deps { @@ -72,8 +72,7 @@ impl MetadataFetcher for CargoMetadataFetcher { .with_context(|| { format!( "Failed to fetch Metadata with `{}` from `{}`", - &self.cargo_bin_path.display(), - working_dir.display() + &self.cargo_bin_path, working_dir ) }) } @@ -81,17 +80,17 @@ impl MetadataFetcher for CargoMetadataFetcher { /// An entity that can generate a lockfile data within a Cargo workspace pub trait LockfileGenerator { - fn generate_lockfile(&self, crate_root_dir: &Path) -> Result; + fn generate_lockfile(&self, crate_root_dir: &Utf8Path) -> Result; } /// A lockfile generator which simply wraps the `cargo generate-lockfile` command struct CargoLockfileGenerator { - cargo_bin_path: PathBuf, + cargo_bin_path: Utf8PathBuf, } impl LockfileGenerator for CargoLockfileGenerator { /// Generate lockfile information from a cargo workspace root - fn generate_lockfile(&self, crate_root_dir: &Path) -> Result { + fn generate_lockfile(&self, crate_root_dir: &Utf8Path) -> Result { let lockfile_path = crate_root_dir.join("Cargo.lock"); // Generate lockfile @@ -99,19 +98,19 @@ impl LockfileGenerator for CargoLockfileGenerator { .arg("generate-lockfile") .current_dir(&crate_root_dir) .output() - .with_context(|| format!("Generating lockfile in {}", crate_root_dir.display()))?; + .with_context(|| format!("Generating lockfile in {}", crate_root_dir))?; if !output.status.success() { anyhow::bail!( "Failed to generate lockfile in {}: {}", - crate_root_dir.display(), + crate_root_dir, String::from_utf8_lossy(&output.stderr) ); } // Load lockfile contents Lockfile::load(&lockfile_path) - .with_context(|| format!("Failed to load lockfile: {}", lockfile_path.display())) + .with_context(|| format!("Failed to load lockfile: {}", lockfile_path)) } } @@ -124,7 +123,7 @@ pub struct RazeMetadata { // The absolute path to the current project's cargo workspace root. Note that the workspace // root in `metadata` will be inside of a temporary directory. For details see: // https://doc.rust-lang.org/cargo/reference/workspaces.html#root-package - pub cargo_workspace_root: PathBuf, + pub cargo_workspace_root: Utf8PathBuf, // The metadata of a lockfile that was generated as a result of fetching metadata pub lockfile: Option, @@ -145,14 +144,14 @@ impl RazeMetadata { /// Create a symlink file on unix systems #[cfg(target_family = "unix")] -fn make_symlink(src: &Path, dest: &Path) -> Result<()> { +fn make_symlink(src: &Utf8Path, dest: &Utf8Path) -> Result<()> { std::os::unix::fs::symlink(src, dest) .with_context(|| "Failed to create symlink for generating metadata") } /// Create a symlink file on windows systems #[cfg(target_family = "windows")] -fn make_symlink(src: &Path, dest: &Path) -> Result<()> { +fn make_symlink(src: &Utf8Path, dest: &Utf8Path) -> Result<()> { std::os::windows::fs::symlink_file(src, dest) .with_context(|| "Failed to create symlink for generating metadata") } @@ -168,13 +167,13 @@ pub struct RazeMetadataFetcher { } impl RazeMetadataFetcher { - pub fn new>( + pub fn new>( cargo_bin_path: P, registry_url: Url, index_url: Url, settings: Option, ) -> RazeMetadataFetcher { - let cargo_bin_pathbuf: PathBuf = cargo_bin_path.into(); + let cargo_bin_pathbuf: Utf8PathBuf = cargo_bin_path.into(); RazeMetadataFetcher { registry_url, index_url, @@ -209,7 +208,7 @@ impl RazeMetadataFetcher { } /// Symlinks the source code of all workspace members into the temp workspace - fn link_src_to_workspace(&self, no_deps_metadata: &Metadata, temp_dir: &Path) -> Result<()> { + fn link_src_to_workspace(&self, no_deps_metadata: &Metadata, temp_dir: &Utf8Path) -> Result<()> { let crate_member_id_re = match consts::OS { "windows" => Regex::new(r".+\(path\+file:///(.+)\)")?, _ => Regex::new(r".+\(path\+file://(.+)\)")?, @@ -226,7 +225,7 @@ impl RazeMetadataFetcher { } // UNWRAP: guarded above - PathBuf::from(crate_member_id_match.unwrap().as_str()) + Utf8PathBuf::from(crate_member_id_match.unwrap().as_str()) }; // Sanity check: The assumption is that any crate with an `id` that matches @@ -238,18 +237,20 @@ impl RazeMetadataFetcher { return Err(anyhow!(format!( "The regex pattern `{}` found a path that did not contain a Cargo.toml file: `{}`", crate_member_id_re.as_str(), - workspace_member_directory.display() + workspace_member_directory ))); } // Copy the Cargo.toml files into the temp directory to match the directory structure on disk - let diff = diff_paths( + let path_diff = diff_paths( &workspace_member_directory, &no_deps_metadata.workspace_root, ) .ok_or_else(|| { anyhow!("All workspace members are expected to be under the workspace root") })?; + let diff = Utf8PathBuf::from_path_buf(path_diff) + .map_err(|_e| anyhow!("Invalid UTF-8 in path diff."))?; let new_path = temp_dir.join(diff); fs::create_dir_all(&new_path)?; fs::copy( @@ -260,17 +261,20 @@ impl RazeMetadataFetcher { // Additionally, symlink everything in some common source directories to ensure specified // library targets can be relied on and won't prevent fetching metadata for dir in vec!["bin", "src"].iter() { - let glob_pattern = format!("{}/**/*.rs", workspace_member_directory.join(dir).display()); + let glob_pattern = format!("{}/**/*.rs", workspace_member_directory.join(dir)); for entry in glob(glob_pattern.as_str()).expect("Failed to read glob pattern") { - let path = entry?; + let path = Utf8PathBuf::from_path_buf(entry?) + .map_err(|_e| anyhow!("Invalid UTF-8 in source directory."))?; // Determine the difference between the workspace root and the current file - let diff = diff_paths(&path, &no_deps_metadata.workspace_root).ok_or_else(|| { + let path_diff = diff_paths(&path, &no_deps_metadata.workspace_root).ok_or_else(|| { anyhow!("All workspace members are expected to be under the workspace root") })?; + let diff = Utf8PathBuf::from_path_buf(path_diff) + .map_err(|_e| anyhow!("Invalid UTF-8 in source directory path diff."))?; // Create a matching directory tree for the current file within the temp workspace - let new_path = temp_dir.join(diff); + let new_path = temp_dir.join(diff.as_path()); if let Some(parent) = new_path.parent() { fs::create_dir_all(parent)?; } @@ -284,7 +288,7 @@ impl RazeMetadataFetcher { } /// Creates a copy workspace in a temporary directory for fetching the metadata of the current workspace - fn make_temp_workspace(&self, cargo_workspace_root: &Path) -> Result<(TempDir, PathBuf)> { + fn make_temp_workspace(&self, cargo_workspace_root: &Utf8Path) -> Result<(TempDir, Utf8PathBuf)> { let temp_dir = TempDir::new()?; // First gather metadata without downloading any dependencies so we can identify any path dependencies. @@ -316,12 +320,14 @@ impl RazeMetadataFetcher { } // Copy over the Cargo.toml files of each workspace member - self.link_src_to_workspace(&no_deps_metadata, temp_dir.as_ref())?; - Ok((temp_dir, no_deps_metadata.workspace_root.into())) + let temp_path = Utf8Path::from_path(temp_dir.as_ref()) + .ok_or_else(|| anyhow!("Invalid UTF-8 in temp path."))?; + self.link_src_to_workspace(&no_deps_metadata, temp_path)?; + Ok((temp_dir, no_deps_metadata.workspace_root)) } /// Download a crate's source code from the current registry url - fn fetch_crate_src(&self, dir: &Path, name: &str, version: &str) -> Result { + fn fetch_crate_src(&self, dir: &Utf8Path, name: &str, version: &str) -> Result { // The registry url should only be the host URL with ports. No path let registry_url = { let mut r_url = self.registry_url.clone(); @@ -359,7 +365,7 @@ impl RazeMetadataFetcher { fn inject_binaries_into_workspace( &self, binary_deps: Vec, - root_toml: &Path, + root_toml: &Utf8Path, ) -> Result<()> { // Read the current manifest let mut manifest = { @@ -385,12 +391,8 @@ impl RazeMetadataFetcher { // cargo_toml::Manifest cannot be serialized direcly. // see: https://gitlab.com/crates.rs/cargo_toml/-/issues/3 let value = toml::Value::try_from(&manifest)?; - std::fs::write(root_toml, toml::to_string(&value)?).with_context(|| { - format!( - "Failed to inject workspace metadata to {}", - root_toml.display() - ) - }) + std::fs::write(root_toml, toml::to_string(&value)?) + .with_context(|| format!("Failed to inject workspace metadata to {}", root_toml)) } /// Look up a crate in a specified crate index to determine it's checksum @@ -429,8 +431,8 @@ impl RazeMetadataFetcher { /// `reused_lockfile` is not provided. fn cargo_generate_lockfile( &self, - reused_lockfile: &Option, - cargo_dir: &Path, + reused_lockfile: &Option, + cargo_dir: &Utf8Path, ) -> Result> { let lockfile_path = cargo_dir.join("Cargo.lock"); @@ -449,12 +451,14 @@ impl RazeMetadataFetcher { /// Gather all information about a Cargo project to use for planning and rendering steps pub fn fetch_metadata( &self, - cargo_workspace_root: &Path, + cargo_workspace_root: &Utf8Path, binary_dep_info: Option<&HashMap>, - reused_lockfile: Option, + reused_lockfile: Option, ) -> Result { let (cargo_dir, cargo_workspace_root) = self.make_temp_workspace(cargo_workspace_root)?; - let cargo_root_toml = cargo_dir.as_ref().join("Cargo.toml"); + let utf8_cargo_dir = Utf8Path::from_path(cargo_dir.as_ref()) + .ok_or_else(|| anyhow!("Cargo dir has invalid UTF-8 in fetch_metadata."))?; + let cargo_root_toml = utf8_cargo_dir.join("Cargo.toml"); // Gather new lockfile data if any binary dependencies were provided let mut checksums: HashMap = HashMap::new(); @@ -464,15 +468,13 @@ impl RazeMetadataFetcher { for (name, info) in binary_dep_info.iter() { let version = info.req(); - let src_dir = self.fetch_crate_src(cargo_dir.as_ref(), name, version)?; + let src_dir = self.fetch_crate_src(utf8_cargo_dir, name, version)?; checksums.insert( package_ident(name, version), self.fetch_crate_checksum(name, version)?, ); if let Some(dirname) = src_dir.file_name() { - if let Some(dirname_str) = dirname.to_str() { - src_dirnames.push(dirname_str.to_string()); - } + src_dirnames.push(dirname.to_string()); } } @@ -480,7 +482,7 @@ impl RazeMetadataFetcher { } } - let output_lockfile = self.cargo_generate_lockfile(&reused_lockfile, cargo_dir.as_ref())?; + let output_lockfile = self.cargo_generate_lockfile(&reused_lockfile, utf8_cargo_dir)?; // Load checksums from the lockfile let workspace_toml_lock = cargo_dir.as_ref().join("Cargo.lock"); @@ -498,7 +500,7 @@ impl RazeMetadataFetcher { let metadata = self .metadata_fetcher - .fetch_metadata(cargo_dir.as_ref(), /*include_deps=*/ true)?; + .fetch_metadata(utf8_cargo_dir, /*include_deps=*/ true)?; // In this function because it's metadata, even though it's not returned by `cargo-metadata` let platform_features = match self.settings.as_ref() { @@ -532,12 +534,13 @@ impl Default for RazeMetadataFetcher { pub struct BinaryDependencyInfo { pub name: String, pub info: cargo_toml::Dependency, - pub lockfile: Option, + pub lockfile: Option, } #[cfg(test)] pub mod tests { use anyhow::Context; + use camino::Utf8PathBuf; use httpmock::MockServer; use tera::Tera; @@ -551,13 +554,13 @@ pub mod tests { } impl DummyCargoMetadataFetcher { - fn render_metadata(&self, mock_workspace_path: &Path) -> Option { + fn render_metadata(&self, mock_workspace_path: &Utf8Path) -> Option { self.metadata_template.as_ref()?; let dir = TempDir::new().unwrap(); let mut renderer = Tera::new(&format!("{}/*", dir.as_ref().display())).unwrap(); - let templates_dir = PathBuf::from(std::file!()) + let templates_dir = Utf8PathBuf::from(std::file!()) .parent() .unwrap() .join("testing/metadata_templates") @@ -583,7 +586,7 @@ pub mod tests { } impl MetadataFetcher for DummyCargoMetadataFetcher { - fn fetch_metadata(&self, working_dir: &Path, include_deps: bool) -> Result { + fn fetch_metadata(&self, working_dir: &Utf8Path, include_deps: bool) -> Result { // Only use the template if the command is looking to reach out to the internet. if include_deps { if let Some(metadata) = self.render_metadata(working_dir) { @@ -601,7 +604,7 @@ pub mod tests { .with_context(|| { format!( "Failed to run `{} metadata` with contents:\n{}", - cargo_bin_path().display(), + cargo_bin_path(), fs::read_to_string(working_dir.join("Cargo.toml")).unwrap() ) }) @@ -614,7 +617,7 @@ pub mod tests { } impl LockfileGenerator for DummyLockfileGenerator { - fn generate_lockfile(&self, _crate_root_dir: &Path) -> Result { + fn generate_lockfile(&self, _crate_root_dir: &Utf8Path) -> Result { match &self.lockfile_contents { Some(contents) => Lockfile::from_str(contents) .with_context(|| format!("Failed to load provided lockfile:\n{}", contents)), @@ -652,7 +655,9 @@ pub mod tests { metadata_template: Some(templates::BASIC_METADATA.to_string()), })); - fetcher.fetch_metadata(dir.as_ref(), None, None).unwrap() + fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .unwrap() } #[test] @@ -666,7 +671,9 @@ pub mod tests { fetcher.set_lockfile_generator(Box::new(DummyLockfileGenerator { lockfile_contents: None, })); - fetcher.fetch_metadata(dir.as_ref(), None, None).unwrap(); + fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .unwrap(); } #[test] @@ -690,7 +697,9 @@ pub mod tests { fetcher.set_lockfile_generator(Box::new(DummyLockfileGenerator { lockfile_contents: None, })); - fetcher.fetch_metadata(dir.as_ref(), None, None).unwrap(); + fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .unwrap(); } #[test] @@ -704,7 +713,9 @@ pub mod tests { } let fetcher = RazeMetadataFetcher::default(); - assert!(fetcher.fetch_metadata(dir.as_ref(), None, None).is_err()); + assert!(fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .is_err()); } #[test] @@ -713,7 +724,7 @@ pub mod tests { let mock = mock_remote_crate("fake-crate", "3.3.3", &mock_server); let path = fetcher - .fetch_crate_src(mock.data_dir.as_ref(), "fake-crate", "3.3.3") + .fetch_crate_src(utf8_path(mock.data_dir.as_ref()), "fake-crate", "3.3.3") .unwrap(); for mock in mock.endpoints.iter() { @@ -737,7 +748,8 @@ pub mod tests { let (fetcher, _mock_server, _index_url) = dummy_raze_metadata_fetcher(); let crate_dir = make_workspace_with_dependency(); - let cargo_toml_path = crate_dir.as_ref().join("Cargo.toml"); + let utf8_crate_dir = utf8_path(crate_dir.as_ref()); + let cargo_toml_path = utf8_crate_dir.join("Cargo.toml"); let mut manifest = cargo_toml::Manifest::from_str(fs::read_to_string(&cargo_toml_path).unwrap().as_str()) .unwrap(); @@ -778,14 +790,18 @@ pub mod tests { let (fetcher, _mock_server, _index_url) = dummy_raze_metadata_fetcher(); let crate_dir = make_workspace_with_dependency(); - let reused_lockfile = crate_dir.as_ref().join("locks_test/Cargo.raze.lock"); + let reused_lockfile = + Utf8PathBuf::from_path_buf(crate_dir.as_ref().join("locks_test/Cargo.raze.lock")).unwrap(); fs::create_dir_all(reused_lockfile.parent().unwrap()).unwrap(); fs::write(&reused_lockfile, "# test_generate_lockfile").unwrap(); // A reuse lockfile was provided so no new lockfile should be returned assert!(fetcher - .cargo_generate_lockfile(&Some(reused_lockfile.clone()), crate_dir.as_ref()) + .cargo_generate_lockfile( + &Some(reused_lockfile.clone()), + utf8_path(crate_dir.as_ref()) + ) .unwrap() .is_none()); @@ -808,7 +824,7 @@ pub mod tests { // A new lockfile should have been created and it should match the expected contents for the advanced_toml workspace assert_eq!( fetcher - .cargo_generate_lockfile(&None, crate_dir.as_ref()) + .cargo_generate_lockfile(&None, Utf8Path::from_path(crate_dir.as_ref()).unwrap()) .unwrap() .unwrap(), Lockfile::from_str(advanced_lock_contents()).unwrap() @@ -823,13 +839,17 @@ pub mod tests { })); let crate_dir = make_workspace(advanced_toml_contents(), None); - let expected_lockfile = crate_dir.as_ref().join("expected/Cargo.expected.lock"); + let expected_lockfile = + Utf8PathBuf::from_path_buf(crate_dir.as_ref().join("expected/Cargo.expected.lock")).unwrap(); fs::create_dir_all(expected_lockfile.parent().unwrap()).unwrap(); fs::write(&expected_lockfile, advanced_lock_contents()).unwrap(); assert!(fetcher - .cargo_generate_lockfile(&Some(expected_lockfile.clone()), crate_dir.as_ref()) + .cargo_generate_lockfile( + &Some(expected_lockfile.clone()), + utf8_path(crate_dir.as_ref()) + ) .unwrap() .is_none()); diff --git a/impl/src/planning.rs b/impl/src/planning.rs index aee78d9f3..2de9f5572 100644 --- a/impl/src/planning.rs +++ b/impl/src/planning.rs @@ -81,7 +81,7 @@ impl BuildPlannerImpl { #[cfg(test)] pub mod tests { - use std::{collections::BTreeMap, collections::HashMap, collections::HashSet, path::PathBuf}; + use std::{collections::BTreeMap, collections::HashMap, collections::HashSet}; use crate::{ metadata::tests::{ @@ -92,6 +92,7 @@ pub mod tests { }; use super::*; + use camino::Utf8PathBuf; use cargo_metadata::PackageId; use indoc::indoc; use itertools::Itertools; @@ -104,7 +105,7 @@ pub mod tests { metadata.resolve = None; RazeMetadata { metadata, - cargo_workspace_root: PathBuf::from("/some/crate"), + cargo_workspace_root: Utf8PathBuf::from("/some/crate"), lockfile: None, checksums: HashMap::new(), features: BTreeMap::new(), @@ -169,7 +170,9 @@ pub mod tests { metadata_template: Some(metadata_template.to_string()), })); - let raze_metadata = fetcher.fetch_metadata(dir.as_ref(), None, None).unwrap(); + let raze_metadata = fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .unwrap(); let mut metadata = raze_metadata.metadata; // Phase 1: Create a workspace package, add it to the packages list. @@ -192,7 +195,7 @@ pub mod tests { RazeMetadata { metadata, - cargo_workspace_root: PathBuf::from("/some/crate"), + cargo_workspace_root: Utf8PathBuf::from("/some/crate"), lockfile: None, checksums: HashMap::new(), features: BTreeMap::new(), @@ -426,7 +429,7 @@ pub mod tests { let mock = mock_remote_crate("some-binary-crate", "3.3.3", &server); let dir = mock_crate_index( &to_index_crates_map(vec![("some-binary-crate", "3.3.3")]), - Some(index_dir.as_ref()), + Some(utf8_path(index_dir.path())), ); assert!(dir.is_none()); @@ -438,7 +441,7 @@ pub mod tests { let dir = make_basic_workspace(); let raze_metadata = fetcher - .fetch_metadata(dir.as_ref(), Some(&settings.binary_deps), None) + .fetch_metadata(utf8_path(dir.as_ref()), Some(&settings.binary_deps), None) .unwrap(); for mock in mock.endpoints.iter() { @@ -651,7 +654,7 @@ pub mod tests { let settings = { let temp_dir = make_workspace(toml_file, None); - let manifest_path = temp_dir.as_ref().join("Cargo.toml"); + let manifest_path = Utf8PathBuf::from_path_buf(temp_dir.as_ref().join("Cargo.toml")).unwrap(); crate::settings::load_settings_from_manifest(&manifest_path, None).unwrap() }; @@ -797,7 +800,7 @@ pub mod tests { } fetcher - .fetch_metadata(crate_dir.as_ref(), None, None) + .fetch_metadata(utf8_path(crate_dir.as_ref()), None, None) .unwrap() } diff --git a/impl/src/planning/subplanners.rs b/impl/src/planning/subplanners.rs index 6dd76d928..6a5fdb1ec 100644 --- a/impl/src/planning/subplanners.rs +++ b/impl/src/planning/subplanners.rs @@ -15,11 +15,11 @@ use std::{ collections::{BTreeMap, HashMap, HashSet}, io, iter, - path::{Path, PathBuf}, str::FromStr, }; use anyhow::{anyhow, bail, Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use cargo_lock::SourceId; use cargo_metadata::{DepKindInfo, DependencyKind, Node, Package, Source}; use cargo_platform::Platform; @@ -281,10 +281,10 @@ impl<'planner> WorkspaceSubplanner<'planner> { impl<'planner> CrateSubplanner<'planner> { /// Builds a crate context from internal state. - fn produce_context(&self, cargo_workspace_root: &Path) -> Result { + fn produce_context(&self, cargo_workspace_root: &Utf8Path) -> Result { let package = self.crate_catalog_entry.package(); - let manifest_path = PathBuf::from(&package.manifest_path); + let manifest_path = Utf8PathBuf::from(&package.manifest_path); assert!(manifest_path.is_absolute()); let package_root = self.find_package_root_for_manifest(&manifest_path)?; @@ -331,9 +331,9 @@ impl<'planner> CrateSubplanner<'planner> { targeted_deps.sort(); - let mut workspace_member_dependents: Vec = Vec::new(); - let mut workspace_member_dev_dependents: Vec = Vec::new(); - let mut workspace_member_build_dependents: Vec = Vec::new(); + let mut workspace_member_dependents: Vec = Vec::new(); + let mut workspace_member_dev_dependents: Vec = Vec::new(); + let mut workspace_member_build_dependents: Vec = Vec::new(); for pkg_id in self.crate_catalog_entry.workspace_member_dependents.iter() { let workspace_member = self @@ -386,13 +386,8 @@ impl<'planner> CrateSubplanner<'planner> { Some(build_file) => Some( cargo_workspace_root .join(&build_file) - .canonicalize() - .with_context(|| { - format!( - "Failed to find additional_build_file: {}", - &build_file.display() - ) - })?, + .canonicalize_utf8() + .with_context(|| format!("Failed to find additional_build_file: {}", &build_file))?, ), None => None, }; @@ -649,7 +644,7 @@ impl<'planner> CrateSubplanner<'planner> { &self, crates_io_template: &str, package: &Package, - package_root: &Path, + package_root: &Utf8Path, binary_dep_spec: Option<&cargo_toml::Dependency>, ) -> Result { let mut git_data = None; @@ -668,7 +663,7 @@ impl<'planner> CrateSubplanner<'planner> { .with_context(|| { anyhow!( "Expected package root `{}` to be a prefix of manifest path dir `{}` but it wasn't", - package_root.display(), + package_root, manifest_parent ) })?; @@ -789,7 +784,7 @@ impl<'planner> CrateSubplanner<'planner> { /// Produces the complete set of build targets specified by this crate. /// This function may access the file system. See #find_package_root_for_manifest for more /// details. - fn produce_targets(&self, package_root_path: &Path) -> Result> { + fn produce_targets(&self, package_root_path: &Utf8Path) -> Result> { let mut targets = Vec::new(); let package = self.crate_catalog_entry.package(); for target in &package.targets { @@ -835,7 +830,7 @@ impl<'planner> CrateSubplanner<'planner> { /// to find the true filesystem root of the dependency. The root cause is that git dependencies /// often aren't solely the crate of interest, but rather a repository that contains the crate of /// interest among others. - fn find_package_root_for_manifest(&self, manifest_path: &Path) -> Result { + fn find_package_root_for_manifest(&self, manifest_path: &Utf8Path) -> Result { let has_git_repo_root = { let is_git = self.source_id.as_ref().map_or(false, SourceId::is_git); is_git && self.settings.genmode == GenMode::Remote @@ -845,7 +840,7 @@ impl<'planner> CrateSubplanner<'planner> { if !has_git_repo_root { // TODO(acmcarther): How do we know parent is valid here? // UNWRAP: Pathbuf guaranteed to succeed from Path - return Ok(PathBuf::from(manifest_path.parent().unwrap())); + return Ok(Utf8PathBuf::from(manifest_path.parent().unwrap())); } // If package is git package it may be nested under a parent repository. We need to find the @@ -856,7 +851,7 @@ impl<'planner> CrateSubplanner<'planner> { let joined = c.join(".git"); if joined.is_dir() { // UNWRAP: Pathbuf guaranteed to succeed from Path - return Ok(PathBuf::from(c)); + return Ok(c.to_path_buf()); } else { check_path = c; } diff --git a/impl/src/rendering.rs b/impl/src/rendering.rs index a89150c03..6b2216374 100644 --- a/impl/src/rendering.rs +++ b/impl/src/rendering.rs @@ -16,7 +16,7 @@ pub mod bazel; use crate::planning::PlannedBuild; use anyhow::Result; -use std::path::PathBuf; +use camino::Utf8PathBuf; pub trait BuildRenderer { fn render_planned_build( @@ -33,17 +33,17 @@ pub trait BuildRenderer { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct FileOutputs { - pub path: PathBuf, + pub path: Utf8PathBuf, pub contents: String, } #[derive(Debug, Clone)] pub struct RenderDetails { - pub cargo_root: PathBuf, - pub path_prefix: PathBuf, + pub cargo_root: Utf8PathBuf, + pub path_prefix: Utf8PathBuf, pub package_aliases_dir: String, pub vendored_buildfile_name: String, - pub bazel_root: PathBuf, + pub bazel_root: Utf8PathBuf, pub rust_rules_workspace_name: String, pub experimental_api: bool, pub render_package_aliases: bool, diff --git a/impl/src/rendering/bazel.rs b/impl/src/rendering/bazel.rs index 0173666df..bbb9595ca 100644 --- a/impl/src/rendering/bazel.rs +++ b/impl/src/rendering/bazel.rs @@ -13,6 +13,7 @@ // limitations under the License. use anyhow::Result; +use camino::Utf8Path; use pathdiff::diff_paths; use tera::{self, Context, Tera}; @@ -23,7 +24,7 @@ use crate::{ rendering::{BuildRenderer, FileOutputs, RenderDetails}, }; -use std::{error::Error, path::Path}; +use std::error::Error; macro_rules! unwind_tera_error { ($err:ident) => {{ @@ -220,11 +221,7 @@ impl BazelRenderer { })?; // Only the root package will have a `crates.bzl` file to export - let is_root_workspace_member = member_path - .to_str() - // Root workspace paths will are represented by an exmpty string - .map(|member_path| member_path.is_empty()) - .unwrap_or(false); + let is_root_workspace_member = member_path.as_str().is_empty(); if is_root_workspace_member { // In remote genmode, a `crates.bzl` file will always be rendered. For // vendored genmode, one is only rendered when using the experimental @@ -249,7 +246,7 @@ impl BazelRenderer { fn render_crates_bzl_package_file( &self, - path_prefix: &Path, + path_prefix: &Utf8Path, file_outputs: &[FileOutputs], ) -> Result> { let crates_bzl_pkg_file = path_prefix.join("BUILD.bazel"); @@ -290,8 +287,7 @@ fn include_additional_build_file( crate_name_opt: Some(package.pkg_name.to_owned()), message: format!( "failed to read additional_build_file '{}': {}", - file_path.display(), - e + file_path, e ), })?; @@ -303,8 +299,7 @@ fn include_additional_build_file( .raze_settings .additional_build_file .as_ref() - .unwrap() - .display(), + .unwrap(), additional_content )) } @@ -478,6 +473,7 @@ impl BuildRenderer for BazelRenderer { #[cfg(test)] mod tests { + use camino::Utf8PathBuf; use hamcrest2::{core::expect, prelude::*}; use semver::Version; @@ -489,7 +485,7 @@ mod tests { planning::PlannedBuild, rendering::{FileOutputs, RenderDetails}, settings::CrateSettings, - testing::basic_lock_contents, + testing::{basic_lock_contents, utf8_path}, }; use super::*; @@ -497,17 +493,16 @@ mod tests { use std::{ collections::{BTreeMap, BTreeSet}, fs, - path::PathBuf, str::FromStr, }; fn dummy_render_details(buildfile_suffix: &str) -> RenderDetails { RenderDetails { - cargo_root: PathBuf::from("/some/cargo/root"), - path_prefix: PathBuf::from("./some_render_prefix"), + cargo_root: Utf8PathBuf::from("/some/cargo/root"), + path_prefix: Utf8PathBuf::from("./some_render_prefix"), package_aliases_dir: "cargo".to_string(), vendored_buildfile_name: buildfile_suffix.to_owned(), - bazel_root: PathBuf::from("/some/bazel/root"), + bazel_root: Utf8PathBuf::from("/some/bazel/root"), rust_rules_workspace_name: "rules_rust".to_owned(), experimental_api: true, render_package_aliases: true, @@ -525,7 +520,7 @@ mod tests { output_buildfile_suffix: "BUILD".to_owned(), // This will typically resolve to: // `/some/cargo/root/some/crate` - workspace_members: vec![PathBuf::from("some/crate")], + workspace_members: vec![Utf8PathBuf::from("some/crate")], }, crate_contexts, workspace_aliases: aliases, @@ -721,7 +716,7 @@ mod tests { let file_outputs = render_crates_for_test(vec![], vec![]); let file_names = file_outputs .iter() - .map(|output| output.path.display().to_string()) + .map(|output| output.path.to_string()) .collect::>(); assert_that!( @@ -740,7 +735,7 @@ mod tests { let file_outputs = render_crates_for_test(vec![dummy_library_crate()], vec![]); let file_names = file_outputs .iter() - .map(|output| output.path.display().to_string()) + .map(|output| output.path.to_string()) .collect::>(); assert_that!( @@ -764,7 +759,7 @@ mod tests { ); let file_names = file_outputs .iter() - .map(|output| output.path.display().to_string()) + .map(|output| output.path.to_string()) .collect::>(); assert_that!( @@ -785,7 +780,7 @@ mod tests { context.is_workspace_member_dependency = true; context .workspace_member_dependents - .push(PathBuf::from("some/crate")); + .push(Utf8PathBuf::from("some/crate")); let aliases = vec![DependencyAlias { target: "target".to_string(), @@ -915,7 +910,7 @@ mod tests { ) .unwrap(); - let additional_build_file = tmp_dir.as_ref().join("some_additional_build_file"); + let additional_build_file = utf8_path(tmp_dir.as_ref()).join("some_additional_build_file"); let file_outputs = render_crates_for_test( vec![CrateContext { @@ -936,7 +931,7 @@ mod tests { expect( crate_build_contents.contains(&format!( "# Additional content from {}", - additional_build_file.display() + additional_build_file )), format!( "expected crate build contents to include additional_build_file, but it just contained \ @@ -959,7 +954,7 @@ mod tests { // Ensure that the lockfiles for binary dependencies get written out properly assert!(render_result.iter().any(|file_output| { - file_output.path == PathBuf::from("/some/bazel/root/./some_render_prefix/Cargo.raze.lock") + file_output.path == Utf8PathBuf::from("/some/bazel/root/./some_render_prefix/Cargo.raze.lock") && file_output.contents == indoc::formatdoc! { r#" # This file is automatically @generated by Cargo. @@ -977,16 +972,16 @@ mod tests { let mut render_details = dummy_render_details("BUILD.bazel"); render_details.experimental_api = true; render_details.render_package_aliases = false; - render_details.bazel_root = PathBuf::from("/tmp/workspace"); - render_details.cargo_root = PathBuf::from("/tmp/workspace"); - render_details.path_prefix = PathBuf::from("cargo"); + render_details.bazel_root = Utf8PathBuf::from("/tmp/workspace"); + render_details.cargo_root = Utf8PathBuf::from("/tmp/workspace"); + render_details.path_prefix = Utf8PathBuf::from("cargo"); // Setup planned build let mut lib_a = dummy_library_crate(); - lib_a.workspace_member_dependents = vec![PathBuf::from("lib_a")]; + lib_a.workspace_member_dependents = vec![Utf8PathBuf::from("lib_a")]; let mut lib_b = dummy_library_crate(); - lib_b.workspace_member_dependents = vec![PathBuf::from("lib_b")]; + lib_b.workspace_member_dependents = vec![Utf8PathBuf::from("lib_b")]; let mut planned_build = dummy_planned_build(vec![lib_a, lib_b], vec![]); @@ -995,7 +990,7 @@ mod tests { workspace_path: "//cargo".to_owned(), gen_workspace_prefix: "raze".to_owned(), output_buildfile_suffix: "BUILD.bazel".to_owned(), - workspace_members: vec![PathBuf::from("lib_a"), PathBuf::from("lib_b")], + workspace_members: vec![Utf8PathBuf::from("lib_a"), Utf8PathBuf::from("lib_b")], }; let file_outputs = BazelRenderer::new() diff --git a/impl/src/settings.rs b/impl/src/settings.rs index 77f88f3ad..697571cb8 100644 --- a/impl/src/settings.rs +++ b/impl/src/settings.rs @@ -18,13 +18,13 @@ use crate::{ util, }; use anyhow::{anyhow, bail, Context, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use cargo_metadata::{Metadata, MetadataCommand, Package}; use semver::VersionReq; use serde::{Deserialize, Serialize}; use std::{ collections::{BTreeMap, HashMap, HashSet}, hash::Hash, - path::{Path, PathBuf}, }; pub type CrateSettingsPerVersion = HashMap; @@ -247,7 +247,7 @@ pub struct CrateSettings { /// Note: This field should be a path to a file relative to the Cargo workspace root. For more /// context, see https://doc.rust-lang.org/cargo/reference/workspaces.html#root-package #[serde(default)] - pub additional_build_file: Option, + pub additional_build_file: Option, } /// Describes how dependencies should be managed in tree. @@ -353,15 +353,15 @@ pub fn format_registry_url(registry_url: &str, name: &str, version: &str) -> Str /// Check that the the `additional_build_file` represents a path to a file from the cargo workspace root fn validate_crate_setting_additional_build_file( - additional_build_file: &Path, - cargo_workspace_root: &Path, + additional_build_file: &Utf8Path, + cargo_workspace_root: &Utf8Path, ) -> Result<()> { let additional_build_file = cargo_workspace_root.join(&additional_build_file); if !additional_build_file.exists() { return Err(anyhow!( "File not found. `{}` should be a relative path from the cargo workspace root: {}", - additional_build_file.display(), - cargo_workspace_root.display() + additional_build_file, + cargo_workspace_root )); } @@ -371,7 +371,7 @@ fn validate_crate_setting_additional_build_file( /// Ensures crate settings associatd with the parsed [RazeSettings](crate::settings::RazeSettings) have valid crate settings fn validate_crate_settings( settings: &RazeSettings, - cargo_workspace_root: &Path, + cargo_workspace_root: &Utf8Path, ) -> Result<(), RazeError> { let mut errors = Vec::new(); @@ -413,7 +413,7 @@ fn validate_crate_settings( /// Verifies that the provided settings make sense. fn validate_settings( settings: &mut RazeSettings, - cargo_workspace_path: &Path, + cargo_workspace_path: &Utf8Path, ) -> Result<(), RazeError> { if !settings.workspace_path.starts_with("//") { return Err(RazeError::Config { @@ -698,7 +698,7 @@ fn parse_raze_settings(metadata: &Metadata) -> Result { /// A cargo command wrapper for gathering cargo metadata used to parse [RazeSettings](crate::settings::RazeSettings) pub struct SettingsMetadataFetcher { - pub cargo_bin_path: PathBuf, + pub cargo_bin_path: Utf8PathBuf, } impl Default for SettingsMetadataFetcher { @@ -710,32 +710,31 @@ impl Default for SettingsMetadataFetcher { } impl MetadataFetcher for SettingsMetadataFetcher { - fn fetch_metadata(&self, working_dir: &Path, _include_deps: bool) -> Result { + fn fetch_metadata(&self, working_dir: &Utf8Path, _include_deps: bool) -> Result { // This fetch does not require network access. MetadataCommand::new() .cargo_path(&self.cargo_bin_path) .no_deps() - .current_dir(working_dir) + .current_dir(working_dir.as_std_path()) .other_options(vec!["--offline".to_owned()]) .exec() .with_context(|| { format!( "Failed to fetch Metadata with `{}` from `{}`", - &self.cargo_bin_path.display(), - working_dir.display() + &self.cargo_bin_path, working_dir ) }) } } /// Load settings from a given Cargo manifest -pub fn load_settings_from_manifest>( +pub fn load_settings_from_manifest>( cargo_toml_path: T, cargo_bin_path: Option, ) -> Result { // Get the path to the cargo binary from either an optional Cargo binary // path or a fallback expected to be found on the system. - let bin_path: PathBuf = if let Some(path) = cargo_bin_path { + let bin_path: Utf8PathBuf = if let Some(path) = cargo_bin_path { path.into() } else { util::cargo_bin_path() @@ -749,7 +748,7 @@ pub fn load_settings_from_manifest>( let cargo_toml_dir = cargo_toml_path.as_ref().parent().ok_or_else(|| { RazeError::Generic(format!( "Failed to find parent directory for cargo toml file: {:?}", - cargo_toml_path.as_ref().display(), + cargo_toml_path.as_ref(), )) })?; let metadata = { @@ -822,7 +821,8 @@ pub mod tests { 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"); + let cargo_toml_path = + Utf8PathBuf::from_path_buf(temp_workspace_dir.path().join("Cargo.toml")).unwrap(); std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); assert!(load_settings_from_manifest(cargo_toml_path, None).is_err()); @@ -857,7 +857,8 @@ pub mod tests { 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"); + let cargo_toml_path = + Utf8PathBuf::from_path_buf(temp_workspace_dir.path().join("Cargo.toml")).unwrap(); std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); let settings = load_settings_from_manifest(cargo_toml_path, None).unwrap(); @@ -893,7 +894,8 @@ pub mod tests { 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"); + let cargo_toml_path = + Utf8PathBuf::from_path_buf(temp_workspace_dir.path().join("Cargo.toml")).unwrap(); std::fs::write(&cargo_toml_path, &toml_contents).unwrap(); let settings = load_settings_from_manifest(cargo_toml_path, /*cargo_bin_path=*/ None).unwrap(); @@ -929,7 +931,11 @@ pub mod tests { std::fs::write(crate_toml, toml_contents).unwrap(); } - let settings = load_settings_from_manifest(dir.as_ref().join("Cargo.toml"), None).unwrap(); + let settings = load_settings_from_manifest( + Utf8PathBuf::from_path_buf(dir.as_ref().join("Cargo.toml")).unwrap(), + None, + ) + .unwrap(); assert_eq!(&settings.workspace_path, "//workspace_path/raze"); assert_eq!(settings.genmode, GenMode::Remote); assert_eq!(settings.crates.len(), 2); diff --git a/impl/src/testing.rs b/impl/src/testing.rs index 440436a23..81fa1c5dc 100644 --- a/impl/src/testing.rs +++ b/impl/src/testing.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use camino::Utf8Path; use cargo_metadata::Metadata; use flate2::Compression; use httpmock::{Method::GET, MockRef, MockServer}; @@ -23,7 +24,6 @@ use std::{ collections::HashMap, fs::{create_dir_all, write, File}, io::Write, - path::Path, }; use crate::{ @@ -273,14 +273,14 @@ pub fn to_index_crates_map(list: Vec<(&str, &str)>) -> HashMap { /// Create a mock cache in a temporary directory that contains a set of given crates pub fn mock_crate_index( crates: &HashMap, - mock_dir: Option<&Path>, + mock_dir: Option<&Utf8Path>, ) -> Option { let index_url_mock_dir = TempDir::new().unwrap(); // If an existing directory is provided, use that instead let index_dir = match mock_dir { Some(dir) => dir, - None => index_url_mock_dir.as_ref(), + None => utf8_path(index_url_mock_dir.path()), }; for (name, version) in crates { @@ -328,10 +328,17 @@ pub fn template_raze_metadata(template_path: &str) -> RazeMetadata { metadata_template: Some(template_path.to_string()), })); - fetcher.fetch_metadata(dir.as_ref(), None, None).unwrap() + fetcher + .fetch_metadata(utf8_path(dir.as_ref()), None, None) + .unwrap() } /// Load a cargo metadata template pub fn template_metadata(template_path: &str) -> Metadata { template_raze_metadata(template_path).metadata } + +/// Coerce a path to UTF-* +pub fn utf8_path(path: &std::path::Path) -> &Utf8Path { + Utf8Path::from_path(path.as_ref()).unwrap() +} diff --git a/impl/src/util.rs b/impl/src/util.rs index 73ccc1971..e9e327dc8 100644 --- a/impl/src/util.rs +++ b/impl/src/util.rs @@ -13,11 +13,9 @@ // limitations under the License. use anyhow::Result; -use std::{ - collections::HashSet, env, fmt, iter::Iterator, path::Path, path::PathBuf, process::Command, - str::FromStr, -}; +use std::{collections::HashSet, env, fmt, iter::Iterator, process::Command, str::FromStr}; +use camino::{Utf8Path, Utf8PathBuf}; use cargo_platform::Cfg; use cfg_expr::{targets::get_builtin_target_by_triple, Expression, Predicate}; @@ -166,14 +164,14 @@ pub fn get_matching_bazel_triples<'a>( } /// Returns whether or not the given path is a Bazel workspace root -pub fn is_bazel_workspace_root(dir: &Path) -> bool { +pub fn is_bazel_workspace_root(dir: &Utf8Path) -> bool { let workspace_files = [dir.join("WORKSPACE.bazel"), dir.join("WORKSPACE")]; workspace_files.iter().any(|x| x.exists()) } /// Returns a path to a Bazel workspace root based on the current working /// directory, otherwise None if not workspace is detected. -pub fn find_bazel_workspace_root(manifest_path: &Path) -> Option { +pub fn find_bazel_workspace_root(manifest_path: &Utf8Path) -> Option { let mut dir = if manifest_path.is_dir() { Some(manifest_path) } else { @@ -182,7 +180,7 @@ pub fn find_bazel_workspace_root(manifest_path: &Path) -> Option { while let Some(current_dir) = dir { if is_bazel_workspace_root(current_dir) { - return Some(PathBuf::from(current_dir)); + return Some(Utf8PathBuf::from(current_dir)); } dir = current_dir.parent(); @@ -290,10 +288,13 @@ fn fetch_attrs(target: &str) -> Result> { ) } -pub fn get_workspace_member_path(manifest_path: &Path, workspace_root: &Path) -> Option { +pub fn get_workspace_member_path( + manifest_path: &Utf8Path, + workspace_root: &Utf8Path, +) -> Option { assert!(manifest_path.ends_with("Cargo.toml")); // UNWRAP: A manifest path should always be a path to a 'Cargo.toml' file which should always have a parent directory - diff_paths(manifest_path.parent().unwrap(), workspace_root) + Utf8PathBuf::from_path_buf(diff_paths(manifest_path.parent().unwrap(), workspace_root)?).ok() } pub fn package_ident(package_name: &str, package_version: &str) -> String { @@ -303,7 +304,10 @@ pub fn package_ident(package_name: &str, package_version: &str) -> String { /// Locates a lockfile for the associated crate. A `Cargo.raze.lock` file in the /// [RazeSettings::workspace_path](crate::settings::RazeSettings::workspace_path) /// directory will take precidence over a standard `Cargo.lock` file. -pub fn find_lockfile(cargo_workspace_root: &Path, raze_output_dir: &Path) -> Option { +pub fn find_lockfile( + cargo_workspace_root: &Utf8Path, + raze_output_dir: &Utf8Path, +) -> Option { // The custom raze lockfile will always take precidence let raze_lockfile = raze_output_dir.join(RAZE_LOCKFILE_NAME); if raze_lockfile.exists() { @@ -321,8 +325,8 @@ pub fn find_lockfile(cargo_workspace_root: &Path, raze_output_dir: &Path) -> Opt } /// Locates a cargo binary form either an evironment variable or PATH -pub fn cargo_bin_path() -> PathBuf { - PathBuf::from(env::var("CARGO").unwrap_or_else(|_| SYSTEM_CARGO_BIN_PATH.to_string())) +pub fn cargo_bin_path() -> Utf8PathBuf { + Utf8PathBuf::from(env::var("CARGO").unwrap_or_else(|_| SYSTEM_CARGO_BIN_PATH.to_string())) } #[cfg(test)] @@ -360,7 +364,7 @@ mod tests { #[test] fn detecting_workspace_root() { let bazel_root = TempDir::new().unwrap(); - let manifest = bazel_root.as_ref().join("Cargo.toml"); + let manifest = Utf8PathBuf::from_path_buf(bazel_root.as_ref().join("Cargo.toml")).unwrap(); // Starting within the temp directory, we'll find that there are no WORKSPACE.bazel files // and thus return None to indicate a Bazel workspace root could not be found. diff --git a/third_party/cargo/crates.bzl b/third_party/cargo/crates.bzl index 745ba3706..9cb209f23 100644 --- a/third_party/cargo/crates.bzl +++ b/third_party/cargo/crates.bzl @@ -13,6 +13,7 @@ load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") # buildifier: di _DEPENDENCIES = { "impl": { "anyhow": "@cargo_raze__anyhow__1_0_40//:anyhow", + "camino": "@cargo_raze__camino__1_0_9//:camino", "cargo-clone-crate": "@cargo_raze__cargo_clone_crate__0_1_6//:cargo_clone_crate", "cargo-lock": "@cargo_raze__cargo_lock__7_0_1//:cargo_lock", "cargo-platform": "@cargo_raze__cargo_platform__0_1_1//:cargo_platform", @@ -562,12 +563,12 @@ def cargo_raze_fetch_remote_crates(): maybe( http_archive, - name = "cargo_raze__camino__1_0_4", - url = "https://crates.io/api/v1/crates/camino/1.0.4/download", + name = "cargo_raze__camino__1_0_9", + url = "https://crates.io/api/v1/crates/camino/1.0.9/download", type = "tar.gz", - sha256 = "d4648c6d00a709aa069a236adcaae4f605a6241c72bf5bee79331a4b625921a9", - strip_prefix = "camino-1.0.4", - build_file = Label("//third_party/cargo/remote:BUILD.camino-1.0.4.bazel"), + sha256 = "869119e97797867fd90f5e22af7d0bd274bd4635ebb9eb68c04f3f513ae6c412", + strip_prefix = "camino-1.0.9", + build_file = Label("//third_party/cargo/remote:BUILD.camino-1.0.9.bazel"), ) maybe( diff --git a/third_party/cargo/remote/BUILD.camino-1.0.4.bazel b/third_party/cargo/remote/BUILD.camino-1.0.9.bazel similarity index 91% rename from third_party/cargo/remote/BUILD.camino-1.0.4.bazel rename to third_party/cargo/remote/BUILD.camino-1.0.9.bazel index bc1974a51..51bb432ac 100644 --- a/third_party/cargo/remote/BUILD.camino-1.0.4.bazel +++ b/third_party/cargo/remote/BUILD.camino-1.0.9.bazel @@ -56,16 +56,12 @@ cargo_build_script( "cargo-raze", "manual", ], - version = "1.0.4", + version = "1.0.9", visibility = ["//visibility:private"], deps = [ ], ) -# Unsupported target "serde" with type "example" omitted - -# Unsupported target "structopt" with type "example" omitted - rust_library( name = "camino", srcs = glob(["**/*.rs"]), @@ -84,7 +80,7 @@ rust_library( "crate-name=camino", "manual", ], - version = "1.0.4", + version = "1.0.9", # buildifier: leave-alone deps = [ ":camino_build_script", diff --git a/third_party/cargo/remote/BUILD.cargo_metadata-0.14.0.bazel b/third_party/cargo/remote/BUILD.cargo_metadata-0.14.0.bazel index 0ba57b61a..2ba701337 100644 --- a/third_party/cargo/remote/BUILD.cargo_metadata-0.14.0.bazel +++ b/third_party/cargo/remote/BUILD.cargo_metadata-0.14.0.bazel @@ -51,7 +51,7 @@ rust_library( version = "0.14.0", # buildifier: leave-alone deps = [ - "@cargo_raze__camino__1_0_4//:camino", + "@cargo_raze__camino__1_0_9//:camino", "@cargo_raze__cargo_platform__0_1_1//:cargo_platform", "@cargo_raze__semver__1_0_3//:semver", "@cargo_raze__serde__1_0_126//:serde",