python / cpython

The Python programming language
https://www.python.org
Other
67.48k stars 32.13k forks source link

NetBSD py_curses.h fix #39234

Closed e4d56fae-63b6-42c8-8ccf-ee6db09f2800 closed 20 years ago

e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago
BPO 806800
Nosy @loewis
Files
  • patch-am: py_curses.h fix
  • py_curses.diff: new py_curses fix / ncurses fix
  • 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: ```python assignee = None closed_at = created_at = labels = [] title = 'NetBSD py_curses.h fix' updated_at = user = 'https://bugs.python.org/marc' ``` bugs.python.org fields: ```python activity = actor = 'loewis' assignee = 'none' closed = True closed_date = None closer = None components = ['None'] creation = creator = 'marc' dependencies = [] files = ['5587', '5588'] hgrepos = [] issue_num = 806800 keywords = ['patch'] message_count = 13.0 messages = ['44637', '44638', '44639', '44640', '44641', '44642', '44643', '44644', '44645', '44646', '44647', '44648', '44649'] nosy_count = 3.0 nosy_names = ['loewis', 'anthonybaxter', 'marc'] pr_nums = [] priority = 'normal' resolution = 'out of date' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue806800' versions = [] ```

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    On NetBSD, [n]curses.h and stdlib.h/wchar.h use different guards against multiple definition of wchar_t and wint_t.

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 21 years ago

    Logged In: YES user_id=21627

    That is a work around a NetBSD bug, right? If so, there should be some procedure to retest presence of the bug in new NetBSD releases, and record the versions for which the bug has been found to be present. Then, when the newest release with the bug is not supported in Python anymore, the code can be removed (which might well be 10 years from now).

    If possible, a code fragment like

    #if (NetBSD_major == 3) or (NetBSD_major==4)
    // bug present, add work-around
    #else
    #error Please check whether the bug is still present
    #endif
    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    Hmm. I won't consider it a bug in NetBSD. Just a clash of declarations.. ;-) The attached patch is needed for 1.6.x and -current (which will be released not sooner than next year). I'm not sure about 1.5, but I'd bet it's the same. So, there's just no restriction possible. It's simply - all

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 21 years ago

    Logged In: YES user_id=21627

    Is it the case that curses.h and wchar.h cannot be simultaneously be included if a certain supported #define is given? If so, it is clearly a bug in the header files - it should not be possible to get conflicting declarations by just including system headers.

    Even if it applies to all released versions of the system, I still need an explicit list of what these versions are, and I need a compile-time failure if a version is encountered that isn't listed. That the next release is only a year away is a good thing - maybe they can fix the bug until then. They won't, of course, fix the bug if nobody tells them there is a bug...

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    Oh. I think we're talking at cross-purposes. The _system_ curses isn't used, it's GNU ncurses. And ncurses and char.h clash if _XOPEN_SOURCE_EXTENDED is defined. Naturally the system curses and wchar.h could be included together.

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    Just one note that might clarify things a bit more... ncurses is installed with the pkgsrc system (like FreeBSD's ports). It doesn't get installed with the base system nor is it in one of the system directories. (It's in /usr/pkg.) NetBSD has a curses implementation, but it has not (yet) all the features Python needs. Because of that we use GNU ncurses (atm 5.3).

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 21 years ago

    Logged In: YES user_id=21627

    Ok, then I still maintain that this is a bug in FreeBSD - namely, in the package. There should then still be a test for versions, so that we know when we can take the work-around out. Otherwise, the work-around will stay in forever, as we have no means of knowing whether it is still needed. That would be bad.

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    NetBSD ;-)
    But, you're right. It's the problem of the ncurses package and not NetBSD's 
    (nor FreeBSD's since it has a similar fix).
    It has also already been fixed in the dev. version of ncurses. I'm going to 
    backport the fix for the NetBSD pkgsrc version of ncurses, but the problem 
    remains for people who are using (or try to use) stock python and ncurses 
    sources. I still don't see a version test here. Digging in CVS shows that wchar_t 
    has been around for ages now (and it's going to stay).
    A solution could be to test for the existence of wchar_t in configure and change 
    the test to something like: 
    if defined(_XOPEN_SOURCE_EXTENDED) && defined(HAS_WCHAR_T) 
    && defined(NCURSES_VERSION)
    #ifndef _WCHAR_T
    #define _WCHAR_T
    [...]

    And thus making it OS independent.

    1a90d5ba-e091-4ed6-a644-41c5568983da commented 21 years ago

    Logged In: YES user_id=29957

    Note that the configure script in the trunk and 2.3 branch already has a workaround for this - it doesn't define the _XOPEN_SOURCE_EXTENDED macro for FreeBSD and Darwin. If the same fix is needed for NetBSD, could you supply a patch for configure.in?

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    ATM i'm doing some tests with a patch that disables _XOPEN_SOURCE_EXTENDEND on NetBSD 1.5/1.5. NetBSD 1.6/1.6. and some older versions of -current. For newer versions of -curernt the definition of _NETBSD_SOURCE 'fixes' the problems introduced by the posix macros.

    The curses problem is more a generic problem. wchar_t / wint_t are defined as "integer types" by POSIX see: http://www.opengroup.org/onlinepubs/007904975/basedefs/stddef.h.html http://www.opengroup.org/onlinepubs/007904975/basedefs/wchar.h.html

    On NetBSD wchar_t is an 'int' and ncurses defines it to 'unsigned long'. Thus the declaration clash. And this can happen on any os that defines wchar_t and uses ncurses for python. Because of that i think the test should be something like: if defined(_XOPEN_SOURCE_EXTENDED) && defined(HAS_WCHAR_T) && defined(NCURSES_VERSION)

    (And maybe a similar with HAS_WINT_Tif we can't assume that the os declares wint_t if it declares wchar_t.)

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 21 years ago

    Logged In: YES user_id=21627

    I disagree that the bug is in ncurses. ncurses supports conditional definition of wchar_t/wint_t, which normally should be defined by the platform. ncurses does so by testing for the _WCHAR_T and wchar_t macros, so for interoperability, the system should define _WCHAR_T (even though there is no standard requiring the definition of _WCHAR_T). This has nothing to do with _XOPEN_SOURCE_EXTENDED.

    Now, that might provide an alternative solution: if the system has wchar_t, and fails to define _WCHAR_T, we should define it ourselves before including ncurses.h.

    e4d56fae-63b6-42c8-8ccf-ee6db09f2800 commented 21 years ago

    Logged In: YES user_id=205

    No, it's a bug in ncurses \<= 5.3. It has been fixed in the current development version of ncurses. And - in the ncurses case - it has to do wich _XOPEN_SOURCE_EXTENDED, because ncurses only defines wchar_t/wint_t then. I followed your suggestion and made a new patch which defines HAVE_WCHAR_T/_WINT and _WCHAR_T / _WINT_T if the system provides them. I'm not 100% sure of the wchar_t test, I'm testing only stddef.h for wchar_t. (POSIX requires it to be defined there...)

    I'm not sure if the system should define _WCHAR_T (and thus polluting the namespace), since the only systems which are using the define are Solaris and Linux. (AFAIK)

    61337411-43fc-4a9c-b8d5-4060aede66d0 commented 20 years ago

    Logged In: YES user_id=21627

    I'm rejecting this patch as outdated. If the problem still exists, please submit a new patch, taking into account the comments in this patch.