Skip to content

Failure to compile with readline-6.3-rc1 #64573

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

Closed
jhl mannequin opened this issue Jan 24, 2014 · 20 comments
Closed

Failure to compile with readline-6.3-rc1 #64573

jhl mannequin opened this issue Jan 24, 2014 · 20 comments
Assignees
Labels
build The build process and cross-build extension-modules C modules in the Modules dir release-blocker

Comments

@jhl
Copy link
Mannequin

jhl mannequin commented Jan 24, 2014

BPO 20374
Nosy @birkenfeld, @larryhastings, @benjaminp, @ned-deily, @serhiy-storchaka, @koobs
Files
  • readline_func_cast.patch
  • issue20374_no_cast_33.patch: 2.7 and 3.3
  • issue20374_no_cast_3x.patch: default
  • 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 = 'https://github.com/ned-deily'
    closed_at = <Date 2014-09-30.17:29:30.903>
    created_at = <Date 2014-01-24.04:26:50.018>
    labels = ['extension-modules', 'build', 'release-blocker']
    title = 'Failure to compile with readline-6.3-rc1'
    updated_at = <Date 2014-09-30.17:37:56.977>
    user = 'https://bugs.python.org/jhl'

    bugs.python.org fields:

    activity = <Date 2014-09-30.17:37:56.977>
    actor = 'georg.brandl'
    assignee = 'ned.deily'
    closed = True
    closed_date = <Date 2014-09-30.17:29:30.903>
    closer = 'ned.deily'
    components = ['Extension Modules']
    creation = <Date 2014-01-24.04:26:50.018>
    creator = 'jhl'
    dependencies = []
    files = ['33682', '33936', '33937']
    hgrepos = []
    issue_num = 20374
    keywords = ['patch']
    message_count = 20.0
    messages = ['209035', '209038', '209055', '209067', '209095', '209096', '209097', '209098', '209225', '209226', '209227', '210355', '210356', '210357', '210358', '210868', '227954', '227955', '227962', '227964']
    nosy_count = 9.0
    nosy_names = ['georg.brandl', 'larry', 'benjamin.peterson', 'ned.deily', 'Arfrever', 'python-dev', 'serhiy.storchaka', 'koobs', 'jhl']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'compile error'
    url = 'https://bugs.python.org/issue20374'
    versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

    @jhl
    Copy link
    Mannequin Author

    jhl mannequin commented Jan 24, 2014

    Python 2.7.6 does not compile with readline 6.3 (rc1).
    It looks like the RL_FUNCTION_TYPEDEF support in Modules/readline.c is not quite complete.

    The following patch works for me, both for 2.7.6 and 3.3.3.

    --- a/Modules/readline.c	2013-11-10 18:36:41.000000000 +1100
    +++ b/Modules/readline.c	2014-01-24 15:11:04.182417214 +1100
    @@ -911,12 +911,27 @@
         rl_bind_key_in_map ('\t', rl_complete, emacs_meta_keymap);
         rl_bind_key_in_map ('\033', rl_complete, emacs_meta_keymap);
         /* Set our hook functions */
    -    rl_startup_hook = (Function *)on_startup_hook;
    +    rl_startup_hook =
    +#if defined(_RL_FUNCTION_TYPEDEF)
    +        (rl_hook_func_t *)on_startup_hook;
    +#else
    +        (Function *)on_startup_hook;
    +#endif
     #ifdef HAVE_RL_PRE_INPUT_HOOK
    -    rl_pre_input_hook = (Function *)on_pre_input_hook;
    +    rl_pre_input_hook = 
    +#if defined(_RL_FUNCTION_TYPEDEF)
    +        (rl_hook_func_t *)on_pre_input_hook;
    +#else
    +        (Function *)on_pre_input_hook;
    +#endif
     #endif
         /* Set our completion function */
    -    rl_attempted_completion_function = (CPPFunction *)flex_complete;
    +    rl_attempted_completion_function =
    +#if defined(_RL_FUNCTION_TYPEDEF)
    +        (rl_completion_func_t *)flex_complete;
    +#else
    +        (CPPFunction *)flex_complete;
    +#endif
         /* Set Python word break characters */
         completer_word_break_characters =
             rl_completer_word_break_characters =

    @jhl jhl mannequin added extension-modules C modules in the Modules dir build The build process and cross-build labels Jan 24, 2014
    @Arfrever Arfrever mannequin added the release-blocker label Jan 24, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 24, 2014

    New changeset 79b82ebc4fd1 by Benjamin Peterson in branch '2.7':
    use new readline function types (closes bpo-20374)
    http://hg.python.org/cpython/rev/79b82ebc4fd1

    New changeset fb2259d9f6b4 by Benjamin Peterson in branch '3.3':
    use new readline function types (closes bpo-20374)
    http://hg.python.org/cpython/rev/fb2259d9f6b4

    New changeset eb251e3624df by Benjamin Peterson in branch 'default':
    merge 3.3 (bpo-20374)
    http://hg.python.org/cpython/rev/eb251e3624df

    @python-dev python-dev mannequin closed this as completed Jan 24, 2014
    @ned-deily
    Copy link
    Member

    The checked-in fixes break builds on OS X that use the Apple-supplied libedit readline compatibility layer. See, for example: http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/1077/steps/compile/logs/stdio

    gcc -fno-strict-aliasing -Werror=declaration-after-statement -g -O0 -Wall -Wstrict-prototypes -I./Include -I. -IInclude -I/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Include -I/Users/buildbot/buildarea/3.x.murray-snowleopard/build -c /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c -o build/temp.macosx-10.6-x86_64-3.4-pydebug/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.o
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c: In function ‘setup_readline’:
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1001: error: ‘rl_hook_func_t’ undeclared (first use in this function)
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1001: error: (Each undeclared identifier is reported only once
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1001: error: for each function it appears in.)
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1001: error: expected expression before ‘)’ token
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1003: error: expected expression before ‘)’ token
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1006: error: ‘rl_completion_func_t’ undeclared (first use in this function)
    /Users/buildbot/buildarea/3.x.murray-snowleopard/build/Modules/readline.c:1006: error: expected expression before ‘)’ token

    @serhiy-storchaka
    Copy link
    Member

    What if remove casts at all? Ned, is it compiled with libedit?

    @ned-deily
    Copy link
    Member

    Serhiy, I'm not sure I understand your question. libedit includes a GNU readline compatibility layer and that is what _readline.so builds and links with on OS X systems (with an ABI of 10.5 and later) since OS X does not ship with GNU readline. This is also the case on some BSD systems. With your patch, _readline.so again builds correctly.

    @serhiy-storchaka
    Copy link
    Member

    With your patch, _readline.so again builds correctly.

    Yes, this is what I wanted to know. Sorry for the awkward formulation.

    Type casting is dangerous because it may hide the signature mismatch, which
    can cause problems later in run time. It is better to rely on static type
    checking if possible.

    @benjaminp
    Copy link
    Contributor

    Okay then. Off we go.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 24, 2014

    New changeset 5e42e5764ac6 by Benjamin Peterson in branch '2.7':
    new plan: just remove typecasts (closes bpo-20374)
    http://hg.python.org/cpython/rev/5e42e5764ac6

    New changeset fc62fcd8e990 by Benjamin Peterson in branch '3.3':
    new plan: just remove typecasts (closes bpo-20374)
    http://hg.python.org/cpython/rev/fc62fcd8e990

    New changeset 3c3624fec6c8 by Benjamin Peterson in branch 'default':
    merge 3.3 (bpo-20374)
    http://hg.python.org/cpython/rev/3c3624fec6c8

    @python-dev python-dev mannequin closed this as completed Jan 24, 2014
    @ned-deily
    Copy link
    Member

    FYI, removing the cast causes the following new warnings when compiling 3.3 with gcc-4.2 on OS X 10.5 and 10.6 (haven't checked elsewhere):

    Modules/readline.c: In function 'setup_readline':
    Modules/readline.c:939: warning: assignment from incompatible pointer type
    Modules/readline.c:941: warning: assignment from incompatible pointer type

    @benjaminp
    Copy link
    Contributor

    It doesn't complain on Linux. I suppose if we don't want any warnings, we'd have to do something like the originally proposed patch.

    @serhiy-storchaka
    Copy link
    Member

    I'm surprised that warnings are emitted at lines 939 and 941, but not 944.

    I think that instead type casting, the more robust way is to change hook functions signatures for on_startup_hook and on_pre_input_hook.

    static int
    #ifdef _RL_FUNCTION_TYPEDEF /* or may be test libedit macro? */
    on_startup_hook(void)
    #else
    on_startup_hook()
    #endif

    @ned-deily
    Copy link
    Member

    Using Serhiy's suggestion, here are patches for 2.7/3.3 and default that condition the hook function signatures. With these patches, all three branches compile without warnings and pass test_readline using OS X libedit, readline 6.3-rc2, and an older readline.

    @ned-deily ned-deily reopened this Feb 6, 2014
    @benjaminp
    Copy link
    Contributor

    That is sure ugly, but okay. :)

    @benjaminp benjaminp assigned ned-deily and unassigned benjaminp Feb 6, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 6, 2014

    New changeset 0b5b0bfcc7b1 by Ned Deily in branch '2.7':
    Issue bpo-20374: Avoid compiler warnings when compiling readline with libedit.
    http://hg.python.org/cpython/rev/0b5b0bfcc7b1

    New changeset 9131a9edcac4 by Ned Deily in branch '3.3':
    Issue bpo-20374: Avoid compiler warnings when compiling readline with libedit.
    http://hg.python.org/cpython/rev/9131a9edcac4

    New changeset 0abf103f5559 by Ned Deily in branch 'default':
    Issue bpo-20374: merge
    http://hg.python.org/cpython/rev/0abf103f5559

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 6, 2014

    New changeset 74c7a6fd8b45 by Ned Deily in branch '2.7':
    Issue bpo-20374: delete spurious empty line
    http://hg.python.org/cpython/rev/74c7a6fd8b45

    New changeset 6616c94d6149 by Ned Deily in branch '3.3':
    Issue bpo-20374: delete spurious empty line
    http://hg.python.org/cpython/rev/6616c94d6149

    New changeset 0b91e764b889 by Ned Deily in branch 'default':
    Issue bpo-20374: merge
    http://hg.python.org/cpython/rev/0b91e764b889

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 10, 2014

    New changeset a7e048674fef by Ned Deily in branch '3.3':
    Issue bpo-20374: Avoid compiler warnings when compiling readline with libedit.
    http://hg.python.org/cpython/rev/a7e048674fef

    New changeset de02d414590d by Ned Deily in branch '3.3':
    Issue bpo-20374: delete spurious empty line
    http://hg.python.org/cpython/rev/de02d414590d

    @serhiy-storchaka
    Copy link
    Member

    The readline module no longer compiled in 3.2. May be we should apply these patches to 3.2.

    @ned-deily
    Copy link
    Member

    This isn't a security issue and in general we do not make changes to branches in security-fix mode to support new releases of external components or operating system releases. There are plenty of other fixes that should be ported to 3.2 if we go down that path. So I'm closing this again unless Georg disagrees.

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Sep 30, 2014

    +1 for backporting build fixes for major platforms (e.g. fix for this issue).

    @birkenfeld
    Copy link
    Member

    I agree with Ned. 3.2 is not maintained anymore, we only provide security fixes.

    (Exceptions prove the rule. One exception I committed today was to stop test_urllibnet from failing, so that I could better see real failures on the buildbots.)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    build The build process and cross-build extension-modules C modules in the Modules dir release-blocker
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants