Skip to content

Commit

Permalink
Revert verbatim changes
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed May 21, 2024
1 parent 733590c commit 610c92d
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 94 deletions.
48 changes: 9 additions & 39 deletions crates/pep508-rs/src/verbatim_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,12 @@ pub struct VerbatimUrl {
#[derivative(Ord = "ignore")]
#[derivative(Hash = "ignore")]
given: Option<String>,
/// Whether the given URL was a relative path.
#[derivative(PartialEq = "ignore")]
#[derivative(Hash = "ignore")]
relative: Option<bool>,
}

impl VerbatimUrl {
/// Create a [`VerbatimUrl`] from a [`Url`].
pub fn from_url(url: Url) -> Self {
Self {
url,
given: None,
relative: None,
}
Self { url, given: None }
}

/// Create a [`VerbatimUrl`] from a file path.
Expand All @@ -64,35 +56,25 @@ impl VerbatimUrl {
url.set_fragment(Some(fragment));
}

Self {
url,
given: None,
relative: None,
}
Self { url, given: None }
}

/// Parse a URL from a string, expanding any environment variables.
pub fn parse_url(given: impl AsRef<str>) -> Result<Self, ParseError> {
let url = Url::parse(given.as_ref())?;
Ok(Self {
url,
given: None,
relative: None,
})
Ok(Self { url, given: None })
}

/// Parse a URL from an absolute or relative path.
#[cfg(feature = "non-pep508-extensions")] // PEP 508 arguably only allows absolute file URLs.
pub fn parse_path(path: impl AsRef<Path>, working_dir: impl AsRef<Path>) -> Self {
let path = path.as_ref();

let relative = path.is_relative();

// Convert the path to an absolute path, if necessary.
let path = if relative {
working_dir.as_ref().join(path)
} else {
let path = if path.is_absolute() {
path.to_path_buf()
} else {
working_dir.as_ref().join(path)
};

// Normalize the path.
Expand All @@ -115,11 +97,7 @@ impl VerbatimUrl {
url.set_fragment(Some(fragment));
}

Self {
url,
given: None,
relative: Some(relative),
}
Self { url, given: None }
}

/// Parse a URL from an absolute path.
Expand Down Expand Up @@ -150,11 +128,7 @@ impl VerbatimUrl {
url.set_fragment(Some(fragment));
}

Ok(Self {
url,
given: None,
relative: Some(false),
})
Ok(Self { url, given: None })
}

/// Set the verbatim representation of the URL.
Expand Down Expand Up @@ -186,11 +160,7 @@ impl VerbatimUrl {
/// This method should be used sparingly (ideally, not at all), as it represents a loss of the
/// verbatim representation.
pub fn unknown(url: Url) -> Self {
Self {
url,
given: None,
relative: None,
}
Self { given: None, url }
}
}

Expand Down
13 changes: 2 additions & 11 deletions crates/uv-resolver/src/resolution/display.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
use std::borrow::Cow;
use std::collections::BTreeSet;

use itertools::Itertools;
use owo_colors::OwoColorize;
use petgraph::visit::EdgeRef;
use petgraph::Direction;

use distribution_types::{
DistributionMetadata, IndexUrl, LocalEditable, Name, SourceAnnotations, Verbatim,
};
use distribution_types::{IndexUrl, LocalEditable, Name, SourceAnnotations, Verbatim};
use pypi_types::HashDigest;
use uv_normalize::PackageName;

Expand Down Expand Up @@ -155,13 +152,7 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
let mut line = match node {
Node::Editable(editable) => format!("-e {}", editable.verbatim()),
Node::Distribution(dist) => {
let dist = if self.include_extras {
Cow::Borrowed(dist)
} else {
dist.without_extras()
};
dist.to_requirements_txt().to_string()

dist.to_requirements_txt(self.include_extras).to_string()
}
};

Expand Down
64 changes: 42 additions & 22 deletions crates/uv-resolver/src/resolution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::path::Path;
use itertools::Itertools;

use distribution_types::{DistributionMetadata, Name, ResolvedDist, Verbatim, VersionOrUrlRef};
use pep508_rs::{split_scheme, Scheme};
use pypi_types::{HashDigest, Metadata23};
use uv_normalize::{ExtraName, PackageName};

Expand Down Expand Up @@ -32,19 +33,52 @@ impl AnnotatedDist {
/// This typically results in a PEP 508 representation of the requirement, but will write an
/// unnamed requirement for relative paths, which can't be represented with PEP 508 (but are
/// supported in `requirements.txt`).
pub(crate) fn to_requirements_txt(&self) -> Cow<str> {
// If the URL is not _definitively_ an absolute `file://` URL, write it as a relative
// path.
pub(crate) fn to_requirements_txt(&self, include_extras: bool) -> Cow<str> {
// If the URL is not _definitively_ an absolute `file://` URL, write it as a relative path.
if let VersionOrUrlRef::Url(url) = self.dist.version_or_url() {
let given = url.verbatim();
if !given.strip_prefix("file://").is_some_and(|path| {
path.starts_with("${PROJECT_ROOT}") || Path::new(path).is_absolute()
}) {
return given;
match split_scheme(&given) {
Some((scheme, path)) => {
match Scheme::parse(scheme) {
Some(Scheme::File) => {
if path
.strip_prefix("//localhost")
.filter(|path| path.starts_with('/'))
.is_some()
{
// Always absolute; nothing to do.
} else if let Some(path) = path.strip_prefix("//") {
// Strip the prefix, to convert, e.g., `file://flask-3.0.3-py3-none-any.whl` to `flask-3.0.3-py3-none-any.whl`.
//
// However, we should allow any of the following:
// - `file://flask-3.0.3-py3-none-any.whl`
// - `file://C:\Users\user\flask-3.0.3-py3-none-any.whl`
// - `file:///C:\Users\user\flask-3.0.3-py3-none-any.whl`
if !path.starts_with("${PROJECT_ROOT}")
&& !Path::new(path).has_root()
{
return Cow::Owned(path.to_string());
}
} else {
// Ex) `file:./flask-3.0.3-py3-none-any.whl`
return given;
}
}
Some(_) => {}
None => {
// Ex) `flask @ C:\Users\user\flask-3.0.3-py3-none-any.whl`
return given;
}
}
}
None => {
// Ex) `flask @ flask-3.0.3-py3-none-any.whl`
return given;
}
}
}

if self.extras.is_empty() {
if self.extras.is_empty() || !include_extras {
self.dist.verbatim()
} else {
let mut extras = self.extras.clone();
Expand All @@ -58,20 +92,6 @@ impl AnnotatedDist {
))
}
}

/// Return the [`AnnotatedDist`] without any extras.
pub(crate) fn without_extras(&self) -> Cow<AnnotatedDist> {
if self.extras.is_empty() {
Cow::Borrowed(self)
} else {
Cow::Owned(AnnotatedDist {
dist: self.dist.clone(),
extras: Vec::new(),
hashes: self.hashes.clone(),
metadata: self.metadata.clone(),
})
}
}
}

impl Name for AnnotatedDist {
Expand Down
Loading

0 comments on commit 610c92d

Please sign in to comment.