Skip to content

Commit

Permalink
Configuration files may now also be stored under sys.prefix (#6268)
Browse files Browse the repository at this point in the history
* Rename kinds.VENV to kinds.SITE and site_config_files to global_config_files
* Add tests for config file options
* Deprecate --venv in pip config
  • Loading branch information
zooba authored and pradyunsg committed Mar 7, 2019
1 parent 61e5970 commit 293c91e
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 54 deletions.
1 change: 1 addition & 0 deletions news/5060.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Configuration files may now also be stored under ``sys.prefix``
60 changes: 43 additions & 17 deletions src/pip/_internal/commands/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
from pip._internal.cli.status_codes import ERROR, SUCCESS
from pip._internal.configuration import Configuration, kinds
from pip._internal.exceptions import PipError
from pip._internal.locations import venv_config_file
from pip._internal.locations import running_under_virtualenv, site_config_file
from pip._internal.utils.deprecation import deprecated
from pip._internal.utils.misc import get_prog

logger = logging.getLogger(__name__)
Expand All @@ -23,7 +24,7 @@ class ConfigurationCommand(Command):
set: Set the name=value
unset: Unset the value associated with name
If none of --user, --global and --venv are passed, a virtual
If none of --user, --global and --site are passed, a virtual
environment configuration file is used if one is active and the file
exists. Otherwise, all modifications happen on the to the user file by
default.
Expand Down Expand Up @@ -73,12 +74,23 @@ def __init__(self, *args, **kwargs):
help='Use the user configuration file only'
)

self.cmd_opts.add_option(
'--site',
dest='site_file',
action='store_true',
default=False,
help='Use the current environment configuration file only'
)

self.cmd_opts.add_option(
'--venv',
dest='venv_file',
action='store_true',
default=False,
help='Use the virtualenv configuration file only'
help=(
'[Deprecated] Use the current environment configuration '
'file in a virtual environment only'
)
)

self.parser.insert_option_group(0, self.cmd_opts)
Expand Down Expand Up @@ -127,27 +139,41 @@ def run(self, options, args):
return SUCCESS

def _determine_file(self, options, need_value):
file_options = {
kinds.USER: options.user_file,
kinds.GLOBAL: options.global_file,
kinds.VENV: options.venv_file
}

if sum(file_options.values()) == 0:
# Convert legacy venv_file option to site_file or error
if options.venv_file and not options.site_file:
if running_under_virtualenv():
options.site_file = True
deprecated(
"The --venv option has been deprecated.",
replacement="--site",
gone_in="19.3",
)
else:
raise PipError(
"Legacy --venv option requires a virtual environment. "
"Use --site instead."
)

file_options = [key for key, value in (
(kinds.USER, options.user_file),
(kinds.GLOBAL, options.global_file),
(kinds.SITE, options.site_file),
) if value]

if not file_options:
if not need_value:
return None
# Default to user, unless there's a virtualenv file.
elif os.path.exists(venv_config_file):
return kinds.VENV
# Default to user, unless there's a site file.
elif os.path.exists(site_config_file):
return kinds.SITE
else:
return kinds.USER
elif sum(file_options.values()) == 1:
# There's probably a better expression for this.
return [key for key in file_options if file_options[key]][0]
elif len(file_options) == 1:
return file_options[0]

raise PipError(
"Need exactly one file to operate upon "
"(--user, --venv, --global) to perform."
"(--user, --site, --global) to perform."
)

def list_values(self, options, args):
Expand Down
14 changes: 6 additions & 8 deletions src/pip/_internal/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@
ConfigurationError, ConfigurationFileCouldNotBeLoaded,
)
from pip._internal.locations import (
legacy_config_file, new_config_file, running_under_virtualenv,
site_config_files, venv_config_file,
global_config_files, legacy_config_file, new_config_file, site_config_file,
)
from pip._internal.utils.misc import ensure_dir, enum
from pip._internal.utils.typing import MYPY_CHECK_RUNNING
Expand Down Expand Up @@ -58,7 +57,7 @@ def _disassemble_key(name):
kinds = enum(
USER="user", # User Specific
GLOBAL="global", # System Wide
VENV="venv", # Virtual Environment Specific
SITE="site", # [Virtual] Environment Specific
ENV="env", # from PIP_CONFIG_FILE
ENV_VAR="env-var", # from Environment Variables
)
Expand All @@ -82,7 +81,7 @@ def __init__(self, isolated, load_only=None):
# type: (bool, Kind) -> None
super(Configuration, self).__init__()

_valid_load_only = [kinds.USER, kinds.GLOBAL, kinds.VENV, None]
_valid_load_only = [kinds.USER, kinds.GLOBAL, kinds.SITE, None]
if load_only not in _valid_load_only:
raise ConfigurationError(
"Got invalid value for load_only - should be one of {}".format(
Expand All @@ -94,7 +93,7 @@ def __init__(self, isolated, load_only=None):

# The order here determines the override order.
self._override_order = [
kinds.GLOBAL, kinds.USER, kinds.VENV, kinds.ENV, kinds.ENV_VAR
kinds.GLOBAL, kinds.USER, kinds.SITE, kinds.ENV, kinds.ENV_VAR
]

self._ignore_env_names = ["version", "help"]
Expand Down Expand Up @@ -351,7 +350,7 @@ def _iter_config_files(self):
yield kinds.ENV, []

# at the base we have any global configuration
yield kinds.GLOBAL, list(site_config_files)
yield kinds.GLOBAL, list(global_config_files)

# per-user configuration next
should_load_user_config = not self.isolated and not (
Expand All @@ -362,8 +361,7 @@ def _iter_config_files(self):
yield kinds.USER, [legacy_config_file, new_config_file]

# finally virtualenv configuration first trumping others
if running_under_virtualenv():
yield kinds.VENV, [venv_config_file]
yield kinds.SITE, [site_config_file]

def _get_parser_to_modify(self):
# type: () -> Tuple[str, RawConfigParser]
Expand Down
4 changes: 2 additions & 2 deletions src/pip/_internal/locations.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ def virtualenv_no_global():
if sys.platform[:6] == 'darwin' and sys.prefix[:16] == '/System/Library/':
bin_py = '/usr/local/bin'

site_config_files = [
global_config_files = [
os.path.join(path, config_basename)
for path in appdirs.site_config_dirs('pip')
]

venv_config_file = os.path.join(sys.prefix, config_basename)
site_config_file = os.path.join(sys.prefix, config_basename)
new_config_file = os.path.join(appdirs.user_config_dir("pip"), config_basename)


Expand Down
32 changes: 16 additions & 16 deletions tests/unit/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from pip._internal.exceptions import ConfigurationError
from pip._internal.locations import (
new_config_file, site_config_files, venv_config_file,
global_config_files, new_config_file, site_config_file,
)
from tests.lib.configuration_helpers import ConfigurationMixin, kinds

Expand All @@ -27,8 +27,8 @@ def test_user_loading(self):
self.configuration.load()
assert self.configuration.get_value("test.hello") == "2"

def test_venv_loading(self):
self.patch_configuration(kinds.VENV, {"test.hello": "3"})
def test_site_loading(self):
self.patch_configuration(kinds.SITE, {"test.hello": "3"})

self.configuration.load()
assert self.configuration.get_value("test.hello") == "3"
Expand Down Expand Up @@ -90,8 +90,8 @@ class TestConfigurationPrecedence(ConfigurationMixin):
# Tests for methods to that determine the order of precedence of
# configuration options

def test_env_overides_venv(self):
self.patch_configuration(kinds.VENV, {"test.hello": "1"})
def test_env_overides_site(self):
self.patch_configuration(kinds.SITE, {"test.hello": "1"})
self.patch_configuration(kinds.ENV, {"test.hello": "0"})
self.configuration.load()

Expand All @@ -111,16 +111,16 @@ def test_env_overides_global(self):

assert self.configuration.get_value("test.hello") == "0"

def test_venv_overides_user(self):
def test_site_overides_user(self):
self.patch_configuration(kinds.USER, {"test.hello": "2"})
self.patch_configuration(kinds.VENV, {"test.hello": "1"})
self.patch_configuration(kinds.SITE, {"test.hello": "1"})
self.configuration.load()

assert self.configuration.get_value("test.hello") == "1"

def test_venv_overides_global(self):
def test_site_overides_global(self):
self.patch_configuration(kinds.GLOBAL, {"test.hello": "3"})
self.patch_configuration(kinds.VENV, {"test.hello": "1"})
self.patch_configuration(kinds.SITE, {"test.hello": "1"})
self.configuration.load()

assert self.configuration.get_value("test.hello") == "1"
Expand All @@ -141,8 +141,8 @@ def test_env_not_overriden_by_environment_var(self):
assert self.configuration.get_value("test.hello") == "1"
assert self.configuration.get_value(":env:.hello") == "5"

def test_venv_not_overriden_by_environment_var(self):
self.patch_configuration(kinds.VENV, {"test.hello": "2"})
def test_site_not_overriden_by_environment_var(self):
self.patch_configuration(kinds.SITE, {"test.hello": "2"})
os.environ["PIP_HELLO"] = "5"

self.configuration.load()
Expand Down Expand Up @@ -182,8 +182,8 @@ def test_no_specific_given_modification(self):
else:
assert False, "Should have raised an error."

def test_venv_modification(self):
self.configuration.load_only = kinds.VENV
def test_site_modification(self):
self.configuration.load_only = kinds.SITE
self.configuration.load()

# Mock out the method
Expand All @@ -192,9 +192,9 @@ def test_venv_modification(self):

self.configuration.set_value("test.hello", "10")

# get the path to venv config file
# get the path to site config file
assert mymock.call_count == 1
assert mymock.call_args[0][0] == venv_config_file
assert mymock.call_args[0][0] == site_config_file

def test_user_modification(self):
# get the path to local config file
Expand Down Expand Up @@ -224,4 +224,4 @@ def test_global_modification(self):

# get the path to user config file
assert mymock.call_count == 1
assert mymock.call_args[0][0] == site_config_files[-1]
assert mymock.call_args[0][0] == global_config_files[-1]
62 changes: 51 additions & 11 deletions tests/unit/test_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

import pip._internal.configuration
from pip._internal import main
from pip._internal.commands import DownloadCommand
from pip._internal.commands import ConfigurationCommand, DownloadCommand
from pip._internal.exceptions import PipError
from tests.lib.options_helpers import AddFakeCommandMixin


Expand Down Expand Up @@ -383,23 +384,62 @@ def test_client_cert(self):
class TestOptionsConfigFiles(object):

def test_venv_config_file_found(self, monkeypatch):
# strict limit on the site_config_files list
# strict limit on the global_config_files list
monkeypatch.setattr(
pip._internal.configuration, 'site_config_files', ['/a/place']
pip._internal.configuration, 'global_config_files', ['/a/place']
)

# If we are running in a virtualenv and all files appear to exist,
# we should see two config files.
monkeypatch.setattr(
pip._internal.configuration,
'running_under_virtualenv',
lambda: True,
)
monkeypatch.setattr(os.path, 'exists', lambda filename: True)
cp = pip._internal.configuration.Configuration(isolated=False)

files = []
for _, val in cp._iter_config_files():
files.extend(val)

assert len(files) == 4

@pytest.mark.parametrize(
"args, expect",
(
([], None),
(["--global"], "global"),
(["--site"], "site"),
(["--user"], "user"),
(["--global", "--user"], PipError),
(["--global", "--site"], PipError),
(["--global", "--site", "--user"], PipError),
)
)
def test_config_file_options(self, monkeypatch, args, expect):
cmd = ConfigurationCommand()
# Replace a handler with a no-op to avoid side effects
monkeypatch.setattr(cmd, "get_name", lambda *a: None)

options, args = cmd.parser.parse_args(args + ["get", "name"])
if expect is PipError:
with pytest.raises(PipError):
cmd._determine_file(options, need_value=False)
else:
assert expect == cmd._determine_file(options, need_value=False)

def test_config_file_venv_option(self, monkeypatch):
cmd = ConfigurationCommand()
# Replace a handler with a no-op to avoid side effects
monkeypatch.setattr(cmd, "get_name", lambda *a: None)

collected_warnings = []

def _warn(message, *a, **kw):
collected_warnings.append(message)
monkeypatch.setattr("warnings.warn", _warn)

options, args = cmd.parser.parse_args(["--venv", "get", "name"])
assert "site" == cmd._determine_file(options, need_value=False)
assert collected_warnings
assert "--site" in collected_warnings[0]

# No warning or error if both "--venv" and "--site" are specified
collected_warnings[:] = []
options, args = cmd.parser.parse_args(["--venv", "--site", "get",
"name"])
assert "site" == cmd._determine_file(options, need_value=False)
assert not collected_warnings

0 comments on commit 293c91e

Please sign in to comment.