From 1d1d218e4f741960899a799f23498f5b19d04a4a Mon Sep 17 00:00:00 2001 From: Nat Noordanus Date: Sun, 28 Aug 2022 14:53:27 +0200 Subject: [PATCH] PoetryExecutor does not use poetry if POETRY_VIRTUALENVS_CREATE=false #65 Also: - PoetryExecutor uses VirtualEnv based execution whenever possible - Fixed issue where poe would use the env from the current project instead of the target project - Update some tests to not avoid encountering missing poetry env --- poethepoet/app.py | 2 +- poethepoet/executor/poetry.py | 67 +++++++++++++---- tests/conftest.py | 12 ++- tests/fixtures/example_project/poetry.lock | 8 ++ .../poetry.toml | 0 tests/test_poe_config.py | 16 +++- tests/test_script_tasks.py | 73 +++++++++++++------ tests/test_task_running.py | 4 +- 8 files changed, 139 insertions(+), 43 deletions(-) create mode 100644 tests/fixtures/example_project/poetry.lock rename tests/fixtures/{scripts_project => example_project}/poetry.toml (100%) diff --git a/poethepoet/app.py b/poethepoet/app.py index fe83b72d..0653fc76 100644 --- a/poethepoet/app.py +++ b/poethepoet/app.py @@ -90,7 +90,7 @@ def resolve_task(self) -> bool: return True def run_task(self, context: Optional[RunContext] = None) -> Optional[int]: - _, *extra_args = self.ui["task"] + extra_args = self.ui["task"][1:] if context is None: context = self.get_run_context() try: diff --git a/poethepoet/executor/poetry.py b/poethepoet/executor/poetry.py index e11e7de9..0905f34a 100644 --- a/poethepoet/executor/poetry.py +++ b/poethepoet/executor/poetry.py @@ -1,5 +1,5 @@ from subprocess import Popen, PIPE -import os +from os import environ from pathlib import Path import shutil import sys @@ -24,14 +24,15 @@ def execute( Execute the given cmd as a subprocess inside the poetry managed dev environment """ - # If this run involves multiple executions then it's worth trying to - # avoid repetative (and expensive) calls to `poetry run` if we can - poetry_env = self._get_poetry_virtualenv(force=self.context.multistage) + poetry_env = self._get_poetry_virtualenv() + project_dir = self.context.config.project_dir - if ( - bool(os.environ.get("POETRY_ACTIVE")) - or self.context.poe_active == PoetryExecutor.__key__ - or sys.prefix == poetry_env + if sys.prefix == poetry_env or ( + ( + bool(environ.get("POETRY_ACTIVE")) + or self.context.poe_active == PoetryExecutor.__key__ + ) + and project_dir == environ.get("POE_ROOT", project_dir) ): # The target venv is already active so we can execute the command unaltered return self._execute_cmd(cmd, input=input, use_exec=use_exec) @@ -46,6 +47,10 @@ def execute( use_exec=use_exec, ) + if self._virtualenv_creation_disabled(): + # There's no poetry env, and there isn't going to be + return self._execute_cmd(cmd, input=input, use_exec=use_exec) + # Run this task with `poetry run` return self._execute_cmd( (self._poetry_cmd(), "run", *cmd), @@ -56,16 +61,52 @@ def execute( def _get_poetry_virtualenv(self, force: bool = True): """ Ask poetry where it put the virtualenv for this project. - This is a relatively expensive operation so uses the context.exec_cache + Invoking poetry is relatively expensive so cache the result """ - if force and "poetry_virtualenv" not in self.context.exec_cache: - self.context.exec_cache["poetry_virtualenv"] = ( - Popen((self._poetry_cmd(), "env", "info", "-p"), stdout=PIPE) + + # TODO: see if there's a more efficient way to do this that doesn't involve + # invoking the poetry cli or relying on undocumented APIs + + exec_cache = self.context.exec_cache + + if force and "poetry_virtualenv" not in exec_cache: + # Need to make sure poetry isn't influenced by whatever virtualenv is + # currently active + clean_env = dict(environ) + clean_env.pop("VIRTUAL_ENV", None) + + exec_cache["poetry_virtualenv"] = ( + Popen( + (self._poetry_cmd(), "env", "info", "-p"), + stdout=PIPE, + cwd=self.context.config.project_dir, + env=clean_env, + ) .communicate()[0] .decode() .strip() ) - return self.context.exec_cache.get("poetry_virtualenv") + + return exec_cache.get("poetry_virtualenv") def _poetry_cmd(self): return shutil.which("poetry") or "poetry" + + def _virtualenv_creation_disabled(self): + exec_cache = self.context.exec_cache + + while "poetry_virtualenvs_create_disabled" not in exec_cache: + # Check env override + env_override = environ.get("POETRY_VIRTUALENVS_CREATE") + if env_override is not None: + exec_cache["poetry_virtualenvs_create_disabled"] = ( + env_override == "false" + ) + break + + # A complete implementation would also check for a local poetry config file + # and a global poetry config file (location for this is platform dependent) + # in that order but just checking the env will do for now. + break + + return exec_cache.get("poetry_virtualenvs_create_disabled", False) diff --git a/tests/conftest.py b/tests/conftest.py index 6d29b39a..1f822a2e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -137,11 +137,17 @@ def run_poe_subproc( output=rf"open(r\"{temp_file}\", \"w\")", ) - env = dict(os.environ, **(env or {})) + subproc_env = dict(os.environ) + subproc_env.pop("VIRTUAL_ENV", None) + if env: + subproc_env.update(env) + if coverage: - env["COVERAGE_PROCESS_START"] = str(PROJECT_TOML) + subproc_env["COVERAGE_PROCESS_START"] = str(PROJECT_TOML) - poeproc = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE, env=env) + poeproc = Popen( + shell_cmd, shell=True, stdout=PIPE, stderr=PIPE, env=subproc_env + ) task_out, task_err = poeproc.communicate() with temp_file.open("rb") as output_file: diff --git a/tests/fixtures/example_project/poetry.lock b/tests/fixtures/example_project/poetry.lock new file mode 100644 index 00000000..d9af7385 --- /dev/null +++ b/tests/fixtures/example_project/poetry.lock @@ -0,0 +1,8 @@ +package = [] + +[metadata] +lock-version = "1.1" +python-versions = "*" +content-hash = "115cf985d932e9bf5f540555bbdd75decbb62cac81e399375fc19f6277f8c1d8" + +[metadata.files] diff --git a/tests/fixtures/scripts_project/poetry.toml b/tests/fixtures/example_project/poetry.toml similarity index 100% rename from tests/fixtures/scripts_project/poetry.toml rename to tests/fixtures/example_project/poetry.toml diff --git a/tests/test_poe_config.py b/tests/test_poe_config.py index e9346ff4..8fc50580 100644 --- a/tests/test_poe_config.py +++ b/tests/test_poe_config.py @@ -1,6 +1,13 @@ import os from pathlib import Path +import pytest import tempfile +import shutil + + +# Setting POETRY_VIRTUALENVS_CREATE stops poetry from creating the virtualenv and +# spamming about it in stdout +no_venv = {"POETRY_VIRTUALENVS_CREATE": "false"} def test_setting_default_task_type(run_poe_subproc, projects, esc_prefix): @@ -10,6 +17,7 @@ def test_setting_default_task_type(run_poe_subproc, projects, esc_prefix): "nat,", r"welcome to " + esc_prefix + "${POE_ROOT}", project="scripts", + env=no_venv, ) assert result.capture == f"Poe => echo-args nat, welcome to {projects['scripts']}\n" assert result.stdout == f"hello nat, welcome to {projects['scripts']}\n" @@ -17,7 +25,7 @@ def test_setting_default_task_type(run_poe_subproc, projects, esc_prefix): def test_setting_default_array_item_task_type(run_poe_subproc): - result = run_poe_subproc("composite_task", project="scripts") + result = run_poe_subproc("composite_task", project="scripts", env=no_venv) assert ( result.capture == f"Poe => poe_test_echo Hello\nPoe => poe_test_echo World!\n" ) @@ -26,7 +34,7 @@ def test_setting_default_array_item_task_type(run_poe_subproc): def test_setting_global_env_vars(run_poe_subproc, is_windows): - result = run_poe_subproc("travel") + result = run_poe_subproc("travel", env=no_venv) if is_windows: assert ( result.capture @@ -74,11 +82,11 @@ def test_partially_decrease_verbosity(run_poe_subproc, high_verbosity_project_pa assert result.stderr == "" -def test_decrease_verbosity(run_poe_subproc, projects, is_windows): +def test_decrease_verbosity(run_poe_subproc, is_windows): result = run_poe_subproc( "-q", "part1", - cwd=projects["example"], + env=no_venv, ) assert result.capture == "" assert result.stderr == "" diff --git a/tests/test_script_tasks.py b/tests/test_script_tasks.py index b1846ac9..f9594f30 100644 --- a/tests/test_script_tasks.py +++ b/tests/test_script_tasks.py @@ -1,8 +1,12 @@ import difflib +import pytest +import shutil + +no_venv = {"POETRY_VIRTUALENVS_CREATE": "false"} def test_script_task_with_hard_coded_args(run_poe_subproc, projects, esc_prefix): - result = run_poe_subproc("static-args-test", project="scripts") + result = run_poe_subproc("static-args-test", project="scripts", env=no_venv) assert result.capture == f"Poe => static-args-test\n" assert result.stdout == ( "str: pat a cake\n" @@ -25,28 +29,28 @@ def test_script_task_with_hard_coded_args(run_poe_subproc, projects, esc_prefix) def test_call_attr_func_with_exec(run_poe_subproc): - result = run_poe_subproc("call_attrs", project="scripts") + result = run_poe_subproc("call_attrs", project="scripts", env=no_venv) assert result.capture == "Poe => call_attrs\n" assert result.stdout == "task!\n" assert result.stderr == "" def test_print_result(run_poe_subproc): - result = run_poe_subproc("print-script-result", project="scripts") + result = run_poe_subproc("print-script-result", project="scripts", env=no_venv) assert result.capture == "Poe => print-script-result\n" assert result.stdout == "determining most random number...\n7\n" assert result.stderr == "" def test_dont_print_result(run_poe_subproc): - result = run_poe_subproc("dont-print-script-result", project="scripts") + result = run_poe_subproc("dont-print-script-result", project="scripts", env=no_venv) assert result.capture == "Poe => dont-print-script-result\n" assert result.stdout == "determining most random number...\n" assert result.stderr == "" def test_automatic_kwargs_from_args(run_poe_subproc): - result = run_poe_subproc("greet", project="scripts") + result = run_poe_subproc("greet", project="scripts", env=no_venv) assert result.capture == "Poe => greet\n" assert result.stdout == "I'm sorry Dave\n" assert result.stderr == "" @@ -58,6 +62,7 @@ def test_script_task_with_cli_args(run_poe_subproc, is_windows): "--greeting=hello", "--user=nat", project="scripts", + env=no_venv, ) assert ( result.capture == f"Poe => greet-passed-args --greeting=hello --user=nat\n" @@ -83,6 +88,7 @@ def test_script_task_with_args_optional(run_poe_subproc, projects, is_windows): "--user=nat", f"--optional=welcome to {named_args_project_path}", project="scripts", + env=no_venv, ) assert result.capture == ( f"Poe => greet-passed-args --greeting=hello --user=nat --optional=welcome " @@ -101,7 +107,7 @@ def test_script_task_with_args_optional(run_poe_subproc, projects, is_windows): def test_script_task_default_arg(run_poe_subproc): - result = run_poe_subproc("greet-full-args", project="scripts") + result = run_poe_subproc("greet-full-args", project="scripts", env=no_venv) assert result.capture == f"Poe => greet-full-args\n" # hi is the default value for --greeting assert result.stdout == "hi None None None\n" @@ -117,6 +123,7 @@ def test_script_task_include_boolean_flag_and_numeric_args(run_poe_subproc): "--age=42", "--height=1.23", project="scripts", + env=no_venv, ) assert result.capture == ( "Poe => greet-full-args --greeting=hello --user=nat --upper --age=42 " @@ -135,6 +142,7 @@ def test_script_task_with_short_args(run_poe_subproc): "109", "-h=1.09", project="scripts", + env=no_venv, ) assert result.capture == ( "Poe => greet-full-args -g=Ciao --user=toni -a 109 -h=1.09\n" @@ -150,24 +158,30 @@ def test_wrong_args_passed(run_poe_subproc): "poe greet-full-args: error:" ) - result = run_poe_subproc("greet-full-args", "--age=lol", project="scripts") + result = run_poe_subproc( + "greet-full-args", "--age=lol", project="scripts", env=no_venv + ) assert result.capture == "" assert result.stdout == "" assert result.stderr == ( f"{base_error} argument --age/-a: invalid int value: 'lol'\n" ) - result = run_poe_subproc("greet-full-args", "--age", project="scripts") + result = run_poe_subproc("greet-full-args", "--age", project="scripts", env=no_venv) assert result.capture == "" assert result.stdout == "" assert result.stderr == (f"{base_error} argument --age/-a: expected one argument\n") - result = run_poe_subproc("greet-full-args", "--age 3 2 1", project="scripts") + result = run_poe_subproc( + "greet-full-args", "--age 3 2 1", project="scripts", env=no_venv + ) assert result.capture == "" assert result.stdout == "" assert result.stderr == (f"{base_error} unrecognized arguments: --age 3 2 1\n") - result = run_poe_subproc("greet-full-args", "--potatoe", project="scripts") + result = run_poe_subproc( + "greet-full-args", "--potatoe", project="scripts", env=no_venv + ) assert result.capture == "" assert result.stdout == "" assert result.stderr == (f"{base_error} unrecognized arguments: --potatoe\n") @@ -175,13 +189,18 @@ def test_wrong_args_passed(run_poe_subproc): def test_required_args(run_poe_subproc): result = run_poe_subproc( - "greet-strict", "--greeting=yo", "--name", "dude", project="scripts" + "greet-strict", + "--greeting=yo", + "--name", + "dude", + project="scripts", + env=no_venv, ) assert result.capture == "Poe => greet-strict --greeting=yo --name dude\n" assert result.stdout == "yo dude\n" assert result.stderr == "" - result = run_poe_subproc("greet-strict", project="scripts") + result = run_poe_subproc("greet-strict", project="scripts", env=no_venv) assert result.capture == "" assert result.stdout == "" assert result.stderr == ( @@ -220,19 +239,23 @@ def test_script_task_bad_content(run_poe_subproc, projects): def test_script_with_positional_args(run_poe_subproc): - result = run_poe_subproc("greet-positional", "help!", "Santa", project="scripts") + result = run_poe_subproc( + "greet-positional", "help!", "Santa", project="scripts", env=no_venv + ) assert result.capture == "Poe => greet-positional help! Santa\n" assert result.stdout == "help! Santa\n" assert result.stderr == "" # Ommission of optional positional arg - result = run_poe_subproc("greet-positional", "Santa", project="scripts") + result = run_poe_subproc( + "greet-positional", "Santa", project="scripts", env=no_venv + ) assert result.capture == "Poe => greet-positional Santa\n" assert result.stdout == "yo Santa\n" assert result.stderr == "" # Ommission of required positional arg - result = run_poe_subproc("greet-positional", project="scripts") + result = run_poe_subproc("greet-positional", project="scripts", env=no_venv) assert result.capture == "" assert result.stdout == "" assert result.stderr == ( @@ -242,7 +265,7 @@ def test_script_with_positional_args(run_poe_subproc): # Too many positional args result = run_poe_subproc( - "greet-positional", "plop", "plop", "plop", project="scripts" + "greet-positional", "plop", "plop", "plop", project="scripts", env=no_venv ) assert result.capture == "" assert result.stdout == "" @@ -254,14 +277,14 @@ def test_script_with_positional_args(run_poe_subproc): def test_script_with_positional_args_and_options(run_poe_subproc): result = run_poe_subproc( - "greet-positional", "help!", "Santa", "--upper", project="scripts" + "greet-positional", "help!", "Santa", "--upper", project="scripts", env=no_venv ) assert result.capture == "Poe => greet-positional help! Santa --upper\n" assert result.stdout == "HELP! SANTA\n" assert result.stderr == "" result = run_poe_subproc( - "greet-positional", "--upper", "help!", "Santa", project="scripts" + "greet-positional", "--upper", "help!", "Santa", project="scripts", env=no_venv ) assert result.capture == "Poe => greet-positional --upper help! Santa\n" assert result.stdout == "HELP! SANTA\n" @@ -283,6 +306,7 @@ def test_script_with_multi_value_args(run_poe_subproc): "v2", "v8", project="scripts", + env=no_venv, ) assert ( result.capture @@ -296,7 +320,7 @@ def test_script_with_multi_value_args(run_poe_subproc): # Test with some args result = run_poe_subproc( - "multiple-value-args", "1", "--engines", "v2", project="scripts" + "multiple-value-args", "1", "--engines", "v2", project="scripts", env=no_venv ) assert result.capture == "Poe => multiple-value-args 1 --engines v2\n" assert result.stdout == ( @@ -306,7 +330,7 @@ def test_script_with_multi_value_args(run_poe_subproc): # Test with minimal args result = run_poe_subproc( - "multiple-value-args", "--engines", "v2", project="scripts" + "multiple-value-args", "--engines", "v2", project="scripts", env=no_venv ) assert result.capture == "Poe => multiple-value-args --engines v2\n" assert result.stdout == ( @@ -316,7 +340,12 @@ def test_script_with_multi_value_args(run_poe_subproc): # Not enough values for option: 0 result = run_poe_subproc( - "multiple-value-args", "--widgets", "--engines", "v2", project="scripts" + "multiple-value-args", + "--widgets", + "--engines", + "v2", + project="scripts", + env=no_venv, ) assert result.capture == "" assert result.stdout == "" @@ -336,6 +365,7 @@ def test_script_with_multi_value_args(run_poe_subproc): "--engines", "v2", project="scripts", + env=no_venv, ) assert result.capture == "" assert result.stdout == "" @@ -351,6 +381,7 @@ def test_script_with_multi_value_args(run_poe_subproc): "--engines", "v2", project="scripts", + env=no_venv, ) assert result.capture == "" assert result.stdout == "" diff --git a/tests/test_task_running.py b/tests/test_task_running.py index dae4297e..d206fc94 100644 --- a/tests/test_task_running.py +++ b/tests/test_task_running.py @@ -1,6 +1,8 @@ def test_ref_task(run_poe_subproc, projects, esc_prefix): # This should be exactly the same as calling the echo task directly - result = run_poe_subproc("also_echo", "foo", "!") + result = run_poe_subproc( + "also_echo", "foo", "!", env={"POETRY_VIRTUALENVS_CREATE": "false"} + ) assert ( result.capture == f"Poe => poe_test_echo POE_ROOT:{projects['example']} Password1, task_args: foo !\n"