Show fully-resolved URLs in non-resolution contexts (#689)

We now show the fully-resolved URL, rather than the URL as given by the
user, _everywhere_ except for the output resolution file (which should
retain relative paths, unexpanded environment variables, etc.).

Closes https://github.com/astral-sh/puffin/issues/687.
This commit is contained in:
Charlie Marsh 2023-12-18 17:10:24 -05:00 committed by GitHub
parent 43c837f7bb
commit 365c860e27
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 123 additions and 39 deletions

View file

@ -1,10 +1,13 @@
use std::fmt::{Display, Formatter};
use std::borrow::Cow;
use std::path::{Path, PathBuf};
use url::Url;
use pep508_rs::VerbatimUrl;
use requirements_txt::EditableRequirement;
use crate::Verbatim;
#[derive(Debug, Clone)]
pub struct LocalEditable {
pub requirement: EditableRequirement,
@ -13,21 +16,30 @@ pub struct LocalEditable {
}
impl LocalEditable {
/// Return the [`VerbatimUrl`] of the editable.
pub fn url(&self) -> &VerbatimUrl {
self.requirement.url()
}
/// Return the underlying [`Url`] of the editable.
pub fn raw(&self) -> &Url {
self.requirement.raw()
}
/// Return the resolved path to the editable.
pub fn path(&self) -> &Path {
&self.path
}
}
impl Display for LocalEditable {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
impl Verbatim for LocalEditable {
fn verbatim(&self) -> Cow<'_, str> {
self.url().verbatim()
}
}
impl std::fmt::Display for LocalEditable {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.requirement.fmt(f)
}
}

View file

@ -36,6 +36,7 @@
//! Since we read this information from [`direct_url.json`](https://packaging.python.org/en/latest/specifications/direct-url-data-structure/), it doesn't match the information [`Dist`] exactly.
//!
//! TODO(konstin): Track all kinds from [`Dist`].
use std::borrow::Cow;
use std::path::{Path, PathBuf};
use std::str::FromStr;
@ -76,6 +77,15 @@ pub enum VersionOrUrl<'a> {
Url(&'a VerbatimUrl),
}
impl Verbatim for VersionOrUrl<'_> {
fn verbatim(&self) -> Cow<'_, str> {
match self {
VersionOrUrl::Version(version) => Cow::Owned(format!("=={version}")),
VersionOrUrl::Url(url) => Cow::Owned(format!(" @ {}", url.verbatim())),
}
}
}
impl std::fmt::Display for VersionOrUrl<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {

View file

@ -1,4 +1,8 @@
use std::borrow::Cow;
use anyhow::Result;
use pep508_rs::VerbatimUrl;
use puffin_cache::CanonicalUrl;
use puffin_normalize::PackageName;
@ -57,7 +61,32 @@ pub trait Identifier {
fn resource_id(&self) -> ResourceId;
}
// Implement `Display` for all known types that implement `DistributionIdentifier`.
pub trait Verbatim {
/// Return the verbatim representation of the distribution.
fn verbatim(&self) -> Cow<'_, str>;
}
impl Verbatim for VerbatimUrl {
fn verbatim(&self) -> Cow<'_, str> {
if let Some(given) = self.given() {
Cow::Borrowed(given)
} else {
Cow::Owned(self.to_string())
}
}
}
impl<T: Metadata> Verbatim for T {
fn verbatim(&self) -> Cow<'_, str> {
Cow::Owned(format!(
"{}{}",
self.name(),
self.version_or_url().verbatim()
))
}
}
// Implement `Display` for all known types that implement `Metadata`.
impl std::fmt::Display for AnyDist {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}{}", self.name(), self.version_or_url())

View file

