fix(py_venv): Activate embedded interpreters #576
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
thesayyn
approved these changes
Jun 3, 2025
arrdem
pushed a commit
that referenced
this pull request
Oct 15, 2025
### Changes are visible to end-users: yes When creating a virtual environment, the `activate` script no longer modifies bash flags. These flags were added in #576 (v1.6.0) (note, they don't appear in the scripts that Python's `venv` module generates). <!-- If no, please delete this section. --> - Searched for relevant documentation and updated as needed: yes (behaviour is not documented) - Breaking change (forces users to change their own code or config):no - Suggested release notes appear below: yes/no ### Test plan - Manual testing; please provide instructions so we can reproduce: Potentially, users could have some tests running the following commands. The new flags introduced in #576 broke this kind of tests: ```sh #!/usr/bin/env bash set -euo pipefail # Create a virtualenvironment bazel run //:venv # Activate it (previous it was setting +e flag) source .venv/bin/activate # Some failing test -- users expects the script to end and fail here false # Some other command that doesn't fail true # !!!! Script returns 0 !!!! ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the
bin/activatescript relied on Bazel (specificallybazel run) defined envvars, or being sourced directly from the binary entrypoint. This meant that if the user created a link (eg via the venv link verb or manually) to the venv defined by a given target, sourcing that venv's activate script would fail to locate a runfiles-based interpreter. On systems where there is no appropriately versioned non-hermetic interpreter available, this meant that venvs which should run with the Bazel-managed interpreter instead failed.The fix is to harden venv initialization so that if the
activatescript is directly used AND the runfiles preconditions are not met an effort will be made to satisfy them.Changes are visible to end-users: yes
Test plan