Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,17 @@ jobs:
if: contains(matrix.os, 'ubuntu')
run: |
cargo fmt -- --check

- name: Check builds Wasm
if: contains(matrix.os, 'ubuntu')
run: |
rustup target add wasm32-unknown-unknown
cargo check --all-features --target wasm32-unknown-unknown

# Some tests mutate the current working directory while some others rely
# on it; we run them sequentially to avoid flakiness.
- name: Cargo test
run: cargo test --locked --release --all-features --bins --tests --examples
run: cargo test --locked --release --all-features --bins --tests --examples -- --test-threads=1

- name: Lint
if: contains(matrix.os, 'ubuntu')
Expand Down
102 changes: 93 additions & 9 deletions src/fs.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright 2018-2025 the Deno authors. MIT license.

use std::borrow::Cow;
use std::io::Error;
use std::io::ErrorKind;
use std::io::Write;
Expand All @@ -18,7 +17,6 @@ use sys_traits::SystemRandom;
use sys_traits::ThreadSleep;

use crate::get_atomic_path;
use crate::normalize_path;

/// Canonicalizes a path which might be non-existent by going up the
/// ancestors until it finds a directory that exists, canonicalizes
Expand All @@ -28,10 +26,8 @@ use crate::normalize_path;
/// subsequently be created along this path by some other code.
pub fn canonicalize_path_maybe_not_exists(
sys: &impl FsCanonicalize,
path: &Path,
mut path: &Path,
) -> std::io::Result<PathBuf> {
let path = normalize_path(Cow::Borrowed(path));

@dsherret dsherret Oct 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is not normalizing the path anymore going to cause issues? Maybe it should still be normalized when not starting with a .?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think path normalization is done by fs_canonicalize anyway, so doing it would just be a duplicate work. Also as you can see we have tests confirming that paths not starting with . are handled expectedly.
I might be overlooking something though

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try running all the tests in the CLI repo with this change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'll do a CI run and test it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks David!

let mut path = path.as_ref();
let mut names_stack = Vec::new();
loop {
match sys.fs_canonicalize(path) {
Expand All @@ -47,6 +43,13 @@ pub fn canonicalize_path_maybe_not_exists(
None => return Err(err),
});
path = match path.parent() {
// When the provided path is a relative path (e.g. `foo/bar.txt`),
// `path.parent()` ends up being the empty string as documented in
// `std::path::Path::parent()` after going up the ancestor path.
// In this case, what this function should return is a path joining
// the current path with the provided path i.e. `{cwd}/foo/bar.txt`,
// so we set `path` to `.` to indicate the current directory.
Some(parent) if parent.as_os_str().is_empty() => Path::new("."),
Some(parent) => parent,
None => return Err(err),
};
Expand Down Expand Up @@ -168,29 +171,110 @@ fn add_file_context_to_err(file_path: &Path, err: Error) -> Error {

#[cfg(test)]
mod test {
use std::path::Path;
use std::path::PathBuf;

use sys_traits::impls::InMemorySys;
use sys_traits::impls::RealSys;
use sys_traits::EnvCurrentDir;
use sys_traits::EnvSetCurrentDir;
use sys_traits::FsCanonicalize;
use sys_traits::FsCreateDirAll;
use sys_traits::FsRead;
use sys_traits::FsSymlinkDir;

use super::atomic_write_file_with_retries;
use super::canonicalize_path_maybe_not_exists;

#[test]
fn test_canonicalize_path_maybe_not_exists() {
fn test_canonicalize_path_maybe_not_exists_in_memory() {
let sys = InMemorySys::default();

// .
// └── a
// └── b (cwd)
// └── c
sys.fs_create_dir_all("/a/b/c").unwrap();
sys.env_set_current_dir("/a/b").unwrap();

let path = canonicalize_path_maybe_not_exists(&sys, Path::new("")).unwrap();
assert_eq!(path, PathBuf::from("/a/b"));
let path =
canonicalize_path_maybe_not_exists(&sys, Path::new(".")).unwrap();
assert_eq!(path, PathBuf::from("/a/b"));
let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("d")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/d"));
let path =
canonicalize_path_maybe_not_exists(&sys, &PathBuf::from("./c")).unwrap();
canonicalize_path_maybe_not_exists(&sys, Path::new("./d")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/d"));
let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("c")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/c"));
let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("./c")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/c"));
let path =
canonicalize_path_maybe_not_exists(&sys, &PathBuf::from("./c/d/e"))
.unwrap();
canonicalize_path_maybe_not_exists(&sys, Path::new("c/d/e")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/c/d/e"));
let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("./c/d/e")).unwrap();
assert_eq!(path, PathBuf::from("/a/b/c/d/e"));
}

// Note: this test mutates the current working directory. Although it will be
// restored at the end, if other tests relying on the cwd are run in parallel
// to this test, they may fail.
#[test]
fn test_canonicalize_path_maybe_not_exists_real() {
let sys = RealSys;
let temp_dir = tempfile::tempdir().unwrap();

// Save the original working directory to restore it later
let original_cwd = sys.env_current_dir().unwrap();

// .
// ├── a
// │ └── b
// │ └── c
// └── link -> a/b/c (cwd)
sys
.fs_create_dir_all(temp_dir.path().join("a/b/c"))
.unwrap();
sys
.fs_symlink_dir(
temp_dir.path().join("a/b/c"),
temp_dir.path().join("link"),
)
.unwrap();
let cwd = temp_dir.path().join("link");
sys.env_set_current_dir(&cwd).unwrap();

let canonicalized_temp_dir_path =
sys.fs_canonicalize(temp_dir.path()).unwrap();

let path =
canonicalize_path_maybe_not_exists(&sys, Path::new(".")).unwrap();
assert_eq!(path, canonicalized_temp_dir_path.join("a/b/c"));

let path =
canonicalize_path_maybe_not_exists(&sys, &PathBuf::from("d")).unwrap();
assert_eq!(path, canonicalized_temp_dir_path.join("a/b/c/d"));

let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("./d")).unwrap();
assert_eq!(path, canonicalized_temp_dir_path.join("a/b/c/d"));

let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("d/e")).unwrap();
assert_eq!(path, canonicalized_temp_dir_path.join("a/b/c/d/e"));

let path =
canonicalize_path_maybe_not_exists(&sys, Path::new("./d/e")).unwrap();
assert_eq!(path, canonicalized_temp_dir_path.join("a/b/c/d/e"));

// Restore the original working directory
sys.env_set_current_dir(&original_cwd).unwrap();
}

#[test]
Expand Down