Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix venv re-population race. (Cherry-pick of #16931) #16936

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Sep 20, 2022

There was a race in venv re-population due to a non-atomic rm, create sequence. There was no real need for the rm and the create is atomic on its own; so just remove the rm which was only attempting to guard "corrupted" venvs in a slapdash way. Now the venv either exists or it doesn't from the script point of view. If the venv exists but has been tampered with, its execution will consistently fail until there is a manual intervention removing the venv dir offline.

Fixes #14618
Fixes #16778

(cherry picked from commit cace851)

There was a race in venv re-population due to a non-atomic `rm`, create
sequence. There was no real need for the `rm` and the create is atomic
on its own; so just remove the `rm` which was only attempting to guard
"corrupted" venvs in a slapdash way. Now the venv either exists or it
doesn't from the script point of view. If the venv exists but has been
tampered with, its execution will consistently fail until there is a
manual intervention removing the venv dir offline.

Fixes pantsbuild#14618
Fixes pantsbuild#16778

(cherry picked from commit cace851)
@jsirois jsirois added the category:bugfix Bug fixes for released features label Sep 20, 2022
@jsirois jsirois merged commit f8564cb into pantsbuild:2.13.x Sep 21, 2022
@jsirois jsirois deleted the 16931/cherry-pick/2.13.x branch September 21, 2022 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants