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

[WIP] Implement --global and try to default to --user when it makes sense #2418

Closed
wants to merge 1 commit into from
Closed
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
14 changes: 14 additions & 0 deletions pip/commands/install.py
Original file line number Diff line number Diff line change
@@ -15,6 +15,7 @@
InstallationError, CommandError, PreviousBuildDirError,
)
from pip import cmdoptions
from pip.utils import default_user_site
from pip.utils.build import BuildDirectory
from pip.utils.deprecation import RemovedInPip7Warning, RemovedInPip8Warning

@@ -126,6 +127,12 @@ def __init__(self, *args, **kw):
action='store_true',
help='Install using the user scheme.')

cmd_opts.add_option(
'--global',
dest='use_user_site',
action='store_false',
help='Install into the global site packages.')

cmd_opts.add_option(
'--egg',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know enough about how command options translate to environment variables and/or config file entries. Does this mean that PIP_GLOBAL or a global setting in the config file would make global the default (once the hard-coded default becomes "user")? If not, how would someone say they wanted to have global as the default?

dest='as_egg',
@@ -224,6 +231,13 @@ def run(self, options, args):

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

# If we don't have an explicitly selected --user or --global then we
# want to execute our fallback code to determine what we're going to
# use.
if options.use_user_site is None:
options.use_user_site = default_user_site()

if options.use_user_site:
if virtualenv_no_global():
raise InstallationError(
34 changes: 33 additions & 1 deletion pip/utils/__init__.py
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@
import os
import posixpath
import shutil
import site
import stat
import subprocess
import sys
@@ -39,7 +40,7 @@
'make_path_relative', 'normalize_path',
'renames', 'get_terminal_size', 'get_prog',
'unzip_file', 'untar_file', 'unpack_file', 'call_subprocess',
'captured_stdout', 'remove_tracebacks']
'captured_stdout', 'remove_tracebacks', 'default_user_site']


logger = logging.getLogger(__name__)
@@ -54,6 +55,37 @@ def get_prog():
return 'pip'


def default_user_site():
# Avoid circular import, the running_under_virtualenv should probably be
# in pip.utils anyways TBH.
from pip.locations import running_under_virtualenv, distutils_scheme

# If we're running inside of a virtual environment, we do not want to
# install to the --user directory.
if running_under_virtualenv():
return False

# If the Python we're running under does not have their user packages
# enabled then we do not want to install to the user directory since it
# may or may not work or exist.
if not site.ENABLE_USER_SITE:
return False

# If any of our potentional locations for writing files is not writable by
# us then we want to use the --user scheme instead of the --global scheme.
# TODO: We should figure out a way to make this work for --root and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if someone intentionally specifies a --root, but they don't have permissions, it should just fail I think, and never switch to a user install. same thing when specifying custom --install-option arguments.

note that our distutils_scheme function reads custom locations from distutils config files, so with your implementation as is, you will be switching users to a user install if any of those custom paths are not writable, which I think is misleading.

# --isolated and such options as well.
# TODO: Figure out if this works when the directories don't exist.
for path in distutils_scheme("").values():
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a concern here with using distutils directly, vs setuptools, since for sdist, that's where the scheme is determined?

if not os.access(path, os.W_OK):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to behave properly on Windows. os.access(r'C:\Windows', os.W_OK) is True for me, but open(r'C:\Windows\test.file', 'w').close() fails with errors on 2.7 and 3.4.

I was expecting this check to happen at a higher level - about the point where pip would terminate with an error but instead retry with --user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retrying with --user seems like the wrong answer to me to be honest. It sounds like it'd be easy to lead to half way installed things and it would be a lot more fragile. If os.access doesn't work we can probably do something based on trying to open file with w to test it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How soon do we know the destination subfolder? Can we switch when trying to create that (and maybe bring creation sooner if necessary)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, but thinking about it more the other side of this is that we don't want pip install foo and pip install bar to install to different locations, so this should ideally be a check that is independent of the thing we're installing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.access currently always returns True for directories on Windows: http://bugs.python.org/issue2528

return True

# If we get to this point, then we will assume that we want to use
# --global, as that is the most backwards compatible policy and it will
# mean that ``sudo pip install`` continues to work as it had previously.
return False


# Retry every half second for up to 3 seconds
@retry(stop_max_delay=3000, wait_fixed=500)
def rmtree(dir, ignore_errors=False):