[proxy] web.archive.org← back | site home | direct (HTTPS) ↗ | proxy home | ◑ dark◐ light
/ cpython Public
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-35321: Set the spec origin to frozen in frozen modules #11732

Merged
merged 2 commits into from Feb 5, 2019

Conversation

Copy link
Contributor

nnja commented Feb 2, 2019

This fix correctly sets the spec origin to
"frozen" for the _frozen_importlib module. Note that the
origin was already correctly set in _frozen_importlib_external.

https://bugs.python.org/issue35321

Copy link
Member

vstinner commented Feb 2, 2019

Are you sure that this PR is related to https://bugs.python.org/issue35537?

This fix correctly sets the spec origin to
"frozen" for the _frozen_importlib module. Note that the
origin was already correctly set in _frozen_importlib_external.
nnja force-pushed the fix_35321_frozen_spec branch from 172f816 to eb3dac6 Compare Feb 2, 2019
nnja changed the title bpo-35537: Set the spec origin to frozen in frozen modules bpo-35321: Set the spec origin to frozen in frozen modules Feb 2, 2019
Copy link
Contributor Author

nnja commented Feb 2, 2019

@vstinner drat, copied and pasted the wrong issue number. It's actually https://bugs.python.org/issue35321. I updated the commit message, GitHub issue title & message.

Copy link
Contributor

maggyero commented Feb 2, 2019

Thank you @nnja for this PR!

warsaw approved these changes Feb 5, 2019
Copy link
Member

warsaw left a comment

Thanks for the PR @nnja - LGTM.

Copy link
Member

warsaw commented Feb 5, 2019

I added the backport tags for 3.7 and 3.6, but I'd like to get @ned-deily 's take on that.

warsaw merged commit 69091cb into python:master Feb 5, 2019
Copy link

bedevere-bot commented Feb 5, 2019

@warsaw: Please replace # with GH- in the commit message next time. Thanks!

Copy link
Contributor

miss-islington commented Feb 5, 2019

Thanks @nnja for the PR, and @warsaw for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒🤖

Copy link
Contributor

miss-islington commented Feb 5, 2019

Sorry, @nnja and @warsaw, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 69091cb497b2f0fe7e2789b30b43cf78caf9de9b 3.7

Copy link
Contributor

miss-islington commented Feb 5, 2019

Sorry, @nnja and @warsaw, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 69091cb497b2f0fe7e2789b30b43cf78caf9de9b 3.6

Copy link
Member

ned-deily commented Feb 5, 2019

AFAICT this is not a 3.6 regression: the behavior has been around for a long time. Even though it is minor and the risk of breakage is small, the benefit seems to be also small. My take is that this doesn't meet the criteria for a security-fix-only branch so it should not go into 3.6 or earlier. Sorry!

Copy link
Member

warsaw commented Feb 5, 2019

Thank you @ned-deily - we'll just back port this to 3.7.

Copy link
Contributor Author

nnja commented Feb 6, 2019

I have a question about how to backport this into 3.7. (maybe @warsaw or @ned-deily can help?)

I used the cherry_picker tool to try to apply this commit to 3.7. When I looked at the merge conflict
I realized it's in Python/importlib.h which is the generated header file for this frozen module.

As expected, this file has irresolvable conflicts.

What's the process for this type of backport?

Copy link
Member

warsaw commented Feb 6, 2019

I'm not sure this is the right way to do it (does the devguide provide any help?). What I generally do is to use the tool to do the merge in the older branch, then revert Python/importlib.h, then make regen-importlib to get a proper importlib.h. Then that's the branch I propose for merging.

I wonder if @Mariatta has any thoughts on adding this to @miss-islington by default?

Copy link
Sponsor Member

Mariatta commented Feb 6, 2019

you can resolve the conflict manually (regenerate the files) and the do cherry_picker --continue.

Currently miss-islington can't do this automatically. There is open ticket here: python/miss-islington#41.

Copy link
Contributor Author

nnja commented Feb 7, 2019

I'll try this approach and follow up if I run into any issues. Thank you @Mariatta and @warsaw!

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

Successfully merging this pull request may close these issues.

None yet

9 participants