From 2439d80a83d9cc23982972b9c9f9c8458774382f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= <mcsinyx@disroot.org> Date: Mon, 13 Jul 2020 11:39:03 +0700 Subject: [PATCH 1/3] Allow specifying verbose/quiet level via config file and env var --- news/8578.bugfix | 4 ++++ src/pip/_internal/cli/parser.py | 36 ++++++++++++++++----------------- 2 files changed, 22 insertions(+), 18 deletions(-) create mode 100644 news/8578.bugfix diff --git a/news/8578.bugfix b/news/8578.bugfix new file mode 100644 index 00000000000..3df7ed078cf --- /dev/null +++ b/news/8578.bugfix @@ -0,0 +1,4 @@ +Allow specifying verbosity and quiet level via configuration files +and environment variables. Previously these options were treated as +boolean values when read from there while through CLI the level can be +specified. diff --git a/src/pip/_internal/cli/parser.py b/src/pip/_internal/cli/parser.py index 04e00b72132..b6b78318a7a 100644 --- a/src/pip/_internal/cli/parser.py +++ b/src/pip/_internal/cli/parser.py @@ -11,6 +11,7 @@ import textwrap from distutils.util import strtobool +from pip._vendor.contextlib2 import suppress from pip._vendor.six import string_types from pip._internal.cli.status_codes import UNKNOWN_ERROR @@ -197,15 +198,27 @@ def _update_defaults(self, defaults): if option is None: continue - if option.action in ('store_true', 'store_false', 'count'): + if option.action in ('store_true', 'store_false'): try: val = strtobool(val) except ValueError: - error_msg = invalid_config_error_message( - option.action, key, val + self.error( + '{} is not a valid value for {} option, ' # noqa + 'please specify a boolean value like yes/no, ' + 'true/false or 1/0 instead.'.format(val, key) + ) + elif option.action == 'count': + with suppress(ValueError): + val = strtobool(val) + with suppress(ValueError): + val = int(val) + if not isinstance(val, int) or val < 0: + self.error( + '{} is not a valid value for {} option, ' # noqa + 'please instead specify either a non-negative integer ' + 'or a boolean value like yes/no or false/true ' + 'which is equivalent to 1/0.'.format(val, key) ) - self.error(error_msg) - elif option.action == 'append': val = val.split() val = [self.check_default(option, key, v) for v in val] @@ -251,16 +264,3 @@ def get_default_values(self): def error(self, msg): self.print_usage(sys.stderr) self.exit(UNKNOWN_ERROR, "{}\n".format(msg)) - - -def invalid_config_error_message(action, key, val): - """Returns a better error message when invalid configuration option - is provided.""" - if action in ('store_true', 'store_false'): - return ("{0} is not a valid value for {1} option, " - "please specify a boolean value like yes/no, " - "true/false or 1/0 instead.").format(val, key) - - return ("{0} is not a valid value for {1} option, " - "please specify a numerical value like 1/0 " - "instead.").format(val, key) From 7a0061d8864d5fab7abc3d0d82ad01a43a0d23cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= <mcsinyx@disroot.org> Date: Wed, 15 Jul 2020 15:26:04 +0700 Subject: [PATCH 2/3] Update docs for setting verbose/quiet in config file or env var Co-authored-by: Paul Moore <p.f.moore@gmail.com> Co-authored-by: Prashant Sharma <prashantsharma161198@gmail.com> Co-authored-by: Xavier Fernandez <xav.fernandez@gmail.com> --- docs/html/user_guide.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/html/user_guide.rst b/docs/html/user_guide.rst index f2e49fadf64..ddaaccd57e7 100644 --- a/docs/html/user_guide.rst +++ b/docs/html/user_guide.rst @@ -442,6 +442,15 @@ and ``--no-cache-dir``, falsy values have to be used: no-compile = no no-warn-script-location = false +For options which can be repeated like ``--verbose`` and ``--quiet``, +a non-negative integer can be used to represent the level to be specified: + +.. code-block:: ini + + [global] + quiet = 0 + verbose = 2 + It is possible to append values to a section within a configuration file such as the pip.ini file. This is applicable to appending options like ``--find-links`` or ``--trusted-host``, which can be written on multiple lines: @@ -488,6 +497,15 @@ is the same as calling:: pip install --find-links=http://mirror1.example.com --find-links=http://mirror2.example.com +Options that do not take a value, but can be repeated (such as ``--verbose``) +can be specified using the number of repetitions, so:: + + export PIP_VERBOSE=3 + +is the same as calling:: + + pip install -vvv + .. note:: Environment variables set to be empty string will not be treated as false. From a85be3f5557e49f9e35e8abcf9365dd3b6cfabab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= <mcsinyx@disroot.org> Date: Wed, 22 Jul 2020 17:58:27 +0700 Subject: [PATCH 3/3] Test verbose/quiet level specified via env var and config file --- tests/unit/test_options.py | 120 +++++++++++++++++++++++++++++++------ 1 file changed, 102 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index ce4fc9c25d1..df174ea67bc 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -1,5 +1,6 @@ import os from contextlib import contextmanager +from tempfile import NamedTemporaryFile import pytest @@ -286,6 +287,107 @@ def test_subcommand_option_before_subcommand_fails(self): main(['--find-links', 'F1', 'fake']) +@contextmanager +def tmpconfig(option, value, section='global'): + with NamedTemporaryFile(mode='w', delete=False) as f: + f.write('[{}]\n{}={}\n'.format(section, option, value)) + name = f.name + try: + yield name + finally: + os.unlink(name) + + +class TestCountOptions(AddFakeCommandMixin): + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(4)) + def test_cli_long(self, option, value): + flags = ['--{}'.format(option)] * value + opt1, args1 = main(flags+['fake']) + opt2, args2 = main(['fake']+flags) + assert getattr(opt1, option) == getattr(opt2, option) == value + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(1, 4)) + def test_cli_short(self, option, value): + flag = '-' + option[0]*value + opt1, args1 = main([flag, 'fake']) + opt2, args2 = main(['fake', flag]) + assert getattr(opt1, option) == getattr(opt2, option) == value + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(4)) + def test_env_var(self, option, value, monkeypatch): + monkeypatch.setenv('PIP_'+option.upper(), str(value)) + assert getattr(main(['fake'])[0], option) == value + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(3)) + def test_env_var_integrate_cli(self, option, value, monkeypatch): + monkeypatch.setenv('PIP_'+option.upper(), str(value)) + assert getattr(main(['fake', '--'+option])[0], option) == value + 1 + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', (-1, 'foobar')) + def test_env_var_invalid(self, option, value, monkeypatch, capsys): + monkeypatch.setenv('PIP_'+option.upper(), str(value)) + with assert_option_error(capsys, expected='a non-negative integer'): + main(['fake']) + + # Undocumented, support for backward compatibility + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', ('no', 'false')) + def test_env_var_false(self, option, value, monkeypatch): + monkeypatch.setenv('PIP_'+option.upper(), str(value)) + assert getattr(main(['fake'])[0], option) == 0 + + # Undocumented, support for backward compatibility + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', ('yes', 'true')) + def test_env_var_true(self, option, value, monkeypatch): + monkeypatch.setenv('PIP_'+option.upper(), str(value)) + assert getattr(main(['fake'])[0], option) == 1 + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(4)) + def test_config_file(self, option, value, monkeypatch): + with tmpconfig(option, value) as name: + monkeypatch.setenv('PIP_CONFIG_FILE', name) + assert getattr(main(['fake'])[0], option) == value + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', range(3)) + def test_config_file_integrate_cli(self, option, value, monkeypatch): + with tmpconfig(option, value) as name: + monkeypatch.setenv('PIP_CONFIG_FILE', name) + assert getattr(main(['fake', '--'+option])[0], option) == value + 1 + + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', (-1, 'foobar')) + def test_config_file_invalid(self, option, value, monkeypatch, capsys): + with tmpconfig(option, value) as name: + monkeypatch.setenv('PIP_CONFIG_FILE', name) + with assert_option_error(capsys, expected='non-negative integer'): + main(['fake']) + + # Undocumented, support for backward compatibility + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', ('no', 'false')) + def test_config_file_false(self, option, value, monkeypatch): + with tmpconfig(option, value) as name: + monkeypatch.setenv('PIP_CONFIG_FILE', name) + assert getattr(main(['fake'])[0], option) == 0 + + # Undocumented, support for backward compatibility + @pytest.mark.parametrize('option', ('verbose', 'quiet')) + @pytest.mark.parametrize('value', ('yes', 'true')) + def test_config_file_true(self, option, value, monkeypatch): + with tmpconfig(option, value) as name: + monkeypatch.setenv('PIP_CONFIG_FILE', name) + assert getattr(main(['fake'])[0], option) == 1 + + class TestGeneralOptions(AddFakeCommandMixin): # the reason to specifically test general options is due to the @@ -310,24 +412,6 @@ def test_require_virtualenv(self): assert options1.require_venv assert options2.require_venv - def test_verbose(self): - options1, args1 = main(['--verbose', 'fake']) - options2, args2 = main(['fake', '--verbose']) - assert options1.verbose == options2.verbose == 1 - - def test_quiet(self): - options1, args1 = main(['--quiet', 'fake']) - options2, args2 = main(['fake', '--quiet']) - assert options1.quiet == options2.quiet == 1 - - options3, args3 = main(['--quiet', '--quiet', 'fake']) - options4, args4 = main(['fake', '--quiet', '--quiet']) - assert options3.quiet == options4.quiet == 2 - - options5, args5 = main(['--quiet', '--quiet', '--quiet', 'fake']) - options6, args6 = main(['fake', '--quiet', '--quiet', '--quiet']) - assert options5.quiet == options6.quiet == 3 - def test_log(self): options1, args1 = main(['--log', 'path', 'fake']) options2, args2 = main(['fake', '--log', 'path'])