@ -37,6 +37,11 @@ impl VerbatimUrl {
})
}
/// Return the original string as given by the user, if available.
pub fn given(&self) -> Option<&str> {
self.given.as_deref()
}
/// Return the underlying [`Url`].
pub fn raw(&self) -> &Url {
&self.url
@ -66,11 +71,7 @@ impl std::str::FromStr for VerbatimUrl {
impl std::fmt::Display for VerbatimUrl {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
if let Some(given) = &self.given {
given.fmt(f)
} else {
self.url.fmt(f)
}
self.url.fmt(f)
}
}

View file

@ -8,7 +8,7 @@ use insta_cmd::get_cargo_bin;
use std::path::PathBuf;
pub(crate) const BIN_NAME: &str = "puffin";
// Not all tests use them and cargo warns otherwise
pub(crate) const INSTA_FILTERS: &[(&str, &str)] = &[
(r"(\d+\.)?\d+(ms|s)", "[TIME]"),
(r"--cache-dir .*", "--cache-dir [CACHE_DIR]"),

View file

@ -432,9 +432,16 @@ fn install_editable() -> Result<()> {
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let current_dir = std::env::current_dir()?
.join("..")
.join("..")
.canonicalize()?;
let mut filters = INSTA_FILTERS.to_vec();
filters.push((current_dir.to_str().unwrap(), "[CURRENT_DIR]"));
// Install the editable package.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -454,13 +461,13 @@ fn install_editable() -> Result<()> {
Downloaded 1 package in [TIME]
Installed 2 packages in [TIME]
+ numpy==1.26.2
+ poetry-editable @ ../../scripts/editable-installs/poetry_editable
+ poetry-editable @ file://[CURRENT_DIR]/scripts/editable-installs/poetry_editable/
"###);
});
// Install it again (no-op).
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -481,7 +488,7 @@ fn install_editable() -> Result<()> {
// Add another, non-editable dependency.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -514,14 +521,14 @@ fn install_editable() -> Result<()> {
+ pathspec==0.12.1
+ platformdirs==4.1.0
- poetry-editable==0.1.0
+ poetry-editable @ ../../scripts/editable-installs/poetry_editable
+ poetry-editable @ file://[CURRENT_DIR]/scripts/editable-installs/poetry_editable/
+ yarl==1.9.4
"###);
});
// Add another, editable dependency.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -542,9 +549,9 @@ fn install_editable() -> Result<()> {
Built 2 editables in [TIME]
Resolved 16 packages in [TIME]
Installed 2 packages in [TIME]
+ maturin-editable @ ../../scripts/editable-installs/maturin_editable
+ maturin-editable @ file://[CURRENT_DIR]/scripts/editable-installs/maturin_editable/
- poetry-editable==0.1.0
+ poetry-editable @ ../../scripts/editable-installs/poetry_editable
+ poetry-editable @ file://[CURRENT_DIR]/scripts/editable-installs/poetry_editable/
"###);
});
@ -557,9 +564,16 @@ fn install_editable_and_registry() -> Result<()> {
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let current_dir = std::env::current_dir()?
.join("..")
.join("..")
.canonicalize()?;
let mut filters = INSTA_FILTERS.to_vec();
filters.push((current_dir.to_str().unwrap(), "[CURRENT_DIR]"));
// Install the registry-based version of Black.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -594,7 +608,7 @@ fn install_editable_and_registry() -> Result<()> {
// Install the editable version of Black. This should remove the registry-based version.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -613,14 +627,14 @@ fn install_editable_and_registry() -> Result<()> {
Resolved 1 package in [TIME]
Installed 1 package in [TIME]
- black==23.12.0
+ black @ ../../scripts/editable-installs/black_editable
+ black @ file://[CURRENT_DIR]/scripts/editable-installs/black_editable/
"###);
});
// Re-install the registry-based version of Black. This should be a no-op, since we have a
// version of Black installed (the editable version) that satisfies the requirements.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")
@ -640,7 +654,7 @@ fn install_editable_and_registry() -> Result<()> {
// Re-install Black at a specific version. This should replace the editable version.
insta::with_settings!({
filters => INSTA_FILTERS.to_vec()
filters => filters.clone()
}, {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.arg("pip-install")

View file

@ -2092,7 +2092,9 @@ fn sync_editable() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let current_dir = std::env::current_dir()?;
let workspace_dir = current_dir.join("..").join("..").canonicalize()?;
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(&formatdoc! {r"
@ -2114,6 +2116,7 @@ fn sync_editable() -> Result<()> {
r"file://.*/../../scripts/editable-installs/poetry_editable",
"file://[TEMP_DIR]/../../scripts/editable-installs/poetry_editable",
),
(workspace_dir.to_str().unwrap(), "[CURRENT_DIR]"),
])
.copied()
.collect::<Vec<_>>();
@ -2139,9 +2142,9 @@ fn sync_editable() -> Result<()> {
Downloaded 2 packages in [TIME]
Installed 4 packages in [TIME]
+ boltons==23.1.1
+ maturin-editable @ ../../scripts/editable-installs/maturin_editable
+ maturin-editable @ file://[CURRENT_DIR]/scripts/editable-installs/maturin_editable/
+ numpy==1.26.2
+ poetry-editable @ file://[TEMP_DIR]/../../scripts/editable-installs/poetry_editable
+ poetry-editable @ file://[CURRENT_DIR]/scripts/editable-installs/poetry_editable
"###);
});
@ -2167,7 +2170,7 @@ fn sync_editable() -> Result<()> {
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- poetry-editable==0.1.0
+ poetry-editable @ file://[TEMP_DIR]/../../scripts/editable-installs/poetry_editable
+ poetry-editable @ file://[CURRENT_DIR]/scripts/editable-installs/poetry_editable
"###);
});
@ -2182,7 +2185,7 @@ fn sync_editable() -> Result<()> {
let command = indoc! {r#"
from maturin_editable import sum_as_string, version
assert version == 1, version
assert sum_as_string(1, 2) == "3", sum_as_string(1, 2)
"#};
@ -2198,7 +2201,7 @@ fn sync_editable() -> Result<()> {
let command = indoc! {r#"
from maturin_editable import sum_as_string, version
from pathlib import Path
assert version == 2, version
assert sum_as_string(1, 2) == "3", sum_as_string(1, 2)
"#};
@ -2235,6 +2238,9 @@ fn sync_editable_and_registry() -> Result<()> {
let cache_dir = assert_fs::TempDir::new()?;
let venv = create_venv_py312(&temp_dir, &cache_dir);
let current_dir = std::env::current_dir()?;
let workspace_dir = current_dir.join("..").join("..").canonicalize()?;
// Install the registry-based version of Black.
let requirements_txt = temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
@ -2245,7 +2251,10 @@ fn sync_editable_and_registry() -> Result<()> {
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.chain(&[
(filter_path.as_str(), "requirements.txt"),
(workspace_dir.to_str().unwrap(), "[CURRENT_DIR]"),
])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
@ -2286,7 +2295,10 @@ fn sync_editable_and_registry() -> Result<()> {
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.chain(&[
(filter_path.as_str(), "requirements.txt"),
(workspace_dir.to_str().unwrap(), "[CURRENT_DIR]"),
])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
@ -2308,7 +2320,7 @@ fn sync_editable_and_registry() -> Result<()> {
Uninstalled 1 package in [TIME]
Installed 1 package in [TIME]
- black==24.1a1
+ black @ ../../scripts/editable-installs/black_editable
+ black @ file://[CURRENT_DIR]/scripts/editable-installs/black_editable/
"###);
});
@ -2323,7 +2335,10 @@ fn sync_editable_and_registry() -> Result<()> {
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.chain(&[
(filter_path.as_str(), "requirements.txt"),
(workspace_dir.to_str().unwrap(), "[CURRENT_DIR]"),
])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({
@ -2355,7 +2370,10 @@ fn sync_editable_and_registry() -> Result<()> {
let filter_path = requirements_txt.display().to_string();
let filters = INSTA_FILTERS
.iter()
.chain(&[(filter_path.as_str(), "requirements.txt")])
.chain(&[
(filter_path.as_str(), "requirements.txt"),
(workspace_dir.to_str().unwrap(), "[CURRENT_DIR]"),
])
.copied()
.collect::<Vec<_>>();
insta::with_settings!({

View file

@ -10,7 +10,7 @@ use pubgrub::type_aliases::SelectedDependencies;
use rustc_hash::FxHashMap;
use url::Url;
use distribution_types::{Dist, LocalEditable, Metadata, PackageId};
use distribution_types::{Dist, LocalEditable, Metadata, PackageId, Verbatim};
use pep440_rs::Version;
use pep508_rs::{Requirement, VerbatimUrl};
use puffin_normalize::{ExtraName, PackageName};
@ -212,9 +212,9 @@ impl std::fmt::Display for ResolutionGraph {
// Print out the dependency graph.
for (index, package) in nodes {
if let Some((editable, _)) = self.editables.get(package.name()) {
writeln!(f, "-e {editable}")?;
writeln!(f, "-e {}", editable.verbatim())?;
} else {
writeln!(f, "{package}")?;
writeln!(f, "{}", package.verbatim())?;
}
let mut edges = self

View file

@ -83,7 +83,7 @@ impl EditableRequirement {
}
}
/// Return the underlying [`Url`].
/// Return the underlying [`Url`] of the editable.
pub fn raw(&self) -> &Url {
match self {
EditableRequirement::Path { url, .. } => url.raw(),
@ -773,7 +773,7 @@ mod test {
.map(ToString::to_string)
.collect::<Vec<_>>();
let expected = &[
"Unsupported URL (expected a `file://` scheme) in ./test-data/requirements-txt/unsupported-editable.txt: http://localhost:8080".to_string()
"Unsupported URL (expected a `file://` scheme) in ./test-data/requirements-txt/unsupported-editable.txt: http://localhost:8080/".to_string()
];
assert_eq!(errors, expected);
}

View file

@ -1 +1 @@
-e http://localhost:8080
-e http://localhost:8080/