fix(pypi): don't resolve python interpreter when not necessary#3727
Conversation
|
This is just the patch posted in the issue blindly applied. Lets see what CI says |
…om/rickeylev/rules_python into fix-pipstar-resolve-py-interpreter
There was a problem hiding this comment.
Code Review
This pull request refactors the _whl_library_impl function in whl_library.bzl to defer Python interpreter resolution and argument construction, skipping them when pipstar extraction is used. A critical issue was identified where setting the interpreter to None would cause failures if wheel patches are present, as the subsequent patching logic requires a valid interpreter.
I am having trouble creating individual review comments. Click here to see my feedback.
python/private/pypi/whl_library.bzl (351)
The condition to skip python interpreter resolution should also check if whl_patches is present. If patches are defined, the patch_whl function is called later (line 406), which requires a valid python_interpreter to execute the patching logic. Setting it to None here will cause a failure when patches need to be applied.
if enable_pipstar_extract and whl_path and not rctx.attr.whl_patches:
Before the PR we would not set the environment for python
interpreter if we would not need the interpreter, but we
still resolve the python interpreter, which works on some
systems that have a minimal Python installation.
With this change we stop resolving the Python interpreter
in cases were we should not need it. To ensure that we are
not accidentally using it,
python_interpreteris set toNone when we use
pipstarand do not do any patching, thatstill requires the Python interpreter.
Fixes #3712