Skip to content

Commit

Permalink
Fix all cases when std canonicalization was used instead of dunce (so…
Browse files Browse the repository at this point in the history
  • Loading branch information
mkaput authored Sep 20, 2023
1 parent e8da195 commit 5ff2b9c
Show file tree
Hide file tree
Showing 13 changed files with 58 additions and 95 deletions.
8 changes: 8 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
disallowed-methods = [
{ path = "std::fs::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
{ path = "std::path::Path::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
{ path = "camino::Utf8Path::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
{ path = "camino::Utf8Path::canonicalize_utf8", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
{ path = "camino::Utf8PathBuf::canonicalize", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
{ path = "camino::Utf8PathBuf::canonicalize_utf8", reason = "use fsx::canonicalize or fsx::canonicalize_utf8" },
]
25 changes: 8 additions & 17 deletions scarb/src/core/manifest/toml_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use crate::core::package::PackageId;
use crate::core::source::{GitReference, SourceId};
use crate::core::{DependencyVersionReq, ManifestBuilder, ManifestCompilerConfig, PackageName};
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;
use crate::internal::serdex::{toml_merge, RelativeUtf8PathBuf};
use crate::internal::to_version::ToVersion;
use crate::{DEFAULT_SOURCE_PATH, MANIFEST_FILE_NAME};
Expand Down Expand Up @@ -621,10 +620,8 @@ impl TomlManifest {
let source_path = target
.source_path
.clone()
.map(|p| root.as_std_path().to_path_buf().join(p))
.map(fsx::canonicalize)
.transpose()?
.map(|p| p.try_into_utf8())
.map(|p| root.join_os(p))
.map(fsx::canonicalize_utf8)
.transpose()?
.unwrap_or(default_source_path.to_path_buf());

Expand Down Expand Up @@ -767,18 +764,12 @@ pub fn readme_for_package(
fn abs_canonical_path(prefix: &Utf8Path, readme: Option<&Utf8Path>) -> Result<Option<Utf8PathBuf>> {
match readme {
None => Ok(None),
Some(readme) => prefix
.parent()
.unwrap()
.join(readme)
.canonicalize_utf8()
.map(Some)
.with_context(|| {
format!(
"failed to find the readme at {}",
prefix.parent().unwrap().join(readme)
)
}),
Some(readme) => {
let path = prefix.parent().unwrap().join(readme);
let path = fsx::canonicalize_utf8(&path)
.with_context(|| format!("failed to find the readme at {path}"))?;
Ok(Some(path))
}
}
}

Expand Down
7 changes: 7 additions & 0 deletions scarb/src/internal/fsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ use anyhow::{anyhow, Context, Result};
use camino::{Utf8Path, Utf8PathBuf};

/// Equivalent to [`fs::canonicalize`] with better error messages.
///
/// Uses [`dunce`] to generate more familiar paths on Windows.
pub fn canonicalize(p: impl AsRef<Path>) -> Result<PathBuf> {
return inner(p.as_ref());

Expand All @@ -17,6 +19,11 @@ pub fn canonicalize(p: impl AsRef<Path>) -> Result<PathBuf> {
}
}

/// Equivalent to [`fs::canonicalize`], but for Utf-8 paths, with better error messages.
pub fn canonicalize_utf8(p: impl AsRef<Path>) -> Result<Utf8PathBuf> {
canonicalize(p)?.try_into_utf8()
}

/// Equivalent to [`fs::create_dir_all`] with better error messages.
pub fn create_dir_all(p: impl AsRef<Path>) -> Result<()> {
return inner(p.as_ref());
Expand Down
3 changes: 1 addition & 2 deletions scarb/src/internal/serdex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use camino::{Utf8Path, Utf8PathBuf};
use serde::{Deserialize, Serialize};

use crate::internal::fsx;
use crate::internal::fsx::PathUtf8Ext;

/// Merge two `toml::Value` serializable structs.
pub fn toml_merge<'de, T, S>(target: &T, source: &S) -> anyhow::Result<T>
Expand Down Expand Up @@ -32,7 +31,7 @@ pub struct RelativeUtf8PathBuf(Utf8PathBuf);

impl RelativeUtf8PathBuf {
pub fn relative_to_directory(&self, root: &Utf8Path) -> Result<Utf8PathBuf> {
fsx::canonicalize(root.join(&self.0))?.try_to_utf8()
fsx::canonicalize_utf8(root.join(&self.0))
}

pub fn relative_to_file(&self, file: &Utf8Path) -> Result<Utf8PathBuf> {
Expand Down
1 change: 1 addition & 0 deletions scarb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
//!
//! [cairo]: https://cairo-lang.org/
#![deny(clippy::disallowed_methods)]
#![deny(rustdoc::broken_intra_doc_links)]
#![deny(rustdoc::private_intra_doc_links)]
#![warn(rust_2018_idioms)]
Expand Down
3 changes: 1 addition & 2 deletions scarb/src/manifest_editor/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use url::Url;

use crate::core::{GitReference, PackageName};
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;
use crate::sources::canonical_url::CanonicalUrl;

use super::tomlx::get_table_mut;
Expand Down Expand Up @@ -115,7 +114,7 @@ impl Dep {
});

let source: Box<dyn Source> = if let Some(path) = op.path {
let path = fsx::canonicalize(path)?.try_into_utf8()?;
let path = fsx::canonicalize_utf8(path)?;
let path = path_value(ctx.manifest_path, &path);

Box::new(PathSource { version, path })
Expand Down
3 changes: 1 addition & 2 deletions scarb/src/manifest_editor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ pub use remove::RemoveDependency;

use crate::core::Config;
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;

mod add;
mod dep_id;
Expand Down Expand Up @@ -40,7 +39,7 @@ pub fn edit(
ops: Vec<Box<dyn Op>>,
opts: EditManifestOptions<'_>,
) -> Result<()> {
let manifest_path = fsx::canonicalize(manifest_path)?.try_into_utf8()?;
let manifest_path = fsx::canonicalize_utf8(manifest_path)?;

let original_raw_manifest = fsx::read_to_string(&manifest_path)?;
let mut doc = Document::from_str(&original_raw_manifest)
Expand Down
4 changes: 2 additions & 2 deletions scarb/src/ops/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use camino::{Utf8Path, Utf8PathBuf};

use crate::core::manifest::TomlManifest;
use crate::internal::fsx;
use crate::internal::fsx::{PathBufUtf8Ext, PathUtf8Ext};
use crate::internal::fsx::PathBufUtf8Ext;
use crate::MANIFEST_FILE_NAME;

#[tracing::instrument(level = "debug")]
Expand All @@ -15,7 +15,7 @@ pub fn find_manifest_path(user_override: Option<&Utf8Path>) -> Result<Utf8PathBu
.unwrap_or_else(|_| user_override.into())
.try_into_utf8()?),
None => {
let pwd = fsx::canonicalize(env::current_dir()?)?.try_to_utf8()?;
let pwd = fsx::canonicalize_utf8(env::current_dir()?)?;
let accept_all = |_| Ok(true);
let manifest_path = try_find_manifest_of_pwd(pwd.clone(), accept_all)?
.unwrap_or_else(|| pwd.join(MANIFEST_FILE_NAME));
Expand Down
7 changes: 1 addition & 6 deletions scarb/src/ops/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use itertools::Itertools;

use crate::core::{Config, PackageName};
use crate::internal::fsx;
use crate::internal::fsx::PathBufUtf8Ext;
use crate::internal::restricted_names;
use crate::{ops, DEFAULT_SOURCE_PATH, DEFAULT_TARGET_DIR_NAME, MANIFEST_FILE_NAME};

Expand Down Expand Up @@ -126,11 +125,7 @@ fn mk(
// Create project directory in case we are called from `new` op.
fsx::create_dir_all(&path)?;

let canonical_path = if let Ok(canonicalize_path) = fsx::canonicalize(&path) {
canonicalize_path.try_into_utf8()?
} else {
path
};
let canonical_path = fsx::canonicalize_utf8(&path).unwrap_or(path);

init_vcs(&canonical_path, version_control)?;
write_vcs_ignore(&canonical_path, config, version_control)?;
Expand Down
4 changes: 1 addition & 3 deletions scarb/src/ops/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,7 @@ fn find_member_paths(
// Look for manifest file, continuing if it does not exist.
let path = path.join(MANIFEST_FILE_NAME);
if path.is_file() {
let path = fsx::canonicalize(path)?;
let path = path.try_into_utf8()?;

let path = fsx::canonicalize_utf8(path)?;
paths.push(path)
} else {
config.ui().warn(format!(
Expand Down
68 changes: 15 additions & 53 deletions scarb/tests/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use std::collections::BTreeMap;

use assert_fs::prelude::*;
use indoc::indoc;
use serde_json::json;

use scarb_metadata::{Cfg, ManifestMetadataBuilder, Metadata, PackageMetadata};
use scarb_test_support::command::{CommandExt, Scarb};
use scarb_test_support::fsx::PathBufUtf8Ext;
use scarb_test_support::fsx;
use scarb_test_support::project_builder::ProjectBuilder;
use scarb_test_support::workspace_builder::WorkspaceBuilder;
use serde_json::json;

fn packages_by_name(meta: Metadata) -> BTreeMap<String, PackageMetadata> {
meta.packages
Expand Down Expand Up @@ -285,10 +286,7 @@ fn manifest_targets_and_metadata() {
.license(Some("MIT License".to_string()))
.license_file(Some("./license.md".to_string()))
.readme(
t.join("README.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("README.md"))
.unwrap()
.into_string()
)
Expand Down Expand Up @@ -617,10 +615,7 @@ fn infer_readme_simple() {
.manifest_metadata
.readme,
Some(
t.join("README")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("README"))
.unwrap()
.into_string()
)
Expand All @@ -643,10 +638,7 @@ fn infer_readme_simple() {
.manifest_metadata
.readme,
Some(
t.join("README.txt")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("README.txt"))
.unwrap()
.into_string()
)
Expand All @@ -669,10 +661,7 @@ fn infer_readme_simple() {
.manifest_metadata
.readme,
Some(
t.join("README.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("README.md"))
.unwrap()
.into_string()
)
Expand Down Expand Up @@ -711,10 +700,7 @@ fn infer_readme_simple() {
.manifest_metadata
.readme,
Some(
t.join("a/b/c/MEREAD.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("a/b/c/MEREAD.md"))
.unwrap()
.into_string()
)
Expand Down Expand Up @@ -797,10 +783,7 @@ fn infer_readme_simple_bool() {
.manifest_metadata
.readme,
Some(
t.join("README.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("README.md"))
.unwrap()
.into_string()
)
Expand Down Expand Up @@ -986,68 +969,47 @@ fn infer_readme_workspace() {
assert_eq!(
packages.get("hello").unwrap().manifest_metadata.readme,
Some(
t.join("MEREAD.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("MEREAD.md"))
.unwrap()
.into_string()
)
);
assert_eq!(
packages.get("t7").unwrap().manifest_metadata.readme,
Some(
t.join("MEREAD.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("MEREAD.md"))
.unwrap()
.into_string()
)
);
assert_eq!(
packages.get("t1").unwrap().manifest_metadata.readme,
Some(
t.join("MEREAD.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.join("MEREAD.md"))
.unwrap()
.into_string()
)
);
assert_eq!(
packages.get("t2").unwrap().manifest_metadata.readme,
Some(
t.child("t2")
.join("README.md")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.child("t2").join("README.md"))
.unwrap()
.into_string()
)
);
assert_eq!(
packages.get("t3").unwrap().manifest_metadata.readme,
Some(
t.child("t3")
.join("README.txt")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.child("t3").join("README.txt"))
.unwrap()
.into_string()
)
);
assert_eq!(
packages.get("t4").unwrap().manifest_metadata.readme,
Some(
t.child("t4")
.join("TEST.txt")
.canonicalize()
.unwrap()
.try_into_utf8()
fsx::canonicalize_utf8(t.child("t4").join("TEST.txt"))
.unwrap()
.into_string()
)
Expand Down
18 changes: 11 additions & 7 deletions scarb/tests/workspace.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use assert_fs::fixture::{PathChild, PathCreateDir};
use assert_fs::TempDir;
use indoc::indoc;
use scarb_metadata::Metadata;

use scarb_metadata::Metadata;
use scarb_test_support::command::{CommandExt, Scarb};
use scarb_test_support::fsx;
use scarb_test_support::project_builder::ProjectBuilder;
use scarb_test_support::workspace_builder::WorkspaceBuilder;

Expand Down Expand Up @@ -79,11 +80,14 @@ fn unify_target_dir() {

assert_eq!(root_metadata.target_dir, pkg_metadata.target_dir);
assert_eq!(
root_metadata
.target_dir
.unwrap()
.to_owned()
.into_std_path_buf(),
t.child("target").canonicalize().unwrap()
fsx::canonicalize(
root_metadata
.target_dir
.unwrap()
.to_owned()
.into_std_path_buf()
)
.unwrap(),
fsx::canonicalize(t.child("target")).unwrap()
);
}
2 changes: 1 addition & 1 deletion utils/scarb-test-support/src/fsx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use camino::Utf8Path;
use itertools::Itertools;
use serde::de::DeserializeOwned;

pub use internal_fsx::{PathBufUtf8Ext, PathUtf8Ext};
pub use internal_fsx::{canonicalize, canonicalize_utf8, PathBufUtf8Ext, PathUtf8Ext};

#[allow(unused)]
#[path = "../../../scarb/src/internal/fsx.rs"]
Expand Down

0 comments on commit 5ff2b9c

Please sign in to comment.