feat: Support tool.uv.extra-build-dependencies in annotation loading#1297
feat: Support tool.uv.extra-build-dependencies in annotation loading#1297konsti-openai wants to merge 17 commits into
tool.uv.extra-build-dependencies in annotation loading#1297Conversation
|
I'm new to rules_py, so I'm interested in feedback in how to expose this best: Should we just load extra build dependencies by default? Should this be a separate command similar to |
tool.uv.extra-build-dependencies in annotation loadingtool.uv.extra-build-dependencies in annotation loading
0e808b5 to
1518240
Compare
| "for sdists that build a wheel. Only demanded when an sbuild is " + | ||
| "guaranteed to be selected (forced builds or sdist-only packages).", | ||
| ), | ||
| "load_annotations": attr.bool( |
There was a problem hiding this comment.
Should we just load extra build dependencies by default?
I assume we should by default if that is what uv does.
Is there any known reason for disabling it? Can we just enable this 100% of the time and drop this attribute?
There was a problem hiding this comment.
I'm not aware of any, dropped!
| { name = "build" }, | ||
| { name = "setuptools" }, | ||
| ] | ||
| # This format is supported, but |
There was a problem hiding this comment.
Should we still be testing both though? Or is this old/deprecated enough we can just delete it (instead of commenting it out like this)?
There was a problem hiding this comment.
I kept it there to not confuse user who come across the old format. imho it's fine to keep both format around and avoid breaking users, the comment should help users to understand what they should use and why they may see something different in existing code.
tamird
left a comment
There was a problem hiding this comment.
This currently accepts only a narrow subset of uv's extra-build-dependencies syntax and silently drops several valid cases. Please either preserve uv's semantics for requirement objects, markers, split versions, and build-only dependencies or explicitly reject unsupported forms, and add an e2e case whose build really depends on the annotation.
| if project.load_annotations: | ||
| extra_build_dependencies = tool_uv.get("extra-build-dependencies", {}) | ||
| for package, extra_deps in extra_build_dependencies.items(): | ||
| target = _resolve(package, None) |
There was a problem hiding this comment.
Resolving the annotated package only through default_versions silently skips every valid split-version lock: that map is populated only when a name has one version, while uv applies the annotation by package name across selected versions. The same issue affects build dependencies below. Please expand the target/dependency resolution over package_versions and add a conflicting-version or platform-fork regression.
There was a problem hiding this comment.
I've added that, though it's important to note that build-system.requires extracts only the name (
rules_py/uv/private/sdist_configure/detect_native.py
Lines 124 to 128 in 5204fc2
tool.uv.extra-build-dependencies and build-system.requires are parsed.
|
Codex also flagged two other problems. They are already present with
|
6bac123 to
7c53c7d
Compare
| package_versions, | ||
| fail_if_missing = False, | ||
| ) | ||
| if not resolved_deps: |
There was a problem hiding this comment.
It's arguable whether this should or shouldn't fail. Codex complains that this fails for wheel-only packages while annotations.toml skips, but OTOH annotating a wheel-only package with requirements missing in the lock sounds bogus.
|
I'm having a hard time scoping this against preexisting gaps and making this too large for review, but I think the current state is ready for a review. |
Currently, `annotations.toml` supports declaring extra build dependencies in a format that runs parallel to `tool.uv.extra-build-dependencies`. This PR adds support for loading `tool.uv.extra-build-dependencies` to `uv.project` by declaring `load_annotations = True`.
… through conflicts
4fd294c to
a6aaf44
Compare
Currently,
annotations.tomlsupports declaring extra build dependencies in a format that runs parallel totool.uv.extra-build-dependencies. This PR adds support for loading uv'stool.uv.extra-build-dependenciestorules_py'suv.project. This moves rules_py closer to uv's interface.Unlike
annotations.toml,tool.uv.extra-build-dependenciessupports the full requirements syntax.Changes are visible to end-users: yes/no
Test plan
Extended the existing e2e snapshot test on top of uv-sdist-fallback to cover both the old and the new syntax for two different packages.