Cross-build _curses failed if host ncurses headers and target ncurses headers have different layouts

[yan12125]

BPO	28190
Nosy	@doko42, @serhiy-storchaka, @ma8ma, @moreati, @yan12125
Files	ncurses.patch cygwin-test_curses.log: ran without skip condition ncurses-headers.patch: Patch version 2, autoreconf necessary no-path-to-ncursesw.patch no-path-to-ncursesw_2.patch

^{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 = None
closed_at = None
created_at = <Date 2016-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 = <Date 2019-12-10.08:03:17.734>
user = 'https://github.com/yan12125'

bugs.python.org fields:

activity = <Date 2019-12-10.08:03:17.734>
actor = 'xdegaye'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Cross-Build']
creation = <Date 2016-09-17.17:20:08.116>
creator = 'yan12125'
dependencies = []
files = ['44718', '44961', '45790', '45831', '45850']
hgrepos = []
issue_num = 28190
keywords = ['patch']
message_count = 23.0
messages = ['276806', '276807', '277430', '278051', '278052', '278060', '282647', '282698', '282782', '282806', '282822', '282838', '282839', '282840', '282844', '282848', '282912', '282915', '282921', '283111', '338884', '338889', '339591']
nosy_count = 6.0
nosy_names = ['doko', 'python-dev', 'serhiy.storchaka', 'masamoto', 'Alex.Willmer', 'yan12125']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = None
type = 'compile error'
url = 'https://bugs.python.org/issue28190'
versions = ['Python 3.6', 'Python 3.7']

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

Added the authors of aforementioned changesets.

[1] https://s3.amazonaws.com/archive.travis-ci.org/jobs/159936249/log.txt

[yan12125]

Hmmm, I don't know why Rietveld failed to recognize changes in configure. Maybe it's because I've modified the patch file manually?

[doko42]

looks good to me, thanks for working on this.

[ma8ma]

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]

Hmm it's surprising for me that an irrelevant patch fixes issues on Cygwin. Does test_curses pass?

[ma8ma]

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]

A clean patch without changes in ./configure. autoreconf necessary

[doko42]

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]

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

#ifdef HAVE_NCURSES_H
#include <ncurses.h>
#else
#include <curses.h>
#ifdef HAVE_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:

AC_CHECK_HEADERS(curses.h ncurses.h ncursesw/ncurses.h ncurses/ncurses.h panel.h ncursesw/panel.h ncurses/panel.h)

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]

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