poetry add/update/install can fail on Windows during temp directory cleanup #1031
Description
If something goes wrong during an installation, a file is often held open for just a little too long for TemporaryDirectory causing it to error out with errors such as:
[OSError]
[WinError 145] The directory is not empty: 'C:\\Users\\KING~1.KYL\\AppData\\Local\\Temp\\tmp774m6ge1'
...
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\contextlib.py in __exit__() at line 119
next(self.gen)
C:\Users\king.kyle\.poetry\lib\poetry\utils\helpers.py in temporary_directory() at line 35
yield name
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\tempfile.py in __exit__() at line 805
self.cleanup()
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\tempfile.py in cleanup() at line 809
_shutil.rmtree(self.name)
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\shutil.py in rmtree() at line 507
return _rmtree_unsafe(path, onerror)
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\shutil.py in _rmtree_unsafe() at line 395
onerror(os.rmdir, path, sys.exc_info())
C:\Users\king.kyle\AppData\Local\Continuum\anaconda2\envs\py37\lib\shutil.py in _rmtree_unsafe() at line 393
os.rmdir(path)
add [-D|--dev] [--git GIT] [--path PATH] [-E|--extras EXTRAS] [--optional] [--python PYTHON] [--platform PLATFORM] [--allow-prereleases] [--dry-run] [--] <name> (<name>)...
This is an incredibly unhelpful error and masks all sorts of other problems.
It is causes by a bug in shutil.rmtree which can fail on Windows from time to time. It can fail for a variety of reasons, but very commonly it's simply a process holding the file active for just a little too long resulting in os.rmdir failing. This is despite the directory actually being empty when you check (i.e. it's just slow).
This is a known error in Python, of which there is no plan to fix: https://bugs.python.org/issue29982
As a result, I suggest the avoidance of using TemporaryDirectory, over which we have no control of the cleanup, and instead use mkdtemp manually, followed by a short loop to try and remove the directory.
In particular in helpers.py:
@contextmanager
def temporary_directory(*args, **kwargs):
try:
from tempfile import TemporaryDirectory
with TemporaryDirectory(*args, **kwargs) as name:
yield name
except ImportError:
name = tempfile.mkdtemp(*args, **kwargs)
yield name
shutil.rmtree(name)should instead be something like:
@contextmanager
def temporary_directory(*args, **kwargs):
name = tempfile.mkdtemp(*args, **kwargs)
yield name
robust_rmtree(name)
def robust_rmtree(path, max_retries=6):
"""Robustly tries to delete paths.
Retries several times if an OSError occurs.
If the final attempt fails, the Exception is propagated
to the caller.
"""
for i in range(max_retries - 1):
try:
shutil.rmtree(path)
return # Only hits this on success
except OSError:
time.sleep(1)
# Final attempt, pass any Exceptions up to caller.
shutil.rmtree(path)This is done in a few other projects:
The latter is where I ripped the example function.
I'm willing to write a pull request later to test this myself. But I'm writing the issue now as it is a problem that should be dealt with.