From 4c3e4e2ef9e237ae74cdd68d6a93b3b7f86f3d57 Mon Sep 17 00:00:00 2001 From: Patrice Neff Date: Wed, 28 Oct 2020 12:23:18 +0000 Subject: [PATCH 1/5] Fix `VIRTUALENV_PYTHON` environment lookup The previous change which introduced the fallback discovery (#1995) broke this behaviour. --- src/virtualenv/discovery/builtin.py | 1 + tests/unit/config/test_env_var.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/src/virtualenv/discovery/builtin.py b/src/virtualenv/discovery/builtin.py index 93e2c7bd5..b66ecb193 100644 --- a/src/virtualenv/discovery/builtin.py +++ b/src/virtualenv/discovery/builtin.py @@ -25,6 +25,7 @@ def add_parser_arguments(cls, parser): "--python", dest="python", metavar="py", + type=str, action="append", default=[], help="interpreter based on what to create environment (path/identifier) " diff --git a/tests/unit/config/test_env_var.py b/tests/unit/config/test_env_var.py index 8eb858d7c..8adf955d1 100644 --- a/tests/unit/config/test_env_var.py +++ b/tests/unit/config/test_env_var.py @@ -4,6 +4,7 @@ import pytest +from virtualenv.config.cli.parser import VirtualEnvOptions from virtualenv.config.ini import IniConfig from virtualenv.run import session_via_cli from virtualenv.util.path import Path @@ -31,6 +32,13 @@ def test_value_bad(monkeypatch, caplog, empty_conf): assert "invalid literal" in caplog.messages[0] +def test_python_via_env_var(monkeypatch): + options = VirtualEnvOptions() + monkeypatch.setenv(str("VIRTUALENV_PYTHON"), str("python3")) + session_via_cli(["venv"], options=options) + assert options.python == ["python3"] + + def test_extra_search_dir_via_env_var(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) value = "a{}0{}b{}c".format(os.linesep, os.linesep, os.pathsep) From e0351cdf53d44d2e2ccd6f22726108de76c99c6c Mon Sep 17 00:00:00 2001 From: Patrice Neff Date: Wed, 28 Oct 2020 12:39:40 +0000 Subject: [PATCH 2/5] Add changelog entry --- docs/changelog/1998.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changelog/1998.bugfix.rst diff --git a/docs/changelog/1998.bugfix.rst b/docs/changelog/1998.bugfix.rst new file mode 100644 index 000000000..9ff36cf4c --- /dev/null +++ b/docs/changelog/1998.bugfix.rst @@ -0,0 +1 @@ +Fix processing of the ``VIRTUALENV_PYTHON`` environment variable From 3a6fe603392e947927b1d4877c125f9ae06e4897 Mon Sep 17 00:00:00 2001 From: Patrice Neff Date: Wed, 28 Oct 2020 14:25:51 +0000 Subject: [PATCH 3/5] Use comma as separator for `VIRTUALENV_PYTHON` Unlike `VIRTUALENV_EXTRA_SEARCH_DIR` where the values are separated by newline, a comma is used for this value. --- docs/changelog/1998.bugfix.rst | 3 ++- docs/cli_interface.rst | 6 ++++++ src/virtualenv/config/convert.py | 21 ++++++++++++++++++--- tests/unit/config/test_env_var.py | 7 +++++++ 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/docs/changelog/1998.bugfix.rst b/docs/changelog/1998.bugfix.rst index 9ff36cf4c..c94c9429d 100644 --- a/docs/changelog/1998.bugfix.rst +++ b/docs/changelog/1998.bugfix.rst @@ -1 +1,2 @@ -Fix processing of the ``VIRTUALENV_PYTHON`` environment variable +Fix processing of the ``VIRTUALENV_PYTHON`` environment variable and make it +multi-value as well (separated by comma) - by :user:`pneff`. diff --git a/docs/cli_interface.rst b/docs/cli_interface.rst index b75247809..3994074f5 100644 --- a/docs/cli_interface.rst +++ b/docs/cli_interface.rst @@ -71,6 +71,12 @@ variable ``VIRTUALENV_PYTHON`` like: env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python virtualenv +Multiple values can be provided separated with a comma: + +.. code-block:: console + + env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python,python3.8 virtualenv + This also works for appending command line options, like :option:`extra-search-dir`, where a literal newline is used to separate the values: diff --git a/src/virtualenv/config/convert.py b/src/virtualenv/config/convert.py index 27821fc06..b74584967 100644 --- a/src/virtualenv/config/convert.py +++ b/src/virtualenv/config/convert.py @@ -46,9 +46,7 @@ def _validate(self): """""" def convert(self, value, flatten=True): - if isinstance(value, (str, bytes)): - value = filter(None, [x.strip() for x in value.splitlines()]) - values = list(value) + values = self.split_values(value) result = [] for value in values: sub_values = value.split(os.pathsep) @@ -56,6 +54,23 @@ def convert(self, value, flatten=True): converted = [self.as_type(i) for i in result] return converted + def split_values(self, value): + """Split the provided value into a list. + + For strings this is a comma-separated. For more complex types (`Path` + for example) this is newline-separated. + """ + if isinstance(value, (str, bytes)): + if self.as_type is str: + # Simple split + value = value.split(",") + else: + value = value.splitlines() + + value = filter(None, [x.strip() for x in value]) + + return list(value) + def convert(value, as_type, source): """Convert the value as a given type where the value comes from the given source""" diff --git a/tests/unit/config/test_env_var.py b/tests/unit/config/test_env_var.py index 8adf955d1..6d4924932 100644 --- a/tests/unit/config/test_env_var.py +++ b/tests/unit/config/test_env_var.py @@ -39,6 +39,13 @@ def test_python_via_env_var(monkeypatch): assert options.python == ["python3"] +def test_python_multi_value_via_env_var(monkeypatch): + options = VirtualEnvOptions() + monkeypatch.setenv(str("VIRTUALENV_PYTHON"), str("python3,python2")) + session_via_cli(["venv"], options=options) + assert options.python == ["python3", "python2"] + + def test_extra_search_dir_via_env_var(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) value = "a{}0{}b{}c".format(os.linesep, os.linesep, os.pathsep) From a59e31cd93e71a8a56ea7ab0915afa6075e49e80 Mon Sep 17 00:00:00 2001 From: Patrice Neff Date: Wed, 28 Oct 2020 15:03:32 +0000 Subject: [PATCH 4/5] Extend comma support to all list values --- docs/cli_interface.rst | 15 ++++++--------- src/virtualenv/config/convert.py | 24 +++++++++++++----------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/docs/cli_interface.rst b/docs/cli_interface.rst index 3994074f5..7f5513ffd 100644 --- a/docs/cli_interface.rst +++ b/docs/cli_interface.rst @@ -71,21 +71,18 @@ variable ``VIRTUALENV_PYTHON`` like: env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python virtualenv -Multiple values can be provided separated with a comma: +Where the option accepts multiple values, for example for :option:`python` or +:option:`extra-search-dir`, the values can be separated either by comma or a +literal newline: .. code-block:: console env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python,python3.8 virtualenv + env VIRTUALENV_EXTRA_SEARCH_DIR=/path/to/dists\n/path/to/other/dists virtualenv -This also works for appending command line options, like :option:`extra-search-dir`, where a literal newline -is used to separate the values: - -.. code-block:: console - - env VIRTUALENV_EXTRA_SEARCH_DIR=/path/to/dists\n/path/to/other/dists virtualenv - -The equivalent CLI-flags based invocation, for the above example, would be: +The equivalent CLI-flags based invocation for the above examples would be: .. code-block:: console + virtualenv --python=/opt/python-3.8/bin/python --python=python3.8 virtualenv --extra-search-dir=/path/to/dists --extra-search-dir=/path/to/other/dists diff --git a/src/virtualenv/config/convert.py b/src/virtualenv/config/convert.py index b74584967..562720a57 100644 --- a/src/virtualenv/config/convert.py +++ b/src/virtualenv/config/convert.py @@ -57,19 +57,21 @@ def convert(self, value, flatten=True): def split_values(self, value): """Split the provided value into a list. - For strings this is a comma-separated. For more complex types (`Path` - for example) this is newline-separated. + First this is done by newlines. If there were no newlines in the text, + then we next try to split by comma. """ if isinstance(value, (str, bytes)): - if self.as_type is str: - # Simple split - value = value.split(",") - else: - value = value.splitlines() - - value = filter(None, [x.strip() for x in value]) - - return list(value) + # Use `splitlines` rather than a custom check for whether there is + # more than one line. This ensures that the full `splitlines()` + # logic is supported here. + values = value.splitlines() + if len(values) <= 1: + values = value.split(",") + values = filter(None, [x.strip() for x in values]) + else: + values = list(value) + + return values def convert(value, as_type, source): From 4f891ef4ab3264aee7b24ac2e21adef1eea11188 Mon Sep 17 00:00:00 2001 From: Patrice Neff Date: Wed, 28 Oct 2020 15:41:08 +0000 Subject: [PATCH 5/5] Verify splitting change with new tests --- docs/cli_interface.rst | 7 +++++-- tests/unit/config/test_env_var.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/docs/cli_interface.rst b/docs/cli_interface.rst index 7f5513ffd..a77c53c20 100644 --- a/docs/cli_interface.rst +++ b/docs/cli_interface.rst @@ -72,8 +72,11 @@ variable ``VIRTUALENV_PYTHON`` like: env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python virtualenv Where the option accepts multiple values, for example for :option:`python` or -:option:`extra-search-dir`, the values can be separated either by comma or a -literal newline: +:option:`extra-search-dir`, the values can be separated either by literal +newlines or commas. Newlines and commas can not be mixed and if both are +present only the newline is used for separating values. Examples for multiple +values: + .. code-block:: console diff --git a/tests/unit/config/test_env_var.py b/tests/unit/config/test_env_var.py index 6d4924932..34b216f4d 100644 --- a/tests/unit/config/test_env_var.py +++ b/tests/unit/config/test_env_var.py @@ -46,6 +46,20 @@ def test_python_multi_value_via_env_var(monkeypatch): assert options.python == ["python3", "python2"] +def test_python_multi_value_newline_via_env_var(monkeypatch): + options = VirtualEnvOptions() + monkeypatch.setenv(str("VIRTUALENV_PYTHON"), str("python3\npython2")) + session_via_cli(["venv"], options=options) + assert options.python == ["python3", "python2"] + + +def test_python_multi_value_prefer_newline_via_env_var(monkeypatch): + options = VirtualEnvOptions() + monkeypatch.setenv(str("VIRTUALENV_PYTHON"), str("python3\npython2,python27")) + session_via_cli(["venv"], options=options) + assert options.python == ["python3", "python2,python27"] + + def test_extra_search_dir_via_env_var(tmp_path, monkeypatch): monkeypatch.chdir(tmp_path) value = "a{}0{}b{}c".format(os.linesep, os.linesep, os.pathsep)