Skip to content

Intentional reuse of deleted root dir in RequirementTracker #5725

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

Closed
gar1t opened this issue Aug 22, 2018 · 12 comments
Closed

Intentional reuse of deleted root dir in RequirementTracker #5725

gar1t opened this issue Aug 22, 2018 · 12 comments
Labels
auto-locked Outdated issues that have been locked by automation

Comments

@gar1t
Copy link

gar1t commented Aug 22, 2018

The constructor uses an environment variable to cache the temp dir it creates:

https://github.com/pypa/pip/blob/18.0/src/pip/_internal/req/req_tracker.py#L17-L25

But RequirementTracker objects are context managers that delete the root dir:

https://github.com/pypa/pip/blob/18.0/src/pip/_internal/req/req_tracker.py#L31-L32
https://github.com/pypa/pip/blob/18.0/src/pip/_internal/req/req_tracker.py#L65-L67

It's not obvious to me looking at the code or associated commit messages what the goal of the root dir reuse is.

@benoit-pierre
Copy link
Member

See #5336.

@gar1t
Copy link
Author

gar1t commented Aug 22, 2018

I see there's a test that removes any PIP_REQ_TRACKER env, but I don't see where anyone sets that. Is the intention that a user will interface with pip via the environment?

I'm happy to put a PR together but I don't understand the intended behavior.

@benoit-pierre
Copy link
Member

No! You should not use that, it's for PIP internal use.

gar1t pushed a commit to guildai/guildai that referenced this issue Aug 22, 2018
@gar1t
Copy link
Author

gar1t commented Aug 22, 2018

Nonetheless, I hope the maintainers can see the bug here, regardless of the intent.

Feel free to ignore and close!

@benoit-pierre
Copy link
Member

I still don't understand what is the bug.

@benoit-pierre
Copy link
Member

The initial pip process (e.g. the one spawned by the user pip install ...) will create a temporary directory, and share it with sub-processes for tracking requirements during the prepare phase.

@gar1t
Copy link
Author

gar1t commented Aug 22, 2018

The class uses the environment to remember the first temporary dir it creates, effectively using the process environment as global state. It never un-remembers this. So that root is stuck forever once it's defined in os.environ.

If a tracker object is used as a context manager, when the context exits, __exit__ is called and the temp directory is deleted. It's never re-created and there's no way to un-remember it.

Code that writes to that directory, assuming it already exists, fails.

@benoit-pierre
Copy link
Member

Again, what's the problem? Unless you're using multiple call to pip's main from python, something that is specifically not supported!

@gar1t
Copy link
Author

gar1t commented Aug 22, 2018

I can see why it's not supported!

I'll go ahead and close this since you don't consider the behavior to be a bug.

@gar1t gar1t closed this as completed Aug 22, 2018
@benoit-pierre
Copy link
Member

See my comment for your use case.

@gar1t
Copy link
Author

gar1t commented Aug 22, 2018

Terrific - thank you!

@lock
Copy link

lock bot commented Jun 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

No branches or pull requests

2 participants