GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-109617: fix ncurses incompatibility on macOS with Xcode 15 by sorcio · Pull Request #111258 · python/cpython

Conversation

sorcio commented Oct 24, 2023 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

macOS 14 ships with a new version of ncurses¹. 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.

Default to using NCURSES_OPAQUE = 0 which, when using ncurses, avoids the use of some exported functions, such as is_pad². The macro versions are used instead.
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?

Issue: Refresh Screen Provided By curses.wrapper Causes Seg Fault (macOS, xcode 15 Apple supplied ncurses 6.0 breakage) #109617

Footnotes

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. ↩
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. ↩

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

sorcio added 4 commits

October 11, 2023 18:53


          use is_pad macro, not function, if available

09c1b6a


          get ncurses_version at runtime from curses_version()

bb2c852


          Merge branch 'main' into curses-mac-fix

137245d


          add news entry

73290a4

sorcio requested review from corona10 and erlend-aasland as code owners

October 24, 2023 11:05

bedevere-app Bot mentioned this pull request

Oct 24, 2023

Refresh Screen Provided By curses.wrapper Causes Seg Fault (macOS, xcode 15 Apple supplied ncurses 6.0 breakage) #109617

Closed

bedevere-app Bot added the awaiting review label

Oct 24, 2023

corona10 requested a review from a team

October 25, 2023 02:21

serhiy-storchaka self-requested a review

October 25, 2023 08:29

vstinner reviewed

Oct 27, 2023

View reviewed changes

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Include/py_curses.h

		@@ -39,7 +39,8 @@
		#ifdef HAVE_NCURSES_H
		/* configure was checking <curses.h>, but we will
		use <ncurses.h>, which has some or all these features. */

vstinner Oct 27, 2023

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sorcio reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

Include/py_curses.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka reviewed

Oct 31, 2023

View reviewed changes

serhiy-storchaka left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sorcio mentioned this pull request

Nov 9, 2023

Restore ncurses widechar support on macOS #111460

Closed

sobolevn mentioned this pull request

Feb 13, 2024

Undeclared function 'is_pad' on macos-13 GitHub runners #115383

Closed


          Merge branch 'main' into curses-mac-fix

71a6504

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reviewed

Feb 13, 2024

View reviewed changes

Comment thread

configure.ac Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_cursesmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

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):

https://github.com/python/cpython/actions/runs/7883963392/job/21511964656?pr=111258

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

https://github.com/python/cpython/actions/runs/7883963392/job/21511964137?pr=111258

And both passed 👍

erlend-aasland reacted with rocket emoji

All reactions

🚀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sorcio added 5 commits

February 13, 2024 11:27


          Merge branch 'main' into curses-mac-fix

c1f2776


          revert py_is_pad to a macro

d925552


          explain version check

d573579


          restrict NCURSES_OPAQUE check to Apple platforms

7c94a46


          Merge remote-tracking branch 'sorcio/curses-mac-fix' into curses-mac-fix

46b25f1

sorcio commented Feb 13, 2024

Copy link

Copy Markdown

Contributor Author

I updated the patch according to feedback:

reverted to the py_is_pad definition as a macro, so that it can build with curses implementation that don't have pechochar(), pnoutrefresh(), prefresh() or subpad()
added some comments to clarify context

I left in the code that parses ncurses version. It addresses a problem that only arises once this fix is available (that is, static version could report 6 but the interpreter could be loading 5 if running on older macOS). I can remove if this feels too controversial, or split to a new issue.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ned-deily mentioned this pull request

Feb 13, 2024

gh-115383: Install ncurses in reusable macos workflow #115389

Closed


          Merge branch 'main' into curses-mac-fix

8ffd117

ambv merged commit 08d169f into python:main

May 4, 2024

bedevere-app Bot removed the awaiting review label

May 4, 2024

SonicField pushed a commit to SonicField/cpython that referenced this pull request

May 8, 2024


          pythongh-109617: fix ncurses incompatibility on macOS with Xcode 15 (p…

027cd28

…ython#111258)

Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>

sorcio deleted the curses-mac-fix branch

May 12, 2024 14:45

ned-deily mentioned this pull request

Apr 7, 2025

curses "insstr" does not handle newline properly on Mac OS #107267

Closed

Reviewers

vstinner

vstinner left review comments

serhiy-storchaka

serhiy-storchaka left review comments

erlend-aasland

erlend-aasland left review comments

corona10

Awaiting requested review from corona10

corona10 is a code owner

+1 more reviewer

ma8ma

ma8ma left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

sorcio commented Oct 24, 2023 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner Oct 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sorcio commented Oct 31, 2023

Uh oh!

hugovk commented Feb 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hugovk commented Feb 13, 2024

Uh oh!

sorcio commented Feb 13, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

sorcio commented Oct 24, 2023 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.