Skip to content

gh-143768: rebuild broken interpreter symlinks#143770

Closed
claydugo wants to merge 6 commits into
python:mainfrom
claydugo:gh-143768-venv-symlink
Closed

gh-143768: rebuild broken interpreter symlinks#143770
claydugo wants to merge 6 commits into
python:mainfrom
claydugo:gh-143768-venv-symlink

Conversation

@claydugo
Copy link
Copy Markdown
Contributor

@claydugo claydugo commented Jan 13, 2026

Summary

Fix venv so re-running it in a directory with stale interpreter symlinks
removes the broken links before recreating them, and add a regression test
to ensure existing environments get their interpreter refreshed without --clear.

Issue number

#143768

try again news

again
@claydugo claydugo force-pushed the gh-143768-venv-symlink branch from 2817914 to 332c799 Compare January 13, 2026 03:26
Comment thread Misc/NEWS.d/next/Library/2025-01-12-10-07-venv-broken-symlinks.rst Outdated
@aisk
Copy link
Copy Markdown
Member

aisk commented Jan 13, 2026

Hi, please avoid force push in the future, see the Dev Guide: https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push

@claydugo
Copy link
Copy Markdown
Contributor Author

Oops sorry about that!

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label May 5, 2026
Copy link
Copy Markdown
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

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

The fix is correct and well-scoped: os.path.islink(dst) and not os.path.exists(dst) targets only dangling symlinks and completes venv's existing overwrite path without adding repair semantics. Background and the virtualenv comparison are in #143768.

The regression test covers symlinks=True only, but the failure in the original report is the --copies path, which raises a different exception (FileNotFoundError [Errno 2] during the copy) than the symlinks path. The fix already handles it, so the test should cover it too; otherwise the exact reported scenario has no regression coverage. See the inline suggestion.

Comment thread Lib/test/test_venv.py
self.assertTrue(os.path.islink(python))
self.assertFalse(os.path.exists(python))

builder = venv.EnvBuilder(with_pip=False, symlinks=True)
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.

Add a symlinks=False (--copies) case so the path from the original report is covered:

builder = venv.EnvBuilder(with_pip=False, symlinks=False)
self.run_with_capture(builder.create, self.env_dir)
self.assertFalse(os.path.islink(python))
self.assertTrue(os.path.exists(python))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for spotting this, I pushed coverage for the --copies path

@claydugo
Copy link
Copy Markdown
Contributor Author

claydugo commented Jun 5, 2026

Hmm, I could use guidance on getting the change detection pipeline passing without force push. I might have gotten it in a bad state using the Github UI

@brettcannon brettcannon self-assigned this Jun 5, 2026
@brettcannon
Copy link
Copy Markdown
Member

Hmm, I could use guidance on getting the change detection pipeline passing without force push. I might have gotten it in a bad state using the Github UI

I don't know how to get is out of this wedged state. Mind closing this PR and opening a new one?

@claydugo
Copy link
Copy Markdown
Contributor Author

claydugo commented Jun 5, 2026

Closing in favor of #150985

@claydugo claydugo closed this Jun 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting review stale Stale PR or inactive for long period of time.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants