This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: packaging install fails to clean up temp files
Type: behavior Stage: resolved
Components: Distutils2, Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: tarek Nosy List: alexis, eric.araujo, python-dev, tarek, vinay.sajip
Priority: normal Keywords: patch

Created on 2011-06-23 07:43 by vinay.sajip, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
initial_patch.diff vinay.sajip, 2011-07-03 21:22 Initial patch to remove temp files review
Repositories containing patches
http://hg.python.org/sandbox/vsajip#fix12391
Messages (5)
msg138860 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-06-23 07:43
There are a number of places in packaging.install where temporary directories are created, but never cleaned up:

1. In _move_files, if no destination path is passed in, one is created using mkdtemp(), but it's not clear where this would be deleted. Moreover, it's never called without a path and not part of the public API, so it would make sense to always expect a destination to be passed in (and update the docstring to match)

2. install_local_project, in the case of an archive, unpacks it into a mkdtemp()'d directory, but never deletes that directory later.

3. install_dists() also calls mkdtemp() if a path is not passed in, but it's not clear where this would be deleted. This should be changed to always require a path to be passed in. The install_from_infos accepts None as an install path and passes that to install_dists, but why are we being so generous? It's not asking a lot to be given an explicit path to install to.

Note: the DistInfo class in packaging.pypi.dist also does this kind of thing (in the download and unpack methods) - it would seem sensible to make similar changes there.
msg139771 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-07-04 14:11
1: Agreed.

2 and 3: Alexis most probably added that behavior as a convenience.  Unless I’m mistaken, the point of $TMP/$TMPDIR is that the OS itself will clean it up, for example on shutdown, so programs that leave stuff here are not strictly wrong.  However, given the realities of Windows behavior (I recall seeing “temporary” directories with tons of stuff never cleaned up) and the low cost of a change (“It's not asking a lot to be given an explicit path to install to” +1), my opinion is that we should take your patch as it is.
msg139777 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2011-07-04 14:38
> Éric Araujo <merwok@netwok.org> added the  comment:

> 2 and 3: Alexis most probably added that  behavior as a convenience.  Unless 
>I’m mistaken, the point of $TMP/$TMPDIR  is that the OS itself will clean it up, 
>for example on shutdown, so programs  that leave stuff here are not strictly 
>wrong.  However, given the realities  of Windows behavior (I recall seeing 
>“temporary” directories with tons of stuff  never cleaned up) and the low cost 
>of a change (“It's not asking a lot to be  given an explicit path to install to” 
>+1), my opinion is that we should take  your patch as it is.

Great. Although /tmp is cleaned up on restart (on Linux at least), waiting for 
that can lead to problems. For example, I came across this problem when I (for 
test purposes) installed every single one of the 400+ packages on PyPI which 
claim to be Py3 compatible, into a virtual env, using "pysetup3 install". I then 
noticed some (slight) performance slowdown and sudden disappearance of free disk 
space ... it was all those archives (and their unpacked contents) in /tmp that 
was the reason.
msg139794 - (view) Author: Alexis Metaireau (alexis) * (Python triager) Date: 2011-07-04 17:19
I'm +1 on applying this patch as well. Removing files in the tmp directory is far better than letting the OS doing so.
msg139971 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2011-07-07 11:59
New changeset a4405b799e1b by Vinay Sajip in branch 'default':
Closes #12391: temporary files are now cleaned up.
http://hg.python.org/cpython/rev/a4405b799e1b
History
Date User Action Args
2022-04-11 14:57:18adminsetgithub: 56600
2011-07-07 11:59:40python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg139971

resolution: fixed
stage: commit review -> resolved
2011-07-04 17:19:01alexissetmessages: + msg139794
2011-07-04 14:38:50vinay.sajipsetmessages: + msg139777
2011-07-04 14:11:01eric.araujosetmessages: + msg139771
stage: commit review
2011-07-03 21:22:53vinay.sajipsetfiles: + initial_patch.diff
keywords: + patch
2011-07-03 21:22:13vinay.sajipsethgrepos: + hgrepo35
2011-06-23 07:43:35vinay.sajipcreate