classification
Title: Fix curses module compilation with ncurses6
Type: compile error Stage: resolved
Components: Extension Modules Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: 31891 Superseder:
Assigned To: Nosy List: donmez, mark.dickinson, masamoto, serhiy.storchaka, twouters, yan12125
Priority: normal Keywords: patch

Created on 2015-11-24 11:39 by donmez, last changed 2017-11-01 12:38 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
curses-ncurses6.patch donmez, 2015-11-24 11:39 review
curses-is_pad.patch masamoto, 2016-10-07 13:36 review
join-test-issue28190-issue25720.patch masamoto, 2016-10-09 13:35 for testing review
Pull Requests
URL Status Linked Edit
PR 1689 closed masamoto, 2017-05-21 03:32
PR 4164 merged masamoto, 2017-10-29 09:03
PR 4212 merged python-dev, 2017-11-01 12:06
PR 4213 merged python-dev, 2017-11-01 12:07
Messages (27)
msg255261 - (view) Author: Ismail Donmez (donmez) * Date: 2015-11-24 11:39
ncurses6 turned on NCURSES_OPAQUE, so now you have to use some helper functions instead of accessing the structs directly. This _should_ be compatible with ncurses5 though I didn't test it.

Original patch is from openSUSE.
msg258136 - (view) Author: Ismail Donmez (donmez) * Date: 2016-01-13 09:26
Any patch review/comment ?
msg258137 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-01-13 09:53
configure.ac directly uses w->_flags in a check. It looks that this check always fails with ncurses6.
msg258142 - (view) Author: Ismail Donmez (donmez) * Date: 2016-01-13 13:53
Thats not an issue for ncurses because Include/py_curses.h does:

  #ifdef HAVE_NCURSES_H
  /* configure was checking <curses.h>, but we will
     use <ncurses.h>, which has all these features. */
  #ifndef WINDOW_HAS_FLAGS
  #define WINDOW_HAS_FLAGS 1
  #endif
  #ifndef MVWDELCH_IS_EXPRESSION
  #define MVWDELCH_IS_EXPRESSION 1
  #endif
  #endif

So it overrides WINDOW_HAS_FLAGS for ncurses case.
msg261920 - (view) Author: Ismail Donmez (donmez) * Date: 2016-03-17 17:18
ping?
msg261923 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-03-17 18:00
I suspect that the patch can break build with non-ncurses implementations or with old ncurses (when is_pad() was added?). Needed more direct feature check.
msg278050 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-10-04 15:32
is_pad is added in ncurses 5.7-20090906 [1]. At least Mac OS X still ships ancient ncurses 5.7-20081102 [2], so an check in configure.ac is necessary. I'm trying it out.

[1] http://invisible-island.net/ncurses/NEWS.html#t20090906
[2] http://opensource.apple.com//source/ncurses/ncurses-46/ncurses/NEWS
msg278054 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-04 15:55
Mark, do you have relation to this?
msg278128 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2016-10-05 14:51
I tried to build curses module on Cygwin (Vista x86) using #25720 patch. And it has been succeeded.
When test_curses ran without skip condition, it was same result as msg278060 (#28190).

I found out build success reasons for cases of applying patch:
#25720 -- implementation of WINDOW is opaque (*but* WINDOW_HAS_FLAGS is defined at Include/py_curses.h:61 ). However, curses module build went well to cover the _flags field from source code by is_pad.
#14598 -- implementation of WINDOW is not opaque (WINDOWS_HAS_FLAGS is defined at configure script). Therefore, curses module build went well because WINDOW has the _flags field.
#28190 -- implementation of WINDOW is opaque (WINDOW_HAS_FLAGS isn't defined: py_curses.h has been cleaned by patch). Hence, curses module build went well to remove the _flags field from source code at preprocessing.

All case tests on Cygwin have failed at unget_wch.
msg278140 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2016-10-05 17:18
> Mark, do you have relation to this?

Sorry, no. Whatever ncurses knowledge I may once have had has long since vanished.
msg278241 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2016-10-07 13:36
I updated the patch that add configuration check for is_pad. the is_pad is wrapped into py_is_pad at Modules/_cursesmodule.c:932 by either of three ways.

Case one -- is_pad is found: Define the macro that is simple wrapping.
Case two -- is_pad is not found, however WINDOW has _flags field: Define the macro using _flags field.
Case three -- is_pad is not found, and WINDOW doesn't have _flags field: Define the macro that is always preprocessed to FALSE.

I succeeded to build curses module on Cygwin (Vista x86) using this patch.
This patch doesn't include that undo fixes for specific platforms.
msg278242 - (view) Author: Ismail Donmez (donmez) * Date: 2016-10-07 13:39
@masamoto thank you!
msg278245 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-07 14:29
Added a comment on Rietveld.
msg278248 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-10-07 15:24
Thanks masamoto! There's another minor issue about this patch: if there's no curses.h (ncurses built with --without-curses-h), the detection always fails.
msg278303 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2016-10-08 14:16
Added comment in review.

Yen,  I tried to build without curses.h file (overwrite to ncurses.h). It was failed on checking header. Hence, I confirmed curses headers on Cygwin and Ubuntu.

I found out that curses.h includes the directive "#include <unctrl.h>". And unctrl.h has "#include <curses.h>". Therefore, I think that some platforms require curses.h.
Would you confirm your platform curses library?
msg278353 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-10-09 09:50
headers.sh in ncurses changes "#include <curses.h>" to the actual path. For example here's a line in my unctrl.h:

#include <ncursesw/ncurses.h>
msg278360 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2016-10-09 13:35
Thank you for confirming, Yen :)
In this case, It seems necessary that resolves headers. I think missing headers issue maybe solve by #28190. I wrote a join test patch for #28190 and #25720.
Would you be able to resolve headers using this?
msg278363 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-10-09 15:40
Thanks, building is fine here.

By the way, testing is broken due to other bugs (/tmp not available on Android). It's unrelated and I'll open a new issue for that.
msg287757 - (view) Author: Ismail Donmez (donmez) * Date: 2017-02-14 10:09
What's the status on this? Can you please create a pull request on Github so we can continue there?
msg294071 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2017-05-21 04:05
Hi, I finished various things and tackle the issue again, I opened PR 1689 at last.

Changes from previous patch:
* If ncurses doesn't have both is_pad function and _flags field of WINDOW, NCURSES_OPAQUE is defined as zero to make WINDOW to non-opaque type before including ncurses.h.
* The conditional compile on function definition are kept, it replaces WINDOW_HAS_FLAGS with py_is_pad macro itself.
* Unindent the blocks that places after section of the conditional compile on function definition.
msg296750 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2017-06-24 02:41
Ping. I updated PR a bit: macOS is joined to new compile condition and remove platform-specific condition.
msg305178 - (view) Author: Masayuki Yamamoto (masamoto) * Date: 2017-10-29 09:09
I opened PR 4164 to improve the is_pad configure check and previous PR was closed.
msg305181 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-29 09:39
I'll try to test this on NetBSD after fixing curses on NetBSD. It uses a different implementation of curses which don't support is_pad.
msg305370 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-01 12:05
New changeset 8bc7d63560024681dce9f40445f2877b2987e92c by Serhiy Storchaka (Masayuki Yamamoto) in branch 'master':
bpo-25720: Fix the method for checking pad state of curses WINDOW (#4164)
https://github.com/python/cpython/commit/8bc7d63560024681dce9f40445f2877b2987e92c
msg305372 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-01 12:35
New changeset ff6ae4de3874f4922a5883f08bb661c93834b060 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) (#4212)
https://github.com/python/cpython/commit/ff6ae4de3874f4922a5883f08bb661c93834b060
msg305373 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-01 12:36
New changeset 6ba0b583d6785a256b17d27431908d67015eeeb6 by Serhiy Storchaka (Miss Islington (bot)) in branch '2.7':
bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) (#4213)
https://github.com/python/cpython/commit/6ba0b583d6785a256b17d27431908d67015eeeb6
msg305374 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-01 12:38
Thank you for your contribution Masayuki!
History
Date User Action Args
2017-11-04 08:56:20serhiy.storchakalinkissue14598 superseder
2017-11-01 12:38:50serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg305374

stage: patch review -> resolved
2017-11-01 12:36:50serhiy.storchakasetmessages: + msg305373
2017-11-01 12:35:43serhiy.storchakasetmessages: + msg305372
2017-11-01 12:07:34python-devsetpull_requests: + pull_request4182
2017-11-01 12:06:34python-devsetpull_requests: + pull_request4181
2017-11-01 12:05:28serhiy.storchakasetmessages: + msg305370
2017-10-29 09:39:47serhiy.storchakasetdependencies: + Make curses compiling on NetBSD 7.1 and tests passing
messages: + msg305181
versions: + Python 2.7, Python 3.6, - Python 3.5
2017-10-29 09:09:34masamotosetmessages: + msg305178
2017-10-29 09:03:13masamotosetpull_requests: + pull_request4133
2017-06-24 02:41:42masamotosetmessages: + msg296750
2017-05-21 04:05:36masamotosetmessages: + msg294071
2017-05-21 03:32:39masamotosetpull_requests: + pull_request1782
2017-02-14 10:09:11donmezsetmessages: + msg287757
2017-01-06 02:17:02berker.peksaglinkissue29170 superseder
2016-11-25 18:15:42berker.peksaglinkissue28802 superseder
2016-10-09 15:40:17yan12125setmessages: + msg278363
2016-10-09 13:35:17masamotosetfiles: + join-test-issue28190-issue25720.patch

messages: + msg278360
2016-10-09 09:50:09yan12125setmessages: + msg278353
2016-10-08 14:16:29masamotosetmessages: + msg278303
2016-10-07 15:24:11yan12125setmessages: + msg278248
2016-10-07 14:29:09serhiy.storchakasetmessages: + msg278245
2016-10-07 13:39:46donmezsetmessages: + msg278242
2016-10-07 13:36:19masamotosetfiles: + curses-is_pad.patch

messages: + msg278241
versions: + Python 3.7
2016-10-05 17:18:01mark.dickinsonsetmessages: + msg278140
2016-10-05 14:51:34masamotosetnosy: + masamoto
messages: + msg278128
2016-10-04 15:55:42serhiy.storchakasetnosy: + mark.dickinson
messages: + msg278054
2016-10-04 15:32:50yan12125setnosy: + yan12125
messages: + msg278050
2016-03-17 18:00:50serhiy.storchakasetmessages: + msg261923
2016-03-17 17:18:06donmezsetmessages: + msg261920
2016-01-13 13:53:39donmezsetmessages: + msg258142
2016-01-13 09:53:13serhiy.storchakasetmessages: + msg258137
2016-01-13 09:26:18donmezsetmessages: + msg258136
2015-11-24 14:00:25serhiy.storchakasetnosy: + twouters, serhiy.storchaka

type: compile error
stage: patch review
2015-11-24 11:39:07donmezcreate