You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Cross-build _curses failed if host ncurses headers and target ncurses headers have different layouts #72377
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee=Noneclosed_at=Nonecreated_at=<Date2016-09-17.17:20:08.116>labels= ['build', '3.7']
title='Cross-build _curses failed if host ncurses headers and target ncurses headers have different layouts'updated_at=<Date2019-12-10.08:03:17.734>user='https://github.com/yan12125'
In changeset 919259054621 (from bpo-12567) and 707761d59a4a (from bpo-15268), /usr/include/ncursesw was added to include paths in setup.py and configure.ac. This is wrong and breaks cross-compiling. For example, if host has /usr/include/ncursesw/ncurses.h and target has $SYSROOT/include/ncurses.h, the build fails. An example can be found in [1].
My patch removes all references to /usr/include/ncursesw and uses a robust detection, which is suitable for both native builds and cross builds.
changed the title [-]Cross-build _curses failed if host ncurses headers and target ncurses headers have different layouts[/-][+]Detect curses headers correctly for cross-compiling[/+]on Sep 25, 2016
Now, Cygwin platform is able to build core interpreter on default branch. But the curses module has been failed to build. Therefore I tried to build curses module on Cygwin (Vista x86) using this patch. And it has been succeeded.
The patch effect at build time removes one compiler option "-I/usr/include/ncursesw". Maybe that modification position is setup.py:1352. I couldn't confirm why it has became success.
yan12125 commented on Oct 4, 2016
yan12125mannequin
MannequinAuthor
More actions
Hmm it's surprising for me that an irrelevant patch fixes issues on Cygwin. Does test_curses pass?
ma8ma commented on Oct 4, 2016
ma8mamannequin
Mannequin
More actions
test_curses has been skipped almost. the skip reason has been written "cygwin's curses mostly just hangs" in test case class.
I removed the skip condition, and ran test. Tests was almost passed. The only, unget_wch was failed by edge case '\U0010FFFF', because the wchar_t size on windows platfrom is two bytes. I'll upload test log together.
yan12125 commented on Dec 7, 2016
yan12125mannequin
MannequinAuthor
More actions
A clean patch without changes in ./configure. autoreconf necessary
the upstream ncurses has the ncursesw subdirinclude name, so you apparently can assume that this directory name is fixed *iff* it exists, however the ncursesw installation can be found in <prefix>/include instead. Can your changes cope with that?
second issue is that you apparently don't do the changes for term.h (and possibly for other headers in other places as well, I only looked at the diff), so you mix ncursesw and ncurses headers. So you have to make these changes in all other places as well.
Third issue is that you can't make these changes for third party extensions, if there are any relying on the ncurses/ncursesw distinction.
second issue is that you apparently don't do the changes for term.h
term.h is included only if ncurses is missing and the system (SysV) curses is used, so I didn't change it. See below:
#ifdef__sgi#include<term.h>#endif#ifdefHAVE_NCURSES_H#include<ncurses.h>#else#include<curses.h>#ifdefHAVE_TERM_H/* for tigetstr, which is not declared in SysV curses */#include<term.h>#endif#endif
Third issue is that you can't make these changes for third party extensions
Did you mean 3rd party extensions that include py_curses.h may get broken with this patch? IMO they shouldn't include this as it's CPython's implementation detail and shouldn't be used outside Modules/_cursesmodule.c and Modules/_curses_panel.c
however the ncursesw installation can be found in <prefix>/include instead
This is what I want to solve at the first place - make _curses compatible with different configurations. In configure.ac, there's a line:
so you apparently can assume that this directory name is fixed *iff* it exists
Did you mean there's no need to check panel.h? I'll try to update the patch.
There's something missing from this patch: if ncurses is built with --enable-reentrant, the library and include paths get an additional 't'. For example, the library name becomes libncursestw.so and includedir becomes $prefix/include/ncursestw. Note that 't' comes before 'w' as ncurses's configure.in handles --enable-widec before --enable-reentrant. I skip such cases as --enable-reentrant is not compatible with Modules/_cursesmodule.c yet (bpo-25720). I'd like to postpone it until bpo-25720 is resolved as it does not make sense to detect incompatible header paths.
xdegaye commented on Dec 9, 2016
xdegayemannequin
Mannequin
More actions
The only change that is needed here is to not include /usr/include/ncursesw in setup.py when cross compiling to ensure that the headers of the build platform are not included. When cross compiling Python, it is the responsability of the packager to set the appropriate CPPFLAGS and LDFLAGS upon invoking configure, so that the curses headers are included and the curses libraries are linked at build time.
The same is true for the other extension modules, readline, openssl, libffi, etc...
10 remaining items
yan12125 commented on Dec 11, 2016
yan12125mannequin
MannequinAuthor
More actions
Sorry I didn't read your patch carefully and it's surprising for me that you didn't remove/modify this line in configure.ac:
CPPFLAGS="$CPPFLAGS -I/usr/include/ncursesw"
With this line left there, feature detections for _curses are broken as ./configure check against native (/usr/include) headers, whose results make little sense for cross-builds.
xdegaye commented on Dec 11, 2016
xdegayemannequin
Mannequin
More actions
Hum, I did miss that line in configure.ac, thanks for pointing that out.
New patch.
Obsolete now. The original bug -- setup.py forcing /usr/include/ncursesw onto the include path, so feature detection used host headers against a target ncurses -- was fixed in 2016 (8c78d844d6f0, 3.6). Since then setup.py module building was removed entirely (3.12); curses is located by autoconf (PY_CHECK_CURSES in configure.ac), which searches the target sysroot when cross-compiling. The affected versions (3.6, 3.7) are EOL.
Closing as out of date -- a cross-build curses failure on a current branch would be a new issue against the configure-based detection.
Activity
yan12125 commentedon Sep 17, 2016
In changeset 919259054621 (from bpo-12567) and 707761d59a4a (from bpo-15268), /usr/include/ncursesw was added to include paths in setup.py and configure.ac. This is wrong and breaks cross-compiling. For example, if host has /usr/include/ncursesw/ncurses.h and target has $SYSROOT/include/ncurses.h, the build fails. An example can be found in [1].
My patch removes all references to /usr/include/ncursesw and uses a robust detection, which is suitable for both native builds and cross builds.
Added the authors of aforementioned changesets.
[1] https://s3.amazonaws.com/archive.travis-ci.org/jobs/159936249/log.txt
yan12125 commentedon Sep 17, 2016
Hmmm, I don't know why Rietveld failed to recognize changes in configure. Maybe it's because I've modified the patch file manually?
[-]Cross-build _curses failed if host ncurses headers and target ncurses headers have different layouts[/-][+]Detect curses headers correctly for cross-compiling[/+]doko42 commentedon Sep 26, 2016
looks good to me, thanks for working on this.
ma8ma commentedon Oct 4, 2016
Now, Cygwin platform is able to build core interpreter on default branch. But the curses module has been failed to build. Therefore I tried to build curses module on Cygwin (Vista x86) using this patch. And it has been succeeded.
The patch effect at build time removes one compiler option "-I/usr/include/ncursesw". Maybe that modification position is setup.py:1352. I couldn't confirm why it has became success.
yan12125 commentedon Oct 4, 2016
Hmm it's surprising for me that an irrelevant patch fixes issues on Cygwin. Does test_curses pass?
ma8ma commentedon Oct 4, 2016
test_curses has been skipped almost. the skip reason has been written "cygwin's curses mostly just hangs" in test case class.
I removed the skip condition, and ran test. Tests was almost passed. The only, unget_wch was failed by edge case '\U0010FFFF', because the wchar_t size on windows platfrom is two bytes. I'll upload test log together.
yan12125 commentedon Dec 7, 2016
A clean patch without changes in ./configure. autoreconf necessary
doko42 commentedon Dec 8, 2016
the upstream ncurses has the ncursesw subdirinclude name, so you apparently can assume that this directory name is fixed *iff* it exists, however the ncursesw installation can be found in <prefix>/include instead. Can your changes cope with that?
second issue is that you apparently don't do the changes for term.h (and possibly for other headers in other places as well, I only looked at the diff), so you mix ncursesw and ncurses headers. So you have to make these changes in all other places as well.
Third issue is that you can't make these changes for third party extensions, if there are any relying on the ncurses/ncursesw distinction.
fyi, "all" ncursesw headers are:
/usr/include/ncursesw/curses.h
/usr/include/ncursesw/cursesapp.h
/usr/include/ncursesw/cursesf.h
/usr/include/ncursesw/cursesm.h
/usr/include/ncursesw/cursesp.h
/usr/include/ncursesw/cursesw.h
/usr/include/ncursesw/cursslk.h
/usr/include/ncursesw/eti.h
/usr/include/ncursesw/etip.h
/usr/include/ncursesw/form.h
/usr/include/ncursesw/menu.h
/usr/include/ncursesw/nc_tparm.h
/usr/include/ncursesw/ncurses_dll.h
/usr/include/ncursesw/panel.h
/usr/include/ncursesw/term.h
/usr/include/ncursesw/term_entry.h
/usr/include/ncursesw/termcap.h
/usr/include/ncursesw/tic.h
/usr/include/ncursesw/unctrl.h
yan12125 commentedon Dec 9, 2016
term.h is included only if ncurses is missing and the system (SysV) curses is used, so I didn't change it. See below:
Did you mean 3rd party extensions that include py_curses.h may get broken with this patch? IMO they shouldn't include this as it's CPython's implementation detail and shouldn't be used outside Modules/_cursesmodule.c and Modules/_curses_panel.c
This is what I want to solve at the first place - make _curses compatible with different configurations. In configure.ac, there's a line:
Did you mean there's no need to check panel.h? I'll try to update the patch.
There's something missing from this patch: if ncurses is built with --enable-reentrant, the library and include paths get an additional 't'. For example, the library name becomes libncursestw.so and includedir becomes $prefix/include/ncursestw. Note that 't' comes before 'w' as ncurses's configure.in handles --enable-widec before --enable-reentrant. I skip such cases as --enable-reentrant is not compatible with Modules/_cursesmodule.c yet (bpo-25720). I'd like to postpone it until bpo-25720 is resolved as it does not make sense to detect incompatible header paths.
xdegaye commentedon Dec 9, 2016
The only change that is needed here is to not include /usr/include/ncursesw in setup.py when cross compiling to ensure that the headers of the build platform are not included. When cross compiling Python, it is the responsability of the packager to set the appropriate CPPFLAGS and LDFLAGS upon invoking configure, so that the curses headers are included and the curses libraries are linked at build time.
The same is true for the other extension modules, readline, openssl, libffi, etc...
10 remaining items
yan12125 commentedon Dec 11, 2016
Sorry I didn't read your patch carefully and it's surprising for me that you didn't remove/modify this line in configure.ac:
CPPFLAGS="$CPPFLAGS -I/usr/include/ncursesw"
With this line left there, feature detections for _curses are broken as ./configure check against native (/usr/include) headers, whose results make little sense for cross-builds.
xdegaye commentedon Dec 11, 2016
Hum, I did miss that line in configure.ac, thanks for pointing that out.
New patch.
python-dev commentedon Dec 13, 2016
New changeset 8c78d844d6f0 by Xavier de Gaye in branch '3.6':
Issue bpo-28190: Cross compiling the _curses module does not use anymore
https://hg.python.org/cpython/rev/8c78d844d6f0
New changeset bdf92b4e02f2 by Xavier de Gaye in branch 'default':
Issue bpo-28190: Merge 3.6.
https://hg.python.org/cpython/rev/bdf92b4e02f2
yan12125 commentedon Mar 26, 2019
As ncurses-related modules in Python alreadu build fine for Android, I consider the issue resolved.
doko42 commentedon Mar 26, 2019
no, please don't assume that if it builds for one cross build variant, that it builds for all. re-opening.
yan12125 commentedon Apr 8, 2019
I created #12587 as a preparation pull request to fix the __sgi issue mentioned in msg282782.
serhiy-storchaka commentedon Jun 28, 2026
Obsolete now. The original bug --
setup.pyforcing/usr/include/ncurseswonto the include path, so feature detection used host headers against a target ncurses -- was fixed in 2016 (8c78d844d6f0, 3.6). Since thensetup.pymodule building was removed entirely (3.12); curses is located by autoconf (PY_CHECK_CURSESinconfigure.ac), which searches the target sysroot when cross-compiling. The affected versions (3.6, 3.7) are EOL.Closing as out of date -- a cross-build curses failure on a current branch would be a new issue against the configure-based detection.