Skip to content
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

Default to --user install in certain conditions #7002

Merged
merged 18 commits into from
Oct 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions news/1668.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Default to doing a user install (as if ``--user`` was passed) when the main
site-packages directory is not writeable and user site-packages are enabled.
86 changes: 75 additions & 11 deletions src/pip/_internal/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import operator
import os
import shutil
import site
from optparse import SUPPRESS_HELP

from pip._vendor import pkg_resources
Expand All @@ -32,7 +33,7 @@
from pip._internal.operations.check import check_install_conflicts
from pip._internal.req import RequirementSet, install_given_reqs
from pip._internal.req.req_tracker import RequirementTracker
from pip._internal.utils.filesystem import check_path_owner
from pip._internal.utils.filesystem import check_path_owner, test_writable_dir
from pip._internal.utils.misc import (
ensure_dir,
get_installed_version,
Expand Down Expand Up @@ -291,17 +292,16 @@ def run(self, options, args):

options.src_dir = os.path.abspath(options.src_dir)
install_options = options.install_options or []

options.use_user_site = decide_user_install(
options.use_user_site,
prefix_path=options.prefix_path,
target_dir=options.target_dir,
root_path=options.root_path,
isolated_mode=options.isolated_mode,
)

if options.use_user_site:
if options.prefix_path:
raise CommandError(
"Can not combine '--user' and '--prefix' as they imply "
"different installation locations"
)
if virtualenv_no_global():
raise InstallationError(
"Can not perform a '--user' install. User site-packages "
"are not visible in this virtualenv."
)
install_options.append('--user')
install_options.append('--prefix=')

Expand Down Expand Up @@ -594,6 +594,70 @@ def get_lib_location_guesses(*args, **kwargs):
return [scheme['purelib'], scheme['platlib']]


def site_packages_writable(**kwargs):
return all(
test_writable_dir(d) for d in set(get_lib_location_guesses(**kwargs))
)


def decide_user_install(
use_user_site, # type: Optional[bool]
prefix_path=None, # type: Optional[str]
target_dir=None, # type: Optional[str]
root_path=None, # type: Optional[str]
isolated_mode=False, # type: bool
):
# type: (...) -> bool
"""Determine whether to do a user install based on the input options.

If use_user_site is False, no additional checks are done.
If use_user_site is True, it is checked for compatibility with other
options.
If use_user_site is None, the default behaviour depends on the environment,
which is provided by the other arguments.
"""
if use_user_site is False:
logger.debug("Non-user install by explicit request")
return False

if use_user_site is True:
if prefix_path:
chrahunt marked this conversation as resolved.
Show resolved Hide resolved
raise CommandError(
"Can not combine '--user' and '--prefix' as they imply "
"different installation locations"
)
if virtualenv_no_global():
raise InstallationError(
"Can not perform a '--user' install. User site-packages "
"are not visible in this virtualenv."
)
logger.debug("User install by explicit request")
return True

# If we are here, user installs have not been explicitly requested/avoided
assert use_user_site is None

# user install incompatible with --prefix/--target
if prefix_path or target_dir:
logger.debug("Non-user install due to --prefix or --target option")
return False

# If user installs are not enabled, choose a non-user install
if not site.ENABLE_USER_SITE:
logger.debug("Non-user install because user site-packages disabled")
return False

# If we have permission for a non-user install, do that,
# otherwise do a user install.
if site_packages_writable(root=root_path, isolated=isolated_mode):
logger.debug("Non-user install because site-packages writeable")
return False

logger.info("Defaulting to user installation because normal site-packages "
"is not writeable")
return True


def create_env_error_message(error, show_traceback, using_user_site):
"""Format an error message for an EnvironmentError

Expand Down
54 changes: 54 additions & 0 deletions src/pip/_internal/utils/filesystem.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import errno
import os
import os.path
import random
import shutil
import stat
from contextlib import contextmanager
Expand Down Expand Up @@ -113,3 +115,55 @@ def replace(src, dest):

else:
replace = _replace_retry(os.replace)


# test_writable_dir and _test_writable_dir_win are copied from Flit,
# with the author's agreement to also place them under pip's license.
def test_writable_dir(path):
# type: (str) -> bool
"""Check if a directory is writable.

Uses os.access() on POSIX, tries creating files on Windows.
"""
# If the directory doesn't exist, find the closest parent that does.
while not os.path.isdir(path):
parent = os.path.dirname(path)
if parent == path:
break # Should never get here, but infinite loops are bad
path = parent

if os.name == 'posix':
return os.access(path, os.W_OK)

return _test_writable_dir_win(path)


def _test_writable_dir_win(path):
# type: (str) -> bool
# os.access doesn't work on Windows: http://bugs.python.org/issue2528
# and we can't use tempfile: http://bugs.python.org/issue22107
basename = 'accesstest_deleteme_fishfingers_custard_'
alphabet = 'abcdefghijklmnopqrstuvwxyz0123456789'
for i in range(10):
name = basename + ''.join(random.choice(alphabet) for _ in range(6))
file = os.path.join(path, name)
try:
fd = os.open(file, os.O_RDWR | os.O_CREAT | os.O_EXCL)
except OSError as e:
if e.errno == errno.EEXIST:
continue
if e.errno == errno.EPERM:
# This could be because there's a directory with the same name.
# But it's highly unlikely there's a directory called that,
# so we'll assume it's because the parent dir is not writable.
return False
raise
else:
os.close(fd)
os.unlink(file)
return True

# This should never be reached
raise EnvironmentError(
'Unexpected condition testing for writable directory'
)
1 change: 0 additions & 1 deletion tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,6 @@ def test_basic_editable_install(script):
in result.stderr
)
assert not result.files_created
assert not result.files_updated


@pytest.mark.svn
Expand Down
4 changes: 3 additions & 1 deletion tests/functional/test_install_upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,9 @@ def test_upgrade_to_same_version_from_url(script):
'https://files.pythonhosted.org/packages/source/I/INITools/INITools-'
'0.3.tar.gz',
)
assert not result2.files_updated, 'INITools 0.3 reinstalled same version'
assert script.site_packages / 'initools' not in result2.files_updated, (
'INITools 0.3 reinstalled same version'
)
result3 = script.pip('uninstall', 'initools', '-y')
assert_all_changes(result, result3, [script.venv / 'build', 'cache'])

Expand Down
35 changes: 34 additions & 1 deletion tests/unit/test_command_install.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import pytest
from mock import Mock, call, patch

from pip._internal.commands.install import build_wheels
from pip._internal.commands.install import build_wheels, decide_user_install


class TestWheelCache:
Expand Down Expand Up @@ -61,3 +62,35 @@ def test_build_wheels__wheel_not_installed(self, is_wheel_installed):
]

assert build_failures == ['a']


class TestDecideUserInstall:
@patch('site.ENABLE_USER_SITE', True)
@patch('pip._internal.commands.install.site_packages_writable')
def test_prefix_and_target(self, sp_writable):
sp_writable.return_value = False

assert decide_user_install(
use_user_site=None, prefix_path='foo'
) is False

assert decide_user_install(
use_user_site=None, target_dir='bar'
) is False

@pytest.mark.parametrize(
"enable_user_site,site_packages_writable,result", [
(True, True, False),
(True, False, True),
(False, True, False),
(False, False, False),
])
def test_most_cases(
self, enable_user_site, site_packages_writable, result, monkeypatch,
):
monkeypatch.setattr('site.ENABLE_USER_SITE', enable_user_site)
monkeypatch.setattr(
'pip._internal.commands.install.site_packages_writable',
lambda **kw: site_packages_writable
)
assert decide_user_install(use_user_site=None) is result