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 PEX_ROOT leak run and repl goals. #12066

Merged
merged 2 commits into from
May 12, 2021
Merged

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented May 12, 2021

Previously we'd leak .cache/pex_root to the workspace. Beyond making it
easier to (mistakenly) corrupt the Pex cache for interactive Pex processes
it also could degrade performance of those processes since this was not
a symlink to the pex_root named cache used by sandboxed processes.

Force user's of PexEnvironment methods that support setting up a process
to declare what type of process they are setting up by moving those
methods to CompletePexEnvironment with one concrete implementation for
sandboxed processes and one for workspace processes.

Fixes #12055

[ci skip-rust]
[ci skip-build-wheels]

Previously we'd leak `.cache/pex_root` to the workspace. Beyond making it
easier to (mistakenly) corrupt the Pex cache for interactive Pex processes
it also could degrade performance of those processes since this was not
a symlink to the `pex_root` named cache used by sandboxed processes.

Force user's of PexEnvironment methods that support setting up a process
to declare what type of process they are setting up by moving those
methods to CompletePexEnvironment with one concrete implementation for
sandboxed processes and one for workspace processes.

Fixes pantsbuild#12055

[ci skip-rust]
[ci skip-build-wheels]
@jsirois jsirois requested review from Eric-Arellano and benjyw May 12, 2021 17:37
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thank you! This modeling looks good.

Could you please also cherry-pick to 2.5? I'll hold off on the release: #12065.

@jsirois
Copy link
Contributor Author

jsirois commented May 12, 2021

Could you please also cherry-pick to 2.5?

Will do.

@jsirois jsirois merged commit 0a01eaf into pantsbuild:main May 12, 2021
@jsirois jsirois deleted the issues/12055 branch May 12, 2021 18:47
jsirois added a commit to jsirois/pants that referenced this pull request May 12, 2021
Previously we'd leak `.cache/pex_root` to the workspace. Beyond making it
easier to (mistakenly) corrupt the Pex cache for interactive Pex processes
it also could degrade performance of those processes since this was not
a symlink to the `pex_root` named cache used by sandboxed processes.

Force user's of PexEnvironment methods that support setting up a process
to declare what type of process they are setting up by moving those
methods to CompletePexEnvironment with one concrete implementation for
sandboxed processes and one for workspace processes.

Fixes pantsbuild#12055

(cherry picked from commit 0a01eaf)

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
jsirois added a commit to jsirois/pants that referenced this pull request May 12, 2021
Previously we'd leak `.cache/pex_root` to the workspace. Beyond making it
easier to (mistakenly) corrupt the Pex cache for interactive Pex processes
it also could degrade performance of those processes since this was not
a symlink to the `pex_root` named cache used by sandboxed processes.

Force user's of PexEnvironment methods that support setting up a process
to declare what type of process they are setting up by moving those
methods to CompletePexEnvironment with one concrete implementation for
sandboxed processes and one for workspace processes.

Fixes pantsbuild#12055

(cherry picked from commit 0a01eaf)

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
jsirois added a commit that referenced this pull request May 12, 2021
Previously we'd leak `.cache/pex_root` to the workspace. Beyond making it
easier to (mistakenly) corrupt the Pex cache for interactive Pex processes
it also could degrade performance of those processes since this was not
a symlink to the `pex_root` named cache used by sandboxed processes.

Force user's of PexEnvironment methods that support setting up a process
to declare what type of process they are setting up by moving those
methods to CompletePexEnvironment with one concrete implementation for
sandboxed processes and one for workspace processes.

Fixes #12055

(cherry picked from commit 0a01eaf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interactive PEX execution leaks PEX_ROOT at $BUILD_ROOT/.cache/pex_root.
2 participants