Add basic `tool.uv.sources` support #3263

konstin · 2024-04-25T11:34:05Z

Introduction

PEP 621 is limited. Specifically, it lacks

Relative path support
Editable support
Workspace support
Index pinning or any sort of index specification

The semantics of urls are a custom extension, PEP 440 does not specify how to use git references or subdirectories, instead pip has a custom stringly format. We need to somehow support these while still stying compatible with PEP 621.

`tool.uv.source`

Drawing inspiration from cargo, poetry and rye, we add tool.uv.sources or (for now stub only) tool.uv.workspace:

[project]
name = "albatross"
version = "0.1.0"
dependencies = [
  "tqdm >=4.66.2,<5",
  "torch ==2.2.2",
  "transformers[torch] >=4.39.3,<5",
  "importlib_metadata >=7.1.0,<8; python_version < '3.10'",
  "mollymawk ==0.1.0"
]

[tool.uv.sources]
tqdm = { git = "https://github.com/tqdm/tqdm", rev = "cc372d09dcd5a5eabdc6ed4cf365bdb0be004d44" }
importlib_metadata = { url = "https://github.com/python/importlib_metadata/archive/refs/tags/v7.1.0.zip" }
torch = { index = "torch-cu118" }
mollymawk = { workspace = true }

[tool.uv.workspace]
include = [
  "packages/mollymawk"
]

[tool.uv.indexes]
torch-cu118 = "https://download.pytorch.org/whl/cu118"

See docs/specifying_dependencies.md for a detailed explanation of the format. The basic gist is that project.dependencies is what ends up on pypi, while tool.uv.sources are your non-published additions. We do support the full range or PEP 508, we just hide it in the docs and prefer the exploded table for easier readability and less confusing with actual url parts.

This format should eventually be able to subsume requirements.txt's current use cases. While we will continue to support the legacy uv pip interface, this is a piece of the uv's own top level interface. Together with uv run and a lockfile format, you should only need to write pyproject.toml and do uv run, which generates/uses/updates your lockfile behind the scenes, no more pip-style requirements involved. It also lays the groundwork for implementing index pinning.

Changes

This PR implements:

Reading and lowering project.dependencies, project.optional-dependencies and tool.uv.sources into a new requirements format, including:
- Git dependencies
- Url dependencies
- Path dependencies, including relative and editable
pip install integration
Error reporting for invalid tool.uv.sources
Json schema integration (works in pycharm, see below)
Draft user-level docs (see docs/specifying_dependencies.md)

It does not implement:

No pip compile testing, deprioritizing towards our own lockfile
Index pinning (stub definitions only)
Development dependencies
Workspace support (stub definitions only)
Overrides in pyproject.toml
Patching/replacing dependencies

One technically breaking change is that we now require user provided pyproject.toml to be valid wrt to PEP 621. Included files still fall back to PEP 517. That means pip install -r requirements.txt requires it to be valid while pip install -r requirements.txt with -e . as content falls back to PEP 517 as before.

Implementation

The pep508 requirement is replaced by a new UvRequirement (name up for bikeshedding, not particularly attached to the uv prefix). The still existing pep508_rs::Requirement type is a url format copied from pip's requirements.txt and doesn't appropriately capture all features we want/need to support. The bulk of the diff is changing the requirement type throughout the codebase.

We still use VerbatimUrl in many places, where we would expect a parsed/decomposed url type, specifically:

Reading core metadata except top level pyproject.toml files, we fail a step later instead if the url isn't supported.
Allowed Urls.
PackageId with a custom CanonicalUrl comparison, instead of canonicalizing urls eagerly.
PubGrubPackage: We eventually convert the VerbatimUrl back to a Dist (Dist::from_url), instead of remembering the url.
Source dist types: We use verbatim url even though we know and require that these are supported urls we can and have parsed.

I tried to make improve the situation be replacing VerbatimUrl, but these changes would require massive invasive changes (see e.g. #3253). A main problem is the ref VersionOrUrl and applying overrides, which assume the same requirement/url type everywhere. In its current form, this PR increases this tech debt.

I've tried to split off PRs and commits, but the main refactoring is still a single monolith commit to make it compile and the tests pass.

Demo

Adding https://gist.githubusercontent.com/konstin/68f3e5ea5e8f1a30a0c9096ae72925c1/raw/d1ae3b85d5a0f3c4bb562c7dfd38c3dafc04ec08/pyproject.json as json schema (v7) to pycharm for pyproject.toml, you can try the IDE support already:

dove.webm

konstin · 2024-04-26T12:35:46Z

Use preview mode for tool.uv.sources #3277
Add basic tool.uv.sources support #3263 👈
main

crates/distribution-types/src/error.rs

crates/distribution-types/src/installed.rs

crates/distribution-types/src/lib.rs

crates/distribution-types/src/parsed_url.rs

crates/distribution-types/src/traits.rs

crates/uv-requirements/src/specification.rs

crates/uv-resolver/src/error.rs

crates/uv-resolver/src/resolution.rs

crates/uv-resolver/src/resolver/locals.rs

crates/uv-resolver/src/resolver/urls.rs

crates/distribution-types/src/uv_requirement.rs

charliermarsh · 2024-04-30T04:49:31Z

crates/distribution-types/src/uv_requirement.rs

+pub enum UvSource {
+    Registry {
+        version: VersionSpecifiers,
+        index: Option<String>,


Should this be IndexUrl?

This is still mostly to-be-designed and mainly a placeholder, but i'd expect this to become the name of an index, e.g. torch, which is then defined elsewhere, either in this file or in a global configuration. This doesn't have any usages yet to be easy to change once we have the design

crates/uv-requirements/src/pyproject.rs

Split out from #3263. No functional changes.

Split out from #3263

Another split out from #3263. This abstracts the copy&pasted check whether an installed distribution satisfies a requirement used by both plan.rs and site_packages.rs into a shared module. It's less useful here than with the new requirement but helps with reducing #3263 diff size.

crates/distribution-types/src/resolution.rs

crates/distribution-types/src/uv_requirement.rs

charliermarsh · 2024-05-01T05:11:01Z

crates/requirements-txt/src/requirement.rs

+            RequirementsTxtRequirement::Uv(requirement) => requirement.name.to_string(),
+            RequirementsTxtRequirement::Unnamed(unnamed) => unnamed.to_string(),
+        }
+    }


Why is this not implementing display?

Which part? There's a impl Display for RequirementsTxtRequirement

crates/uv-dev/src/generate_json_schema.rs

charliermarsh · 2024-05-03T20:20:00Z

crates/uv/tests/pip_compile.rs

@@ -5224,7 +5224,7 @@ fn unsupported_scheme() -> Result<()> {
    ----- stdout -----

    ----- stderr -----
-    error: Unsupported scheme `bzr+https` on URL: bzr+https://example.com/anyio (Bazaar is not supported)
+    error: Unsupported URL prefix `bzr` in URL: `bzr+https://example.com/anyio`


I feel like this error message got worse, can we restore it?

At least the part at the end.

#3403

I'd also like to track the origin of the invalid url, but that's a larger task.

charliermarsh · 2024-05-03T20:55:59Z

crates/uv-resolver/src/pubgrub/dependencies.rs

                ));
            };

            if !Urls::is_allowed(expected, url) {
                return Err(ResolveError::ConflictingUrlsTransitive(
                    requirement.name.clone(),
                    expected.verbatim().to_string(),
-                    url.verbatim().to_string(),


Why lose the verbatim here?

charliermarsh · 2024-05-04T02:48:39Z

crates/uv-requirements/src/pyproject.rs

+            };
+
+            let mut url = Url::parse(&format!("git+{git}"))?;
+            let mut given = git.to_string();


This is incorrect. Don't we need the actual string here?

@konstin - I guess we need to find a way to preserve the exact string from the TOML, right? Otherwise, how will we preserve environment variables?

imho toml shouldn't support env vars

charliermarsh · 2024-05-04T02:48:56Z

crates/uv-requirements/src/pyproject.rs

+                _ => return Err(LoweringError::MoreThanOneGitRef),
+            };
+
+            let mut url = Url::parse(&format!("git+{git}"))?;


The docs on RequirementSource::Git say it's the URL without the git+ prefix. Is this correct?

Fixed here: #3365

charliermarsh · 2024-05-04T02:49:23Z

crates/uv-installer/src/satisfies.rs

+                    return Ok(Self::Mismatch);
+                }
+
+                if &requested_url.to_string() != installed_url


