Skip to content

Commit

Permalink
Use relative paths for user display (#2559)
Browse files Browse the repository at this point in the history
## Summary

This PR changes our user-facing representation for paths to use relative
paths, when the path is within the current working directory. This
mirrors what we do in Ruff. (If the path is _outside_ the current
working directory, we print an absolute path.)

Before:

```shell
❯ uv venv .venv2
Using Python 3.12.2 interpreter at: /Users/crmarsh/workspace/uv/.venv/bin/python3
Creating virtualenv at: .venv2
Activate with: source .venv2/bin/activate
```

After:

```shell
❯ cargo run venv .venv2
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s
     Running `target/debug/uv venv .venv2`
Using Python 3.12.2 interpreter at: .venv/bin/python3
Creating virtualenv at: .venv2
Activate with: source .venv2/bin/activate
```

Note that we still want to use the existing `.simplified_display()`
anywhere that the path is being simplified, but _still_ intended for
machine consumption (e.g., when passing to `.current_dir()`).
  • Loading branch information
charliermarsh authored Mar 20, 2024
1 parent 204b159 commit 00fc440
Show file tree
Hide file tree
Showing 30 changed files with 149 additions and 183 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 2 additions & 6 deletions crates/distribution-types/src/installed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,8 @@ impl InstalledDist {
pub fn metadata(&self) -> Result<pypi_types::Metadata23> {
let path = self.path().join("METADATA");
let contents = fs::read(&path)?;
pypi_types::Metadata23::parse_metadata(&contents).with_context(|| {
format!(
"Failed to parse METADATA file at: {}",
path.simplified_display()
)
})
pypi_types::Metadata23::parse_metadata(&contents)
.with_context(|| format!("Failed to parse METADATA file at: {}", path.user_display()))
}

/// Return the `INSTALLER` of the distribution.
Expand Down
4 changes: 2 additions & 2 deletions crates/install-wheel-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub enum Error {
#[error(transparent)]
Io(#[from] io::Error),
/// Custom error type to add a path to error reading a file from a zip
#[error("Failed to reflink {} to {}", from.simplified_display(), to.simplified_display())]
#[error("Failed to reflink {} to {}", from.user_display(), to.user_display())]
Reflink {
from: PathBuf,
to: PathBuf,
Expand Down Expand Up @@ -82,7 +82,7 @@ pub enum Error {
DirectUrlJson(#[from] serde_json::Error),
#[error("No .dist-info directory found")]
MissingDistInfo,
#[error("Cannot uninstall package; RECORD file not found at: {}", _0.simplified_display())]
#[error("Cannot uninstall package; RECORD file not found at: {}", _0.user_display())]
MissingRecord(PathBuf),
#[error("Multiple .dist-info directories found: {0}")]
MultipleDistInfo(String),
Expand Down
2 changes: 1 addition & 1 deletion crates/install-wheel-rs/src/wheel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ pub(crate) fn windows_script_launcher(
}

let python = python_executable.as_ref();
let python_path = python.simplified().to_string_lossy();
let python_path = python.simplified_display().to_string();

let mut launcher: Vec<u8> = Vec::with_capacity(launcher_bin.len() + payload.len());
launcher.extend_from_slice(launcher_bin);
Expand Down
54 changes: 22 additions & 32 deletions crates/requirements-txt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ impl RequirementsTxt {
if data == Self::default() {
warn_user!(
"Requirements file {} does not contain any dependencies",
requirements_txt.simplified_display()
requirements_txt.user_display()
);
}

Expand Down Expand Up @@ -1024,35 +1024,35 @@ impl Display for RequirementsTxtFileError {
write!(
f,
"Invalid URL in `{}` at position {start}: `{url}`",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::InvalidEditablePath(given) => {
write!(
f,
"Invalid editable path in `{}`: {given}",
self.file.simplified_display()
self.file.user_display()
)
}
RequirementsTxtParserError::UnsupportedUrl(url) => {
write!(
f,
"Unsupported URL (expected a `file://` scheme) in `{}`: `{url}`",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::MissingRequirementPrefix(given) => {
write!(
f,
"Requirement `{given}` in `{}` looks like a requirements file but was passed as a package name. Did you mean `-r {given}`?",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::MissingEditablePrefix(given) => {
write!(
f,
"Requirement `{given}` in `{}` looks like a directory but was passed as a package name. Did you mean `-e {given}`?",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::Parser {
Expand All @@ -1063,28 +1063,28 @@ impl Display for RequirementsTxtFileError {
write!(
f,
"{message} at {}:{line}:{column}",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::UnsupportedRequirement { start, .. } => {
write!(
f,
"Unsupported requirement in {} at position {start}",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::Pep508 { start, .. } => {
write!(
f,
"Couldn't parse requirement in `{}` at position {start}",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::Subfile { start, .. } => {
write!(
f,
"Error parsing included file in `{}` at position {start}",
self.file.simplified_display(),
self.file.user_display(),
)
}
RequirementsTxtParserError::NonUnicodeUrl { url } => {
Expand All @@ -1099,7 +1099,7 @@ impl Display for RequirementsTxtFileError {
write!(
f,
"Error while accessing remote requirements file {}: {err}",
self.file.simplified_display(),
self.file.user_display(),
)
}
}
Expand Down Expand Up @@ -1276,9 +1276,8 @@ mod test {
.take(2)
.join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let missing_txt = regex::escape(&missing_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let missing_txt = regex::escape(&missing_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(missing_txt.as_str(), "<MISSING_TXT>"),
Expand Down Expand Up @@ -1313,8 +1312,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1350,8 +1348,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1387,8 +1384,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1419,8 +1415,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![(requirement_txt.as_str(), "<REQUIREMENTS_TXT>")];
insta::with_settings!({
filters => filters
Expand Down Expand Up @@ -1453,8 +1448,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1488,8 +1482,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1528,8 +1521,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1692,8 +1684,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down Expand Up @@ -1746,8 +1737,7 @@ mod test {
.unwrap_err();
let errors = anyhow::Error::new(error).chain().join("\n");

let requirement_txt =
regex::escape(&requirements_txt.path().simplified_display().to_string());
let requirement_txt = regex::escape(&requirements_txt.path().user_display().to_string());
let filters = vec![
(requirement_txt.as_str(), "<REQUIREMENTS_TXT>"),
(r"\\", "/"),
Expand Down
1 change: 1 addition & 0 deletions crates/uv-fs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ encoding_rs_io = { workspace = true }
fs-err = { workspace = true }
fs2 = { workspace = true }
junction = { workspace = true }
once_cell = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, optional = true }
tracing = { workspace = true }
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub async fn write_atomic(path: impl AsRef<Path>, data: impl AsRef<[u8]>) -> std
std::io::ErrorKind::Other,
format!(
"Failed to persist temporary file to {}: {}",
path.simplified_display(),
path.user_display(),
err.error
),
)
Expand All @@ -135,7 +135,7 @@ pub fn write_atomic_sync(path: impl AsRef<Path>, data: impl AsRef<[u8]>) -> std:
std::io::ErrorKind::Other,
format!(
"Failed to persist temporary file to {}: {}",
path.simplified_display(),
path.user_display(),
err.error
),
)
Expand Down Expand Up @@ -288,7 +288,7 @@ impl LockedFile {
warn_user!(
"Waiting to acquire lock for {} (lockfile: {})",
resource,
path.simplified_display(),
path.user_display(),
);
file.file().lock_exclusive()?;
Ok(Self(file))
Expand Down
16 changes: 15 additions & 1 deletion crates/uv-fs/src/path.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
use std::borrow::Cow;
use std::path::{Component, Path, PathBuf};

use once_cell::sync::Lazy;

pub static CWD: Lazy<PathBuf> = Lazy::new(|| std::env::current_dir().unwrap());

pub trait Simplified {
/// Simplify a [`Path`].
///
/// On Windows, this will strip the `\\?\` prefix from paths. On other platforms, it's a no-op.
fn simplified(&self) -> &Path;

/// Render a [`Path`] for user-facing display.
/// Render a [`Path`] for display.
///
/// 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;

/// 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;
}

impl<T: AsRef<Path>> Simplified for T {
Expand All @@ -22,6 +31,11 @@ impl<T: AsRef<Path>> Simplified for T {
fn simplified_display(&self) -> std::path::Display {
dunce::simplified(self.as_ref()).display()
}

fn user_display(&self) -> std::path::Display {
let path = dunce::simplified(self.as_ref());
path.strip_prefix(&*CWD).unwrap_or(path).display()
}
}

pub trait PythonExt {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-git/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl GitRemote {
let reference = locked_ref.as_ref().unwrap_or(reference);
if let Some(mut db) = db {
fetch(&mut db.repo, self.url.as_str(), reference, strategy, client)
.with_context(|| format!("failed to fetch into: {}", into.simplified_display()))?;
.with_context(|| format!("failed to fetch into: {}", into.user_display()))?;

let resolved_commit_hash = match locked_rev {
Some(rev) => db.contains(rev).then_some(rev),
Expand All @@ -190,7 +190,7 @@ impl GitRemote {
paths::create_dir_all(into)?;
let mut repo = init(into, true)?;
fetch(&mut repo, self.url.as_str(), reference, strategy, client)
.with_context(|| format!("failed to clone into: {}", into.simplified_display()))?;
.with_context(|| format!("failed to clone into: {}", into.user_display()))?;
let rev = match locked_rev {
Some(rev) => rev,
None => reference.resolve(&repo)?,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-installer/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ async fn worker(
Ok(()) => {
debug!(
"Bytecode compilation `python` at {} stderr:\n{}\n---",
interpreter.simplified_display(),
interpreter.user_display(),
stderr
);
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-installer/src/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ impl<'a> Planner<'a> {
if !wheel.filename.is_compatible(tags) {
bail!(
"A path dependency is incompatible with the current platform: {}",
wheel.path.simplified_display()
wheel.path.user_display()
);
}

Expand Down
6 changes: 3 additions & 3 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -485,20 +485,20 @@ impl InterpreterInfo {
debug!(
"Cached interpreter info for Python {}, skipping probing: {}",
cached.data.markers.python_full_version,
executable.simplified_display()
executable.user_display()
);
return Ok(cached.data);
}

debug!(
"Ignoring stale cached markers for: {}",
executable.simplified_display()
executable.user_display()
);
}
Err(err) => {
warn!(
"Broken cache entry at {}, removing: {err}",
cache_entry.path().simplified_display()
cache_entry.path().user_display()
);
let _ = fs_err::remove_file(cache_entry.path());
}
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-virtualenv/src/bare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn create_bare_venv(
if metadata.is_file() {
return Err(Error::IO(io::Error::new(
io::ErrorKind::AlreadyExists,
format!("File exists at `{}`", location.simplified_display()),
format!("File exists at `{}`", location.user_display()),
)));
} else if metadata.is_dir() {
if location.join("pyvenv.cfg").is_file() {
Expand All @@ -103,7 +103,7 @@ pub fn create_bare_venv(
io::ErrorKind::AlreadyExists,
format!(
"The directory `{}` exists, but it's not a virtualenv",
location.simplified_display()
location.user_display()
),
)));
}
Expand Down
Loading

0 comments on commit 00fc440

Please sign in to comment.