Better handling of conflicting --copies and --symlinks

Introduce priority of where the option is set to follow the order:
cli, env var, file, hard coded. If both set at same level prefer copy
over symlink.

Signed-off-by: Bernat Gabor <bgabor8@bloomberg.net>

docs/changelog/1784.bugfix.rst

@@ -0,0 +1,3 @@
+Better handling of optionlicting :option:`copies` and :option:`symlinks`. Introduce priority of where the option is set
+to follow the order: CLI, env var, file, hardcoded. If both set at same level prefers copy over symlink. - by
+user:`gaborbernat`.

setup.cfg

@@ -92,7 +92,7 @@ testing =
  packaging>=20.0;python_version>"3.4"
  pytest >= 4.0.0, <6
  coverage >= 4.5.1, <6
- pytest-mock >= 3.0.0, <4
+ pytest-mock >= 2.0.0, <4
  pytest-env >= 0.6.2, <1
  pytest-timeout >= 1.3.4, <2
  xonsh >= 0.9.17, <1; python_version > '3.4'

src/virtualenv/__main__.py

@@ -1,11 +1,11 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
-import argparse
 import logging
 import os
 import sys
 from datetime import datetime
 
+from virtualenv.config.cli.parser import VirtualEnvOptions
 from virtualenv.util.six import ensure_text
 
 
@@ -46,7 +46,7 @@ def __str__(self):
 
 
 def run_with_catch(args=None):
- options = argparse.Namespace()
+ options = VirtualEnvOptions()
  try:
  run(args, options)
  except (KeyboardInterrupt, Exception) as exception:

src/virtualenv/config/cli/parser.py

@@ -1,6 +1,6 @@
 from __future__ import absolute_import, unicode_literals
 
-from argparse import SUPPRESS, ArgumentDefaultsHelpFormatter, ArgumentParser
+from argparse import SUPPRESS, ArgumentDefaultsHelpFormatter, ArgumentParser, Namespace
 from collections import OrderedDict
 
 from virtualenv.config.convert import get_type
@@ -9,6 +9,38 @@
 from ..ini import IniConfig
 
 
+class VirtualEnvOptions(Namespace):
+ def __init__(self, **kwargs):
+ super(VirtualEnvOptions, self).__init__(**kwargs)
+ self._src = None
+ self._sources = {}
+
+ def set_src(self, key, value, src):
+ setattr(self, key, value)
+ if src.startswith("env var"):
+ src = "env var"
+ self._sources[key] = src
+
+ def __setattr__(self, key, value):
+ if getattr(self, "_src", None) is not None:
+ self._sources[key] = self._src
+ super(VirtualEnvOptions, self).__setattr__(key, value)
+
+ def get_source(self, key):
+ return self._sources.get(key)
+
+ @property
+ def verbosity(self):
+ if not hasattr(self, "verbose") and not hasattr(self, "quiet"):
+ return None
+ return max(self.verbose - self.quiet, 0)
+
+ def __repr__(self):
+ return "{}({})".format(
+ type(self).__name__, ", ".join("{}={}".format(k, v) for k, v in vars(self).items() if not k.startswith("_"))
+ )
+
+
 class VirtualEnvConfigParser(ArgumentParser):
  """
  Custom option parser which updates its defaults by checking the configuration files and environmental variables
@@ -24,8 +56,9 @@ def __init__(self, options=None, *args, **kwargs):
  super(VirtualEnvConfigParser, self).__init__(*args, **kwargs)
  self._fixed = set()
  self._elements = None
- self._verbosity = None
- self._options = options
+ if options is not None and not isinstance(options, VirtualEnvOptions):
+ raise TypeError("options must be of type VirtualEnvOptions")
+ self.options = VirtualEnvOptions() if options is None else options
  self._interpreter = None
  self._app_data = None
 
@@ -52,18 +85,25 @@ def _fix_default(self, action):
  break
  if outcome is not None:
  action.default, action.default_source = outcome
+ else:
+ outcome = action.default, "default"
+ self.options.set_src(action.dest, *outcome)
 
  def enable_help(self):
  self._fix_defaults()
  self.add_argument("-h", "--help", action="help", default=SUPPRESS, help="show this help message and exit")
 
  def parse_known_args(self, args=None, namespace=None):
+ if namespace is None:
+ namespace = self.options
+ elif namespace is not self.options:
+ raise ValueError("can only pass in parser.options")
  self._fix_defaults()
- return super(VirtualEnvConfigParser, self).parse_known_args(args, namespace=namespace)
-
- def parse_args(self, args=None, namespace=None):
- self._fix_defaults()
- return super(VirtualEnvConfigParser, self).parse_args(args, namespace=namespace)
+ self.options._src = "cli"
+ try:
+  return super(VirtualEnvConfigParser, self).parse_known_args(args, namespace=namespace)
+ finally:
+  self.options._src = None
 
 
 class HelpFormatter(ArgumentDefaultsHelpFormatter):

src/virtualenv/create/via_global_ref/api.py

@@ -34,11 +34,26 @@ def can_symlink(self):
 class ViaGlobalRefApi(Creator):
  def __init__(self, options, interpreter):
  super(ViaGlobalRefApi, self).__init__(options, interpreter)
- copies = getattr(options, "copies", False)
- symlinks = getattr(options, "symlinks", False)
- self.symlinks = symlinks is True and copies is False
+ self.symlinks = self._should_symlink(options)
  self.enable_system_site_package = options.system_site
 
+ @staticmethod
+ def _should_symlink(options):
+ # Priority of where the option is set to follow the order: CLI, env var, file, hardcoded.
+ # If both set at same level prefers copy over symlink.
+ copies, symlinks = getattr(options, "copies", False), getattr(options, "symlinks", False)
+ copy_src, sym_src = options.get_source("copies"), options.get_source("symlinks")
+ for level in ["cli", "env var", "file", "default"]:
+ s_opt = symlinks if sym_src == level else None
+ c_opt = copies if copy_src == level else None
+ if s_opt is True and c_opt is True:
+ return False
+ if s_opt is True:
+ return True
+ if c_opt is True:
+ return False
+ return False # fallback to copy
+
  @classmethod
  def add_parser_arguments(cls, parser, interpreter, meta, app_data):
  super(ViaGlobalRefApi, cls).add_parser_arguments(parser, interpreter, meta, app_data)
@@ -50,6 +65,8 @@ def add_parser_arguments(cls, parser, interpreter, meta, app_data):
  help="give the virtual environment access to the system site-packages dir",
  )
  group = parser.add_mutually_exclusive_group()
+ if not meta.can_symlink and not meta.can_copy:
+ raise RuntimeError("neither symlink or copy method supported")
  if meta.can_symlink:
  group.add_argument(
  "--symlinks",

src/virtualenv/report.py

@@ -18,8 +18,7 @@
 LOGGER = logging.getLogger()
 
 
-def setup_report(verbose, quiet):
- verbosity = max(verbose - quiet, 0)
+def setup_report(verbosity):
  _clean_handlers(LOGGER)
  if verbosity > MAX_LEVEL:
  verbosity = MAX_LEVEL # pragma: no cover

src/virtualenv/run/__init__.py

@@ -1,6 +1,5 @@
 from __future__ import absolute_import, unicode_literals
 
-import argparse
 import logging
 
 from virtualenv.run.app_data import AppDataAction
@@ -19,23 +18,21 @@ def cli_run(args, options=None):
  """Create a virtual environment given some command line interface arguments
 
  :param args: the command line arguments
