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
58 changes: 52 additions & 6 deletions crates/uv-fs/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,17 +206,36 @@ pub fn normalize_absolute_path(path: &Path) -> Result<PathBuf, std::io::Error> {
Ok(ret)
}

/// Normalize a [`Path`], removing things like `.` and `..`.
/// Normalize a [`Path`], removing `.`, `..`, and trailing slashes.
pub fn normalize_path(path: &Path) -> Cow<'_, Path> {
// Fast path: if the path is already normalized, return it as-is.
if path.components().all(|component| match component {
// A path with `.` or `..` is not normalized.
if !path.components().all(|component| match component {
Component::Prefix(_) | Component::RootDir | Component::Normal(_) => true,
Component::ParentDir | Component::CurDir => false,

@samypr100 samypr100 Apr 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note (might be intentional), .components() strips Component::CurDir at least on windows so on paths only with foo\.\bar, normalize_path actually doesn't normalize it to foo\bar as it goes to the Cow::Borrowed(path) path.

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.

Oh no, I didn't realize this, thanks for the note!

The same thing is happening with foo//bar and for\\bar also-

}) {
Cow::Borrowed(path)
} else {
Cow::Owned(normalized(path))
return Cow::Owned(normalized(path));
}

// A path with a trailing path separator is not normalized.
if path
.as_os_str()
.as_encoded_bytes()
.last()
.is_some_and(|trailing| {
if cfg!(windows) {
*trailing == b'\\' || *trailing == b'/'
} else if cfg!(unix) {
*trailing == b'/'
} else {
unimplemented!("Only Windows and Unix are supported")
}
})
{
return Cow::Owned(normalized(path));
}

// Fast path: if the path is already normalized, return it as-is.
Cow::Borrowed(path)
}

/// Normalize a [`PathBuf`], removing things like `.` and `..`.
Expand Down Expand Up @@ -564,4 +583,31 @@ mod tests {
assert_eq!(normalize_path(Path::new(input)), Path::new(expected));
}
}

#[test]
fn test_normalize_trailing_path_separator() {
let cases = [
(
"/home/ferris/projects/python/",
"/home/ferris/projects/python",
),
("python/", "python"),
("/", "/"),
];
for (input, expected) in cases {
assert_eq!(normalize_path(Path::new(input)), Path::new(expected));
}
}

#[test]
#[cfg(windows)]
fn test_normalize_trailing_path_separator_windows() {
let cases = [(
r"C:\Users\Ferris\projects\python\",
r"C:\Users\Ferris\projects\python",
)];
for (input, expected) in cases {
assert_eq!(normalize_path(Path::new(input)), Path::new(expected));
}
}
}
18 changes: 6 additions & 12 deletions crates/uv-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,7 @@ impl Workspace {
let path = std::path::absolute(path)
.map_err(WorkspaceError::Normalize)?
.clone();
// Remove `.` and `..`
let path = uv_fs::normalize_path(&path);
// Trim trailing slashes.
let path = path.components().collect::<PathBuf>();

let project_path = path
.ancestors()
Expand Down Expand Up @@ -1400,10 +1397,7 @@ impl ProjectWorkspace {
let project_path = std::path::absolute(install_path)
.map_err(WorkspaceError::Normalize)?
.clone();
// Remove `.` and `..`
let project_path = uv_fs::normalize_path(&project_path);
// Trim trailing slashes.
let project_path = project_path.components().collect::<PathBuf>();

// Check if workspaces are explicitly disabled for the project.
if project_pyproject_toml
Expand All @@ -1414,7 +1408,7 @@ impl ProjectWorkspace {
== Some(false)
{
debug!("Project `{}` is marked as unmanaged", project.name);
return Err(WorkspaceError::NonWorkspace(project_path));
return Err(WorkspaceError::NonWorkspace(project_path.to_path_buf()));
}

// Check if the current project is also an explicit workspace root.
Expand All @@ -1425,7 +1419,7 @@ impl ProjectWorkspace {
.and_then(|uv| uv.workspace.as_ref())
.map(|workspace| {
(
project_path.clone(),
project_path.to_path_buf(),
workspace.clone(),
project_pyproject_toml.clone(),
)
Expand All @@ -1438,7 +1432,7 @@ impl ProjectWorkspace {
}

let current_project = WorkspaceMember {
root: project_path.clone(),
root: project_path.to_path_buf(),
project: project.clone(),
pyproject_toml: project_pyproject_toml.clone(),
};
Expand All @@ -1461,10 +1455,10 @@ impl ProjectWorkspace {
)?;

return Ok(Self {
project_root: project_path.clone(),
project_root: project_path.to_path_buf(),
project_name: project.name.clone(),
workspace: Workspace {
install_path: project_path.clone(),
install_path: project_path.to_path_buf(),
packages: current_project_as_members,
required_members,
// There may be package sources, but we don't need to duplicate them into the
Expand Down Expand Up @@ -1492,7 +1486,7 @@ impl ProjectWorkspace {
.await?;

Ok(Self {
project_root: project_path,
project_root: project_path.to_path_buf(),
project_name: project.name.clone(),
workspace,
})
Expand Down
Loading