Skip to content

bpo-35321: Set the spec origin to frozen in frozen modules#11732

Merged
warsaw merged 2 commits into
python:masterfrom
nnja:fix_35321_frozen_spec
Feb 5, 2019
Merged

bpo-35321: Set the spec origin to frozen in frozen modules#11732
warsaw merged 2 commits into
python:masterfrom
nnja:fix_35321_frozen_spec

Conversation

@nnja

@nnja nnja commented Feb 2, 2019

Copy link
Copy Markdown
Contributor

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

@vstinner

vstinner commented Feb 2, 2019

Copy link
Copy Markdown
Member

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 nnja force-pushed the fix_35321_frozen_spec branch from 172f816 to eb3dac6 Compare February 2, 2019 20:13
@nnja 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
@nnja

nnja commented Feb 2, 2019

Copy link
Copy Markdown
Contributor Author

@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.

@geryogam

geryogam commented Feb 2, 2019

Copy link
Copy Markdown
Contributor

Thank you @nnja for this PR!

@warsaw warsaw 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.

Thanks for the PR @nnja - LGTM.

@warsaw

warsaw commented Feb 5, 2019

Copy link
Copy Markdown
Member

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

@warsaw warsaw merged commit 69091cb into python:master Feb 5, 2019
@bedevere-bot

Copy link
Copy Markdown

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

@miss-islington

Copy link
Copy Markdown
Contributor

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

@miss-islington

Copy link
Copy Markdown
Contributor

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

@miss-islington

Copy link
Copy Markdown
Contributor

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

@ned-deily

Copy link
Copy Markdown
Member

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!

@warsaw

warsaw commented Feb 5, 2019

Copy link
Copy Markdown
Member

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

@nnja

nnja commented Feb 6, 2019

Copy link
Copy Markdown
Contributor Author

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?

@warsaw

warsaw commented Feb 6, 2019

Copy link
Copy Markdown
Member

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?

@Mariatta

Mariatta commented Feb 6, 2019

Copy link
Copy Markdown
Member

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.

@nnja

nnja commented Feb 7, 2019

Copy link
Copy Markdown
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants