Bias towards local directories for bare editable requirements (#3995)

## Summary

Given `install -e dagster`, we need to assume that the user meant
`install -e ./dagster`, even though `install dagster` should _not_ be
treated as `install ./dagster`. I suspect pip will change this in the
future (since `pip install dagster` does _not_ meant `pip install
./dagster`) but for now it's what users expect.

Closes https://github.com/astral-sh/uv/issues/3994.
This commit is contained in:
Charlie Marsh 2024-06-04 15:37:05 -04:00 committed by GitHub
parent 420333a40e
commit 8eea470d49
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 64 additions and 19 deletions

View File

@ -44,7 +44,6 @@ use tracing::instrument;
use unscanny::{Pattern, Scanner};
use url::Url;
use crate::requirement::EditableError;
use distribution_types::{UnresolvedRequirement, UnresolvedRequirementSpecification};
use pep508_rs::{
expand_env_vars, split_scheme, strip_host, Pep508Error, RequirementOrigin, Scheme, VerbatimUrl,
@ -57,11 +56,12 @@ use uv_configuration::{NoBinary, NoBuild, PackageNameSpecifier};
use uv_fs::{normalize_url_path, Simplified};
use uv_warnings::warn_user;
use crate::requirement::EditableError;
pub use crate::requirement::RequirementsTxtRequirement;
mod requirement;
/// We emit one of those for each requirements.txt entry
/// We emit one of those for each `requirements.txt` entry.
enum RequirementsTxtStatement {
/// `-r` inclusion filename
Requirements {
@ -500,7 +500,8 @@ fn parse_entry(
Some(requirements_txt)
};
let (requirement, hashes) = parse_requirement_and_hashes(s, content, source, working_dir)?;
let (requirement, hashes) =
parse_requirement_and_hashes(s, content, source, working_dir, true)?;
let requirement =
requirement
.into_editable()
@ -579,7 +580,8 @@ fn parse_entry(
Some(requirements_txt)
};
let (requirement, hashes) = parse_requirement_and_hashes(s, content, source, working_dir)?;
let (requirement, hashes) =
parse_requirement_and_hashes(s, content, source, working_dir, false)?;
RequirementsTxtStatement::RequirementEntry(RequirementEntry {
requirement,
hashes,
@ -643,6 +645,7 @@ fn parse_requirement_and_hashes(
content: &str,
source: Option<&Path>,
working_dir: &Path,
editable: bool,
) -> Result<(RequirementsTxtRequirement, Vec<String>), RequirementsTxtParserError> {
// PEP 508 requirement
let start = s.cursor();
@ -699,7 +702,7 @@ fn parse_requirement_and_hashes(
}
}
let requirement = RequirementsTxtRequirement::parse(requirement, working_dir)
let requirement = RequirementsTxtRequirement::parse(requirement, working_dir, editable)
.map(|requirement| {
if let Some(source) = source {
requirement.with_origin(RequirementOrigin::File(source.to_path_buf()))

View File

@ -119,10 +119,22 @@ impl RequirementsTxtRequirement {
pub fn parse(
input: &str,
working_dir: impl AsRef<Path>,
editable: bool,
) -> Result<Self, Box<Pep508Error<VerbatimParsedUrl>>> {
// Attempt to parse as a PEP 508-compliant requirement.
match pep508_rs::Requirement::parse(input, &working_dir) {
Ok(requirement) => Ok(Self::Named(requirement)),
Ok(requirement) => {
// As a special-case, interpret `dagster` as `./dagster` if we're in editable mode.
if editable && requirement.version_or_url.is_none() {
Ok(Self::Unnamed(UnnamedRequirement::parse(
input,
&working_dir,
&mut TracingReporter,
)?))
} else {
Ok(Self::Named(requirement))
}
}
Err(err) => match err.message {
Pep508ErrorSource::UnsupportedRequirement(_) => {
// If that fails, attempt to parse as a direct URL requirement.

View File

@ -86,16 +86,18 @@ impl RequirementsSpecification {
) -> Result<Self> {
Ok(match source {
RequirementsSource::Package(name) => {
let requirement = RequirementsTxtRequirement::parse(name, std::env::current_dir()?)
.with_context(|| format!("Failed to parse: `{name}`"))?;
let requirement =
RequirementsTxtRequirement::parse(name, std::env::current_dir()?, false)
.with_context(|| format!("Failed to parse: `{name}`"))?;
Self {
requirements: vec![UnresolvedRequirementSpecification::from(requirement)],
..Self::default()
}
}
RequirementsSource::Editable(name) => {
let requirement = RequirementsTxtRequirement::parse(name, std::env::current_dir()?)
.with_context(|| format!("Failed to parse: `{name}`"))?;
let requirement =
RequirementsTxtRequirement::parse(name, std::env::current_dir()?, true)
.with_context(|| format!("Failed to parse: `{name}`"))?;
Self {
requirements: vec![UnresolvedRequirementSpecification::from(
requirement.into_editable()?,

View File

@ -1114,22 +1114,50 @@ fn install_editable_pep_508_cli() {
}
#[test]
fn invalid_editable_no_version() -> Result<()> {
fn install_editable_bare_cli() {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str("-e black")?;
let packages_dir = context.workspace_root.join("scripts/packages");
uv_snapshot!(context.filters(), context.install()
.arg("-r")
.arg("requirements.txt"), @r###"
success: false
exit_code: 2
.arg("-e")
.arg("black_editable")
.current_dir(&packages_dir), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
error: Unsupported editable requirement in `requirements.txt`
Caused by: Editable `black` must refer to a local directory
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
+ black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
"###
);
}
#[test]
fn install_editable_bare_requirements_txt() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str("-e black_editable")?;
let packages_dir = context.workspace_root.join("scripts/packages");
uv_snapshot!(context.filters(), context.install()
.arg("-r")
.arg(requirements_txt.path())
.current_dir(&packages_dir), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
+ black==0.1.0 (from file://[WORKSPACE]/scripts/packages/black_editable)
"###
);