This seems off. Shouldn't we instead parse the URL, and use CanonicalUrl to compare these? Why are we comparing as strings?

(Fixed in #3373)

Fixup for #3263 (comment)

When using `tool.uv.sources`, we enforce that requirements have a bound, i.e. at least a lower version constraint. When using a library, the symbols you import were introduced in different versions, creating an implicit lower bound. This forces to make that bound explicit. This is crucial to prevent backtracking resolvers from selecting an ancient versions that is not compatible (or worse, doesn't build), and a performance optimization on top. This feature is gated to `tool.uv.sources` (as it should have been for #3263/#3443) to not unnecessarily break legacy workflows. It is also helpful specifically when using a `tool.uv.sources` section that contains constraints that are not published to pypi, e.g. for workspace dependencies. We can adjust those later to e.g. not constrain workspace dependencies with `publish = false`, but i think it's the right setting to start with.

When using `tool.uv.sources`, we warn that requirements have a bound, i.e. at least a lower version constraint. When using a library, the symbols you import were introduced in different versions, creating an implicit lower bound. This forces to make that bound explicit. This is crucial to prevent backtracking resolvers from selecting an ancient versions that is not compatible (or worse, doesn't build), and a performance optimization on top. This feature is gated to `tool.uv.sources` (as it should have been for #3263/#3443) to not unnecessarily break legacy workflows. It is also helpful specifically when using a `tool.uv.sources` section that contains constraints that are not published to pypi, e.g. for workspace dependencies. We can adjust those later to e.g. not constrain workspace dependencies with `publish = false`, but i think it's the right setting to start with.

When using `tool.uv.sources`, we warn that requirements have a bound, i.e. at least a lower version constraint. When using a library, the symbols you import were introduced in different versions, creating an implicit lower bound. This warning makes this explicit. This is crucial to prevent backtracking resolvers from selecting an ancient versions that is not compatible (or worse, doesn't build), and a performance optimization on top. This feature is gated to `tool.uv.sources` (as it should have been to begin with for #3263/#3443) to not unnecessarily break legacy workflows. It is also helpful specifically when using a `tool.uv.sources` section that contains constraints that are not published to pypi, e.g. for workspace dependencies. We can adjust those later to e.g. not constrain workspace dependencies with `publish = false`, but i think it's the right setting to start with.

konstin added the enhancement New feature or request label Apr 25, 2024

konstin force-pushed the konsti/tool-uv-sources-analysis branch 2 times, most recently from 46d6c68 to 33883ed Compare April 26, 2024 12:35

konstin mentioned this pull request Apr 26, 2024

Use preview mode for tool.uv.sources #3277

Merged

konstin added the preview Experimental behavior label Apr 26, 2024

konstin requested a review from charliermarsh April 28, 2024 19:53

charliermarsh reviewed Apr 30, 2024

View reviewed changes

konstin added a commit that referenced this pull request Apr 30, 2024

Refactor Dist constructors

6f14550

Split out from #3263. No functional changes.

konstin mentioned this pull request Apr 30, 2024

Refactor Dist constructors #3320

Merged

konstin added a commit that referenced this pull request Apr 30, 2024

Refactor Dist constructors (#3320)

71fbd25

Split out from #3263. No functional changes.

konstin force-pushed the konsti/tool-uv-sources-analysis branch from 33883ed to b8cb0db Compare April 30, 2024 11:22

konstin added a commit that referenced this pull request Apr 30, 2024

Fix typos

d2e3c8e

Split out from #3263

konstin mentioned this pull request Apr 30, 2024

Fix typos #3323

Merged

konstin force-pushed the konsti/tool-uv-sources-analysis branch from b8cb0db to 8c11163 Compare April 30, 2024 14:32

konstin added a commit that referenced this pull request Apr 30, 2024

Fix typos (#3323)

80ce32b

Split out from #3263

konstin mentioned this pull request Apr 30, 2024

Centralize installed dist satisfies requirement check #3324

Merged

konstin force-pushed the konsti/tool-uv-sources-analysis branch 2 times, most recently from 348d11d to 1fae847 Compare April 30, 2024 16:29

konstin force-pushed the konsti/tool-uv-sources-analysis branch 2 times, most recently from 868ef3d to 2431020 Compare April 30, 2024 17:19

charliermarsh reviewed May 1, 2024

View reviewed changes

crates/distribution-types/src/resolution.rs Outdated Show resolved Hide resolved

charliermarsh reviewed May 1, 2024

View reviewed changes

crates/distribution-types/src/uv_requirement.rs Outdated Show resolved Hide resolved

charliermarsh reviewed May 1, 2024

View reviewed changes

crates/uv-dev/src/generate_json_schema.rs Outdated Show resolved Hide resolved

konstin added 9 commits May 3, 2024 17:22

Improve URL handling

bc02edc

Remove Uv naming everywhere

eec41bf

Use VerbatimUrl::parse_path

d2661b6

Fix clippy and tests

14215e3

url test error message

ceeaa81

Fix failing tests rebasing

33d934d

Use pep508_rs::Requirement in requirements.txt

89afc8f

Rename Requirement::from_requirement to Requirement::from_pep508

f90f618

Cleanup and docs

83b6fe0

konstin force-pushed the konsti/tool-uv-sources-analysis branch from b83969b to 83b6fe0 Compare May 3, 2024 15:25

charliermarsh reviewed May 3, 2024

View reviewed changes

Tweaks to docs

2ee61ab

charliermarsh reviewed May 3, 2024

View reviewed changes

Small tweaks to comments, etc.

b768af1

charliermarsh approved these changes May 3, 2024

View reviewed changes

charliermarsh enabled auto-merge (squash) May 3, 2024 21:06

charliermarsh merged commit 4f87edb into main May 3, 2024
43 checks passed

charliermarsh deleted the konsti/tool-uv-sources-analysis branch May 3, 2024 21:10

charliermarsh reviewed May 4, 2024

View reviewed changes

konstin mentioned this pull request May 6, 2024

Restore verbatim in error message #3402

Merged

konstin added a commit that referenced this pull request May 6, 2024

Restore verbatim in error message (#3402)

d0c3146

Fixup for #3263 (comment)

konstin mentioned this pull request May 8, 2024

Warn when missing minimal bounds when using tool.uv.sources #3452

Merged

zanieb mentioned this pull request Jun 4, 2024

Implement uv remove #3960

Closed

zanieb mentioned this pull request Jul 2, 2024

Tracking: uv top-level commands and user experience #2917

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic `tool.uv.sources` support #3263

Add basic `tool.uv.sources` support #3263

konstin commented Apr 25, 2024

konstin commented Apr 26, 2024 •

edited

Loading

charliermarsh Apr 30, 2024

konstin Apr 30, 2024

charliermarsh May 1, 2024

konstin May 2, 2024

charliermarsh May 3, 2024

charliermarsh May 3, 2024

konstin May 6, 2024

charliermarsh May 3, 2024

charliermarsh May 4, 2024

charliermarsh May 4, 2024

konstin May 6, 2024

charliermarsh May 4, 2024

charliermarsh May 4, 2024

konstin May 6, 2024

charliermarsh May 4, 2024

konstin May 6, 2024

Add basic tool.uv.sources support #3263

Add basic tool.uv.sources support #3263

Conversation

konstin commented Apr 25, 2024

Introduction

tool.uv.source

Changes

Implementation

Demo

konstin commented Apr 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Add basic `tool.uv.sources` support #3263

Add basic `tool.uv.sources` support #3263

`tool.uv.source`

konstin commented Apr 26, 2024 •

edited

Loading