Allow semicolons directly after direct URLs (#8836)

## Summary

Like pip, we now allow the semicolon to directly proceed the URL (but
require that it's either preceded or followed by a space):

```
# OK
./test.whl; sys_platform == 'darwin'

# OK
./test.whl ;sys_platform == 'darwin'

# Error
./test.whl;sys_platform == 'darwin'
```

Closes https://github.com/astral-sh/uv/issues/8831.
This commit is contained in:
Charlie Marsh 2024-11-05 16:07:07 -05:00 committed by GitHub
parent 515993c743
commit d238642d76
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 290 additions and 114 deletions

View file

@ -709,18 +709,17 @@ fn parse_url<T: Pep508Url>(
len += c.len_utf8();
// If we see a top-level semicolon or hash followed by whitespace, we're done.
match c {
';' if cursor.peek_char().is_some_and(char::is_whitespace) => {
if cursor.peek_char().is_some_and(|c| matches!(c, ';' | '#')) {
let mut cursor = cursor.clone();
cursor.next();
if cursor.peek_char().is_some_and(char::is_whitespace) {
break;
}
'#' if cursor.peek_char().is_some_and(char::is_whitespace) => {
break;
}
_ => {}
}
}
(start, len)
};
let url = cursor.slice(start, len);
if url.is_empty() {
return Err(Pep508Error {
@ -731,19 +730,6 @@ fn parse_url<T: Pep508Url>(
});
}
for c in [';', '#'] {
if url.ends_with(c) {
return Err(Pep508Error {
message: Pep508ErrorSource::String(format!(
"Missing space before '{c}', the end of the URL is ambiguous"
)),
start: start + len - 1,
len: 1,
input: cursor.to_string(),
});
}
}
let url = T::parse_url(url, working_dir).map_err(|err| Pep508Error {
message: Pep508ErrorSource::UrlError(err),
start,
@ -970,8 +956,13 @@ fn parse_pep508_requirement<T: Pep508Url>(
// wsp*
cursor.eat_whitespace();
if let Some((pos, char)) = cursor.next() {
let message = if marker.is_none() {
if let Some((pos, char)) = cursor.next().filter(|(_, c)| *c != '#') {
let message = if char == '#' {
format!(
r#"Expected end of input or `;`, found `{char}`; comments must be preceded by a leading space"#
)
} else if marker.is_none() {
format!(r#"Expected end of input or `;`, found `{char}`"#)
} else {
format!(r#"Expected end of input, found `{char}`"#)

View file

@ -545,42 +545,6 @@ fn error_extras_not_closed() {
);
}
#[test]
fn error_no_space_after_url() {
assert_snapshot!(
parse_pep508_err(r"name @ https://example.com/; extra == 'example'"),
@r#"
Missing space before ';', the end of the URL is ambiguous
name @ https://example.com/; extra == 'example'
^
"#
);
}
#[test]
fn error_no_space_after_file_url() {
assert_snapshot!(
parse_pep508_err(r"name @ file:///test.whl; extra == 'example'"),
@r###"
Missing space before ';', the end of the URL is ambiguous
name @ file:///test.whl; extra == 'example'
^
"###
);
}
#[test]
fn error_no_space_after_file_path() {
assert_snapshot!(
parse_pep508_err(r"name @ ./test.whl; extra == 'example'"),
@r###"
Missing space before ';', the end of the URL is ambiguous
name @ ./test.whl; extra == 'example'
^
"###
);
}
#[test]
fn error_name_at_nothing() {
assert_snapshot!(

View file

@ -174,7 +174,11 @@ fn parse_unnamed_requirement<Url: UnnamedRequirementUrl>(
// wsp*
cursor.eat_whitespace();
if let Some((pos, char)) = cursor.next() {
let message = if marker.is_none() {
let message = if char == '#' {
format!(
r#"Expected end of input or `;`, found `{char}`; comments must be preceded by a leading space"#
)
} else if marker.is_none() {
format!(r#"Expected end of input or `;`, found `{char}`"#)
} else {
format!(r#"Expected end of input, found `{char}`"#)
@ -388,15 +392,11 @@ fn parse_unnamed_url<Url: UnnamedRequirementUrl>(
len += c.len_utf8();
// If we see a top-level semicolon or hash followed by whitespace, we're done.
if depth == 0 {
match c {
';' if cursor.peek_char().is_some_and(char::is_whitespace) => {
break;
}
'#' if cursor.peek_char().is_some_and(char::is_whitespace) => {
break;
}
_ => {}
if depth == 0 && cursor.peek_char().is_some_and(|c| matches!(c, ';' | '#')) {
let mut cursor = cursor.clone();
cursor.next();
if cursor.peek_char().is_some_and(char::is_whitespace) {
break;
}
}
}
@ -413,19 +413,6 @@ fn parse_unnamed_url<Url: UnnamedRequirementUrl>(
});
}
for c in [';', '#'] {
if url.ends_with(c) {
return Err(Pep508Error {
message: Pep508ErrorSource::String(format!(
"Missing space before '{c}', the end of the URL is ambiguous"
)),
start: start + len - 1,
len: 1,
input: cursor.to_string(),
});
}
}
let url = preprocess_unnamed_url(url, working_dir, cursor, start, len)?;
Ok(url)

View file

@ -334,6 +334,106 @@ RequirementsTxt {
),
hashes: [],
},
RequirementEntry {
requirement: Unnamed(
UnnamedRequirement {
url: VerbatimParsedUrl {
parsed_url: Directory(
ParsedDirectoryUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: 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: [],
marker: true,
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
),
hashes: [],
},
RequirementEntry {
requirement: Unnamed(
UnnamedRequirement {
url: VerbatimParsedUrl {
parsed_url: Directory(
ParsedDirectoryUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: 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: [],
marker: true,
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
),
hashes: [],
},
],
index_url: None,
extra_index_urls: [],

View file

@ -7,7 +7,7 @@ RequirementsTxtFileError {
error: Pep508 {
source: Pep508Error {
message: String(
"Missing space before '#', the end of the URL is ambiguous",
"Expected end of input or `;`, found `#`; comments must be preceded by a leading space",
),
start: 10,
len: 1,

View file

@ -6,14 +6,17 @@ RequirementsTxtFileError {
file: "<REQUIREMENTS_DIR>/semicolon.txt",
error: Pep508 {
source: Pep508Error {
message: String(
"Missing space before ';', the end of the URL is ambiguous",
message: UrlError(
MissingExtensionPath(
"./editable;python_version >= \"3.9\" and os_name == \"posix\"",
Dist,
),
),
start: 10,
len: 1,
input: "./editable; python_version >= \"3.9\" and os_name == \"posix\"",
start: 0,
len: 57,
input: "./editable;python_version >= \"3.9\" and os_name == \"posix\"",
},
start: 50,
end: 108,
end: 107,
},
}

View file

@ -1,5 +1,5 @@
---
source: crates/requirements-txt/src/lib.rs
source: crates/uv-requirements-txt/src/lib.rs
expression: actual
---
RequirementsTxt {
@ -334,6 +334,106 @@ RequirementsTxt {
),
hashes: [],
},
RequirementEntry {
requirement: Unnamed(
UnnamedRequirement {
url: VerbatimParsedUrl {
parsed_url: Directory(
ParsedDirectoryUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: 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: [],
marker: true,
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
),
hashes: [],
},
RequirementEntry {
requirement: Unnamed(
UnnamedRequirement {
url: VerbatimParsedUrl {
parsed_url: Directory(
ParsedDirectoryUrl {
url: Url {
scheme: "file",
cannot_be_a_base: false,
username: "",
password: None,
host: None,
port: None,
path: "/<REQUIREMENTS_DIR>/editable",
query: None,
fragment: None,
},
install_path: "<REQUIREMENTS_DIR>/editable",
editable: true,
virtual: false,
},
),
verbatim: 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: [],
marker: true,
origin: Some(
File(
"<REQUIREMENTS_DIR>/editable.txt",
),
),
},
),
hashes: [],
},
],
index_url: None,
extra_index_urls: [],

View file

@ -15,3 +15,9 @@
# OK (unterminated)
-e ./editable[d
# OK
-e ./editable # comment
# OK
-e ./editable #comment

View file

@ -1,2 +1,2 @@
# Disallowed (missing whitespace before colon)
-e ./editable; python_version >= "3.9" and os_name == "posix"
-e ./editable;python_version >= "3.9" and os_name == "posix"

View file

@ -295,31 +295,6 @@ dependencies = ["flask==1.0.x"]
Ok(())
}
#[test]
fn trailing_semicolon() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str("./flask.whl; sys_platform == 'win32'")?;
uv_snapshot!(context.pip_install()
.arg("-r")
.arg("requirements.txt")
.arg("--strict"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Couldn't parse requirement in `requirements.txt` at position 0
Caused by: Missing space before ';', the end of the URL is ambiguous
./flask.whl; sys_platform == 'win32'
^
"###
);
Ok(())
}
#[test]
fn missing_pip() {
uv_snapshot!(Command::new(get_bin()).arg("install"), @r###"

View file

@ -5638,3 +5638,53 @@ fn sanitize() -> Result<()> {
Ok(())
}
/// Allow semicolons attached to markers, as long as they're preceded by a space.
#[test]
fn semicolon_trailing_space() -> Result<()> {
let context = TestContext::new("3.12");
let requirements = context.temp_dir.child("requirements.txt");
requirements.write_str("iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl; python_version > '3.10'")?;
uv_snapshot!(context.pip_sync()
.arg("requirements.txt"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0 (from https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl)
"###
);
Ok(())
}
/// Treat a semicolon that's not whitespace-separated as a part of the URL.
#[test]
fn semicolon_no_space() -> Result<()> {
let context = TestContext::new("3.12");
let requirements = context.temp_dir.child("requirements.txt");
requirements.write_str("iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl;python_version > '3.10'")?;
uv_snapshot!(context.pip_sync()
.arg("requirements.txt"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Couldn't parse requirement in `requirements.txt` at position 0
Caused by: Expected direct URL (`https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl;python_version%20%3E%20'3.10'`) to end in a supported file extension: `.whl`, `.tar.gz`, `.zip`, `.tar.bz2`, `.tar.lz`, `.tar.lzma`, `.tar.xz`, `.tar.zst`, `.tar`, `.tbz`, `.tgz`, `.tlz`, or `.txz`
iniconfig @ https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl;python_version > '3.10'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"###
);
Ok(())
}