From 8d566e553d044bfc4e2ff8d6b5aa9c29e97e168b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 23 May 2024 23:02:42 -0400 Subject: [PATCH] Use a cross-platform representation for relative paths in `pip compile` (#3804) ## Summary I haven't tested on Windows yet, but the idea here is that we should use a portable representation when printing paths. I decided to limit the scope here to paths that we write to output files. Closes https://github.com/astral-sh/uv/issues/3800. --- Cargo.lock | 10 +++- Cargo.toml | 3 +- crates/distribution-types/src/annotation.rs | 8 +-- crates/uv-configuration/Cargo.toml | 2 +- crates/uv-configuration/src/overrides.rs | 2 +- crates/uv-fs/Cargo.toml | 2 + crates/uv-fs/src/path.rs | 58 ++++++++++++++++++--- crates/uv-interpreter/src/discovery.rs | 8 +-- crates/uv/src/commands/pip/compile.rs | 2 +- crates/uv/tests/pip_compile.rs | 6 +-- 10 files changed, 75 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ed2fa624f..a5f1a00c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2486,6 +2486,12 @@ dependencies = [ "once_cell", ] +[[package]] +name = "path-slash" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e91099d4268b0e11973f036e885d652fb0b21fedcf69738c627f94db6a44f42" + [[package]] name = "pathdiff" version = "0.2.1" @@ -4665,7 +4671,7 @@ dependencies = [ "anyhow", "clap", "distribution-types", - "itertools 0.13.0", + "either", "pep508_rs", "platform-tags", "rustc-hash", @@ -4811,12 +4817,14 @@ dependencies = [ "backoff", "cachedir", "dunce", + "either", "encoding_rs_io", "fs-err", "fs2", "junction", "once_cell", "path-absolutize", + "path-slash", "tempfile", "tracing", "urlencoding", diff --git a/Cargo.toml b/Cargo.toml index aed9f74c7..518ed1667 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,7 +73,7 @@ derivative = { version = "2.2.0" } directories = { version = "5.0.1" } dirs-sys = { version = "0.4.1" } dunce = { version = "1.0.4" } -either = { version = "1.9.0" } +either = { version = "1.12.0" } encoding_rs_io = { version = "0.1.7" } flate2 = { version = "1.0.28", default-features = false } fs-err = { version = "2.11.0" } @@ -98,6 +98,7 @@ nanoid = { version = "0.4.0" } once_cell = { version = "1.19.0" } owo-colors = { version = "4.0.0" } path-absolutize = { version = "3.1.1" } +path-slash = { version = "0.2.1" } pathdiff = { version = "0.2.1" } petgraph = { version = "0.6.4" } platform-info = { version = "2.0.2" } diff --git a/crates/distribution-types/src/annotation.rs b/crates/distribution-types/src/annotation.rs index 2080cc97e..f6cd207f5 100644 --- a/crates/distribution-types/src/annotation.rs +++ b/crates/distribution-types/src/annotation.rs @@ -22,17 +22,17 @@ impl std::fmt::Display for SourceAnnotation { match self { Self::Requirement(origin) => match origin { RequirementOrigin::File(path) => { - write!(f, "-r {}", path.user_display()) + write!(f, "-r {}", path.portable_display()) } RequirementOrigin::Project(path, project_name) => { - write!(f, "{project_name} ({})", path.user_display()) + write!(f, "{project_name} ({})", path.portable_display()) } }, Self::Constraint(origin) => { - write!(f, "-c {}", origin.path().user_display()) + write!(f, "-c {}", origin.path().portable_display()) } Self::Override(origin) => { - write!(f, "--override {}", origin.path().user_display()) + write!(f, "--override {}", origin.path().portable_display()) } } } diff --git a/crates/uv-configuration/Cargo.toml b/crates/uv-configuration/Cargo.toml index 34f05896d..b9a9b8769 100644 --- a/crates/uv-configuration/Cargo.toml +++ b/crates/uv-configuration/Cargo.toml @@ -20,7 +20,7 @@ uv-auth = { workspace = true } uv-normalize = { workspace = true } clap = { workspace = true, features = ["derive"], optional = true } -itertools = { workspace = true } +either = { workspace = true } rustc-hash = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true } diff --git a/crates/uv-configuration/src/overrides.rs b/crates/uv-configuration/src/overrides.rs index 0ad3e0ae1..37a5f0531 100644 --- a/crates/uv-configuration/src/overrides.rs +++ b/crates/uv-configuration/src/overrides.rs @@ -1,6 +1,6 @@ use std::hash::BuildHasherDefault; -use itertools::Either; +use either::Either; use rustc_hash::FxHashMap; use distribution_types::Requirement; diff --git a/crates/uv-fs/Cargo.toml b/crates/uv-fs/Cargo.toml index 57f490155..7efdd437d 100644 --- a/crates/uv-fs/Cargo.toml +++ b/crates/uv-fs/Cargo.toml @@ -18,11 +18,13 @@ uv-warnings = { workspace = true } backoff = { workspace = true } cachedir = { workspace = true } dunce = { workspace = true } +either = { workspace = true } encoding_rs_io = { workspace = true } fs-err = { workspace = true } fs2 = { workspace = true } once_cell = { workspace = true } path-absolutize = { workspace = true } +path-slash = { workspace = true } tempfile = { workspace = true } tracing = { workspace = true } urlencoding = { workspace = true } diff --git a/crates/uv-fs/src/path.rs b/crates/uv-fs/src/path.rs index b790549cd..f09ac071f 100644 --- a/crates/uv-fs/src/path.rs +++ b/crates/uv-fs/src/path.rs @@ -1,7 +1,9 @@ +use either::Either; use std::borrow::Cow; use std::path::{Component, Path, PathBuf}; use once_cell::sync::Lazy; +use path_slash::PathExt; /// The current working directory. pub static CWD: Lazy = @@ -25,7 +27,7 @@ pub trait Simplified { /// /// On Windows, this will strip the `\\?\` prefix from paths. On other platforms, it's /// equivalent to [`std::path::Display`]. - fn simplified_display(&self) -> std::path::Display; + fn simplified_display(&self) -> impl std::fmt::Display; /// Canonicalize a path without a `\\?\` prefix on Windows. fn simple_canonicalize(&self) -> std::io::Result; @@ -33,7 +35,18 @@ pub trait Simplified { /// Render a [`Path`] for user-facing display. /// /// Like [`simplified_display`], but relativizes the path against the current working directory. - fn user_display(&self) -> std::path::Display; + fn user_display(&self) -> impl std::fmt::Display; + + /// Render a [`Path`] for user-facing display, where the [`Path`] is relative to a base path. + /// + /// If the [`Path`] is not relative to the base path, will attempt to relativize the path + /// against the current working directory. + fn user_display_from(&self, base: impl AsRef) -> impl std::fmt::Display; + + /// Render a [`Path`] for user-facing display using a portable representation. + /// + /// Like [`user_display`], but uses a portable representation for relative paths. + fn portable_display(&self) -> impl std::fmt::Display; } impl> Simplified for T { @@ -41,7 +54,7 @@ impl> Simplified for T { dunce::simplified(self.as_ref()) } - fn simplified_display(&self) -> std::path::Display { + fn simplified_display(&self) -> impl std::fmt::Display { dunce::simplified(self.as_ref()).display() } @@ -49,17 +62,48 @@ impl> Simplified for T { dunce::canonicalize(self.as_ref()) } - fn user_display(&self) -> std::path::Display { + fn user_display(&self) -> impl std::fmt::Display { let path = dunce::simplified(self.as_ref()); // Attempt to strip the current working directory, then the canonicalized current working // directory, in case they differ. - path.strip_prefix(CWD.simplified()) - .unwrap_or_else(|_| { + let path = path.strip_prefix(CWD.simplified()).unwrap_or_else(|_| { + path.strip_prefix(CANONICAL_CWD.simplified()) + .unwrap_or(path) + }); + + path.display() + } + + fn user_display_from(&self, base: impl AsRef) -> impl std::fmt::Display { + let path = dunce::simplified(self.as_ref()); + + // Attempt to strip the base, then the current working directory, then the canonicalized + // current working directory, in case they differ. + let path = path.strip_prefix(base.as_ref()).unwrap_or_else(|_| { + path.strip_prefix(CWD.simplified()).unwrap_or_else(|_| { path.strip_prefix(CANONICAL_CWD.simplified()) .unwrap_or(path) }) - .display() + }); + + path.display() + } + + fn portable_display(&self) -> impl std::fmt::Display { + let path = dunce::simplified(self.as_ref()); + + // Attempt to strip the current working directory, then the canonicalized current working + // directory, in case they differ. + let path = path.strip_prefix(CWD.simplified()).unwrap_or_else(|_| { + path.strip_prefix(CANONICAL_CWD.simplified()) + .unwrap_or(path) + }); + + // Use a portable representation for relative paths. + path.to_slash() + .map(Either::Left) + .unwrap_or_else(|| Either::Right(path.display())) } } diff --git a/crates/uv-interpreter/src/discovery.rs b/crates/uv-interpreter/src/discovery.rs index c5784b7e0..d52575538 100644 --- a/crates/uv-interpreter/src/discovery.rs +++ b/crates/uv-interpreter/src/discovery.rs @@ -1256,17 +1256,11 @@ impl fmt::Display for InterpreterNotFound { path.user_display() ), Self::ExecutableNotFoundInDirectory(directory, executable) => { - let executable = if let Ok(relative_executable) = executable.strip_prefix(directory) - { - relative_executable.display() - } else { - executable.user_display() - }; write!( f, "Interpreter directory `{}` does not contain Python executable at `{}`", directory.user_display(), - executable + executable.user_display_from(directory) ) } Self::ExecutableNotFoundInSearchPath(name) => { diff --git a/crates/uv/src/commands/pip/compile.rs b/crates/uv/src/commands/pip/compile.rs index 32dbad9a9..cc28619fa 100644 --- a/crates/uv/src/commands/pip/compile.rs +++ b/crates/uv/src/commands/pip/compile.rs @@ -707,7 +707,7 @@ fn cmd( } let args = env::args_os() .skip(1) - .map(|arg| arg.user_display().to_string()) + .map(|arg| arg.to_string_lossy().to_string()) .scan(None, move |skip_next, arg| { if matches!(skip_next, Some(true)) { // Reset state; skip this iteration. diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index b056d3f47..ca57e0d7e 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -7511,7 +7511,7 @@ fn local_version_of_remote_package() -> Result<()> { exit_code: 0 ----- stdout ----- # This file was autogenerated by uv via the following command: - # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z [TEMP_DIR]/requirements.in anyio==4.3.0 # via -r requirements.in idna==3.6 @@ -7548,7 +7548,7 @@ fn local_version_of_remote_package() -> Result<()> { exit_code: 0 ----- stdout ----- # This file was autogenerated by uv via the following command: - # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z [TEMP_DIR]/requirements.in anyio==4.3.0 # via -r requirements.in idna==3.6 @@ -7580,7 +7580,7 @@ fn local_version_of_remote_package() -> Result<()> { exit_code: 0 ----- stdout ----- # This file was autogenerated by uv via the following command: - # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z requirements.in --output-file requirements.txt + # uv pip compile --cache-dir [CACHE_DIR] --exclude-newer 2024-03-25T00:00:00Z [TEMP_DIR]/requirements.in --output-file [TEMP_DIR]/requirements.txt anyio==4.3.0 # via -r requirements.in idna==3.6