Skip to content

Commit

Permalink
Revert "PoetryExecutor does not use poetry if POETRY_VIRTUALENVS_CREA…
Browse files Browse the repository at this point in the history
…TE=false #65"

This is to address the bug reported as #88

This reverts commit 1d1d218.
  • Loading branch information
nat-n committed Sep 2, 2022
1 parent 210ae45 commit e6e162e
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 139 deletions.
2 changes: 1 addition & 1 deletion poethepoet/app.py
Expand Up @@ -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"][1:]
_, *extra_args = self.ui["task"]
if context is None:
context = self.get_run_context()
try:
Expand Down
67 changes: 13 additions & 54 deletions poethepoet/executor/poetry.py
@@ -1,5 +1,5 @@
from subprocess import Popen, PIPE
from os import environ
import os
from pathlib import Path
import shutil
import sys
Expand All @@ -24,15 +24,14 @@ def execute(
Execute the given cmd as a subprocess inside the poetry managed dev environment
"""

poetry_env = self._get_poetry_virtualenv()
project_dir = self.context.config.project_dir
# 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)

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)
if (
bool(os.environ.get("POETRY_ACTIVE"))
or self.context.poe_active == PoetryExecutor.__key__
or sys.prefix == poetry_env
):
# The target venv is already active so we can execute the command unaltered
return self._execute_cmd(cmd, input=input, use_exec=use_exec)
Expand All @@ -47,10 +46,6 @@ 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),
Expand All @@ -61,52 +56,16 @@ def execute(
def _get_poetry_virtualenv(self, force: bool = True):
"""
Ask poetry where it put the virtualenv for this project.
Invoking poetry is relatively expensive so cache the result
This is a relatively expensive operation so uses the context.exec_cache
"""

# 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,
)
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)
.communicate()[0]
.decode()
.strip()
)

return exec_cache.get("poetry_virtualenv")
return self.context.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)
12 changes: 3 additions & 9 deletions tests/conftest.py
Expand Up @@ -137,17 +137,11 @@ def run_poe_subproc(
output=rf"open(r\"{temp_file}\", \"w\")",
)

subproc_env = dict(os.environ)
subproc_env.pop("VIRTUAL_ENV", None)
if env:
subproc_env.update(env)

env = dict(os.environ, **(env or {}))
if coverage:
subproc_env["COVERAGE_PROCESS_START"] = str(PROJECT_TOML)
env["COVERAGE_PROCESS_START"] = str(PROJECT_TOML)

poeproc = Popen(
shell_cmd, shell=True, stdout=PIPE, stderr=PIPE, env=subproc_env
)
poeproc = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE, env=env)
task_out, task_err = poeproc.communicate()

with temp_file.open("rb") as output_file:
Expand Down
8 changes: 0 additions & 8 deletions tests/fixtures/example_project/poetry.lock

This file was deleted.

File renamed without changes.
16 changes: 4 additions & 12 deletions tests/test_poe_config.py
@@ -1,13 +1,6 @@
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):
Expand All @@ -17,15 +10,14 @@ 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"
assert result.stderr == ""


def test_setting_default_array_item_task_type(run_poe_subproc):
result = run_poe_subproc("composite_task", project="scripts", env=no_venv)
result = run_poe_subproc("composite_task", project="scripts")
assert (
result.capture == f"Poe => poe_test_echo Hello\nPoe => poe_test_echo World!\n"
)
Expand All @@ -34,7 +26,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", env=no_venv)
result = run_poe_subproc("travel")
if is_windows:
assert (
result.capture
Expand Down Expand Up @@ -82,11 +74,11 @@ def test_partially_decrease_verbosity(run_poe_subproc, high_verbosity_project_pa
assert result.stderr == ""


def test_decrease_verbosity(run_poe_subproc, is_windows):
def test_decrease_verbosity(run_poe_subproc, projects, is_windows):
result = run_poe_subproc(
"-q",
"part1",
env=no_venv,
cwd=projects["example"],
)
assert result.capture == ""
assert result.stderr == ""
Expand Down

0 comments on commit e6e162e

Please sign in to comment.