From 79a70289d248057d8cee855dfa0a96a1d4820b7c Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 15 Feb 2019 11:24:39 -0800 Subject: [PATCH 1/8] Fixes #5060: Configuration files may now also be stored under `sys.prefix` --- news/5060.feature | 1 + src/pip/_internal/utils/appdirs.py | 4 ++++ 2 files changed, 5 insertions(+) create mode 100644 news/5060.feature diff --git a/news/5060.feature b/news/5060.feature new file mode 100644 index 00000000000..67765ae82dc --- /dev/null +++ b/news/5060.feature @@ -0,0 +1 @@ +Configuration files may now also be stored under ``sys.prefix`` \ No newline at end of file diff --git a/src/pip/_internal/utils/appdirs.py b/src/pip/_internal/utils/appdirs.py index 9af9fa7b582..1dec9efce08 100644 --- a/src/pip/_internal/utils/appdirs.py +++ b/src/pip/_internal/utils/appdirs.py @@ -190,6 +190,10 @@ def site_config_dirs(appname): # always look in /etc directly as well pathlist.append('/etc') + # allow configuration in sys.prefix without any appname + # see + pathlist.append(sys.prefix) + return pathlist From ffac44574e26c1ec5086ce88896d52f4b1d3b21b Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 15 Feb 2019 11:31:57 -0800 Subject: [PATCH 2/8] Fix tests that passed against environment pip --- tests/unit/test_appdirs.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_appdirs.py b/tests/unit/test_appdirs.py index 0980cf16aad..7edb480d522 100644 --- a/tests/unit/test_appdirs.py +++ b/tests/unit/test_appdirs.py @@ -100,7 +100,8 @@ def _get_win_folder(base): monkeypatch.setattr(appdirs, "WINDOWS", True) monkeypatch.setattr(os, "path", ntpath) - assert appdirs.site_config_dirs("pip") == ["C:\\ProgramData\\pip"] + assert appdirs.site_config_dirs("pip") == ["C:\\ProgramData\\pip", + sys.prefix] assert _get_win_folder.calls == [pretend.call("CSIDL_COMMON_APPDATA")] def test_site_config_dirs_osx(self, monkeypatch): @@ -110,7 +111,7 @@ def test_site_config_dirs_osx(self, monkeypatch): monkeypatch.setattr(sys, "platform", "darwin") assert appdirs.site_config_dirs("pip") == \ - ["/Library/Application Support/pip"] + ["/Library/Application Support/pip", sys.prefix] def test_site_config_dirs_linux(self, monkeypatch): monkeypatch.setattr(appdirs, "WINDOWS", False) @@ -120,7 +121,8 @@ def test_site_config_dirs_linux(self, monkeypatch): assert appdirs.site_config_dirs("pip") == [ '/etc/xdg/pip', - '/etc' + '/etc', + sys.prefix ] def test_site_config_dirs_linux_override(self, monkeypatch): @@ -134,7 +136,8 @@ def test_site_config_dirs_linux_override(self, monkeypatch): '/spam/pip', '/etc/pip', '/etc/xdg/pip', - '/etc' + '/etc', + sys.prefix ] From bcb97974c157c71db40d812591f84e64b76f9ee5 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 15 Feb 2019 11:41:37 -0800 Subject: [PATCH 3/8] Make sys.prefix configuration lower-priority for 'pip config set' --- src/pip/_internal/utils/appdirs.py | 3 ++- tests/unit/test_appdirs.py | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/utils/appdirs.py b/src/pip/_internal/utils/appdirs.py index 1dec9efce08..75cae548158 100644 --- a/src/pip/_internal/utils/appdirs.py +++ b/src/pip/_internal/utils/appdirs.py @@ -191,8 +191,9 @@ def site_config_dirs(appname): pathlist.append('/etc') # allow configuration in sys.prefix without any appname + # but put it first so that it is not preferred for "pip config set" # see - pathlist.append(sys.prefix) + pathlist.insert(0, sys.prefix) return pathlist diff --git a/tests/unit/test_appdirs.py b/tests/unit/test_appdirs.py index 7edb480d522..d9236eac2aa 100644 --- a/tests/unit/test_appdirs.py +++ b/tests/unit/test_appdirs.py @@ -100,8 +100,8 @@ def _get_win_folder(base): monkeypatch.setattr(appdirs, "WINDOWS", True) monkeypatch.setattr(os, "path", ntpath) - assert appdirs.site_config_dirs("pip") == ["C:\\ProgramData\\pip", - sys.prefix] + assert appdirs.site_config_dirs("pip") == [sys.prefix, + "C:\\ProgramData\\pip"] assert _get_win_folder.calls == [pretend.call("CSIDL_COMMON_APPDATA")] def test_site_config_dirs_osx(self, monkeypatch): @@ -111,7 +111,7 @@ def test_site_config_dirs_osx(self, monkeypatch): monkeypatch.setattr(sys, "platform", "darwin") assert appdirs.site_config_dirs("pip") == \ - ["/Library/Application Support/pip", sys.prefix] + [sys.prefix, "/Library/Application Support/pip"] def test_site_config_dirs_linux(self, monkeypatch): monkeypatch.setattr(appdirs, "WINDOWS", False) @@ -120,9 +120,9 @@ def test_site_config_dirs_linux(self, monkeypatch): monkeypatch.setattr(sys, "platform", "linux2") assert appdirs.site_config_dirs("pip") == [ + sys.prefix, '/etc/xdg/pip', '/etc', - sys.prefix ] def test_site_config_dirs_linux_override(self, monkeypatch): @@ -133,11 +133,11 @@ def test_site_config_dirs_linux_override(self, monkeypatch): monkeypatch.setattr(sys, "platform", "linux2") assert appdirs.site_config_dirs("pip") == [ + sys.prefix, '/spam/pip', '/etc/pip', '/etc/xdg/pip', '/etc', - sys.prefix ] From 24cf5e7128eda1ca502a7b258164f0da8107ed89 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sat, 16 Feb 2019 15:07:10 -0800 Subject: [PATCH 4/8] Use venv configuration set for site settings even outside of a virtual environment --- src/pip/_internal/configuration.py | 6 ++---- src/pip/_internal/utils/appdirs.py | 5 ----- tests/unit/test_appdirs.py | 11 ++++------- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/pip/_internal/configuration.py b/src/pip/_internal/configuration.py index fe6df9b7518..44ed9ac4c7c 100644 --- a/src/pip/_internal/configuration.py +++ b/src/pip/_internal/configuration.py @@ -22,8 +22,7 @@ ConfigurationError, ConfigurationFileCouldNotBeLoaded, ) from pip._internal.locations import ( - legacy_config_file, new_config_file, running_under_virtualenv, - site_config_files, venv_config_file, + legacy_config_file, new_config_file, site_config_files, venv_config_file, ) from pip._internal.utils.misc import ensure_dir, enum from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -363,8 +362,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.VENV, [venv_config_file] def _get_parser_to_modify(self): # type: () -> Tuple[str, RawConfigParser] diff --git a/src/pip/_internal/utils/appdirs.py b/src/pip/_internal/utils/appdirs.py index 75cae548158..9af9fa7b582 100644 --- a/src/pip/_internal/utils/appdirs.py +++ b/src/pip/_internal/utils/appdirs.py @@ -190,11 +190,6 @@ def site_config_dirs(appname): # always look in /etc directly as well pathlist.append('/etc') - # allow configuration in sys.prefix without any appname - # but put it first so that it is not preferred for "pip config set" - # see - pathlist.insert(0, sys.prefix) - return pathlist diff --git a/tests/unit/test_appdirs.py b/tests/unit/test_appdirs.py index d9236eac2aa..0980cf16aad 100644 --- a/tests/unit/test_appdirs.py +++ b/tests/unit/test_appdirs.py @@ -100,8 +100,7 @@ def _get_win_folder(base): monkeypatch.setattr(appdirs, "WINDOWS", True) monkeypatch.setattr(os, "path", ntpath) - assert appdirs.site_config_dirs("pip") == [sys.prefix, - "C:\\ProgramData\\pip"] + assert appdirs.site_config_dirs("pip") == ["C:\\ProgramData\\pip"] assert _get_win_folder.calls == [pretend.call("CSIDL_COMMON_APPDATA")] def test_site_config_dirs_osx(self, monkeypatch): @@ -111,7 +110,7 @@ def test_site_config_dirs_osx(self, monkeypatch): monkeypatch.setattr(sys, "platform", "darwin") assert appdirs.site_config_dirs("pip") == \ - [sys.prefix, "/Library/Application Support/pip"] + ["/Library/Application Support/pip"] def test_site_config_dirs_linux(self, monkeypatch): monkeypatch.setattr(appdirs, "WINDOWS", False) @@ -120,9 +119,8 @@ def test_site_config_dirs_linux(self, monkeypatch): monkeypatch.setattr(sys, "platform", "linux2") assert appdirs.site_config_dirs("pip") == [ - sys.prefix, '/etc/xdg/pip', - '/etc', + '/etc' ] def test_site_config_dirs_linux_override(self, monkeypatch): @@ -133,11 +131,10 @@ def test_site_config_dirs_linux_override(self, monkeypatch): monkeypatch.setattr(sys, "platform", "linux2") assert appdirs.site_config_dirs("pip") == [ - sys.prefix, '/spam/pip', '/etc/pip', '/etc/xdg/pip', - '/etc', + '/etc' ] From a3852be7d9a66b4f2d8f6fde812e37393cc2bb35 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Sat, 16 Feb 2019 15:49:00 -0800 Subject: [PATCH 5/8] Remove now unnecessary monkeypatching in test --- tests/unit/test_options.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 3215a954038..7911d904683 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -388,14 +388,6 @@ def test_venv_config_file_found(self, monkeypatch): pip._internal.configuration, 'site_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 = [] From 3a408e79c7183ace8494aca64219918308e8e188 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 20 Feb 2019 13:31:37 -0800 Subject: [PATCH 6/8] Rename kinds.VENV to kinds.SITE and site_config_files to global_config_files --- src/pip/_internal/commands/configuration.py | 57 +++++++++++++++------ src/pip/_internal/configuration.py | 12 ++--- src/pip/_internal/locations.py | 4 +- tests/unit/test_configuration.py | 32 ++++++------ tests/unit/test_options.py | 4 +- 5 files changed, 66 insertions(+), 43 deletions(-) diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py index 826c08dccdf..f112de735e3 100644 --- a/src/pip/_internal/commands/configuration.py +++ b/src/pip/_internal/commands/configuration.py @@ -6,7 +6,7 @@ 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.misc import get_prog logger = logging.getLogger(__name__) @@ -23,7 +23,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. @@ -73,12 +73,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) @@ -127,27 +138,39 @@ 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: + if running_under_virtualenv(): + options.site_file = True + logger.warn( + "The --venv option is deprecated. Use --site instead." + ) + else: + raise PipError( + "Legacy --venv option requires a virtual environment. " + "Use --site instead." + ) + + file_options = [key for key in [ + kinds.USER if options.user_file else None, + kinds.GLOBAL if options.global_file else None, + kinds.SITE if options.site_file else None, + ] if key] + + 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): diff --git a/src/pip/_internal/configuration.py b/src/pip/_internal/configuration.py index 44ed9ac4c7c..33642af2d6f 100644 --- a/src/pip/_internal/configuration.py +++ b/src/pip/_internal/configuration.py @@ -22,7 +22,7 @@ ConfigurationError, ConfigurationFileCouldNotBeLoaded, ) from pip._internal.locations import ( - legacy_config_file, new_config_file, 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 @@ -58,7 +58,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 ) @@ -82,7 +82,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( @@ -94,7 +94,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"] @@ -351,7 +351,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 ( @@ -362,7 +362,7 @@ def _iter_config_files(self): yield kinds.USER, [legacy_config_file, new_config_file] # finally virtualenv configuration first trumping others - yield kinds.VENV, [venv_config_file] + yield kinds.SITE, [site_config_file] def _get_parser_to_modify(self): # type: () -> Tuple[str, RawConfigParser] diff --git a/src/pip/_internal/locations.py b/src/pip/_internal/locations.py index c6e2a3e484f..dc6d2f76f56 100644 --- a/src/pip/_internal/locations.py +++ b/src/pip/_internal/locations.py @@ -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) diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 9fe5bd98a6d..01a9a777987 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -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 @@ -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" @@ -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() @@ -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" @@ -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() @@ -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 @@ -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 @@ -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] diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 7911d904683..42f1e20d842 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -383,9 +383,9 @@ 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'] ) cp = pip._internal.configuration.Configuration(isolated=False) From 571e3884ca434839008bf5a1c21485ae8ee89e84 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Fri, 22 Feb 2019 08:31:54 -0800 Subject: [PATCH 7/8] Add tests for config file options --- src/pip/_internal/commands/configuration.py | 4 +- tests/unit/test_options.py | 49 ++++++++++++++++++++- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py index f112de735e3..caf9ed02033 100644 --- a/src/pip/_internal/commands/configuration.py +++ b/src/pip/_internal/commands/configuration.py @@ -139,10 +139,10 @@ def run(self, options, args): def _determine_file(self, options, need_value): # Convert legacy venv_file option to site_file or error - if options.venv_file: + if options.venv_file and not options.site_file: if running_under_virtualenv(): options.site_file = True - logger.warn( + logger.warning( "The --venv option is deprecated. Use --site instead." ) else: diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 42f1e20d842..1802b5078ce 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -1,3 +1,4 @@ +import logging import os from contextlib import contextmanager @@ -5,7 +6,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 @@ -395,3 +397,48 @@ def test_venv_config_file_found(self, monkeypatch): 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) + + warnings = [] + logger = logging.getLogger("pip._internal.commands.configuration") + monkeypatch.setattr(logger, "warning", warnings.append) + + options, args = cmd.parser.parse_args(["--venv", "get", "name"]) + assert "site" == cmd._determine_file(options, need_value=False) + assert warnings + assert "Use --site" in warnings[0] + + # No warning or error if both "--venv" and "--site" are specified + warnings[:] = [] + options, args = cmd.parser.parse_args(["--venv", "--site", "get", + "name"]) + assert "site" == cmd._determine_file(options, need_value=False) + assert not warnings From 28d41704ad42c4a42b102ac428a63f44d97318aa Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 6 Mar 2019 10:30:24 -0800 Subject: [PATCH 8/8] Switch to proper deprecation warning. Simplify selection of the best file option. --- src/pip/_internal/commands/configuration.py | 17 ++++++++++------- tests/unit/test_options.py | 17 +++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/pip/_internal/commands/configuration.py b/src/pip/_internal/commands/configuration.py index caf9ed02033..950e2057368 100644 --- a/src/pip/_internal/commands/configuration.py +++ b/src/pip/_internal/commands/configuration.py @@ -7,6 +7,7 @@ from pip._internal.configuration import Configuration, kinds from pip._internal.exceptions import PipError 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__) @@ -142,8 +143,10 @@ def _determine_file(self, options, need_value): if options.venv_file and not options.site_file: if running_under_virtualenv(): options.site_file = True - logger.warning( - "The --venv option is deprecated. Use --site instead." + deprecated( + "The --venv option has been deprecated.", + replacement="--site", + gone_in="19.3", ) else: raise PipError( @@ -151,11 +154,11 @@ def _determine_file(self, options, need_value): "Use --site instead." ) - file_options = [key for key in [ - kinds.USER if options.user_file else None, - kinds.GLOBAL if options.global_file else None, - kinds.SITE if options.site_file else None, - ] if key] + 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: diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 1802b5078ce..1bcee00bf9b 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -1,4 +1,3 @@ -import logging import os from contextlib import contextmanager @@ -427,18 +426,20 @@ def test_config_file_venv_option(self, monkeypatch): # Replace a handler with a no-op to avoid side effects monkeypatch.setattr(cmd, "get_name", lambda *a: None) - warnings = [] - logger = logging.getLogger("pip._internal.commands.configuration") - monkeypatch.setattr(logger, "warning", warnings.append) + 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 warnings - assert "Use --site" in warnings[0] + assert collected_warnings + assert "--site" in collected_warnings[0] # No warning or error if both "--venv" and "--site" are specified - warnings[:] = [] + collected_warnings[:] = [] options, args = cmd.parser.parse_args(["--venv", "--site", "get", "name"]) assert "site" == cmd._determine_file(options, need_value=False) - assert not warnings + assert not collected_warnings