Skip to content

Commit

Permalink
Unify editable and unnamed URL parsing (#3946)
Browse files Browse the repository at this point in the history
## Summary

This will help prevent bugs like #3934 by unifying the implementations
for editables and non-editable unnamed requirements. Specifically, both
of these now go through the same parsing paths and use the same struct
representations (with the exception that the editable flag is flipped in
the first case):

```
-e ./foo/bar
./foo/bar
```

We also now support PEP 508 in editable URLs. It turns out this is just
a limitation in pip, so it's correct to support it. For example, this
now works:

```
-e black[d] @ file://${PROJECT_ROOT}/scripts/packages/black_editable
```

Closes #3941.

Closes #3942.
  • Loading branch information
charliermarsh committed May 31, 2024
1 parent 8c11f99 commit 7b7da80
Show file tree
Hide file tree
Showing 12 changed files with 1,137 additions and 831 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 38 additions & 1 deletion crates/pep508-rs/src/unnamed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ fn parse_unnamed_requirement<Url: UnnamedRequirementUrl>(

/// Create a `VerbatimUrl` to represent the requirement, and extracts any extras at the end of the
/// URL, to comply with the non-PEP 508 extensions.
///
/// For example:
/// - `file:///home/ferris/project/scripts/...`
/// - `file:../editable/`
/// - `../editable/`
/// - `https://download.pytorch.org/whl/torch_stable.html`
fn preprocess_unnamed_url<Url: UnnamedRequirementUrl>(
url: &str,
#[cfg_attr(not(feature = "non-pep508-extensions"), allow(unused))] working_dir: Option<&Path>,
Expand Down Expand Up @@ -356,8 +362,39 @@ fn parse_unnamed_url<Url: UnnamedRequirementUrl>(
) -> Result<(Url, Vec<ExtraName>), Pep508Error<Url>> {
// wsp*
cursor.eat_whitespace();

// <URI_reference>
let (start, len) = cursor.take_while(|char| !char.is_whitespace());
let (start, len) = {
let start = cursor.pos();
let mut len = 0;
let mut backslash = false;
let mut depth = 0u32;
while let Some((_, c)) = cursor.next() {
if backslash {
backslash = false;
} else if c == '\\' {
backslash = true;
} else if c == '[' {
depth = depth.saturating_add(1);
} else if c == ']' {
depth = depth.saturating_sub(1);
}

// If we see top-level whitespace, we're done.
if depth == 0 && c.is_whitespace() {
break;
}

// If we see a line break, we're done.
if matches!(c, '\r' | '\n') {
break;
}

len += c.len_utf8();
}
(start, len)
};

let url = cursor.slice(start, len);
if url.is_empty() {
return Err(Pep508Error {
Expand Down
1 change: 1 addition & 0 deletions crates/requirements-txt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ fs-err = { workspace = true }
regex = { workspace = true }
reqwest = { workspace = true, optional = true }
reqwest-middleware = { workspace = true, optional = true }
thiserror = { workspace = true }
tracing = { workspace = true }
unscanny = { workspace = true }
url = { workspace = true }
Expand Down
Loading

0 comments on commit 7b7da80

Please sign in to comment.