Skip to content
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

[3.10] gh-97032: avoid test_squeezer crash on macOS buildbots #115508

Merged
merged 1 commit into from
Feb 19, 2024

Conversation

ned-deily
Copy link
Member

@ned-deily ned-deily commented Feb 15, 2024

@bedevere-app bedevere-app bot added the tests Tests in the Lib/test dir label Feb 15, 2024
@ned-deily ned-deily changed the title avoid test_squeezer crash on macOS buildbots [3.10] avoid test_squeezer crash on macOS buildbots Feb 15, 2024
@ned-deily
Copy link
Member Author

!buildbot x86-64 macOS PR

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ned-deily for commit 08ecd09 🤖

The command will test the builders whose names match following regular expression: x86-64 macOS PR

The builders matched are:

  • x86-64 macOS PR

@ned-deily
Copy link
Member Author

!buildbot ARM64 MacOS

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @ned-deily for commit 08ecd09 🤖

The command will test the builders whose names match following regular expression: ARM64 MacOS

The builders matched are:

  • ARM64 MacOS M1 Refleaks NoGIL PR
  • ARM64 macOS PR
  • ARM64 MacOS M1 NoGIL PR

@ned-deily
Copy link
Member Author

@encukou @ambv

@ned-deily ned-deily deleted the branch python:3.10 February 15, 2024 10:49
@ned-deily ned-deily closed this Feb 15, 2024
@ned-deily ned-deily deleted the 3.10 branch February 15, 2024 10:49
@ned-deily ned-deily restored the 3.10 branch February 15, 2024 10:57
@ned-deily ned-deily reopened this Feb 15, 2024
@encukou
Copy link
Member

encukou commented Feb 15, 2024

!buildbot ARM64 MacOS

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 08ecd09 🤖

The command will test the builders whose names match following regular expression: ARM64 MacOS

The builders matched are:

  • ARM64 MacOS M1 Refleaks NoGIL PR
  • ARM64 macOS PR
  • ARM64 MacOS M1 NoGIL PR

@encukou
Copy link
Member

encukou commented Feb 15, 2024

PR buildbots for old branches aren't particularly stable, so it's better to look at the issues and ignore unrelated ones.
There's no tkinter-related crash here, so I'd count them as successful.

@pablogsal, IMO this is the best way to fix the 3.10 & 3.9 buildbot failures. There's a more robust fix in later branches.
The other approaches considered are:

  • change library detection logic, which isn't ideal in source-only security releases
  • configuring the buildbot to use Homebrew tkinter -- too much work for a library that isn't really tested anyway

@encukou encukou marked this pull request as ready for review February 15, 2024 13:46
@encukou encukou requested a review from terryjreedy as a code owner February 15, 2024 13:46
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

Running IDLE on Mac with Cocoa and Apple tkinter is not supported. When I user does so, IDLE warns about stability and points to the web page about installing better tkinter.

The crash makes no sense to me; do whatever whoever thinks best.

@encukou encukou changed the title [3.10] avoid test_squeezer crash on macOS buildbots [3.10] gh-97032: avoid test_squeezer crash on macOS buildbots Feb 16, 2024
@encukou
Copy link
Member

encukou commented Feb 16, 2024

The test failure is SIGABRT that, AFAIK, comes from inside tkinter.

macOS 12 (1207) or later required, have instead 12 (1206) !
Fatal Python error: Aborted

That is, the tkinter that comes with the system now crashes the process at startup. AFAIK, this started happening after a system upgrade.
The right thing Python could do about this is not select this tkinter in ./configure. Later branches do this; for 3.9-3.10, changing the logic would IMO be too invasive.
Hence this PR: avoid starting tkinter at all if we're not testing GUI.

@pablogsal
Copy link
Member

While I understand the fix and the rationale, I have to say that I still find it a bit "hacky" (for the lack of a better term). Given that this is to fix CI in old branches I am fine merging it as it has no user impact, but ideally we should fix this in the build system in main if we detect that we are using an incompatible tkinter .

I any case, I am merging this as I think the proper fix will be more aggressive and we need to have the CI green to avoid hiding other problems. Thanks @encukou and @ned-deily for the PR and the explanation.

@pablogsal pablogsal merged commit 17a6533 into python:3.10 Feb 19, 2024
22 of 25 checks passed
@miss-islington-app
Copy link

Thanks @ned-deily for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2024
…ythonGH-115508)

avoid test_squeezer crash on macOS buildbots
(cherry picked from commit 17a6533)

Co-authored-by: Ned Deily <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 19, 2024

GH-115655 is a backport of this pull request to the 3.9 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.9 only security fixes label Feb 19, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 19, 2024
…ythonGH-115508)

avoid test_squeezer crash on macOS buildbots
(cherry picked from commit 17a6533)

Co-authored-by: Ned Deily <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Feb 19, 2024

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

@encukou
Copy link
Member

encukou commented Feb 19, 2024

Thanks!
I agree it's hacky, but I don't see a way toward a better fix than what's in main now -- using Tk that the user has control over, and making them responsible for providing something that doesn't crash.
I don't think we want e.g. to try creating GUI windows in configure/make to see if Tk crashes.

@bedevere-app
Copy link

bedevere-app bot commented Feb 21, 2024

GH-115655 is a backport of this pull request to the 3.9 branch.

ambv pushed a commit that referenced this pull request Feb 21, 2024
ambv pushed a commit that referenced this pull request Feb 21, 2024
@ned-deily ned-deily deleted the 3.10 branch February 26, 2024 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-mac skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants