gh-109617: fix ncurses incompatibility on macOS with Xcode 15 by sorcio · Pull Request #111258 · python/cpython · GitHub
Skip to content

gh-109617: fix ncurses incompatibility on macOS with Xcode 15 - #111258

Merged
ambv merged 11 commits into
python:mainfrom
sorcio:curses-mac-fix
May 4, 2024
Merged

ambv merged 11 commits into
python:mainfrom
sorcio:curses-mac-fix

Conversation

@sorcio

@sorcio sorcio commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

macOS 14 ships with a new version of ncurses1. But their new SDK tries to support both the old and the new version at build time in a way that CPython's configure script could not correctly detect.

  1. Default to using NCURSES_OPAQUE = 0 which, when using ncurses, avoids the use of some exported functions, such as is_pad2. The macro versions are used instead.
  2. Since the same build can dynamically link against significantly different ncurses implementations, query the version information at runtime instead of hardcoding it.

Question: do we want to limit the change to macOS, or possibly make it conditional to MACOSX_DEPLOYMENT_TARGET < 14.0?

Footnotes

  1. for a long time, macOS has shipped a libncurses version based on ncurses 5.7.20081102. Since macOS 14.0, the version seems to be 6.0.20150808 plus some patches.

  2. see https://invisible-island.net/ncurses/man/curs_opaque.3x.html. Opaque mode would be necessary to use the alternative libncursest (ncurses thread-safer ABI) but Python does not use it.

Comment thread Modules/_cursesmodule.c Outdated
Comment thread Modules/_cursesmodule.c Outdated
Comment thread Modules/_cursesmodule.c Outdated
Comment thread Modules/_cursesmodule.c
Comment thread Include/py_curses.h

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.

Can you explain the version check in the comment?

Comment thread Include/py_curses.h

@serhiy-storchaka serhiy-storchaka 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.

The current code do not use pechochar(), pnoutrefresh(), prefresh() and subpad() if neither HAVE_CURSES_IS_PAD nor WINDOW_HAS_FLAGS are defined. It allows to build with the curses implementation that does not support pads and does not have these function.

This PR makes the above functions required.

Comment thread Modules/_cursesmodule.c
@sorcio

sorcio commented Oct 31, 2023

Copy link
Copy Markdown
Contributor Author

The current code do not use pechochar(), pnoutrefresh(), prefresh() and subpad() if neither HAVE_CURSES_IS_PAD nor WINDOW_HAS_FLAGS are defined. It allows to build with the curses implementation that does not support pads and does not have these function.

This PR makes the above functions required.

Thanks, I missed that. I will revert to the macro version. Do you have a suggestion on how to test for those implementations?

@hugovk

hugovk commented Feb 13, 2024

Copy link
Copy Markdown
Member

Updated from main to trigger the CI and see if this fixes the new #115383.

Comment thread configure.ac Outdated
Comment thread Modules/_cursesmodule.c Outdated
Comment thread Modules/_cursesmodule.c Outdated
@hugovk

hugovk commented Feb 13, 2024

Copy link
Copy Markdown
Member

We got lucky with GitHub Actions runner image allocation.

The free-threaded build ran on the older 20240114.1 with Xcode 14 (which has recently started failing):

And the regular build ran on the newer 20240204.1 with Xcode 15:

And both passed 👍

@sorcio

sorcio commented Feb 13, 2024

Copy link
Copy Markdown
Contributor Author

@ambv
ambv merged commit 08d169f into python:main May 4, 2024
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
…ython#111258)

Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Sign up for free to 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.

7 participants