Skip to content

Commit

Permalink
Avoid calling normalize_path on possibly-relative paths
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Apr 12, 2024
1 parent f61b97e commit 460a029
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 12 deletions.
2 changes: 2 additions & 0 deletions crates/pep508-rs/src/verbatim_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ impl VerbatimUrl {
}

/// Create a [`VerbatimUrl`] from a file path.
///
/// Assumes that the path is absolute.
pub fn from_path(path: impl AsRef<Path>) -> Self {
let path = path.as_ref();

Expand Down
16 changes: 12 additions & 4 deletions crates/uv-fs/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,17 @@ pub fn normalize_url_path(path: &str) -> Cow<'_, str> {
/// Normalize a path, removing things like `.` and `..`.
///
/// Source: <https://github.com/rust-lang/cargo/blob/b48c41aedbd69ee3990d62a0e2006edbb506a480/crates/cargo-util/src/paths.rs#L76C1-L109C2>
pub fn normalize_path(path: impl AsRef<Path>) -> PathBuf {
let mut components = path.as_ref().components().peekable();
let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().copied() {
///
/// CAUTION: Assumes that the path is already absolute.
///
/// CAUTION: This does not resolve symlinks (unlike
/// [`std::fs::canonicalize`]). This may cause incorrect or surprising
/// behavior at times. This should be used carefully. Unfortunately,
/// [`std::fs::canonicalize`] can be hard to use correctly, since it can often
/// fail, or on Windows returns annoying device paths.
pub fn normalize_path(path: &Path) -> PathBuf {
let mut components = path.components().peekable();
let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() {
components.next();
PathBuf::from(c.as_os_str())
} else {
Expand Down Expand Up @@ -235,7 +243,7 @@ mod tests {
use super::*;

#[test]
fn normalize() {
fn normalize_url() {
if cfg!(windows) {
assert_eq!(
normalize_url_path("/C:/Users/ferris/wheel-0.42.0.tar.gz"),
Expand Down
5 changes: 1 addition & 4 deletions crates/uv-interpreter/src/find_python.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use std::path::PathBuf;
use tracing::{debug, instrument};

use uv_cache::Cache;
use uv_fs::normalize_path;
use uv_toolchain::PythonVersion;

use crate::interpreter::InterpreterInfoError;
Expand Down Expand Up @@ -52,9 +51,7 @@ pub fn find_requested_python(request: &str, cache: &Cache) -> Result<Option<Inte
Interpreter::query(executable, cache).map(Some)
} else {
// `-p /home/ferris/.local/bin/python3.10`
let executable = normalize_path(request);

Interpreter::query(executable, cache).map(Some)
Interpreter::query(request, cache).map(Some)
}
}

Expand Down
7 changes: 3 additions & 4 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,16 +488,15 @@ impl InterpreterInfo {
/// unless the Python executable changes, so we use the executable's last modified
/// time as a cache key.
pub(crate) fn query_cached(executable: &Path, cache: &Cache) -> Result<Self, Error> {
let executable_bytes = executable.as_os_str().as_encoded_bytes();
let canonical = uv_fs::canonicalize_executable(executable)?;
let modified = Timestamp::from_path(canonical)?;

let cache_entry = cache.entry(
CacheBucket::Interpreter,
"",
format!("{}.msgpack", digest(&executable_bytes)),
format!("{}.msgpack", digest(&canonical)),
);

let modified = Timestamp::from_path(uv_fs::canonicalize_executable(executable)?)?;

// Read from the cache.
if cache
.freshness(&cache_entry, None)
Expand Down

0 comments on commit 460a029

Please sign in to comment.