Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Poetry breaks when there are spaces in path to Python #1479

Closed
3 tasks done
absassi opened this issue Oct 18, 2019 · 5 comments
Closed
3 tasks done

Poetry breaks when there are spaces in path to Python #1479

absassi opened this issue Oct 18, 2019 · 5 comments
Labels
kind/bug Something isn't working as expected

Comments

@absassi
Copy link

absassi commented Oct 18, 2019

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • OS version and name: Windows 10
  • Poetry version: 1.0.0b1
  • Link of a Gist with the contents of your pyproject.toml file:

Issue

Cannot use Python interpreter at "C:\Program Files\Python38\python.exe", because Poetry makes a subprocess call using shell without properly quoting arguments. Note that, while I found this issue in Windows, any operating system is affected. It's just that spaces in paths are not so common in other systems. The above path is the default installation folder of Python for Windows when installed for all users.

>poetry env use -vvv "C:\Program Files\Python38\python.exe"
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

[EnvCommandError]
Command C:\Program Files\Python38\python.exe -c "import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))" errored with the following return code 1, and output:


Traceback (most recent call last):
  File "C:\Users\asassi\.poetry\lib\poetry\_vendor\py3.6\clikit\console_application.py", line 132, in run
    status_code = command.handle(parsed_args, io)
  File "C:\Users\asassi\.poetry\lib\poetry\_vendor\py3.6\clikit\api\command\command.py", line 119, in handle
    status_code = self._do_handle(args, io)
  File "C:\Users\asassi\.poetry\lib\poetry\_vendor\py3.6\clikit\api\command\command.py", line 167, in _do_handle
    return getattr(handler, handler_method)(args, io, self)
  File "C:\Users\asassi\.poetry\lib\poetry\_vendor\py3.6\cleo\commands\command.py", line 92, in wrap_handle
    return self.handle()
  File "C:\Users\asassi\.poetry\lib\poetry\console\commands\env\use.py", line 24, in handle
    env = manager.activate(self.argument('python'), poetry.file.parent, self._io)
  File "C:\Users\asassi\.poetry\lib\poetry\utils\env.py", line 177, in activate
    raise EnvCommandError(e)

Simply removing the " ".join and shell=True from https://github.com/sdispater/poetry/blob/master/poetry/utils/env.py#L185 and other subprocess calls throughout this file, and possibly in many other files, should fix this issue.

Poetry should always use the argument list form (without shell=True) to call subprocesses, which has no ambiguity or other issues with escaping special characters.

@absassi absassi added the kind/bug Something isn't working as expected label Oct 18, 2019
@finswimmer
Copy link
Member

Hello,

I'm not sure about the removing of shell=True as poetry might need access to the correct environment variables within the shell.

The main cause for the problem is, that the path to the python interpreter should be quoted, e.g. like this:

try:
    python_version = decode(
        subprocess.check_output(
            " ".join(
                [
                    "{}".format(shlex.quote(python)),
                    "-c",
                    "\"import sys; print('.'.join([str(s) for s in sys.version_info[:3]]))\"",
                ]
            ),
            shell=True,
        )
    )
except CalledProcessError as e:
    raise EnvCommandError(e)

@absassi
Copy link
Author

absassi commented Oct 18, 2019

Unfortunately, shlex.quote is for POSIX systems only. The most realiable way to quote arguments for the current system is to pass them as a list to subprocess functions and they will quote them for you in Windows (because CreateProcess takes arguments as a string) or pass as is for execv in POSIX systems.

In CPython, shell=True is just a shortcut for either ["/bin/sh", "-c", args] or '{} /c "{}"'.format(comspec, args) (where comspec is usually 'cmd.exe').

I don't see how shell=True would access different environment variables, as any relevant variables were already inherited by Poetry and will be inherited by the subprocess, and a non-interactive sh should not read any startup file.

@Caligatio
Copy link
Contributor

@finswimmer are your concerns about environment variables targeted at this particular problem or generic? I can't think of any sane circumstance where it would be an issue here.

The only thing I can think of is if the shell launched by Python via shell=True sources a different set of environment variables than what were defined before Python was executed. However, the function in question shouldn't be dependent on any external variables.

@finswimmer
Copy link
Member

Solved by #1774

Copy link

github-actions bot commented Mar 3, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

No branches or pull requests

3 participants