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

Fix find_python_executable error #331

Merged

Conversation

Edward-Knight
Copy link
Contributor

Fixes #330

I also took the liberty of adding type hints.

Behaviour before:

$ flit install --python pyhton
Python executable None not found
$ flit install --python ls
ls: cannot access 'import sys; print(sys.executable)': No such file or directory
Traceback (most recent call last):
  File "/PATH/bin/flit", line 8, in <module>
    sys.exit(main())
  File "/PATH/lib/python3.7/site-packages/flit/__init__.py", line 168, in main
    python = find_python_executable(args.python)
  File "/PATH/lib/python3.7/site-packages/flit/__init__.py", line 33, in find_python_executable
    universal_newlines=True,
  File "/PATH/lib/python3.7/subprocess.py", line 411, in check_output
    **kwargs).stdout
  File "/PATH/lib/python3.7/subprocess.py", line 512, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['/PATH/ls', '-c', 'import sys; print(sys.executable)']' returned non-zero exit status 2.
$ flit install --python .
Python executable None not found

Behaviour after:

$ flit install --python pyhton
Python executable 'pyhton' not found
$ flit install --python ls
ls: cannot access 'import sys; print(sys.executable)': No such file or directory
CalledProcessError occurred trying to find the absolute filepath of Python executable 'ls'
$ flit install --python .
PermissionError occurred trying to find the absolute filepath of Python executable '.'

@Edward-Knight
Copy link
Contributor Author

I can add tests for those exceptions and error messages if needed :)

Copy link
Member

@takluyver takluyver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Comments inline.

Yes, one or two tests of handling bad cases would be good. It doesn't need to be exhaustive, and I try to avoid testing the precise wording of human-readable error messages.

flit/__init__.py Outdated Show resolved Hide resolved
flit/__init__.py Outdated Show resolved Hide resolved
flit/__init__.py Show resolved Hide resolved
flit/__init__.py Show resolved Hide resolved
@takluyver
Copy link
Member

Thanks - are you still interested in restoring the which() call and adding a test? Or should I pick it up?

@Edward-Knight
Copy link
Contributor Author

Hey, yeah I'll add it in tonight if that's okay? Been a bit busy the past couple of days

@takluyver
Copy link
Member

takluyver commented Apr 1, 2020 via email

@Edward-Knight
Copy link
Contributor Author

Will have to postpone till tomorrow, apologies!

@Edward-Knight
Copy link
Contributor Author

I'll have some time on the weekend to add in the tests hopefully :)

@takluyver takluyver merged commit 28d74f5 into pypa:master Apr 7, 2020
@takluyver
Copy link
Member

Thanks, that looks good.

@takluyver takluyver added this to the 2.3 milestone Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python executable None not found
2 participants