Fix VIRTUALENV_PYTHON environment lookup (#1998)

docs/changelog/1998.bugfix.rst

@@ -0,0 +1,2 @@
+Fix processing of the ``VIRTUALENV_PYTHON`` environment variable and make it
+multi-value as well (separated by comma) - by :user:`pneff`.

docs/cli_interface.rst

@@ -71,15 +71,21 @@ variable ``VIRTUALENV_PYTHON`` like:
 
    env VIRTUALENV_PYTHON=/opt/python-3.8/bin/python virtualenv
 
-This also works for appending command line options, like :option:`extra-search-dir`, where a literal newline
-is used to separate the values:
+Where the option accepts multiple values, for example for :option:`python` or
+: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
 
-  env VIRTUALENV_EXTRA_SEARCH_DIR=/path/to/dists\n/path/to/other/dists virtualenv
+   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
 
-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

src/virtualenv/config/convert.py

@@ -46,16 +46,33 @@ 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)
             result.extend(sub_values)
         converted = [self.as_type(i) for i in result]
         return converted
 
+    def split_values(self, value):
+        """Split the provided value into a list.
+
+        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)):
+            # 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):
     """Convert the value as a given type where the value comes from the given source"""

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) "

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,34 @@ 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_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_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)