Fix issues on windows and restore changes from 0.16.1 (#106)

* Fix various issues manifesting on windows

- Make the tests that break on python37 on windows only run for python >= 3.8
- Reenable disabled test matrix item for windows-python37

In shell tasks:
- check for sh installed with git
- trust git bash over bash on the path which might be a decoy put there to trick us
- add test for the default powershell, as distinct from pwsh

* Revert "Revert "PoetryExecutor does not use poetry if POETRY_VIRTUALENVS_CREATE=false #65""

This reverts commit e6e162e.

* Always explicitly use the poetry virtualenv if possible in the poetry executor

This removes an optimization that probably does add much value anymore, but
does seem to cause tasks to be run without the correct virtual_env in some
scenarios on windows.

Also make process of updaating the path env var to enable a venv try to avoid
redundant updates in case the virtualenv is already active.

.github/workflows/ci.yml

@@ -39,10 +39,6 @@ jobs:
       matrix:
         os: [Ubuntu, MacOS, Windows]
         python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
-        # TODO: remove this when apparent issue with GitHub actions is fixed
-        exclude:
-        - os: Windows
-          python-version: '3.7'
     runs-on: ${{ matrix.os }}-latest
     steps:
       - uses: actions/checkout@v3

poethepoet/executor/poetry.py

@@ -1,6 +1,5 @@
-import os
 import shutil
-import sys
+from os import environ
 from pathlib import Path
 from subprocess import PIPE, Popen
 from typing import Dict, Optional, Sequence, Type
@@ -25,17 +24,8 @@ 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)
-
-        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)
+        poetry_env = self._get_poetry_virtualenv()
+        project_dir = self.context.config.project_dir
 
         if poetry_env:
             # Execute the task in the virtualenv from poetry
@@ -47,6 +37,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),
@@ -57,16 +51,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)

poethepoet/task/shell.py

@@ -109,12 +109,21 @@ def _locate_interpreter(self, interpreter: str) -> Optional[str]:
         elif interpreter == "sh":
             result = which("sh") or which("/bin/sh")
 
-        elif interpreter == "bash":
-            result = which("bash") or which("/bin/bash")
-
-            # Specifically look for git bash on windows
+            # Specifically look for git sh on windows
             if result is None and self._is_windows:
-                result = which(f"{prog_files}\\Git\\bin\\bash.exe")
+                result = which(f"{prog_files}\\Git\\bin\\sh.exe")
+
+        elif interpreter == "bash":
+            if self._is_windows:
+                # Specifically look for git bash on windows as the preferred option
+                # Don't trust bash from the path becuase it might be a useless decoy
+                result = (
+                    which(f"{prog_files}\\Git\\bin\\bash.exe")
+                    or which("/bin/bash")
+                    or which("bash")
+                )
+            else:
+                result = which("bash") or which("/bin/bash")
 
         elif interpreter == "zsh":
             result = which("zsh") or which("/bin/zsh")

poethepoet/virtualenv.py

@@ -76,12 +76,16 @@ def valid(self) -> bool:
         )
 
     def get_env_vars(self, base_env: Mapping[str, str]) -> Dict[str, str]:
-        path_delim = ";" if self._is_windows else ":"
-        result = dict(
-            base_env,
-            VIRTUAL_ENV=str(self.path),
-            PATH=f"{self.bin_dir()}{path_delim}{os.environ.get('PATH', '')}",
-        )
+        bin_dir = str(self.bin_dir())
+        path_var = os.environ.get("PATH", "")
+
+        if not path_var.startswith(bin_dir):
+            path_delim = ";" if self._is_windows else ":"
+            path_var = bin_dir + path_delim + path_var
+
+        result = dict(base_env, VIRTUAL_ENV=str(self.path), PATH=path_var)
+
         if "PYTHONHOME" in result:
             result.pop("PYTHONHOME")
+
         return result

tests/conftest.py

@@ -139,11 +139,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:

tests/fixtures/shells_project/pyproject.toml

@@ -35,6 +35,12 @@ poe_test_echo "bam bam baaam bam"
   env = { test_var =  "roflcopter"}
 
 
+  [tool.poe.tasks.echo_powershell]
+  interpreter = "powershell"
+  shell = "poe_test_echo $ENV:test_var"
+  env = { test_var =  "roflcopter"}
+
+
   [tool.poe.tasks.echo_python]
   interpreter = "python"
   shell = """

tests/test_poe_config.py

@@ -1,7 +1,14 @@
 import os
+import shutil
 import tempfile
 from pathlib import Path
 
+import pytest
+
+# 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):
     # Also tests passing of extra_args to sys.argv
@@ -10,14 +17,15 @@ 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")
+    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 == ""

tests/test_poetry_plugin.py

@@ -15,7 +15,7 @@ def setup_poetry_project_empty_prefix(run_poetry, projects):
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_poetry_help(run_poetry, projects):
     result = run_poetry([], cwd=projects["poetry_plugin"])
     assert result.stdout.startswith("Poetry (version ")
@@ -25,7 +25,7 @@ def test_poetry_help(run_poetry, projects):
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_task_with_cli_dependency(
     run_poetry, projects, setup_poetry_project, is_windows
 ):
@@ -47,7 +47,7 @@ def test_task_with_cli_dependency(
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_task_with_lib_dependency(
     run_poetry, projects, setup_poetry_project, is_windows
 ):
@@ -59,7 +59,7 @@ def test_task_with_lib_dependency(
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_task_accepts_any_args(run_poetry, projects, setup_poetry_project):
     result = run_poetry(
         ["poe", "echo", "--lol=:D", "--version", "--help"],
@@ -72,7 +72,7 @@ def test_task_accepts_any_args(run_poetry, projects, setup_poetry_project):
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_poetry_help_without_poe_command_prefix(
     run_poetry, projects, setup_poetry_project_empty_prefix
 ):
@@ -84,7 +84,7 @@ def test_poetry_help_without_poe_command_prefix(
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_running_tasks_without_poe_command_prefix(
     run_poetry, projects, setup_poetry_project_empty_prefix
 ):
@@ -99,7 +99,7 @@ def test_running_tasks_without_poe_command_prefix(
 
 
 @pytest.mark.slow
-@pytest.mark.skipif(version_info < (3, 7), reason="dependencies require python>=3.7")
+@pytest.mark.skipif(version_info < (3, 8), reason="dependencies require python>=3.8")
 def test_running_poetry_command_with_hooks(run_poetry, projects, setup_poetry_project):
     result = run_poetry(["env", "info"], cwd=projects["poetry_plugin"])
     assert "THIS IS YOUR ENV!" in result.stdout