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

bpo-34725: Adds _Py_SetProgramFullPath so embedders may override sys.executable #9860

Merged
merged 5 commits into from
Nov 18, 2018

Conversation

zooba
Copy link
Member

@zooba zooba commented Oct 13, 2018

I also made prefix and exec_prefix consistent between platforms, so there is less optional stuff going on in the configuration.

https://bugs.python.org/issue34725

@zooba
Copy link
Member Author

zooba commented Oct 13, 2018

I have a backport of this for 3.7 that adds the API as a private one, and does not include the exec_prefix change.

@zooba zooba requested a review from vstinner October 14, 2018 00:31
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Please don't add yet another Py_SetXXX() to configuration Python before it's initialization. There is an on-going project, the PEP 432 of Nick Coghlan, to add a consistent and unique way to configure Python. Until this PEP is fully implemented, please only add private methods: rename it to _Py_SetProgramFullPath().

Why no calling it directly Py_SetExecutable() and set core_config->executable? Sorry, I only read quickly the bug and the PR.

@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@zooba
Copy link
Member Author

zooba commented Oct 22, 2018

I have made the requested changes; please review again

@vstinner I added it the way I did because that's the only way I could make it work without the value being overwritten. If you can see a better way to make it happen, please show me, but when I tried your suggestion it didn't end up as neat as this way.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@vstinner: please review the changes made to this pull request.

@zooba zooba changed the title bpo-34725: Adds Py_SetProgramFullPath so embedders may override sys.executable bpo-34725: Adds _Py_SetProgramFullPath so embedders may override sys.executable Nov 17, 2018
@zooba
Copy link
Member Author

zooba commented Nov 17, 2018

Given the lack of additional feedback, I'll run CI one more time against the latest master and then merge (in a day or so, once I'm done with international travel…).

@zooba zooba merged commit 177a41a into python:master Nov 18, 2018
@zooba zooba deleted the bpo-34725 branch November 18, 2018 04:41
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.

4 participants