[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-38722: Runpy use io.open_code() #17234

Merged
merged 5 commits into from Nov 18, 2019

Conversation

Copy link
Contributor

jsnklln commented Nov 18, 2019

Jason Killen added 2 commits Nov 15, 2019
This should be working but test.test_tools is failing when
doing the full test.  Switching branches to work on something
else right now.
Copy link
Member

brandtbucher left a comment

Thanks @jsnklln!

Copy link
Member

brandtbucher commented Nov 18, 2019

@zooba

@@ -0,0 +1 @@
:mod:`runpy` now uses :meth:`io.open_code` which will trigger auditing events.
Copy link
Contributor

taleinat Nov 18, 2019

Choose a reason for hiding this comment

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

open() and io.open() also trigger auditing events. And as far as I can tell, this won't change anything WRT auditing; it only allows setting an "open code hook" using PyFile_SetOpenCodeHook. So this text needs to be revised.

Copy link
Contributor Author

jsnklln Nov 18, 2019

Choose a reason for hiding this comment

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

I've updated with a better description of what the change really means/does.

Copy link

bedevere-bot commented Nov 18, 2019

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

taleinat removed the type-security A security issue label Nov 18, 2019
taleinat changed the title bpo-38722: Runpy use io open code bpo-38722: Runpy use io.open_code() Nov 18, 2019
taleinat added the 🤖 automerge PR will be merged once it's been approved and all CI passed label Nov 18, 2019
zooba approved these changes Nov 18, 2019
Copy link
Member

zooba left a comment

Looks good to me, but let's wait on the backport discussion before merging.

Edit: Missed the automerge tag - oh well :) We can still apply the backport tag if we decide to add to 3.8.1.

Copy link
Contributor

miss-islington commented Nov 18, 2019

Thanks @jsnklln for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒🤖

Copy link

bedevere-bot commented Nov 18, 2019

GH-17241 is a backport of this pull request to the 3.8 branch.

Copy link
Member

brandtbucher commented Nov 18, 2019

Thanks @jsnklln!

Copy link
Contributor

taleinat commented Nov 18, 2019

Thanks, @zooba! Since this is apparently a security-related issue, I've backported it.

Copy link
Contributor

miss-islington commented Nov 18, 2019

Thanks @jsnklln for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒🤖

Copy link

bedevere-bot commented Nov 18, 2019

GH-17244 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Nov 18, 2019
https://bugs.python.org/issue38722

Automerge-Triggered-By: @taleinat
(cherry picked from commit e243bae)

Co-authored-by: jsnklln <jsnklln@gmail.com>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 automerge PR will be merged once it's been approved and all CI passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants