Skip to content

gh-128636: Fix crash in PyREPL when os.environ is overwritten with an invalid value for mac - #138089

Merged
ambv merged 16 commits into
python:mainfrom
yihong0618:hy/close_issue_128636_for_mac
Sep 15, 2025
Merged

gh-128636: Fix crash in PyREPL when os.environ is overwritten with an invalid value for mac#138089
ambv merged 16 commits into
python:mainfrom
yihong0618:hy/close_issue_128636_for_mac

Conversation

@yihong0618

@yihong0618yihong0618 commented Aug 23, 2025

Copy link
Copy Markdown
Contributor

for mac if environ is valid and it will retrigger all of it

so I checked the code base in repr the os is from init
so make it in__init__ is better and can avoid

and that is not enough, we also need fix it the same way in

_can_colorize theme

cc @pablogsal

in my env it fixed the issue, and I wonder it maybe a litter faster

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618

Copy link
Copy Markdown
ContributorAuthor

the news is the same so maybe we can ignore it?

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@picnixzpicnixz changed the title [3.13] gh-128636: Fix crash in PyREPL when os.environ is overwritten with an invalid value for mac(GH-128653) gh-128636: Fix crash in PyREPL when os.environ is overwritten with an invalid value for macAug 23, 2025
@yihong0618

Copy link
Copy Markdown
ContributorAuthor

Why only 3.13?

only up to 3.13 new repr has this issue follow this patch title #129186

@picnixz

Copy link
Copy Markdown
Member

the news is the same so maybe we can ignore it?

No, because we have already shipped a few releases since then, so we need a fresh one.

Comment threadLib/_pyrepl/unix_console.py Outdated
@yihong0618

Copy link
Copy Markdown
ContributorAuthor

the news is the same so maybe we can ignore it?

No, because we have already shipped a few releases since then, so we need a fresh one.

learned that, thank you very much

@picnixz

Copy link
Copy Markdown
Member

only up to 3.13 new repr has this issue follow this patch title #129186

That's the backport. All PRs must first be done against main and we backport them when needed. We only use [3.x] when the fix is specific to the version.

@yihong0618

Copy link
Copy Markdown
ContributorAuthor

only up to 3.13 new repr has this issue follow this patch title #129186

That's the backport. All PRs must first be done against main and we backport them when needed. We only use [3.x] when the fix is specific to the version.

copy that, sorry for my wrong understand at first

Signed-off-by: yihong0618 <zouzou0208@gmail.com>

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

I'm not entirely sure but caching THEME() seems to go against the comment in THEME().

Comment threadLib/_pyrepl/reader.py Outdated
Comment threadLib/_pyrepl/unix_console.py
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618

yihong0618 commented Aug 23, 2025

Copy link
Copy Markdown
ContributorAuthor

I'm not entirely sure but caching THEME() seems to go against the comment in THEME().

addressed thank you

and add tests

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
picnixz
picnixz previously requested changes Aug 24, 2025
Comment threadLib/test/test_pyrepl/test_unix_console.py Outdated
Comment threadLib/test/test_pyrepl/test_unix_console.py Outdated
Comment threadLib/test/test_pyrepl/test_unix_console.py Outdated
@bedevere-app

Copy link
Copy Markdown

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.

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@picnixz
picnixz dismissed their stale reviewAugust 25, 2025 12:48

I'm letting Łukasz shepherd this as I'm not a REPL expert

Comment threadLib/_colorize.py Outdated
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618

Copy link
Copy Markdown
ContributorAuthor

fixed thank you for the help

@yihong0618
yihong0618 requested a review from ambvAugust 25, 2025 13:49
Comment threadLib/test/test_pyrepl/test_unix_console.py Outdated
Comment threadLib/test/test_pyrepl/test_unix_console.py Outdated
Comment threadLib/_colorize.py
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
yihong0618 requested a review from hugovkAugust 25, 2025 20:55
@picnixz
picnixz removed their request for review September 6, 2025 14:40
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@ambv
ambv merged commit 8ef7735 into python:mainSep 15, 2025
53 checks passed
@ambvambv added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels Sep 15, 2025
@miss-islington-app

Copy link
Copy Markdown

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

@miss-islington-app

Copy link
Copy Markdown

Thanks @yihong0618 for the PR, and @ambv 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 15, 2025
…ith an invalid value for macOS (pythonGH-138089)
(cherry picked from commit 8ef7735)
Co-authored-by: yihong <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@miss-islington-app

Copy link
Copy Markdown

Sorry, @yihong0618 and @ambv, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8ef7735c536e0ffe4a60224e59b7587288f53e9e 3.13

@bedevere-app

Copy link
Copy Markdown

GH-138938 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appBot removed the needs backport to 3.14 bugs and security fixes label Sep 15, 2025
ambv pushed a commit to ambv/cpython that referenced this pull request Sep 15, 2025
…ith an invalid value for macOS (pythonGH-138089)
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
(cherry picked from commit 8ef7735)
ambv added a commit to ambv/cpython that referenced this pull request Sep 15, 2025
…itten with an invalid value for macOS (pythonGH-138089)
(cherry picked from commit 8ef7735)
Co-authored-by: yihong <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@bedevere-app

Copy link
Copy Markdown

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

@bedevere-appbedevere-appBot removed the needs backport to 3.13 bugs and security fixes label Sep 15, 2025
ambv added a commit that referenced this pull request Sep 15, 2025
…with an invalid value for macOS (GH-138089) (GH-138942)
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
(cherry picked from commit 8ef7735)
hugovk pushed a commit that referenced this pull request Sep 16, 2025
…with an invalid value for macOS (GH-138089) (#138938)
Co-authored-by: yihong <zouzou0208@gmail.com>
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Sign up for freeto 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.

4 participants

@yihong0618@picnixz@ambv@hugovk