gh-148240: skip importing readline in check_extension_modules.py on macOS#148279
gh-148240: skip importing readline in check_extension_modules.py on macOS#148279MazinSharaf wants to merge 3 commits intopython:mainfrom
readline in check_extension_modules.py on macOS#148279Conversation
Add temporary fix to skip readline module on macOS.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 684734ca54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if sys.platform == "darwin" and modinfo.name == "readline": | ||
| logger.error("Skipping readline module for macOS") | ||
| return |
There was a problem hiding this comment.
Keep strict builds from silently passing skipped readline
Returning early for readline on macOS makes check_module_import() report success without ever executing bootstrap_load(spec). In ModuleChecker.check(), any non-exception path is treated as a successful import and added to shared_ok, so --strict/PYTHONSTRICTEXTENSIONBUILD can no longer detect a broken readline module on macOS. This undermines the strict-build guarantee and can let a bad extension ship undetected.
Useful? React with 👍 / 👎.
Raise ImportError for readline module on macOS instead of logging an error.
readline in check_extension_modules.py on macOS
readline in check_extension_modules.py on macOSreadline in check_extension_modules.py on macOS
|
I don't think this is the correct fix. The CI won't be happy otherwise and we are missing checks. I'm closing it for now and we should discuss on the issue more before opening a fix (and please, don't open PRs unless consensus is reached) |
Readline Fix
check_extension_modules.pyhangs build due to SIGTTIN #148240A fix for hanging when importing
readlinemodule on macOS by checking ifreadlinemodule is being imported on macOS and skips the import if it is to avoid hanging.