diff --git a/docs/changelog/1784.bugfix.rst b/docs/changelog/1784.bugfix.rst new file mode 100644 index 000000000..9171bb8bf --- /dev/null +++ b/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`. diff --git a/src/virtualenv/__main__.py b/src/virtualenv/__main__.py index edc7f9512..e5a54f6bd 100644 --- a/src/virtualenv/__main__.py +++ b/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: diff --git a/src/virtualenv/config/cli/parser.py b/src/virtualenv/config/cli/parser.py index ef7539421..1511deb98 100644 --- a/src/virtualenv/config/cli/parser.py +++ b/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): diff --git a/src/virtualenv/create/via_global_ref/api.py b/src/virtualenv/create/via_global_ref/api.py index 237a51252..a99b077d8 100644 --- a/src/virtualenv/create/via_global_ref/api.py +++ b/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", diff --git a/src/virtualenv/report.py b/src/virtualenv/report.py index e03ccf979..7ae2f2414 100644 --- a/src/virtualenv/report.py +++ b/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 diff --git a/src/virtualenv/run/__init__.py b/src/virtualenv/run/__init__.py index 6983e33ed..7c63ab9a6 100644 --- a/src/virtualenv/run/__init__.py +++ b/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,11 +18,9 @@ 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() @@ -31,11 +28,11 @@ def cli_run(args, options=None): # 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) diff --git a/src/virtualenv/run/plugin/discovery.py b/src/virtualenv/run/plugin/discovery.py index 43d5eb2e0..03f0590f3 100644 --- a/src/virtualenv/run/plugin/discovery.py +++ b/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 == "": options.app_data = TempAppData() if options.clear_app_data: diff --git a/tests/unit/config/test_env_var.py b/tests/unit/config/test_env_var.py index 95c40e304..8eb858d7c 100644 --- a/tests/unit/config/test_env_var.py +++ b/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 diff --git a/tests/unit/config/test_ini.py b/tests/unit/config/test_ini.py new file mode 100644 index 000000000..6b54808df --- /dev/null +++ b/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 diff --git a/tests/unit/create/test_creator.py b/tests/unit/create/test_creator.py index 60f5b8666..39fcbbed6 100644 --- a/tests/unit/create/test_creator.py +++ b/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: @@ -386,6 +388,7 @@ def test_create_long_path(current_fastest, tmp_path): subprocess.check_call([str(result.creator.script("pip")), "--version"]) +@pytest.mark.timeout(timeout=60) @pytest.mark.parametrize("creator", set(PythonInfo.current_system().creators().key_to_class) - {"builtin"}) def test_create_distutils_cfg(creator, tmp_path, monkeypatch): result = cli_run([ensure_text(str(tmp_path / "venv")), "--activators", "", "--creator", creator])