diff --git a/Cargo.lock b/Cargo.lock index 608346e82bbd4..9e399a48d22da 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -669,6 +669,15 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4964518bd3b4a8190e832886cdc0da9794f12e8e6c1613a9e90ff331c4c8724b" +[[package]] +name = "camino" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "176c40258709f5883ce0f352f30b2065c64112843b33e3e318e1c1cc22960979" +dependencies = [ + "serde", +] + [[package]] name = "cargo-platform" version = "0.1.1" @@ -726,9 +735,9 @@ dependencies = [ [[package]] name = "cfg-expr" -version = "0.6.0" +version = "0.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb4f9cf6cb58661f5cdcda0240ab42788e009bd957ba56c1367aa01c7c6fbc05" +checksum = "2b81b9640fc656040fbf0d3f2bafacadcf17d55f2b0dc89589c1b80b0658fd5a" dependencies = [ "smallvec", ] @@ -1382,11 +1391,11 @@ dependencies = [ [[package]] name = "determinator" -version = "0.2.1" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f862ffb07525089b200b5bd75d2be5a35eebb529cf1118511827a3929358b21" +checksum = "33ec6ed6a28572d35f1c9d512fe0381b6bd83bf45e273b60b9c1df883234645c" dependencies = [ - "cfg-if 1.0.0", + "camino", "globset", "guppy", "itertools 0.10.0", @@ -3317,10 +3326,11 @@ dependencies = [ [[package]] name = "guppy" -version = "0.7.2" +version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf1c6ce38a18b386305494e4b38a137b2c1a48f368cce1e65e8efbf4dbeeb6f8" +checksum = "ec4cbddce827d660864974dc3dc62a1c810e813d17e518cc5830716b977cd110" dependencies = [ + "camino", "cargo_metadata", "fixedbitset", "guppy-summaries", @@ -3340,10 +3350,11 @@ dependencies = [ [[package]] name = "guppy-summaries" -version = "0.3.2" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "811b7332020d2a53cfe64049a52fd46d0761082b12d8b6b8f2c5b702f8bb2a24" +checksum = "c526b51262ef5ed4de421ee6679bf0927a20310d5cc048abe89464b710a2a299" dependencies = [ + "camino", "cfg-if 1.0.0", "diffus", "semver 0.11.0", @@ -3373,11 +3384,12 @@ dependencies = [ [[package]] name = "hakari" -version = "0.1.1" +version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "717795bc5eb8ef223c45564609318cb5af8b13e285bb5b033c67232861fda383" +checksum = "136693d8798c61025d90e5c1f588eeec33e52d0e8d42f8a24ac2d79c590757ca" dependencies = [ "atomicwrites", + "camino", "cfg-if 1.0.0", "diffy", "guppy", @@ -6996,9 +7008,9 @@ checksum = "36474e732d1affd3a6ed582781b3683df3d0563714c59c39591e8ff707cf078e" [[package]] name = "target-spec" -version = "0.6.1" +version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "39920b3286e8e9585b3ba77becb3e682888f63b2be5326cd16264d8ad292d78e" +checksum = "ac395241acf0f3d224dd2b07b334994adf6f37c904d1826867332ab8c6ddedbf" dependencies = [ "cfg-expr", "serde", @@ -8052,6 +8064,7 @@ name = "x" version = "0.1.0" dependencies = [ "anyhow", + "camino", "chrono", "colored-diff", "determinator", @@ -8076,6 +8089,7 @@ dependencies = [ name = "x-core" version = "0.1.0" dependencies = [ + "camino", "determinator", "guppy", "hakari", @@ -8092,6 +8106,7 @@ dependencies = [ name = "x-lint" version = "0.1.0" dependencies = [ + "camino", "guppy", "hakari", "once_cell", diff --git a/devtools/x-core/Cargo.toml b/devtools/x-core/Cargo.toml index 71b7382259a1b..53ba181fd6c0a 100644 --- a/devtools/x-core/Cargo.toml +++ b/devtools/x-core/Cargo.toml @@ -8,10 +8,11 @@ publish = false license = "Apache-2.0" [dependencies] -determinator = "0.2.1" -guppy = "0.7.2" +camino = { version = "1.0.0", features = ["serde1"] } +determinator = "0.3.0" +guppy = "0.8.0" indoc = "1.0.3" -hakari = { version = "0.1.1", features = ["summaries"] } +hakari = { version = "0.2.0", features = ["summaries"] } hex = "0.4.2" log = "0.4.14" toml = "0.5.8" diff --git a/devtools/x-core/src/errors.rs b/devtools/x-core/src/errors.rs index 8a82374f2905f..71a94d16011bc 100644 --- a/devtools/x-core/src/errors.rs +++ b/devtools/x-core/src/errors.rs @@ -10,6 +10,7 @@ use std::{ path::{Path, PathBuf}, process::ExitStatus, result, + str::Utf8Error, }; /// Type alias for the return type for `run` methods. @@ -48,6 +49,10 @@ pub enum SystemError { context: Cow<'static, str>, err: io::Error, }, + NonUtf8Path { + path: Vec, + err: Utf8Error, + }, Serde { context: Cow<'static, str>, err: Box, @@ -149,6 +154,9 @@ impl fmt::Display for SystemError { None => write!(f, "'{}' terminated by signal", cmd), }, SystemError::GitRoot(s) => write!(f, "git root error: {}", s), + SystemError::NonUtf8Path { path, .. } => { + write!(f, "non-UTF-8 path \"{}\"", String::from_utf8_lossy(path)) + } SystemError::FromHex { context, .. } | SystemError::Io { context, .. } | SystemError::Serde { context, .. } @@ -171,6 +179,7 @@ impl error::Error for SystemError { SystemError::Guppy { err, .. } => Some(err), SystemError::HakariCargoToml { err, .. } => Some(err), SystemError::HakariTomlOut { err, .. } => Some(err), + SystemError::NonUtf8Path { err, .. } => Some(err), SystemError::TargetSpec { err, .. } => Some(err), SystemError::Serde { err, .. } => Some(err.as_ref()), } diff --git a/devtools/x-core/src/git.rs b/devtools/x-core/src/git.rs index e7f5580604875..eae2aa5101ed3 100644 --- a/devtools/x-core/src/git.rs +++ b/devtools/x-core/src/git.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::errors::*; -use determinator::Paths0; +use determinator::Utf8Paths0; use guppy::{graph::PackageGraph, MetadataCommand}; use indoc::formatdoc; use log::{debug, info}; @@ -24,7 +24,7 @@ use std::{ pub struct GitCli { root: &'static Path, // Caches. - tracked_files: OnceCell, + tracked_files: OnceCell, } impl GitCli { @@ -41,7 +41,7 @@ impl GitCli { /// Returns the files tracked by Git in this working copy. /// /// The return value can be iterated on to get a list of paths. - pub fn tracked_files(&self) -> Result<&Paths0> { + pub fn tracked_files(&self) -> Result<&Utf8Paths0> { self.tracked_files.get_or_try_init(|| { // TODO: abstract out SCM and command-running functionality. let output = self @@ -57,8 +57,8 @@ impl GitCli { }); } - // TODO: Get this working on Windows. - Ok(Paths0::new_unix(output.stdout)) + Utf8Paths0::from_bytes(output.stdout) + .map_err(|(path, err)| SystemError::NonUtf8Path { path, err }) }) } @@ -93,7 +93,7 @@ impl GitCli { new: impl Into>>, // TODO: make this more well-typed/express more of the diff model in Rust diff_filter: Option<&str>, - ) -> Result { + ) -> Result { let mut command = self.git_command(); command.args(&["diff", "-z", "--name-only"]); if let Some(diff_filter) = diff_filter { @@ -114,7 +114,8 @@ impl GitCli { }); } - Ok(Paths0::new_unix(output.stdout)) + Utf8Paths0::from_bytes(output.stdout) + .map_err(|(path, err)| SystemError::NonUtf8Path { path, err }) } /// Returns a package graph for the given commit, using a scratch repo if necessary. diff --git a/devtools/x-core/src/workspace_subset.rs b/devtools/x-core/src/workspace_subset.rs index e25edd1f33e26..2c9722fc75e95 100644 --- a/devtools/x-core/src/workspace_subset.rs +++ b/devtools/x-core/src/workspace_subset.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{core_config::SubsetConfig, Result, SystemError}; +use camino::Utf8PathBuf; use guppy::{ graph::{ cargo::{CargoOptions, CargoResolverVersion, CargoSet}, @@ -11,11 +12,7 @@ use guppy::{ PackageId, }; use serde::Deserialize; -use std::{ - collections::BTreeMap, - fs, - path::{Path, PathBuf}, -}; +use std::{collections::BTreeMap, fs, path::Path}; use toml::de; /// Contains information about all the subsets specified in this workspace. @@ -94,7 +91,7 @@ impl<'g> WorkspaceSubsets<'g> { // Helper methods // --- - fn read_default_members(project_root: &Path) -> Result> { + fn read_default_members(project_root: &Path) -> Result> { #[derive(Deserialize)] struct RootToml { workspace: Workspace, @@ -103,7 +100,7 @@ impl<'g> WorkspaceSubsets<'g> { #[derive(Deserialize)] struct Workspace { #[serde(rename = "default-members")] - default_members: Vec, + default_members: Vec, } let root_toml = project_root.join("Cargo.toml"); diff --git a/devtools/x-lint/Cargo.toml b/devtools/x-lint/Cargo.toml index 52565bc18c8b8..d16dfaa6d6311 100644 --- a/devtools/x-lint/Cargo.toml +++ b/devtools/x-lint/Cargo.toml @@ -8,8 +8,9 @@ publish = false license = "Apache-2.0" [dependencies] -guppy = "0.7.2" -hakari = "0.1.1" +camino = "1.0.0" +guppy = "0.8.0" +hakari = "0.2.0" once_cell = "1.6.0" toml = "0.5.8" serde = { version = "1.0.123", features = ["derive"] } diff --git a/devtools/x-lint/src/file_path.rs b/devtools/x-lint/src/file_path.rs index 690b0162e7162..ba5b0c486c555 100644 --- a/devtools/x-lint/src/file_path.rs +++ b/devtools/x-lint/src/file_path.rs @@ -2,7 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{prelude::*, LintContext}; -use std::{ffi::OsStr, fs, io, path::Path}; +use camino::Utf8Path; +use std::{fs, io, path::Path}; /// Represents a linter that runs once per file path. pub trait FilePathLinter: Linter { @@ -18,12 +19,12 @@ pub trait FilePathLinter: Linter { #[derive(Clone, Debug)] pub struct FilePathContext<'l> { project_ctx: &'l ProjectContext<'l>, - file_path: &'l Path, + file_path: &'l Utf8Path, } impl<'l> FilePathContext<'l> { /// Constructs a new context. - pub fn new(project_ctx: &'l ProjectContext<'l>, file_path: &'l Path) -> Self { + pub fn new(project_ctx: &'l ProjectContext<'l>, file_path: &'l Utf8Path) -> Self { Self { project_ctx, file_path, @@ -36,14 +37,13 @@ impl<'l> FilePathContext<'l> { } /// Returns the path of this file, relative to the root of the repository. - pub fn file_path(&self) -> &'l Path { + pub fn file_path(&self) -> &'l Utf8Path { &self.file_path } - /// Returns the extension of the file. Returns `None` if there's no extension or if the - /// extension isn't valid UTF-8. + /// Returns the extension of the file. Returns `None` if there's no extension. pub fn extension(&self) -> Option<&'l str> { - self.file_path.extension().map(OsStr::to_str).flatten() + self.file_path.extension() } /// Loads this file and turns it into a `ContentContext`. diff --git a/devtools/x-lint/src/lib.rs b/devtools/x-lint/src/lib.rs index 91102b2ff52db..dcf5f5ad852de 100644 --- a/devtools/x-lint/src/lib.rs +++ b/devtools/x-lint/src/lib.rs @@ -12,8 +12,9 @@ pub mod package; pub mod project; pub mod runner; +use camino::Utf8Path; use guppy::PackageId; -use std::{borrow::Cow, fmt, path::Path}; +use std::{borrow::Cow, fmt}; /// Represents a linter. pub trait Linter: Send + Sync + fmt::Debug { @@ -81,12 +82,12 @@ pub enum RunStatus<'l> { #[derive(Clone, Debug, Eq, PartialEq)] #[non_exhaustive] pub enum SkipReason<'l> { - /// This file was not valid UTF-8. - NonUtf8, + /// This file's content was not valid UTF-8. + NonUtf8Content, /// This extension was unsupported. UnsupportedExtension(Option<&'l str>), /// The given file was unsupported by this linter. - UnsupportedFile(&'l Path), + UnsupportedFile(&'l Utf8Path), /// The given package was unsupported by this linter. UnsupportedPackage(&'l PackageId), /// The given file was excepted by a glob rule @@ -162,10 +163,10 @@ pub enum LintKind<'l> { Project, Package { name: &'l str, - workspace_path: &'l Path, + workspace_path: &'l Utf8Path, }, - FilePath(&'l Path), - Content(&'l Path), + FilePath(&'l Utf8Path), + Content(&'l Utf8Path), } impl<'l> fmt::Display for LintKind<'l> { @@ -175,9 +176,9 @@ impl<'l> fmt::Display for LintKind<'l> { LintKind::Package { name, workspace_path, - } => write!(f, "package '{}' (at {})", name, workspace_path.display()), - LintKind::FilePath(path) => write!(f, "file path {}", path.display()), - LintKind::Content(path) => write!(f, "content {}", path.display()), + } => write!(f, "package '{}' (at {})", name, workspace_path), + LintKind::FilePath(path) => write!(f, "file path {}", path), + LintKind::Content(path) => write!(f, "content {}", path), } } } diff --git a/devtools/x-lint/src/package.rs b/devtools/x-lint/src/package.rs index 530779a7eb35d..e812b9e86bd8c 100644 --- a/devtools/x-lint/src/package.rs +++ b/devtools/x-lint/src/package.rs @@ -2,8 +2,8 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{prelude::*, LintContext}; +use camino::Utf8Path; use guppy::graph::{PackageGraph, PackageMetadata}; -use std::path::Path; use x_core::WorkspaceStatus; /// Represents a linter that runs once per package. @@ -22,7 +22,7 @@ pub struct PackageContext<'l> { // PackageContext requires the package graph to be computed and available, though ProjectContext // does not. package_graph: &'l PackageGraph, - workspace_path: &'l Path, + workspace_path: &'l Utf8Path, metadata: PackageMetadata<'l>, is_default_member: bool, } @@ -31,7 +31,7 @@ impl<'l> PackageContext<'l> { pub fn new( project_ctx: &'l ProjectContext<'l>, package_graph: &'l PackageGraph, - workspace_path: &'l Path, + workspace_path: &'l Utf8Path, metadata: PackageMetadata<'l>, ) -> Result { let default_members = project_ctx.default_members()?; @@ -55,7 +55,7 @@ impl<'l> PackageContext<'l> { } /// Returns the relative path for this package in the workspace. - pub fn workspace_path(&self) -> &'l Path { + pub fn workspace_path(&self) -> &'l Utf8Path { self.workspace_path } diff --git a/devtools/x-lint/src/runner.rs b/devtools/x-lint/src/runner.rs index 9eddc41b25a2e..672dfbf1b3ee2 100644 --- a/devtools/x-lint/src/runner.rs +++ b/devtools/x-lint/src/runner.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::{prelude::*, LintContext}; -use std::path::Path; +use camino::Utf8Path; use x_core::XCoreContext; /// Configuration for the lint engine. @@ -238,7 +238,7 @@ impl<'cfg> LintEngine<'cfg> { // Helper methods // --- - fn file_list(&self) -> Result + 'cfg> { + fn file_list(&self) -> Result + 'cfg> { let tracked_files = self.config.core.git_cli().tracked_files()?; // TODO: make global exclusions configurable Ok(tracked_files diff --git a/devtools/x/Cargo.toml b/devtools/x/Cargo.toml index d01bcf2ac0908..00b054059a79e 100644 --- a/devtools/x/Cargo.toml +++ b/devtools/x/Cargo.toml @@ -8,14 +8,15 @@ publish = false license = "Apache-2.0" [dependencies] -determinator = "0.2.1" +camino = "1.0.0" +determinator = "0.3.0" serde = { version = "1.0.123", features = ["derive"] } serde_json = "1.0.62" structopt = "0.3.21" anyhow = "1.0.38" colored-diff = "0.2.2" -guppy = { version = "0.7.2", features = ["summaries"] } -hakari = "0.1.1" +guppy = { version = "0.8.0", features = ["summaries"] } +hakari = "0.2.0" indoc = "1.0.3" toml = "0.5.8" env_logger = "0.8.3" diff --git a/devtools/x/src/lint/allowed_paths.rs b/devtools/x/src/lint/allowed_paths.rs index 17361f3a2e459..2fec09529fe58 100644 --- a/devtools/x/src/lint/allowed_paths.rs +++ b/devtools/x/src/lint/allowed_paths.rs @@ -32,14 +32,7 @@ impl FilePathLinter for AllowedPaths { ctx: &FilePathContext<'l>, out: &mut LintFormatter<'l, '_>, ) -> Result> { - let file_path = match ctx.file_path().to_str() { - Some(file_path) => file_path, - None => { - out.write(LintLevel::Error, "path isn't valid Unicode"); - return Ok(RunStatus::Executed); - } - }; - if !self.allowed_regex.is_match(file_path) { + if !self.allowed_regex.is_match(ctx.file_path().as_str()) { out.write( LintLevel::Error, format!( diff --git a/devtools/x/src/lint/guppy.rs b/devtools/x/src/lint/guppy.rs index 8a304c3e5a822..5671a885bc424 100644 --- a/devtools/x/src/lint/guppy.rs +++ b/devtools/x/src/lint/guppy.rs @@ -158,15 +158,11 @@ impl PackageLinter for CrateNamesPaths { } let workspace_path = ctx.workspace_path(); - if let Some(path) = workspace_path.to_str() { - if path.contains('_') { - out.write( - LintLevel::Error, - "workspace path contains '_' (use '-' instead)", - ); - } - } else { - // Workspace path is invalid UTF-8. A different lint should catch this. + if workspace_path.as_str().contains('_') { + out.write( + LintLevel::Error, + "workspace path contains '_' (use '-' instead)", + ); } for build_target in ctx.metadata().build_targets() { diff --git a/devtools/x/src/lint/license.rs b/devtools/x/src/lint/license.rs index 869f7a1fe803c..3de932ffb1f79 100644 --- a/devtools/x/src/lint/license.rs +++ b/devtools/x/src/lint/license.rs @@ -37,7 +37,7 @@ impl ContentLinter for LicenseHeader { Some(content) => content, None => { // This is not a UTF-8 file -- don't analyze it. - return Ok(RunStatus::Skipped(SkipReason::NonUtf8)); + return Ok(RunStatus::Skipped(SkipReason::NonUtf8Content)); } }; diff --git a/devtools/x/src/lint/toml.rs b/devtools/x/src/lint/toml.rs index a9d7ba27fbbc1..6a11c5f078316 100644 --- a/devtools/x/src/lint/toml.rs +++ b/devtools/x/src/lint/toml.rs @@ -1,9 +1,9 @@ // Copyright (c) The Diem Core Contributors // SPDX-License-Identifier: Apache-2.0 +use camino::Utf8Path; use colored_diff::PrettyDifference; use serde::{Deserialize, Serialize}; -use std::path::Path; use toml::{de, ser}; use x_lint::prelude::*; @@ -20,7 +20,7 @@ impl Linter for RootToml { impl ContentLinter for RootToml { fn pre_run<'l>(&self, file_ctx: &FilePathContext<'l>) -> Result> { let file_path = file_ctx.file_path(); - if file_path == Path::new("Cargo.toml") { + if file_path == "Cargo.toml" { Ok(RunStatus::Executed) } else { Ok(RunStatus::Skipped(SkipReason::UnsupportedFile(file_path))) @@ -112,6 +112,6 @@ struct RootTomlContents<'a> { #[derive(Clone, Debug, Deserialize, Serialize)] struct Workspace<'a> { #[serde(borrow)] - members: Vec<&'a Path>, + members: Vec<&'a Utf8Path>, // Add other fields as necessary. } diff --git a/devtools/x/src/lint/whitespace.rs b/devtools/x/src/lint/whitespace.rs index ec45fb5b9d718..4c40bae42ec31 100644 --- a/devtools/x/src/lint/whitespace.rs +++ b/devtools/x/src/lint/whitespace.rs @@ -34,7 +34,7 @@ impl<'cfg> ContentLinter for EofNewline<'cfg> { ) -> Result> { let content = match ctx.content() { Some(text) => text, - None => return Ok(RunStatus::Skipped(SkipReason::NonUtf8)), + None => return Ok(RunStatus::Skipped(SkipReason::NonUtf8Content)), }; if !content.is_empty() && !content.ends_with('\n') { out.write(LintLevel::Error, "missing newline at EOF"); @@ -72,7 +72,7 @@ impl<'cfg> ContentLinter for TrailingWhitespace<'cfg> { ) -> Result> { let content = match ctx.content() { Some(text) => text, - None => return Ok(RunStatus::Skipped(SkipReason::NonUtf8)), + None => return Ok(RunStatus::Skipped(SkipReason::NonUtf8Content)), }; for (ln, line) in content.lines().enumerate().map(|(ln, line)| (ln + 1, line)) { diff --git a/devtools/x/src/lint/workspace_hack.rs b/devtools/x/src/lint/workspace_hack.rs index 6d915d663fdb5..de170fcf38bc1 100644 --- a/devtools/x/src/lint/workspace_hack.rs +++ b/devtools/x/src/lint/workspace_hack.rs @@ -54,8 +54,7 @@ impl ProjectLinter for GenerateWorkspaceHack { hakari_package .source() .workspace_path() - .expect("hakari package is in workspace") - .display(), + .expect("hakari package is in workspace"), diff ), );