McSinyx
/
pip
mirror of https://github.com/pypa/pip
1
1
Fork 0
Code Issues Releases Activity

AdjacentTempDirectory should fail on unwritable directory (#6215) 

Based on #6225
This commit is contained in:
Pradyun Gedam 2019-02-08 14:08:27 +05:30 committed by GitHub
parent ab53c8209c c64503e58c
commit 7eb79b13d0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 310 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
.gitignore vendored
View File

@ -32,9 +32,13 @@ pip-wheel-metadata
# Misc
*~
.*.sw?
.env/
# For IntelliJ IDEs (basically PyCharm)
.idea/
# For Visual Studio Code
.vscode/
# Scratch Pad for experiments
.scratch/

1
news/6169.bugfix Normal file
View File

@ -0,0 +1 @@
``AdjacentTempDirectory`` fails on unwritable directory instead of locking up the uninstall command.

2
news/6194.bugfix
View File

@ -1 +1 @@
Make failed uninstalls roll back more reliably and better at avoiding naming conflicts.
Make failed uninstalls roll back more reliably and better at avoiding naming conflicts.

160
src/pip/_internal/req/req_uninstall.py
View File

@ -17,7 +17,7 @@ from pip._internal.utils.misc import (
FakeFile, ask, dist_in_usersite, dist_is_local, egg_link_path, is_local,
normalize_path, renames, rmtree,
)
from pip._internal.utils.temp_dir import AdjacentTempDirectory
from pip._internal.utils.temp_dir import AdjacentTempDirectory, TempDirectory
logger = logging.getLogger(__name__)
@ -127,7 +127,7 @@ def compress_for_rename(paths):
# If all the files we found are in our remaining set of files to
# remove, then remove them from the latter set and add a wildcard
# for the directory.
if len(all_files - remaining) == 0:
if not (all_files - remaining):
remaining.difference_update(all_files)
wildcards.add(root + os.sep)
@ -183,6 +183,111 @@ def compress_for_output_listing(paths):
return will_remove, will_skip
class StashedUninstallPathSet(object):
"""A set of file rename operations to stash files while
tentatively uninstalling them."""
def __init__(self):
# Mapping from source file root to [Adjacent]TempDirectory
# for files under that directory.
self._save_dirs = {}
# (old path, new path) tuples for each move that may need
# to be undone.
self._moves = []
def _get_directory_stash(self, path):
"""Stashes a directory.
Directories are stashed adjacent to their original location if
possible, or else moved/copied into the user's temp dir."""
try:
save_dir = AdjacentTempDirectory(path)
save_dir.create()
except OSError:
save_dir = TempDirectory(kind="uninstall")
save_dir.create()
self._save_dirs[os.path.normcase(path)] = save_dir
return save_dir.path
def _get_file_stash(self, path):
"""Stashes a file.
If no root has been provided, one will be created for the directory
in the user's temp directory."""
path = os.path.normcase(path)
head, old_head = os.path.dirname(path), None
save_dir = None
while head != old_head:
try:
save_dir = self._save_dirs[head]
break
except KeyError:
pass
head, old_head = os.path.dirname(head), head
else:
# Did not find any suitable root
head = os.path.dirname(path)
save_dir = TempDirectory(kind='uninstall')
save_dir.create()
self._save_dirs[head] = save_dir
relpath = os.path.relpath(path, head)
if relpath and relpath != os.path.curdir:
return os.path.join(save_dir.path, relpath)
return save_dir.path
def stash(self, path):
"""Stashes the directory or file and returns its new location.
"""
if os.path.isdir(path):
new_path = self._get_directory_stash(path)
else:
new_path = self._get_file_stash(path)
self._moves.append((path, new_path))
if os.path.isdir(path) and os.path.isdir(new_path):
# If we're moving a directory, we need to
# remove the destination first or else it will be
# moved to inside the existing directory.
# We just created new_path ourselves, so it will
# be removable.
os.rmdir(new_path)
renames(path, new_path)
return new_path
def commit(self):
"""Commits the uninstall by removing stashed files."""
for _, save_dir in self._save_dirs.items():
save_dir.cleanup()
self._moves = []
self._save_dirs = {}
def rollback(self):
"""Undoes the uninstall by moving stashed files back."""
for p in self._moves:
logging.info("Moving to %s\n from %s", *p)
for new_path, path in self._moves:
try:
logger.debug('Replacing %s from %s', new_path, path)
if os.path.isfile(new_path):
os.unlink(new_path)
elif os.path.isdir(new_path):
rmtree(new_path)
renames(path, new_path)
except OSError as ex:
logger.error("Failed to restore %s", new_path)
logger.debug("Exception: %s", ex)
self.commit()
@property
def can_rollback(self):
return bool(self._moves)
class UninstallPathSet(object):
"""A set of file paths to be removed in the uninstallation of a
requirement."""
@ -191,8 +296,7 @@ class UninstallPathSet(object):
self._refuse = set()
self.pth = {}
self.dist = dist
self._save_dirs = []
self._moved_paths = []
self._moved_paths = StashedUninstallPathSet()
def _permitted(self, path):
"""
@ -230,22 +334,6 @@ class UninstallPathSet(object):
else:
self._refuse.add(pth_file)
def _stash(self, path):
best = None
for save_dir in self._save_dirs:
if not path.startswith(save_dir.original + os.sep):
continue
if not best or len(save_dir.original) > len(best.original):
best = save_dir
if best is None:
best = AdjacentTempDirectory(os.path.dirname(path))
best.create()
self._save_dirs.append(best)
relpath = os.path.relpath(path, best.original)
if not relpath or relpath == os.path.curdir:
return best.path
return os.path.join(best.path, relpath)
def remove(self, auto_confirm=False, verbose=False):
"""Remove paths in ``self.paths`` with confirmation (unless
``auto_confirm`` is True)."""
@ -264,18 +352,14 @@ class UninstallPathSet(object):
with indent_log():
if auto_confirm or self._allowed_to_proceed(verbose):
for path in sorted(compact(compress_for_rename(self.paths))):
new_path = self._stash(path)
moved = self._moved_paths
for_rename = compress_for_rename(self.paths)
for path in sorted(compact(for_rename)):
moved.stash(path)
logger.debug('Removing file or directory %s', path)
self._moved_paths.append((path, new_path))
if os.path.isdir(path) and os.path.isdir(new_path):
# If we're moving a directory, we need to
# remove the destination first or else it will be
# moved to inside the existing directory.
# We just created new_path ourselves, so it will
# be removable.
os.rmdir(new_path)
renames(path, new_path)
for pth in self.pth.values():
pth.remove()
@ -312,28 +396,20 @@ class UninstallPathSet(object):
def rollback(self):
"""Rollback the changes previously made by remove()."""
if not self._save_dirs:
if not self._moved_paths.can_rollback:
logger.error(
"Can't roll back %s; was not uninstalled",
self.dist.project_name,
)
return False
logger.info('Rolling back uninstall of %s', self.dist.project_name)
for path, tmp_path in self._moved_paths:
logger.debug('Replacing %s', path)
if os.path.isdir(tmp_path) and os.path.isdir(path):
rmtree(path)
renames(tmp_path, path)
self._moved_paths.rollback()
for pth in self.pth.values():
pth.rollback()
for save_dir in self._save_dirs:
save_dir.cleanup()
def commit(self):
"""Remove temporary save dir: rollback will no longer be possible."""
for save_dir in self._save_dirs:
save_dir.cleanup()
self._moved_paths = []
self._moved_paths.commit()
@classmethod
def from_dist(cls, dist):

17
src/pip/_internal/utils/temp_dir.py
View File

@ -1,5 +1,6 @@
from __future__ import absolute_import
import errno
import itertools
import logging
import os.path
@ -118,22 +119,30 @@ class AdjacentTempDirectory(TempDirectory):
package).
"""
for i in range(1, len(name)):
if name[i] in cls.LEADING_CHARS:
continue
for candidate in itertools.combinations_with_replacement(
cls.LEADING_CHARS, i - 1):
new_name = '~' + ''.join(candidate) + name[i:]
if new_name != name:
yield new_name
# If we make it this far, we will have to make a longer name
for i in range(len(cls.LEADING_CHARS)):
for candidate in itertools.combinations_with_replacement(
cls.LEADING_CHARS, i):
new_name = '~' + ''.join(candidate) + name
if new_name != name:
yield new_name
def create(self):
root, name = os.path.split(self.original)
for candidate in self._generate_names(name):
path = os.path.join(root, candidate)
try:
os.mkdir(path)
except OSError:
pass
except OSError as ex:
# Continue if the name exists already
if ex.errno != errno.EEXIST:
raise
else:
self.path = os.path.realpath(path)
break

97
tests/unit/test_req_uninstall.py
View File

@ -5,8 +5,8 @@ from mock import Mock
import pip._internal.req.req_uninstall
from pip._internal.req.req_uninstall import (
UninstallPathSet, compact, compress_for_output_listing,
compress_for_rename, uninstallation_paths,
StashedUninstallPathSet, UninstallPathSet, compact,
compress_for_output_listing, compress_for_rename, uninstallation_paths,
)
from tests.lib import create_file
@ -175,3 +175,96 @@ class TestUninstallPathSet(object):
ups.add(path1)
ups.add(path2)
assert ups.paths == {path1}
class TestStashedUninstallPathSet(object):
WALK_RESULT = [
("A", ["B", "C"], ["a.py"]),
("A/B", ["D"], ["b.py"]),
("A/B/D", [], ["c.py"]),
("A/C", [], ["d.py", "e.py"]),
("A/E", ["F"], ["f.py"]),
("A/E/F", [], []),
("A/G", ["H"], ["g.py"]),
("A/G/H", [], ["h.py"]),
]
@classmethod
def mock_walk(cls, root):
for dirname, subdirs, files in cls.WALK_RESULT:
dirname = os.path.sep.join(dirname.split("/"))
if dirname.startswith(root):
yield dirname[len(root) + 1:], subdirs, files
def test_compress_for_rename(self, monkeypatch):
paths = [os.path.sep.join(p.split("/")) for p in [
"A/B/b.py",
"A/B/D/c.py",
"A/C/d.py",
"A/E/f.py",
"A/G/g.py",
]]
expected_paths = [os.path.sep.join(p.split("/")) for p in [
"A/B/", # selected everything below A/B
"A/C/d.py", # did not select everything below A/C
"A/E/", # only empty folders remain under A/E
"A/G/g.py", # non-empty folder remains under A/G
]]
monkeypatch.setattr('os.walk', self.mock_walk)
actual_paths = compress_for_rename(paths)
assert set(expected_paths) == set(actual_paths)
@classmethod
def make_stash(cls, tmpdir, paths):
for dirname, subdirs, files in cls.WALK_RESULT:
root = os.path.join(tmpdir, *dirname.split("/"))
if not os.path.exists(root):
os.mkdir(root)
for d in subdirs:
os.mkdir(os.path.join(root, d))
for f in files:
with open(os.path.join(root, f), "wb"):
pass
pathset = StashedUninstallPathSet()
paths = [os.path.join(tmpdir, *p.split('/')) for p in paths]
stashed_paths = [(p, pathset.stash(p)) for p in paths]
return pathset, stashed_paths
def test_stash(self, tmpdir):
pathset, stashed_paths = self.make_stash(tmpdir, [
"A/B/", "A/C/d.py", "A/E/", "A/G/g.py",
])
for old_path, new_path in stashed_paths:
assert not os.path.exists(old_path)
assert os.path.exists(new_path)
assert stashed_paths == pathset._moves
def test_commit(self, tmpdir):
pathset, stashed_paths = self.make_stash(tmpdir, [
"A/B/", "A/C/d.py", "A/E/", "A/G/g.py",
])
pathset.commit()
for old_path, new_path in stashed_paths:
assert not os.path.exists(old_path)
assert not os.path.exists(new_path)
def test_rollback(self, tmpdir):
pathset, stashed_paths = self.make_stash(tmpdir, [
"A/B/", "A/C/d.py", "A/E/", "A/G/g.py",
])
pathset.rollback()
for old_path, new_path in stashed_paths:
assert os.path.exists(old_path)
assert not os.path.exists(new_path)

87
tests/unit/test_utils.py
View File

@ -553,6 +553,10 @@ class TestTempDirectory(object):
"ABC.dist-info",
"_+-",
"_package",
"A......B",
"AB",
"A",
"2",
])
def test_adjacent_directory_names(self, name):
def names():
@ -566,17 +570,82 @@ class TestTempDirectory(object):
# result that works, provided there are many of those
# and that shorter names result in totally unique sets,
# it's okay to skip part of the test.)
some_names = list(itertools.islice(names(), 10000))
assert len(some_names) == len(set(some_names))
some_names = list(itertools.islice(names(), 1000))
# We should always get at least 1000 names
assert len(some_names) == 1000
# Ensure original name does not appear
assert not any(n == name for n in names())
# Ensure original name does not appear early in the set
assert name not in some_names
# Check the first group are correct
expected_names = ['~' + name[1:]]
expected_names.extend('~' + c + name[2:] for c in chars)
for x, y in zip(some_names, expected_names):
assert x == y
if len(name) > 2:
# Names should be at least 90% unique (given the infinite
# range of inputs, and the possibility that generated names
# may already exist on disk anyway, this is a much cheaper
# criteria to enforce than complete uniqueness).
assert len(some_names) > 0.9 * len(set(some_names))
# Ensure the first few names are the same length as the original
same_len = list(itertools.takewhile(
lambda x: len(x) == len(name),
some_names
))
assert len(same_len) > 10
# Check the first group are correct
expected_names = ['~' + name[1:]]
expected_names.extend('~' + c + name[2:] for c in chars)
for x, y in zip(some_names, expected_names):
assert x == y
else:
# All names are going to be longer than our original
assert min(len(x) for x in some_names) > 1
# All names are going to be unqiue
assert len(some_names) == len(set(some_names))
if len(name) == 2:
# All but the first name are going to end with our original
assert all(x.endswith(name) for x in some_names[1:])
else:
# All names are going to end with our original
assert all(x.endswith(name) for x in some_names)
@pytest.mark.parametrize("name", [
"A",
"ABC",
"ABC.dist-info",
"_+-",
"_package",
])
def test_adjacent_directory_exists(self, name, tmpdir):
block_name, expect_name = itertools.islice(
AdjacentTempDirectory._generate_names(name), 2)
original = os.path.join(tmpdir, name)
blocker = os.path.join(tmpdir, block_name)
ensure_dir(original)
ensure_dir(blocker)
with AdjacentTempDirectory(original) as atmp_dir:
assert expect_name == os.path.split(atmp_dir.path)[1]
def test_adjacent_directory_permission_error(self, monkeypatch):
name = "ABC"
def raising_mkdir(*args, **kwargs):
raise OSError("Unknown OSError")
with TempDirectory() as tmp_dir:
original = os.path.join(tmp_dir.path, name)
ensure_dir(original)
monkeypatch.setattr("os.mkdir", raising_mkdir)
with pytest.raises(OSError):
with AdjacentTempDirectory(original):
pass
class TestGlibc(object):