Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix curses module compilation with ncurses6 #69906

Closed
donmez mannequin opened this issue Nov 24, 2015 · 27 comments
Closed

Fix curses module compilation with ncurses6 #69906

donmez mannequin opened this issue Nov 24, 2015 · 27 comments
Labels
3.7 (EOL) end of life build The build process and cross-build extension-modules C modules in the Modules dir

Comments

@donmez
Copy link
Mannequin

donmez mannequin commented Nov 24, 2015

BPO 25720
Nosy @Yhg1s, @mdickinson, @serhiy-storchaka, @ma8ma, @yan12125
PRs
  • bpo-25720: curses: Fix check for whether WINDOW is a pad #1689
  • bpo-25720: Fix the method for checking pad state of curses WINDOW #4164
  • [3.6] bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) #4212
  • [2.7] bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) #4213
  • Dependencies
  • bpo-31891: Make curses compiling on NetBSD 7.1 and tests passing
  • Files
  • curses-ncurses6.patch
  • curses-is_pad.patch
  • join-test-issue28190-issue25720.patch: for testing
  • 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 = <Date 2017-11-01.12:38:50.867>
    created_at = <Date 2015-11-24.11:39:07.962>
    labels = ['extension-modules', 'build', '3.7']
    title = 'Fix curses module compilation with ncurses6'
    updated_at = <Date 2017-11-01.12:38:50.865>
    user = 'https://bugs.python.org/donmez'

    bugs.python.org fields:

    activity = <Date 2017-11-01.12:38:50.865>
    actor = 'serhiy.storchaka'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-11-01.12:38:50.867>
    closer = 'serhiy.storchaka'
    components = ['Extension Modules']
    creation = <Date 2015-11-24.11:39:07.962>
    creator = 'donmez'
    dependencies = ['31891']
    files = ['41148', '45002', '45037']
    hgrepos = []
    issue_num = 25720
    keywords = ['patch']
    message_count = 27.0
    messages = ['255261', '258136', '258137', '258142', '261920', '261923', '278050', '278054', '278128', '278140', '278241', '278242', '278245', '278248', '278303', '278353', '278360', '278363', '287757', '294071', '296750', '305178', '305181', '305370', '305372', '305373', '305374']
    nosy_count = 6.0
    nosy_names = ['twouters', 'mark.dickinson', 'donmez', 'serhiy.storchaka', 'masamoto', 'yan12125']
    pr_nums = ['1689', '4164', '4212', '4213']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue25720'
    versions = ['Python 2.7', 'Python 3.6', 'Python 3.7']

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Nov 24, 2015

    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.

    @donmez donmez mannequin added the extension-modules C modules in the Modules dir label Nov 24, 2015
    @serhiy-storchaka serhiy-storchaka added the build The build process and cross-build label Nov 24, 2015
    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Jan 13, 2016

    Any patch review/comment ?

    @serhiy-storchaka
    Copy link
    Member

    configure.ac directly uses w->_flags in a check. It looks that this check always fails with ncurses6.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Jan 13, 2016

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Mar 17, 2016

    ping?

    @serhiy-storchaka
    Copy link
    Member

    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.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Oct 4, 2016

    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

    @serhiy-storchaka
    Copy link
    Member

    Mark, do you have relation to this?

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Oct 5, 2016

    I tried to build curses module on Cygwin (Vista x86) using bpo-25720 patch. And it has been succeeded.
    When test_curses ran without skip condition, it was same result as msg278060 (bpo-28190).

    I found out build success reasons for cases of applying patch:
    bpo-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.
    bpo-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.
    bpo-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.

    @mdickinson
    Copy link
    Member

    Mark, do you have relation to this?

    Sorry, no. Whatever ncurses knowledge I may once have had has long since vanished.

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Oct 7, 2016

    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.

    @ma8ma ma8ma mannequin added the 3.7 (EOL) end of life label Oct 7, 2016
    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Oct 7, 2016

    @masamoto thank you!

    @serhiy-storchaka
    Copy link
    Member

    Added a comment on Rietveld.

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Oct 7, 2016

    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.

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Oct 8, 2016

    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?

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Oct 9, 2016

    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>

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Oct 9, 2016

    Thank you for confirming, Yen :)
    In this case, It seems necessary that resolves headers. I think missing headers issue maybe solve by bpo-28190. I wrote a join test patch for bpo-28190 and bpo-25720.
    Would you be able to resolve headers using this?

    @yan12125
    Copy link
    Mannequin

    yan12125 mannequin commented Oct 9, 2016

    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.

    @donmez
    Copy link
    Mannequin Author

    donmez mannequin commented Feb 14, 2017

    What's the status on this? Can you please create a pull request on Github so we can continue there?

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented May 21, 2017

    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.

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Jun 24, 2017

    Ping. I updated PR a bit: macOS is joined to new compile condition and remove platform-specific condition.

    @ma8ma
    Copy link
    Mannequin

    ma8ma mannequin commented Oct 29, 2017

    I opened PR 4164 to improve the is_pad configure check and previous PR was closed.

    @serhiy-storchaka
    Copy link
    Member

    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.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 8bc7d63 by Serhiy Storchaka (Masayuki Yamamoto) in branch 'master':
    bpo-25720: Fix the method for checking pad state of curses WINDOW (bpo-4164)
    8bc7d63

    @serhiy-storchaka
    Copy link
    Member

    New changeset ff6ae4d by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
    bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) (bpo-4212)
    ff6ae4d

    @serhiy-storchaka
    Copy link
    Member

    New changeset 6ba0b58 by Serhiy Storchaka (Miss Islington (bot)) in branch '2.7':
    bpo-25720: Fix the method for checking pad state of curses WINDOW (GH-4164) (bpo-4213)
    6ba0b58

    @serhiy-storchaka
    Copy link
    Member

    Thank you for your contribution Masayuki!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    aphedges added a commit to aphedges/pyenv that referenced this issue Oct 5, 2023
    XCode Command Line Tools 15.0 was recently released, and it contains a
    broken version of ncurses 6.0. Some uses of Python's `curses` module
    will segfault when compiled with it. The solution is to switch to using
    the version of ncurses from Homebrew, which is currently 6.4. Support
    for ncurses 6 was added to Python 3.7 and was backported to 3.6 and 2.7,
    so this change should not break any recently supported Python versions.
    
    See python/cpython#109617 and
    python/cpython#69906 for more information.
    aphedges added a commit to aphedges/pyenv that referenced this issue Oct 5, 2023
    XCode Command Line Tools 15.0 was recently released, and it contains a
    broken version of ncurses 6.0. Some uses of Python's `curses` module
    will segfault when compiled with it. The solution is to switch to using
    the version of ncurses from Homebrew, which is currently 6.4. Support
    for ncurses 6 was added to Python 3.7 and was backported to 3.6 and 2.7,
    so this change should not break any recently supported Python versions.
    
    I tested this commit with Python 3.12, 3.11, and 2.7, and all tests in
    the `test.test_curses` module passed without issue.
    
    See python/cpython#109617 and
    python/cpython#69906 for more information.
    native-api pushed a commit to pyenv/pyenv that referenced this issue Oct 6, 2023
    XCode Command Line Tools 15.0 was recently released, and it contains a
    broken version of ncurses 6.0. Some uses of Python's `curses` module
    will segfault when compiled with it. The solution is to switch to using
    the version of ncurses from Homebrew, which is currently 6.4. Support
    for ncurses 6 was added to Python 3.7 and was backported to 3.6 and 2.7,
    so this change should not break any recently supported Python versions.
    
    Tested with Python 3.12, 3.11, and 2.7, and all tests in
    the `test.test_curses` module pass without issue.
    
    See python/cpython#109617 and
    python/cpython#69906 for more information.
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life build The build process and cross-build extension-modules C modules in the Modules dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants