fix(pypi): support source-less wheels with dependencies - #4054
fix(pypi): support source-less wheels with dependencies#4054hartikainen wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an analysis-time failure in pip.parse-generated per-wheel repositories when consuming source-less wheels that still declare dependencies. It prevents the generated wrapper py_library from pointing its srcs at an empty source target (which Bazel rejects), while still keeping the source target in deps so PyInfo continues to propagate.
Changes:
- Plumbs a computed
wrapper_srcsvalue out ofwhl_library_srcs()and intowhl_library_deps_targets()so the wrapperpy_library(srcs=...)is only populated when it would be source-producing. - Adds regression tests covering sourceless wheels, normal sourceful wheels, and the “generated namespace-package init files” case.
- Adds a news entry documenting the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
python/private/pypi/whl_library_targets.bzl |
Computes and forwards wrapper srcs to avoid analysis errors for source-less wheels while preserving PyInfo propagation via deps. |
tests/pypi/whl_library_targets/whl_library_targets_tests.bzl |
Adds targeted regression coverage for sourceless/sourceful/namespace-init scenarios by asserting the generated py_library calls. |
news/sourceless-wheel-srcs.fixed.md |
Documents the bugfix in the changelog/news system. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`rules_python` `2.3.x` wraps source-less wheels in a `py_library` whose `srcs` target has no Python files. Forward the target only when it has valid sources, and cover the wrapper behavior with unit tests.
eb2978b to
920f5ad
Compare
|
I think it would be good to see if we can use //python:none target in these cases. It provides pyinfo provider afterall. |
| dep_template = dep_template, # only needed if requires_dist is present | ||
| repo = None, # set aliases in the same repo | ||
| aliases = {}, | ||
| srcs = wrapper_srcs, |
There was a problem hiding this comment.
Hmmm. This particular approach means that defining the dependency graphs and the srcs in separate repositories is not going to be possible.
Thoughts:
- I am OK to merge this because this will fix a regression.
- I don't wanna merge it because it is one step back. I am curious how we can resolve this.
I'd prefer a fix in the py_library validation logic itself, TBH. Any thoughts?
There was a problem hiding this comment.
Hmm, looking at the tests it does not seem that we are generating the message or at least I cannot find it. So I am thinking that we should attempt solving this with //python:none instead.
There was a problem hiding this comment.
Thanks for the comments. I'll try poking around if I can get //python:none working.
There was a problem hiding this comment.
From what I can tell, //python:none provides SentinelInfo, not PyInfo.
I moved the fix into py_library validation in 74616c5. It now allows only empty PyInfo targets while non-Python outputs still fail. I verified that the py-spy example linked in the issue. What do you think?
Source-less wheel wrappers can reference an empty `py_library` from srcs, which Starlark validation rejected during analysis. This change allows empty PyInfo-providing source targets while preserving validation for non-Python outputs and split source/dependency repositories.
Source-less wheels with dependency metadata fail analysis because the generated wrapper
py_libraryputs an empty:srcstarget in itssrcsattribute.This forwards
:srcsthrough the wrapper only when it produces Python sources, while retaining it indepsforPyInfopropagation. Before this change,py-spy==0.4.1dependency fails during analysis. Afterward, it can be used as a dependency.Adds regression coverage for source-less, sourceful, and generated namespace-package source targets, along with a news entry.
Largely implemented with Opus.
Fixes #4053.