From bb26d8c97c1bd2da85ac6b995d8598b0f1e05ff3 Mon Sep 17 00:00:00 2001 From: Tyler Levy Conde Date: Thu, 27 Jun 2024 16:25:53 -0600 Subject: [PATCH] replace pyvenv with builtin venv --- salt/modules/virtualenv_mod.py | 136 ++++++++++++++++------ tests/unit/modules/test_virtualenv_mod.py | 34 +++--- 2 files changed, 117 insertions(+), 53 deletions(-) diff --git a/salt/modules/virtualenv_mod.py b/salt/modules/virtualenv_mod.py index cb0a6322e7d..0e07263d3ff 100644 --- a/salt/modules/virtualenv_mod.py +++ b/salt/modules/virtualenv_mod.py @@ -27,7 +27,10 @@ KNOWN_BINARY_NAMES = frozenset( log = logging.getLogger(__name__) -__opts__ = {"venv_bin": salt.utils.path.which_bin(KNOWN_BINARY_NAMES) or "virtualenv"} +__opts__ = { + "venv_bin": salt.utils.path.which_bin(KNOWN_BINARY_NAMES) + or f"{sys.executable} -m venv" +} __pillar__ = {} @@ -82,6 +85,8 @@ def create( never_download=None, prompt=None, pip=False, + symlinks=None, + upgrade=None, user=None, use_vt=False, saltenv="base", @@ -99,7 +104,7 @@ def create( Defaults to ``virtualenv``. system_site_packages : False - Passthrough argument given to virtualenv + Passthrough argument given to virtualenv or venv distribute : False Passthrough argument given to virtualenv @@ -109,7 +114,7 @@ def create( ``distribute=True`` clear : False - Passthrough argument given to virtualenv + Passthrough argument given to virtualenv or venv python : None (default) Passthrough argument given to virtualenv @@ -123,6 +128,12 @@ def create( prompt : None (default) Passthrough argument given to virtualenv if not None + symlinks : None + Passthrough argument given to venv if True + + upgrade : None + Passthrough argument given to venv if True + user : None Set ownership for the virtualenv @@ -165,47 +176,98 @@ def create( - env: - VIRTUALENV_ALWAYS_COPY: 1 """ - # TODO venv_bin can be "sys.executable -m venv" if venv_bin is None: - venv_bin = __opts__.get("venv_bin") or __pillar__.get("venv_bin") + venv_bin = __pillar__.get("venv_bin") or __opts__.get("venv_bin") cmd = [venv_bin] - virtualenv_version_info = virtualenv_ver(venv_bin, user=user, **kwargs) - - if distribute: - if virtualenv_version_info >= (1, 10): - log.info( - "The virtualenv '--distribute' option has been " - "deprecated in virtualenv(>=1.10), as such, the " - "'distribute' option to `virtualenv.create()` has " - "also been deprecated and it's not necessary anymore." + if "venv" not in venv_bin: + # ----- Stop the user if venv only options are used -----------------> + # If any of the following values are not None, it means that the user + # is actually passing a True or False value. Stop Him! + if upgrade is not None: + raise CommandExecutionError( + "The `upgrade`(`--upgrade`) option is not supported by '{}'".format( + venv_bin + ) ) - else: - cmd.append("--distribute") - - if python is not None and python.strip() != "": - if not salt.utils.path.which(python): - raise CommandExecutionError(f"Cannot find requested python ({python}).") - cmd.append(f"--python={python}") - if extra_search_dir is not None: - if isinstance(extra_search_dir, str) and extra_search_dir.strip() != "": - extra_search_dir = [e.strip() for e in extra_search_dir.split(",")] - for entry in extra_search_dir: - cmd.append(f"--extra-search-dir={entry}") - if never_download is True: - if (1, 10) <= virtualenv_version_info < (14, 0, 0): - log.info( - "--never-download was deprecated in 1.10.0, but reimplemented in" - " 14.0.0. If this feature is needed, please install a supported" - " virtualenv version." + elif symlinks is not None: + raise CommandExecutionError( + "The `symlinks`(`--symlinks`) option is not supported by '{}'".format( + venv_bin + ) ) - else: - cmd.append("--never-download") - if prompt is not None and prompt.strip() != "": - cmd.append(f"--prompt='{prompt}'") + # <---- Stop the user if venv only options are used ------------------ - # Common options to virtualenv + virtualenv_version_info = virtualenv_ver(venv_bin, user=user, **kwargs) + + if distribute: + if virtualenv_version_info >= (1, 10): + log.info( + "The virtualenv '--distribute' option has been " + "deprecated in virtualenv(>=1.10), as such, the " + "'distribute' option to `virtualenv.create()` has " + "also been deprecated and it's not necessary anymore." + ) + else: + cmd.append("--distribute") + + if python is not None and python.strip() != "": + if not salt.utils.path.which(python): + raise CommandExecutionError(f"Cannot find requested python ({python}).") + cmd.append(f"--python={python}") + if extra_search_dir is not None: + if isinstance(extra_search_dir, str) and extra_search_dir.strip() != "": + extra_search_dir = [e.strip() for e in extra_search_dir.split(",")] + for entry in extra_search_dir: + cmd.append(f"--extra-search-dir={entry}") + if never_download is True: + if (1, 10) <= virtualenv_version_info < (14, 0, 0): + log.info( + "--never-download was deprecated in 1.10.0, but reimplemented in" + " 14.0.0. If this feature is needed, please install a supported" + " virtualenv version." + ) + else: + cmd.append("--never-download") + if prompt is not None and prompt.strip() != "": + cmd.append(f"--prompt='{prompt}'") + else: + # venv module from the Python >= 3.3 standard library + + # ----- Stop the user if virtualenv only options are being used -----> + # If any of the following values are not None, it means that the user + # is actually passing a True or False value. Stop Him! + if python is not None and python.strip() != "": + raise CommandExecutionError( + "The `python`(`--python`) option is not supported by '{}'".format( + venv_bin + ) + ) + elif extra_search_dir is not None and extra_search_dir.strip() != "": + raise CommandExecutionError( + "The `extra_search_dir`(`--extra-search-dir`) option is not " + "supported by '{}'".format(venv_bin) + ) + elif never_download is not None: + raise CommandExecutionError( + "The `never_download`(`--never-download`) option is not " + "supported by '{}'".format(venv_bin) + ) + elif prompt is not None and prompt.strip() != "": + raise CommandExecutionError( + "The `prompt`(`--prompt`) option is not supported by '{}'".format( + venv_bin + ) + ) + # <---- Stop the user if virtualenv only options are being used ------ + + if upgrade is True: + cmd.append("--upgrade") + if symlinks is True: + cmd.append("--symlinks") + + # Common options to virtualenv and venv if clear is True: cmd.append("--clear") if system_site_packages is True: diff --git a/tests/unit/modules/test_virtualenv_mod.py b/tests/unit/modules/test_virtualenv_mod.py index 552a93264b2..bd682b6a22a 100644 --- a/tests/unit/modules/test_virtualenv_mod.py +++ b/tests/unit/modules/test_virtualenv_mod.py @@ -17,6 +17,8 @@ from tests.support.mixins import LoaderModuleMockMixin from tests.support.mock import MagicMock, patch from tests.support.unit import TestCase +VENV_BIN = f"{sys.executable} -m venv" + class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): def setup_loader_modules(self): @@ -146,7 +148,7 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): ) def test_unapplicable_options(self): - # ----- Virtualenv using pyvenv options -----------------------------> + # ----- Virtualenv using venv options -------------------------------> mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) with patch.dict(virtualenv_mod.__salt__, {"cmd.run_all": mock}): self.assertRaises( @@ -166,19 +168,19 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): venv_bin="virtualenv", symlinks=True, ) - # <---- Virtualenv using pyvenv options ------------------------------ + # <---- Virtualenv using venv options -------------------------------- - # ----- pyvenv using virtualenv options -----------------------------> + # ----- venv using virtualenv options -------------------------------> mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) with patch.dict( virtualenv_mod.__salt__, - {"cmd.run_all": mock, "cmd.which_bin": lambda _: "pyvenv"}, + {"cmd.run_all": mock, "cmd.which_bin": lambda _: VENV_BIN}, ): self.assertRaises( CommandExecutionError, virtualenv_mod.create, "/tmp/foo", - venv_bin="pyvenv", + venv_bin=VENV_BIN, python="python2.7", ) @@ -187,7 +189,7 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): CommandExecutionError, virtualenv_mod.create, "/tmp/foo", - venv_bin="pyvenv", + venv_bin=VENV_BIN, prompt="PY Prompt", ) @@ -196,7 +198,7 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): CommandExecutionError, virtualenv_mod.create, "/tmp/foo", - venv_bin="pyvenv", + venv_bin=VENV_BIN, never_download=True, ) @@ -205,10 +207,10 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): CommandExecutionError, virtualenv_mod.create, "/tmp/foo", - venv_bin="pyvenv", + venv_bin=VENV_BIN, extra_search_dir="/tmp/bar", ) - # <---- pyvenv using virtualenv options ------------------------------ + # <---- venv using virtualenv options -------------------------------- def test_get_virtualenv_version_from_shell(self): with ForceImportErrorOn("virtualenv"): @@ -321,30 +323,30 @@ class VirtualenvTestCase(TestCase, LoaderModuleMockMixin): ) def test_upgrade_argument(self): - # We test for pyvenv only because with virtualenv this is un + # We test for venv only because with virtualenv this is un # unsupported option. mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) with patch.dict(virtualenv_mod.__salt__, {"cmd.run_all": mock}): - virtualenv_mod.create("/tmp/foo", venv_bin="pyvenv", upgrade=True) + virtualenv_mod.create("/tmp/foo", venv_bin=VENV_BIN, upgrade=True) mock.assert_called_once_with( - ["pyvenv", "--upgrade", "/tmp/foo"], runas=None, python_shell=False + [VENV_BIN, "--upgrade", "/tmp/foo"], runas=None, python_shell=False ) def test_symlinks_argument(self): - # We test for pyvenv only because with virtualenv this is un + # We test for venv only because with virtualenv this is un # unsupported option. mock = MagicMock(return_value={"retcode": 0, "stdout": ""}) with patch.dict(virtualenv_mod.__salt__, {"cmd.run_all": mock}): - virtualenv_mod.create("/tmp/foo", venv_bin="pyvenv", symlinks=True) + virtualenv_mod.create("/tmp/foo", venv_bin=VENV_BIN, symlinks=True) mock.assert_called_once_with( - ["pyvenv", "--symlinks", "/tmp/foo"], runas=None, python_shell=False + [VENV_BIN, "--symlinks", "/tmp/foo"], runas=None, python_shell=False ) def test_virtualenv_ver(self): """ test virtualenv_ver when there is no ImportError """ - ret = virtualenv_mod.virtualenv_ver(venv_bin="pyvenv") + ret = virtualenv_mod.virtualenv_ver(venv_bin=VENV_BIN) assert ret == (1, 9, 1) def test_virtualenv_ver_importerror(self):