-
Notifications
You must be signed in to change notification settings - Fork 137
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
Make Python command absolute before calling subprocess #300
Make Python command absolute before calling subprocess #300
Conversation
This works around bpo-38905 with suggestion from Steve Dower. https://bugs.python.org/issue38905
That's rather frustrating. I've worked on a bunch of code that relies on If we're using |
I was just about to post a grumpy message to that Python issue, but on reading it more carefully, I think I see it's not |
flit/__init__.py
Outdated
@@ -22,6 +23,7 @@ def find_python_executable(python): | |||
return sys.executable | |||
if os.path.isabs(python): # sys.executable is absolute too | |||
return python | |||
python = shutil.which(python) or os.path.abspath(python) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the abspath
call? I think which()
already handles relative paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not aware of this, thanks! TIL 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What should we fall back to if the which
call fails though? (It returns None
if there’s no matching executable found.) Just the unresolved python
? Because otherwise subprocess.check_call
would fail with a cryptic TypeError
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably throw an error, I'd guess - that means you've given it a name which isn't on PATH, or a path which doesn't exist or isn't executable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added an exception for this.
Thanks, this LGTM. |
bpo-38905
On Python 3.7.2 or later,
sys.executable
returns the wrong executable inside venv on Windows unless you invoke the interpreter with an absolute path. It seems that the behaviour is expected and unlikely to be fixed (at least not any time soon). Core devs seem to approve theshutil.which()
workaround.