X Tutup
Skip to content

gh-123275: Support -Xgil=1 and PYTHON_GIL=1 on non-free-threaded builds#123276

Merged
corona10 merged 5 commits intopython:mainfrom
ZeroIntensity:support-xgil-nonfreethread
Sep 5, 2024
Merged

gh-123275: Support -Xgil=1 and PYTHON_GIL=1 on non-free-threaded builds#123276
corona10 merged 5 commits intopython:mainfrom
ZeroIntensity:support-xgil-nonfreethread

Conversation

@ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Aug 23, 2024 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Aug 23, 2024
Closed
@Eclips4 Eclips4 requested a review from colesbury August 24, 2024 06:01
@ZeroIntensity @Eclips4
Update Misc/NEWS.d/next/Core_and_Builtins/2024-08-23-18-31-10.gh-issu…
f78a7e3 
…e-123275.DprIrj.rst

Co-authored-by: Kirill Podoprigora <kirill.bast9@mail.ru>
godlygeek
godlygeek reviewed Aug 25, 2024
View reviewed changes
@ZeroIntensity @godlygeek
Update cmdline.rst
7d3c769 
Co-authored-by: Matt Wozniski <godlygeek@gmail.com>
colesbury
colesbury approved these changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

I don't have a strong opinion about this one way or the other. This seems fine to me.

corona10
corona10 approved these changes Sep 5, 2024
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@corona10 corona10 added the needs backport to 3.13 bugs and security fixes label Sep 5, 2024
@corona10 corona10 merged commit 84ad264 into python:main Sep 5, 2024
@miss-islington-app
Copy link

miss-islington-app bot commented Sep 5, 2024

Thanks @ZeroIntensity for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 5, 2024
@ZeroIntensity @miss-islington
pythongh-123275: Support -Xgil=1 and PYTHON_GIL=1 on non-free-thr…
efa87cf 
…eaded builds (pythongh-123276)

(cherry picked from commit 84ad264)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 5, 2024

GH-123753 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 5, 2024
@ZeroIntensity ZeroIntensity deleted the support-xgil-nonfreethread branch September 5, 2024 23:54
@corona10
Copy link
Member

corona10 commented Sep 6, 2024

@ZeroIntensity I think that we forgot to update test code.

cpython/Lib/test/test_cmd_line.py

Lines 885 to 914 in b5aa271

def test_python_gil(self):
cases = [
# (env, opt, expected, msg)
(None, None, 'None', "no options set"),
('0', None, '0', "PYTHON_GIL=0"),
('1', None, '1', "PYTHON_GIL=1"),
('1', '0', '0', "-X gil=0 overrides PYTHON_GIL=1"),
(None, '0', '0', "-X gil=0"),
(None, '1', '1', "-X gil=1"),
]
code = "import sys; print(sys.flags.gil)"
environ = dict(os.environ)
for env, opt, expected, msg in cases:
with self.subTest(msg, env=env, opt=opt):
environ.pop('PYTHON_GIL', None)
if env is not None:
environ['PYTHON_GIL'] = env
extra_args = []
if opt is not None:
extra_args = ['-X', f'gil={opt}']
proc = subprocess.run([sys.executable, *extra_args, '-c', code],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
text=True, env=environ)
self.assertEqual(proc.returncode, 0, proc)
self.assertEqual(proc.stdout.rstrip(), expected)
self.assertEqual(proc.stderr, '')

Can you update it too?

@corona10 corona10 mentioned this pull request Sep 6, 2024
Merged
@ZeroIntensity
Copy link
Member Author

ZeroIntensity commented Sep 6, 2024

Sure, I'll submit a hotfix. Sorry about that!

@corona10
Copy link
Member

corona10 commented Sep 6, 2024

Sure, I'll submit a hotfix. Sorry about that!

No problem, it was my fault since I approved and merged this PR without checking test code.
Thanks for the work!

@ZeroIntensity
Copy link
Member Author

ZeroIntensity commented Sep 6, 2024

FWIW, that test doesn't actually run on default builds. I'll just add a new test next to it to make sure that it works.

@corona10
Copy link
Member

corona10 commented Sep 6, 2024

FWIW, that test doesn't actually run on default builds. I'll just add a new test next to it to make sure that it works.

Yeah, I know, but we should add the test when we want to support this feature at the default build, that was what I wanted to mean :)

corona10 pushed a commit that referenced this pull request Sep 6, 2024
@miss-islington @ZeroIntensity
[3.13] gh-123275: Support -Xgil=1 and PYTHON_GIL=1 on non-free-th…
92b9c44 
…readed builds (gh-123276) (gh-123753)

gh-123275: Support `-Xgil=1` and `PYTHON_GIL=1` on non-free-threaded builds (gh-123276)
(cherry picked from commit 84ad264)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4 Eclips4 Eclips4 left review comments

@colesbury colesbury colesbury approved these changes

@corona10 corona10 corona10 approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

+1 more reviewer

@godlygeek godlygeek godlygeek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ZeroIntensity @corona10 @godlygeek @colesbury @Eclips4 @hugovk
X Tutup