-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Don't follow symlinks when uninstalling files #2552
Changes from 4 commits
09f2b34
7e9a42d
e1989f0
9f66f31
7982ecc
7034545
068dfd7
94c63a7
b5d84a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import os | ||
import shutil | ||
import sys | ||
import tempfile | ||
|
||
import pytest | ||
from mock import Mock | ||
|
||
from pip.locations import running_under_virtualenv | ||
from pip.req.req_uninstall import UninstallPathSet | ||
|
||
class TestUninstallPathSet(object): | ||
def setup(self): | ||
if running_under_virtualenv(): | ||
# Construct tempdir in sys.prefix, otherwise UninstallPathSet | ||
# will reject paths. | ||
self.tempdir = tempfile.mkdtemp(prefix=sys.prefix) | ||
else: | ||
self.tempdir = tempfile.mkdtemp() | ||
|
||
def teardown(self): | ||
shutil.rmtree(self.tempdir, ignore_errors=True) | ||
|
||
def test_add(self): | ||
file_extant = os.path.join(self.tempdir, 'foo') | ||
file_nonexistant = os.path.join(self.tempdir, 'nonexistant') | ||
with open(file_extant, 'w'): pass | ||
|
||
ups = UninstallPathSet(dist=Mock()) | ||
assert ups.paths == set() | ||
ups.add(file_extant) | ||
assert ups.paths == set([file_extant]) | ||
|
||
ups.add(file_nonexistant) | ||
assert ups.paths == set([file_extant]) | ||
|
||
@pytest.mark.skipif("sys.platform == 'win32'") | ||
def test_add_symlink(self): | ||
f = os.path.join(self.tempdir, 'foo') | ||
with open(f, 'w'): pass | ||
l = os.path.join(self.tempdir, 'foo_link') | ||
os.symlink(f, l) | ||
|
||
ups = UninstallPathSet(dist=Mock()) | ||
ups.add(l) | ||
assert ups.paths == set([l]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ | |
|
||
from mock import Mock, patch | ||
from pip.utils import (egg_link_path, Inf, get_installed_distributions, | ||
untar_file, unzip_file, rmtree) | ||
untar_file, unzip_file, rmtree, normalize_path) | ||
from pip.operations.freeze import freeze_excludes | ||
|
||
|
||
|
@@ -369,3 +369,36 @@ def test_rmtree_retries_for_3sec(tmpdir, monkeypatch): | |
monkeypatch.setattr(shutil, 'rmtree', Failer(duration=5).call) | ||
with pytest.raises(OSError): | ||
rmtree('foo') | ||
|
||
class Test_normalize_path(object): | ||
def setup(self): | ||
self.tempdir = tempfile.mkdtemp() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in general, we're trying to use pytests's "tmpdir" fixture for temp dirs |
||
self.orig_working_dir = os.getcwd() | ||
os.chdir(self.tempdir) | ||
|
||
def teardown(self): | ||
os.chdir(self.orig_working_dir) | ||
shutil.rmtree(self.tempdir, ignore_errors=True) | ||
|
||
# Technically, symlinks are possible on Windows, but you need a special | ||
# permission bit to create them, and Python 2 doesn't support it anyway, so | ||
# it's easiest just to skip this test on Windows altogether. | ||
@pytest.mark.skipif("sys.platform == 'win32'") | ||
def test_resolve_symlinks(self): | ||
d = os.path.join('foo', 'bar') | ||
f = os.path.join(d, 'file1') | ||
os.makedirs(d) | ||
with open(f, 'w'): pass # Create the file | ||
|
||
os.symlink(d, 'dir_link') | ||
os.symlink(f, 'file_link') | ||
|
||
assert normalize_path('dir_link/file1', resolve_symlinks=True) \ | ||
== os.path.join(self.tempdir, f) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Travis pep8 tests are failing on this line (and the similar lines below), with:
I can't find an indentation that makes it happy - one extra space of indentation is still 'over-indented', and zero spaces gives another warning about 'missing indentation'. What is the desired formatting for a statement like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. assert normalize_path(
"dir_link/file1", resolve_symlinks=True,
) == os.path.join(self.tempdir, f) Will probably work. |
||
assert normalize_path('dir_link/file1', resolve_symlinks=False) \ | ||
== os.path.join(self.tempdir, 'dir_link', 'file1') | ||
|
||
assert normalize_path('file_link', resolve_symlinks=True) \ | ||
== os.path.join(self.tempdir, f) | ||
assert normalize_path('file_link', resolve_symlinks=False) \ | ||
== os.path.join(self.tempdir, 'file_link') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather not have the pip test suite creating temp in sys.prefix.
how about a monkeypatch for
is_local
insteadThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use pytest's "monkeypatch"