Fix virtualenv update for new bash activation script #36
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
Member
Author
|
virtualenv 20.36.0 upgrades pip to 25.3 pypa/virtualenv/pull/2989 which includes support for PEP 660 (via pypa/setuptools#3265). PEP 660 adds editable finder files which modules by their path. These now also need to be changed. |
ymilki
force-pushed
the
fix_virtualenv_update_for_new_bash_activation_script
branch
from
454a3eb to
41f42b0
Compare
ymilki
force-pushed
the
fix_virtualenv_update_for_new_bash_activation_script
branch
2 times, most recently
from
28df0ad to
81698c8
Compare
ymilki
force-pushed
the
fix_virtualenv_update_for_new_bash_activation_script
branch
from
81698c8 to
d105f20
Compare
| path = os.path.join(bin_dir, fname) | ||
| if fname in ACTIVATION_SCRIPTS and activation: | ||
| update_activation_script(path, new_path) | ||
| update_activation_script(path, orig_path, new_path) |
There was a problem hiding this comment.
ignore my last comment, i missed that this is the same function 🤦♀️
There was a problem hiding this comment.
oh wait it isn't; yeah i think maybe old_path in update_activation_script is no longer necessary? unless i'm missing something obvious
naomiven
reviewed
Jan 20, 2026
Comment on lines
+84
to
+91
| if line != new_line: | ||
| lines[idx] = new_line | ||
| changed = True | ||
| else: | ||
| new_line = _activation_path_re.sub(_handle_sub, line) | ||
| if line != new_line: | ||
| lines[idx] = new_line | ||
| changed = True |
Member
There was a problem hiding this comment.
not a big deal, but this repeated code can be taken out and be placed after the if-else:
if line != new_line:
lines[idx] = new_line
changed = True
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.
Problem
pypa/virtualenv#2996 (
virtualenv==20.36.0) changes how theVIRTUAL_ENVvariable is defined in the bash activate script. Our tool explicitly reads and parses the activate script for the value ofVIRTUAL_ENV=, which no longer works with the change and breaks withAdditionally, the upstream changes add an explicit exists check for
VIRTUAL_ENV. This must also be changed.Fixes #34.
Solution
Because there are now multiple references to
VIRTUALENV_ENVpath in the bash activation, we must replace them all. We will handle this script differently from the rest of the scripts uses bash-specific regexes.Validation
We added a new test
test_get_orig_pathandtest_update_activation_script. During development, we tested against both older and newer virtualenv. Our change is compatible with both so we don't need to explicitly test different virtualenvs.Note
HLFH in #34 provided their own patch in their fork for this change in HLFH@4971532, but it seems to be targeting a different form of the activation script.
Notes
This is an alternative approach to #35