Skip to content

bpo-46890: Fix setting of sys._base_executable with framework builds on macOS#31958

Merged
ned-deily merged 4 commits into
python:mainfrom
ronaldoussoren:bpo-469890
Apr 5, 2022
Merged

bpo-46890: Fix setting of sys._base_executable with framework builds on macOS#31958
ned-deily merged 4 commits into
python:mainfrom
ronaldoussoren:bpo-469890

Conversation

@ronaldoussoren

@ronaldoussoren ronaldoussoren commented Mar 17, 2022

Copy link
Copy Markdown
Contributor

The side effect of this bug was that venv environments directly
used the main interpreter instead of the intermediate stub executable,
which can cause problems when a script uses system APIs that
require the use of an application bundle.

https://bugs.python.org/issue46890

…on macOS

The side effect of this bug was that venv environments directly
used the main interpreter instead of the intermediate stub executable,
which can cause problems when a script uses system APIs that
require the use of an application bundle.
@ronaldoussoren ronaldoussoren self-assigned this Mar 17, 2022

@zooba zooba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Couple of suggestions that might make it easier for reading/understanding the code, but nothing major.

Comment thread Modules/getpath.py Outdated
Comment thread Modules/getpath.c Outdated
@ned-deily

Copy link
Copy Markdown
Member

I would like to review and play with the PR a bit but I likely won't be able to before Monday. If you don't hear anything by then, please go ahead. Thanks!

ronaldoussoren and others added 2 commits March 18, 2022 14:25
* Ensure WITH_NEXT_FRAMEWORK is set unconditionally
  while running getpath.py
* Remove duplicate code in the WITH_NEXT_FRAMEWORK
  case.

@ned-deily ned-deily left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After spending some time reviewing and testing it, I am also not 100% confident that this is the best fix since this area is so complex but it does seem to work as advertised and does fix a serious problem with venvs in a framework environment. Since we are nearing the end of the alpha phase of 3.11, it's probably best to get this in now for more exposure. I pushed a fix for a few non-code typos. Since I have been sitting on this and with 3.11.0a7 going to be tagged very soon, I'll merge it now. Thanks, Ronald, and thanks, Steve, for diving into such murky waters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants