8000 GH-93899: fix checks for eventfd flags by kumaraditya303 · Pull Request #95170 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

GH-93899: fix checks for eventfd flags #95170

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jul 27, 2022
Merged

GH-93899: fix checks for eventfd flags #95170

merged 9 commits into from
Jul 27, 2022

Conversation

kumaraditya303
Copy link
Contributor
@kumaraditya303 kumaraditya303 commented Jul 23, 2022 •
edited by miss-islington
Loading

@kumaraditya303 kumaraditya303 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes type-bug An unexpected behavior, bug, or error labels Jul 23, 2022
erlend-aasland
erlend-aasland requested changes Jul 24, 2022
View reviewed changes
Copy link
Contributor
@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

LGTM, but os_eventfd_impl could also use a wrapper, and some of the tests should skip if these flags are not present.

@bedevere-bot
Copy link

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.

@kumaraditya303
Copy link
Contributor Author
kumaraditya303 commented Jul 24, 2022 •
edited
Loading

but os_eventfd_impl could also use a wrapper,

Added guard to os.eventfd.

and some of the tests should skip if these flags are not present.

The tests are already skipped on older kernel versions.

cpython/Lib/test/test_os.py

Lines 3686 to 3690 in 3c94d33

@unittest.skipUnless(hasattr(os, 'eventfd'), 'requires os.eventfd')
@support.requires_linux_version(2, 6, 30)
class EventfdTests(unittest.TestCase):
def test_eventfd_initval(self):
def pack(value):

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@erlend-aasland: please review the changes made to this pull request.

tiran
tiran requested changes Jul 24, 2022
View reviewed changes
Copy link
Member
@tiran tiran left a comment

Choose a reason for hiding this comment

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

If you actually want to support ancient Kernels, then let's use consistent checks.

@bedevere-bot
Copy link

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.

@kumaraditya303 kumaraditya303 requested a review from tiran July 25, 2022 07:18
erlend-aasland
erlend-aasland approved these changes Jul 25, 2022
View reviewed changes
Copy link
Contributor
@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

LGTM. You may consider to make the NEWS entry a little bit more specific ("older kernel versions" is a bit vague).

@kumaraditya303 kumaraditya303 removed the request for review from tiran July 27, 2022 12:23
@kumaraditya303
Copy link
Contributor Author

LGTM. You may consider to make the NEWS entry a little bit more specific ("older kernel versions" is a bit vague).

Thanks for the review! I have reworded news entry, should be better now!

@erlend-aasland
Copy link
Contributor
erlend-aasland commented Jul 27, 2022 •
edited
Loading

Thanks, I'm fine with that. I'll land it as soon as the CI is green and I get a final approval from @tiran. Thanks!

@miss-islington miss-islington merged commit 4dd099b into python:main Jul 27, 2022
@miss-islington
Copy link
Contributor

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry, @kumaraditya303, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 4dd099bafff14639ef5d2185965016d8f253353f 3.11

@miss-islington
Copy link
Contributor

Sorry @kumaraditya303, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 4dd099bafff14639ef5d2185965016d8f253353f 3.10

@erlend-aasland
Copy link
Contributor

Will you fix the backports, Kumar? It's probably just a clinic regen that's missing. (Can we teach Miss I to do that?)

@kumaraditya303 kumaraditya303 deleted the gh-93899 branch July 27, 2022 18:11
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 27, 2022
@bedevere-bot
Copy link

GH-95342 is a backport of this pull request to the 3.11 branch.

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request Jul 27, 2022
@kumaraditya303
[3.11] pythonGH-93899: fix checks for eventfd flags (pythonGH-95170).
a1a745c 
(cherry picked from commit 4dd099b)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bedevere-bot
Copy link

GH-95345 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 27, 2022
@kumaraditya303
Copy link
Contributor Author

Will you fix the backports, Kumar?

Done

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request Jul 27, 2022
@kumaraditya303
[3.10] pythonGH-93899: fix checks for eventfd flags (pythonGH-95170).
b8c540d 
(cherry picked from commit 4dd099b)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@erlend-aasland erlend-aasland mentioned this pull request Jul 27, 2022
Closed
erlend-aasland pushed a commit that referenced this pull request Jul 28, 2022
@kumaraditya303
[3.11] GH-93899: fix checks for eventfd flags (GH-95170). (#95342)
7813d97 
(cherry picked from commit 4dd099b)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
erlend-aasland pushed a commit that referenced this pull request Jul 28, 2022
@kumaraditya303
[3.10] GH-93899: fix checks for eventfd flags (GH-95170). (#95345)
4ad2229 
(cherry picked from commit 4dd099b)

Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiran tiran tiran approved these changes

@erlend-aasland erlend-aasland erlend-aasland approved these changes

Assignees

@miss-islington miss-islington

Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kumaraditya303 @bedevere-bot @erlend-aasland @miss-islington @tiran
0