From 7f73f7b3c2bbc85ecd61f8c79983af7d3633881a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 16 May 2024 16:53:52 -0400 Subject: [PATCH] Discard markers on editable requirements (#3622) ## 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 https://github.com/astral-sh/uv/issues/3604. --- crates/requirements-txt/src/lib.rs | 34 ++- ..._txt__test__line-endings-editable.txt.snap | 79 ------- ...ts_txt__test__parse-unix-editable.txt.snap | 194 ++++++++++++++++++ ...txt__test__parse-windows-editable.txt.snap | 194 ++++++++++++++++++ .../test-data/requirements-txt/editable.txt | 32 ++- 5 files changed, 431 insertions(+), 102 deletions(-) delete mode 100644 crates/requirements-txt/src/snapshots/requirements_txt__test__line-endings-editable.txt.snap create mode 100644 crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-editable.txt.snap create mode 100644 crates/requirements-txt/src/snapshots/requirements_txt__test__parse-windows-editable.txt.snap diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index e99e41c3a9c6..82ffb4e8d876 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -199,6 +199,11 @@ impl EditableRequirement { origin: Option<&Path>, working_dir: impl AsRef, ) -> Result { + // 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| { @@ -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 { @@ -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"); @@ -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); @@ -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); diff --git a/crates/requirements-txt/src/snapshots/requirements_txt__test__line-endings-editable.txt.snap b/crates/requirements-txt/src/snapshots/requirements_txt__test__line-endings-editable.txt.snap deleted file mode 100644 index 63c3db751b30..000000000000 --- a/crates/requirements-txt/src/snapshots/requirements_txt__test__line-endings-editable.txt.snap +++ /dev/null @@ -1,79 +0,0 @@ ---- -source: crates/requirements-txt/src/lib.rs -expression: actual ---- -RequirementsTxt { - requirements: [ - RequirementEntry { - requirement: Named( - Requirement { - name: PackageName( - "numpy", - ), - extras: [], - version_or_url: None, - marker: None, - origin: Some( - File( - "/editable.txt", - ), - ), - }, - ), - hashes: [], - }, - RequirementEntry { - requirement: Named( - Requirement { - name: PackageName( - "pandas", - ), - extras: [ - ExtraName( - "tabulate", - ), - ], - version_or_url: Some( - Url( - VerbatimUrl { - url: Url { - scheme: "https", - cannot_be_a_base: false, - username: "", - password: None, - host: Some( - Domain( - "github.com", - ), - ), - port: None, - path: "/pandas-dev/pandas", - query: None, - fragment: None, - }, - given: Some( - "https://github.com/pandas-dev/pandas", - ), - }, - ), - ), - marker: None, - origin: Some( - File( - "/editable.txt", - ), - ), - }, - ), - hashes: [], - }, - ], - constraints: [], - editables: [], - index_url: None, - extra_index_urls: [], - find_links: [], - no_index: false, - no_binary: None, - only_binary: None, -} diff --git a/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-editable.txt.snap b/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-editable.txt.snap new file mode 100644 index 000000000000..11ec84230b76 --- /dev/null +++ b/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-unix-editable.txt.snap @@ -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: "/editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "/editable;", + query: None, + fragment: None, + }, + given: Some( + "./editable;", + ), + }, + extras: [], + path: "/editable;", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + ], + index_url: None, + extra_index_urls: [], + find_links: [], + no_index: false, + no_binary: None, + only_binary: None, +} diff --git a/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-windows-editable.txt.snap b/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-windows-editable.txt.snap new file mode 100644 index 000000000000..16fdf73553e4 --- /dev/null +++ b/crates/requirements-txt/src/snapshots/requirements_txt__test__parse-windows-editable.txt.snap @@ -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: "//editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "//editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "//editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "//editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [ + ExtraName( + "d", + ), + ExtraName( + "dev", + ), + ], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "//editable", + query: None, + fragment: None, + }, + given: Some( + "./editable", + ), + }, + extras: [], + path: "/editable", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + EditableRequirement { + url: VerbatimUrl { + url: Url { + scheme: "file", + cannot_be_a_base: false, + username: "", + password: None, + host: None, + port: None, + path: "//editable;", + query: None, + fragment: None, + }, + given: Some( + "./editable;", + ), + }, + extras: [], + path: "/editable;", + origin: Some( + File( + "/editable.txt", + ), + ), + }, + ], + index_url: None, + extra_index_urls: [], + find_links: [], + no_index: false, + no_binary: None, + only_binary: None, +} diff --git a/crates/requirements-txt/test-data/requirements-txt/editable.txt b/crates/requirements-txt/test-data/requirements-txt/editable.txt index c1e99017e28a..eb059486c1c9 100644 --- a/crates/requirements-txt/test-data/requirements-txt/editable.txt +++ b/crates/requirements-txt/test-data/requirements-txt/editable.txt @@ -1,25 +1,17 @@ +# OK (standard) +-e ./editable[d,dev] +# OK (whitespace between extras; disallowed by pip) +-e ./editable[d, dev] - # # - # # - # # - # # - # # - # # - # # +# OK +-e ./editable[d,dev] ; python_version >= "3.9" and python_ver - ## +# OK (whitespace between extras; disallowed by pip) +-e ./editable[d, dev] ; python_version >= "3.9" and python_ver -# +# OK +-e ./editable ; python_version >= "3.9" and python_ver - numpy # # - -\ - -pandas [tabulate] @ https://github.com/pandas-dev/pandas \ - # üh - - - # - # 안녕 - # +# Disallowed (missing whitespace before colon) +-e ./editable; python_version >= "3.9" and python_ver