Skip to content

feat(venv): make wheel scripts runnable in venv#3743

Open
rickeylev wants to merge 48 commits intobazel-contrib:mainfrom
rickeylev:venv.bin.runnable
Open

feat(venv): make wheel scripts runnable in venv#3743
rickeylev wants to merge 48 commits intobazel-contrib:mainfrom
rickeylev:venv.bin.runnable

Conversation

@rickeylev
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev commented Apr 27, 2026

Currently, #!python based scripts in a wheel are put into the venv
as-is, so aren't actually runnable.

To fix, rewrite the scripts to re-exec themselves with the python3
binary in the same directory.

DO NOT MERGE: await #3726

ref https://packaging.python.org/en/latest/specifications/binary-distribution-format/#installing-a-wheel-distribution-1-0-py32-none-any-whl

for generic data scheme: these are usually installed to the venv root,
but that seems ill-advised. So we install into a data sub-directory
of the venv.
Uses os.name to correctly assert the Scripts/ and Include/ directory
paths when creating the virtual environment on Windows.
Also add debug info to venvs_site_packages_libs_test to help diagnose
Windows CI failures.
…addition

These changes seemed to cause many regressions in WORKSPACE mode in CI.
Also revert redundant bazel_skylib additions to example WORKSPACE files.
The original target was intended to be internal-only, so it has been
renamed and all references updated. No alias was added as per
instructions.
rickeylev and others added 16 commits April 26, 2026 10:01
The `whl_from_dir_repo` repository rule previously relied on the Unix
`zip` utility to create wheels. Because this command isn't natively
available on Windows, any tests that depended on repositories generated
by this rule had to be explicitly skipped on Windows hosts.

To fix this and expand our test coverage, this adds a native Windows
fallback. When running on Windows, the rule now invokes a helper
PowerShell script that uses .NET compression APIs to create the
archive. This script ensures the resulting wheel remains uncompressed
and uses zeroed-out timestamps to match the deterministic behavior of
the original `zip -0X` command.

With this constraint removed, the Unix-only compatibility flags
(`SUPPORTS_BZLMOD_UNIXY`) have been dropped, enabling several namespace
package and wheel-related integration tests to finally run on Windows.
@rickeylev rickeylev requested review from aignas and groodt as code owners April 27, 2026 06:10
@rickeylev rickeylev added the do not merge Tag that prevents merging label Apr 27, 2026
@rickeylev rickeylev removed the request for review from groodt April 27, 2026 06:10
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request ensures that wheel data files (bin, include, and data) are included as dependencies and correctly populated into virtual environments when --venv_site_packages=yes is enabled. It introduces a binary rewriter mechanism to adjust shebangs in wheel-provided scripts to use the venv's interpreter. Feedback identifies several critical issues: the rewriter scripts hardcode the interpreter name as python3, the PowerShell rewriter incorrectly generates POSIX shell wrappers, and the selection logic for the rewriter does not properly account for cross-compilation scenarios. Additionally, hardcoded script generation settings in the wheel installer may limit cross-platform compatibility.

Comment thread python/private/venv_bin_rewriter.sh
Comment thread python/private/venv_bin_rewriter.ps1
Comment on lines +35 to +47
if rewriter_file.path.endswith(".ps1"):
# powershell.exe is used for broader compatibility
# It is installed by default on most Windows versions
action_exe = "powershell.exe"
action_args.add_all([
"-ExecutionPolicy",
"Bypass",
"-NoProfile",
"-File",
rewriter_file,
])
else:
action_exe = ctx.attr._venv_bin_rewriter[DefaultInfo].files_to_run
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The rewriter selection is based on the execution platform (cfg = "exec"), but the script it generates must be compatible with the target platform. When cross-compiling (e.g., Linux host to Windows target), the current logic will use the host's rewriter (shell script) to generate a script for the target, which will be incompatible. The rewriter logic should be aware of the target OS and the correct interpreter name (e.g. python.exe vs python3).

Comment thread python/private/pypi/whl_installer/wheel.py
@rickeylev
Copy link
Copy Markdown
Collaborator Author

Oh neat, it looks like entry points already work? I'm guessing something in whl_library is generating something from the entry points config?

@rickeylev
Copy link
Copy Markdown
Collaborator Author

Hm, after poking around a bit, i'm guessing the entry points stuff is actually relying on repo-phase code generating the scripts. It shouldn't be too hard to replace that with a build-time generation rule.

@aignas
Copy link
Copy Markdown
Collaborator

aignas commented Apr 27, 2026

I have actually removed the entry point parsing/generation at head with #3735. If you need the code feel free to restore some of it. Should be easy to do a partial revert and just plumb through the entry point script names to create py_console_script_binary targets for.

@aignas
Copy link
Copy Markdown
Collaborator

aignas commented Apr 27, 2026

That said, I was thinking that it would be best to return the console scripts via the PyInfo provider (or some other provider) so that all we have to do in py_console_script_binary is get the file from the provider instead of parsing an entry_points.txt file.

I think it is very doable to also use the same provider to expand the console script template on the fly in the venv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Tag that prevents merging

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants