The .pth mechanism breaks subprocess

[rdbisme]

My current poetry version (1.1.14) has this poetry.utils._compat.py:

    def run(*popenargs, **kwargs):
        """Run command with arguments and return a CompletedProcess instance.
        The returned instance will have attributes args, returncode, stdout and
        stderr. By default, stdout and stderr are not captured, and those attributes
        will be None. Pass stdout=PIPE and/or stderr=PIPE in order to capture them.
        If check is True and the exit code was non-zero, it raises a
        CalledProcessError. The CalledProcessError object will have the return code
        in the returncode attribute, and output & stderr attributes if those streams
        were captured.
        If timeout is given, and the process takes too long, a TimeoutExpired
        exception will be raised.
        There is an optional argument "input", allowing you to
        pass a string to the subprocess's stdin.  If you use this argument
        you may not also use the Popen constructor's "stdin" argument, as
        it will be used internally.
        The other arguments are the same as for the Popen constructor.
        If universal_newlines=True is passed, the "input" argument must be a
        string and stdout/stderr in the returned object will be strings rather than
        bytes.
        """
        input = kwargs.pop("input", None)
        timeout = kwargs.pop("timeout", None)
        check = kwargs.pop("check", False)
        if input is not None:
            if "stdin" in kwargs:
                raise ValueError("stdin and input arguments may not both be used.")
            kwargs["stdin"] = PIPE

        process = Popen(*popenargs, **kwargs)
        try:
            process.__enter__()  # No-Op really... illustrate "with in 2.4"
            try:
                stdout, stderr = process.communicate(input, timeout=timeout)
            except TimeoutExpired:
                process.kill()
                stdout, stderr = process.communicate()
                raise TimeoutExpired(
                    process.args, timeout, output=stdout, stderr=stderr
                )
            except:
                process.kill()
                process.wait()
                raise
            retcode = process.poll()
            if check and retcode:
                raise CalledProcessError(
                    retcode, process.args, output=stdout, stderr=stderr
                )
        finally:
            # None because our context manager __exit__ does not use them.
            process.__exit__(None, None, None)

        return CompletedProcess(process.args, retcode, stdout, stderr)

    subprocess.run = run
    subprocess.CalledProcessError = CalledProcessError

The problem is that when poetry-dynamic-versioning gets injected into the environment, it imports poetry:

def _apply_patches() -> None:
    # Use the least invasive patching if possible; otherwise, wait until
    # Poetry is available to be patched.
    need_fallback_patches = False

    if not _state.patched_poetry_create:
        try:
            from poetry import factory as factory_mod

            _patch_poetry_create(factory_mod)
            _state.patched_poetry_create = True
        except ImportError:
            need_fallback_patches = True

    if not _state.patched_core_poetry_create:
        try:
            from poetry.core import factory as core_factory_mod

            _patch_poetry_create(core_factory_mod)
            _state.patched_core_poetry_create = True
        except ImportError:
            need_fallback_patches = True

    if not _state.patched_poetry_command_run:
        try:
            from poetry.console.commands import run as run_mod

            _patch_poetry_command_run(run_mod)
            _state.patched_poetry_command_run = True
        except ImportError:
            need_fallback_patches = True

    if not _state.patched_poetry_command_shell:
        try:
            from poetry.console.commands import shell as shell_mod

            _patch_poetry_command_shell(shell_mod)
            _state.patched_poetry_command_shell = True
        except ImportError:
            need_fallback_patches = True

    if need_fallback_patches:
        _patch_builtins_import()

That means now that in my virtual environment, subprocess.run is monkey patched with the poetry.utils._compat.run version, and in particular it doesn't support the capture_output kwarg.

To reproduce:

1. Create a virtualenv
2. Install poetry and poetry-dynamic-versioning
3. Run something like:

import subprocess

subprocess.run(["python", "-V"], capture_output=True])

Traceback (most recent call last):
  File "/home/***/git/qupynt/src/qupynt/run.py", line 3, in <module>
    subprocess.run(["python", "-V"], capture_output=True)
  File "/home/***/mambaforge/envs/qupynt/lib/python3.10/site-packages/poetry/utils/_compat.py", line 192, in run
    process = Popen(*popenargs, **kwargs)
TypeError: Popen.__init__() got an unexpected keyword argument 'capture_output'

I guess the problem here is that poetry and poetry-dynamic-versioning should not be installed in the development environment, but...

[mtkennerly]

I think you should consider reporting this to Poetry as an issue for anyone using non-isolated installs of Poetry, although since they probably expect it to be used as a CLI only, they may not consider it a problem. Still, I think they should try to make their monkey patch compatible with the standard function or at least gracefully fall back to it when necessary.

As far as poetry-dynamic-versioning goes, it may be able to skip patching Poetry if it was able to patch Poetry Core. We'd just need to make sure that that wouldn't lose any functionality. That could also run into a similar issue if Poetry Core ever introduced its own monkey patching, but that seems less likely.

[mtkennerly]

I noticed this comment from a Poetry maintainer:

At Poetry we consider CLI as the API and nothing more.

Also, the latest Poetry installation instructions no longer suggest pip install --user poetry as an option. Now, even the "manually (advanced)" section indicates that a virtual environment is required.

Based on that, I think this kind of subprocess disruption and other similar changes could be expected in the future when attempting to import poetry like the plugin does. I think the only reasonably safe thing to do is say that the plugin should also be installed in the same virtual environment as Poetry, so that it can't interfere with subprocess/etc in your global Python environment. I'm going to update the plugin documentation to no longer recommend global installs.

[mtkennerly]

I've gone a step further and replaced the import hacks entirely in v0.18.0. The README has the new install instructions. This prevents the plugin from inadvertently activating any of Poetry's monkey patches on a global basis.

[rdbisme]

Thanks a lot for the effort :)