- :param options: passing in a ``argparse.Namespace`` object allows return of the parsed options
+ :param options: passing in a ``VirtualEnvOptions`` object allows return of the parsed options
  :return: the session object of the creation (its structure for now is experimental and might change on short notice)
  """
- if options is None:
- options = argparse.Namespace()
  session = session_via_cli(args, options)
  with session:
  session.run()
  return session
 
 
 # noinspection PyProtectedMember
-def session_via_cli(args, options):
+def session_via_cli(args, options=None):
  parser = build_parser(args, options)
- parser.parse_args(args, namespace=parser._options)
- creator, seeder, activators = tuple(e.create(parser._options) for e in parser._elements) # create types
- session = Session(parser._verbosity, options.app_data, parser._interpreter, creator, seeder, activators)
+ options = parser.parse_args(args)
+ creator, seeder, activators = tuple(e.create(options) for e in parser._elements) # create types
+ session = Session(options.verbosity, options.app_data, parser._interpreter, creator, seeder, activators)
  return session
 
 
@@ -50,7 +47,7 @@ def build_parser(args=None, options=None):
  default=False,
  help="on failure also display the stacktrace internals of virtualenv",
  )
- parser._options, parser._verbosity = _do_report_setup(parser, args)
+ _do_report_setup(parser, args)
  # here we need a write-able application data (e.g. the zipapp might need this for discovery cache)
  default_app_data = AppDataAction.default()
  parser.add_argument(
@@ -67,7 +64,7 @@ def build_parser(args=None, options=None):
  help="start with empty app data folder",
  default=False,
  )
- discover = get_discover(parser, args, parser._options)
+ discover = get_discover(parser, args)
  parser._interpreter = interpreter = discover.interpreter
  if interpreter is None:
  raise RuntimeError("failed to find interpreter for {}".format(discover))
@@ -76,9 +73,9 @@ def build_parser(args=None, options=None):
  SeederSelector(interpreter, parser),
  ActivationSelector(interpreter, parser),
  ]
- parser.parse_known_args(args, namespace=parser._options)
+ options, _ = parser.parse_known_args(args)
  for element in parser._elements:
- element.handle_selected_arg_parse(parser._options)
+ element.handle_selected_arg_parse(options)
  parser.enable_help()
  return parser
 
@@ -103,6 +100,5 @@ def _do_report_setup(parser, args):
  verbosity = verbosity_group.add_mutually_exclusive_group()
  verbosity.add_argument("-v", "--verbose", action="count", dest="verbose", help="increase verbosity", default=2)
  verbosity.add_argument("-q", "--quiet", action="count", dest="quiet", help="decrease verbosity", default=0)
- options, _ = parser.parse_known_args(args, namespace=parser._options)
- verbosity_value = setup_report(options.verbose, options.quiet)
- return options, verbosity_value
+ option, _ = parser.parse_known_args(args)
+ setup_report(option.verbosity)

src/virtualenv/run/plugin/discovery.py

@@ -9,7 +9,7 @@ class Discovery(PluginLoader):
  """"""
 
 
-def get_discover(parser, args, options):
+def get_discover(parser, args):
  discover_types = Discovery.entry_points_for("virtualenv.discovery")
  discovery_parser = parser.add_argument_group(
  title="discovery", description="discover and provide a target interpreter"
@@ -21,7 +21,7 @@ def get_discover(parser, args, options):
  required=False,
  help="interpreter discovery method",
  )
- options, _ = parser.parse_known_args(args, namespace=options)
+ options, _ = parser.parse_known_args(args)
  if options.app_data == "<temp folder>":
  options.app_data = TempAppData()
  if options.clear_app_data:

tests/unit/config/test_env_var.py

@@ -1,7 +1,6 @@
 from __future__ import absolute_import, unicode_literals
 
 import os
-from argparse import Namespace
 
 import pytest
 
@@ -10,11 +9,6 @@
 from virtualenv.util.path import Path
 
 
-def parse_cli(args):
- options = Namespace()
- return session_via_cli(args, options)
-
-
 @pytest.fixture()
 def empty_conf(tmp_path, monkeypatch):
  conf = tmp_path / "conf.ini"
@@ -24,13 +18,13 @@ def empty_conf(tmp_path, monkeypatch):
 
 def test_value_ok(monkeypatch, empty_conf):
  monkeypatch.setenv(str("VIRTUALENV_VERBOSE"), str("5"))
- result = parse_cli(["venv"])
+ result = session_via_cli(["venv"])
  assert result.verbosity == 5
 
 
 def test_value_bad(monkeypatch, caplog, empty_conf):
  monkeypatch.setenv(str("VIRTUALENV_VERBOSE"), str("a"))
- result = parse_cli(["venv"])
+ result = session_via_cli(["venv"])
  assert result.verbosity == 2
  assert len(caplog.messages) == 1
  assert "env var VIRTUALENV_VERBOSE failed to convert" in caplog.messages[0]
@@ -44,7 +38,7 @@ def test_extra_search_dir_via_env_var(tmp_path, monkeypatch):
  (tmp_path / "a").mkdir()
  (tmp_path / "b").mkdir()
  (tmp_path / "c").mkdir()
- result = parse_cli(["venv"])
+ result = session_via_cli(["venv"])
  assert result.seeder.extra_search_dir == [Path("a").resolve(), Path("b").resolve(), Path("c").resolve()]
 
 
@@ -65,5 +59,5 @@ def func(self, action):
  monkeypatch.delenv(str("SYMLINKS"), raising=False)
  monkeypatch.delenv(str("VIRTUALENV_COPIES"), raising=False)
  monkeypatch.setenv(str("VIRTUALENV_ALWAYS_COPY"), str("1"))
- result = parse_cli(["venv"])
+ result = session_via_cli(["venv"])
  assert result.creator.symlinks is False

tests/unit/config/test_ini.py

@@ -0,0 +1,28 @@
+from __future__ import unicode_literals
+
+from textwrap import dedent
+
+import pytest
+
+from virtualenv.info import fs_supports_symlink
+from virtualenv.run import session_via_cli
+from virtualenv.util.six import ensure_str
+
+
+@pytest.mark.skipif(not fs_supports_symlink(), reason="symlink is not supported")
+def test_ini_can_be_overwritten_by_flag(tmp_path, monkeypatch):
+ custom_ini = tmp_path / "conf.ini"
+ custom_ini.write_text(
+ dedent(
+ """
+ [virtualenv]
+ copies = True
+ """
+ )
+ )
+ monkeypatch.setenv(ensure_str("VIRTUALENV_CONFIG_FILE"), str(custom_ini))
+
+ result = session_via_cli(["venv", "--symlinks"])
+
+ symlinks = result.creator.symlinks
+ assert symlinks is True

tests/unit/create/test_creator.py

@@ -349,7 +349,9 @@ def test_cross_major(cross_python, coverage_env, tmp_path, session_app_data, cur
 
 def test_create_parallel(tmp_path, monkeypatch, temp_app_data):
  def create(count):
- subprocess.check_call([sys.executable, "-m", "virtualenv", str(tmp_path / "venv{}".format(count))])
+ subprocess.check_call(
+ [sys.executable, "-m", "virtualenv", "-vvv", str(tmp_path / "venv{}".format(count)), "--without-pip"]
+ )
 
  threads = [Thread(target=create, args=(i,)) for i in range(1, 4)]
  for thread in threads: