feat!: multiple improvements to py_package and py_wheel
Each commit should be self-contained but at a high level:
py_packagenow returnsPyInfowith collected files and imports.py_packagenow has anexcludeattr that's applied after thepackages.- Automatic third-party requirement collection in
py_wheelviaauto_add_requirements. - Respect
importsofpy_librarytargets added to thepy_wheel. This aligns with how Bazel assembles the PYTHONPATH under the runfiles. - Added missing
importsattributes to theexamples/wheel.
It's only a breaking change for users not setting imports or some other behaviour that we cannot predict.
Despite it introducing a breaking change, the fixes users should apply are not complex and can be done by combining imports attributes and other features introduced by this PR.
AssemblyAI gently sponsored this work.
The current test failure is due to files clashing inside the action. I'll patch an alternative.
Generally LGTM, it would be good to show an example of use and have a test that confirms that we build wheels with some dependencies automatically included.
Thanks for the reviews, @alexeagle, @rickeylev and @chrislovecnm. I believe I addressed most of the comments. I only pushed back the ones I really thought didn't make sense.
re: using ctx.actions.symlink instead of a custom program to perform essentially the same
Trust me; I tried all the obvious solutions. CI would always fail for one of the platforms.
Well, still, would you mind elaborating a bit, for the rest of us, please?
ctx.actions.symlink should be platform agnostic, and should work with both files and directories. A specialized program to perform an otherwise standard operation isn't bad per-se, but it does mean the specialized program needs to explain what about the standard method is lacking that justifies needing a specialized program.
re: why all the logic about symlink vs copying and error handling:
Because the filesystem might not accept symlinks and we have to fallback to copying.
What filesystem or platform is that? For Windows, we basically require that symlinks are enabled now; I'm pretty sure there's a few other places that rely on symlink support now. Mac and Linux's support symlinks.
This is the sort of detail that belongs in a comment -- OSError is a generic class of errors. Is this due to some esoteric permission issue? Something to do with hardlinks? Some esoteric platform or system that doesn't accept symlinks? While you may have gone through a gauntlet and know the answer, other readers don't. All they see is error handling code for code that doesn't look like it should be raising an error.
re: magic pypi tags:
Nope, they are very magical.
Ok, so, we can't have magic tags that affect behavior without some sort of explanation about that. For example, I can trace through the code and can see how they are literally used, but that doesn't really provide an explanation. It just leaves me with several questions. For example:
- Why are there two tags? If both aren't present, they are ignored. The two tags are, in the end concatenated into a single value -- why are they separate tags?
- Why does the last value win? Is this intentional, or just accidental?
- Why can a target have only one requirement via its tags?
- Why are tags used instead an augmenting target in
dataand/or having RequirementsInfo provided directly?
I'm not saying we need a whole design doc or page long .md file about this. We do, however, need enough to understand the feature. That, and any publicly undocumented feature needs some sort of implementation comment about it, otherwise it's just a haunted graveyard people are afraid to touch.
re: visiting srcs and gathering RequirementsInfo from them:
Because srcs may be plain source files, but it's not mandatory.
Ok, but that still doesn't really answer the question: providing a requirement at an individual source-file level looks gratitious. A source file doesn't really do much in isolation; it's part of some unit (e.g. a py_library), which, as a unit, has requirements as to its needs.
@f0rmiga ping
This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days. Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!
This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"