Skip to content

Commit

Permalink
Discard markers on editable requirements (#3622)
Browse files Browse the repository at this point in the history
## Summary

If a user includes markers after an editable, we now ignore them (rather
than including them in the parsed URL). This matches pip's behavior. In
the future, we could further improve by respecting them, but that
_would_ be a deviation from pip.

For example, given:

```
-e ./scripts/packages/black_editable ; python_version >= "3.9" and python_ver
```

We now split at the first whitespace (just before the `;`), parse
everything before, and throw out everything after.

This logic also extends to extras. So given:

```
-e ./scripts/packages/black_editable[dev, colorama]
```

We'll now parse this as the URL
`./scripts/packages/black_editable[dev,`, and throw out ` colorama]`.
Instead, you need to do:

```
-e ./scripts/packages/black_editable[dev,colorama]
```

(I.e., remove the space.)

This _also_ matches pip's behavior. I could "fix" this but I'm unsure if
I should -- it means requirements files will be parseable by uv that
won't work with pip. Open to input. My gut reaction is that we _should_
properly support `-e ./scripts/packages/black_editable[dev, colorama]`
even if pip would reject it, but `requirements.txt` is
implementation-defined so it'd be a "deviation".

Closes #3604.
  • Loading branch information
charliermarsh authored May 16, 2024
1 parent 3b8e8de commit 7f73f7b
Show file tree
Hide file tree
Showing 5 changed files with 431 additions and 102 deletions.
34 changes: 31 additions & 3 deletions crates/requirements-txt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ impl EditableRequirement {
origin: Option<&Path>,
working_dir: impl AsRef<Path>,
) -> Result<Self, RequirementsTxtParserError> {
// Identify (but discard) any markers, to match pip.
let given = Self::split_markers(given)
.map(|(requirement, _)| requirement)
.unwrap_or(given);

// Identify the extras.
let (requirement, extras) = if let Some((requirement, extras)) = Self::split_extras(given) {
let extras = Extras::parse(extras).map_err(|err| {
Expand Down Expand Up @@ -300,6 +305,28 @@ impl EditableRequirement {

Some(given.split_at(index))
}

/// Identify the markers in an editable URL (e.g., `../editable ; python_version > "3.8"`).
pub fn split_markers(given: &str) -> Option<(&str, &str)> {
// Take until we see whitespace, unless it's escaped with a backslash, or within brackets
// (which would indicate an extra).
let mut backslash = false;
let mut depth = 0;
for (index, c) in given.char_indices() {
if backslash {
backslash = false;
} else if c == '\\' {
backslash = true;
} else if c == '[' {
depth += 1;
} else if c == ']' {
depth -= 1;
} else if depth == 0 && c.is_whitespace() {
return Some(given.split_at(index));
}
}
None
}
}

impl Display for EditableRequirement {
Expand Down Expand Up @@ -1427,7 +1454,6 @@ mod test {
#[test_case(Path::new("poetry-with-hashes.txt"))]
#[test_case(Path::new("small.txt"))]
#[test_case(Path::new("whitespace.txt"))]
#[test_case(Path::new("editable.txt"))]
#[tokio::test]
async fn line_endings(path: &Path) {
let working_dir = workspace_test_data_dir().join("requirements-txt");
Expand Down Expand Up @@ -1469,8 +1495,9 @@ mod test {

#[cfg(unix)]
#[test_case(Path::new("bare-url.txt"))]
#[test_case(Path::new("editable.txt"))]
#[tokio::test]
async fn parse_unnamed_unix(path: &Path) {
async fn parse_unix(path: &Path) {
let working_dir = workspace_test_data_dir().join("requirements-txt");
let requirements_txt = working_dir.join(path);

Expand All @@ -1490,8 +1517,9 @@ mod test {

#[cfg(windows)]
#[test_case(Path::new("bare-url.txt"))]
#[test_case(Path::new("editable.txt"))]
#[tokio::test]
async fn parse_unnamed_windows(path: &Path) {
async fn parse_windows(path: &Path) {
let working_dir = workspace_test_data_dir().join("requirements-txt");
let requirements_txt = working_dir.join(path);

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
---
source: crates/requirements-txt/src/lib.rs
expression: actual
---
RequirementsTxt {
requirements: [],
constraints: [],
editables: [
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
given: Some(
"./editable",
),
},
extras: [
ExtraName(
"d",
),
ExtraName(
"dev",
),
],
path: "<REQUIREMENTS_DIR>/editable",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
given: Some(
"./editable",
),
},
extras: [
ExtraName(
"d",
),
ExtraName(
"dev",
),
],
path: "<REQUIREMENTS_DIR>/editable",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
given: Some(
"./editable",
),
},
extras: [
ExtraName(
"d",
),
ExtraName(
"dev",
),
],
path: "<REQUIREMENTS_DIR>/editable",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
given: Some(
"./editable",
),
},
extras: [
ExtraName(
"d",
),
ExtraName(
"dev",
),
],
path: "<REQUIREMENTS_DIR>/editable",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
given: Some(
"./editable",
),
},
extras: [],
path: "<REQUIREMENTS_DIR>/editable",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
EditableRequirement {
url: VerbatimUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable;",
query: None,
fragment: None,
},
given: Some(
"./editable;",
),
},
extras: [],
path: "<REQUIREMENTS_DIR>/editable;",
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
],
index_url: None,
extra_index_urls: [],
find_links: [],
no_index: false,
no_binary: None,
only_binary: None,
}
Loading

0 comments on commit 7f73f7b

Please sign in to comment.