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

Export Readline version and expect ANSI sequence for version < 0x0600 #66962

Closed
DavidEdelsohn mannequin opened this issue Oct 31, 2014 · 19 comments
Closed

Export Readline version and expect ANSI sequence for version < 0x0600 #66962

DavidEdelsohn mannequin opened this issue Oct 31, 2014 · 19 comments
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@DavidEdelsohn
Copy link
Mannequin

DavidEdelsohn mannequin commented Oct 31, 2014

BPO 22773
Nosy @pitrou, @vstinner, @ned-deily
Files
  • Issue22773.txt
  • Issue22773.txt: v2
  • Issue22773.txt: v3
  • Issue22773.txt: v4
  • 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 2014-11-04.14:14:52.336>
    created_at = <Date 2014-10-31.14:01:28.065>
    labels = ['extension-modules', 'type-bug']
    title = 'Export Readline version and expect ANSI sequence for version < 0x0600'
    updated_at = <Date 2014-11-04.14:14:52.335>
    user = 'https://bugs.python.org/DavidEdelsohn'

    bugs.python.org fields:

    activity = <Date 2014-11-04.14:14:52.335>
    actor = 'pitrou'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-11-04.14:14:52.336>
    closer = 'pitrou'
    components = ['Extension Modules']
    creation = <Date 2014-10-31.14:01:28.065>
    creator = 'David.Edelsohn'
    dependencies = []
    files = ['37084', '37085', '37090', '37092']
    hgrepos = []
    issue_num = 22773
    keywords = []
    message_count = 19.0
    messages = ['230340', '230341', '230342', '230344', '230346', '230347', '230366', '230368', '230371', '230382', '230383', '230384', '230386', '230387', '230388', '230392', '230404', '230621', '230623']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'vstinner', 'ned.deily', 'Arfrever', 'python-dev', 'David.Edelsohn']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue22773'
    versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    The patch for bpo-19884 to set enable-meta-key to "off" does not work when readline-devel package is libreadline5, which includes SLES 11.

    @DavidEdelsohn DavidEdelsohn mannequin added the extension-modules C modules in the Modules dir label Oct 31, 2014
    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    Patch attached to export version and skip test if version less than 0x600.

    @DavidEdelsohn DavidEdelsohn mannequin added the type-bug An unexpected behavior, bug, or error label Oct 31, 2014
    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    Revised patch with leading underscores to make version objects private.

    @vstinner
    Copy link
    Member

    I would prefer to see the bug fixed instead of being skipped :-/ Is there a way to fix the issue, but differently on readline < 6.0?

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    The portion of the patch to skip the test is necessary because, as mentioned in bpo-19884, setting enable-meta-key does not work in older releases of libreadline. The particular combination of default TERM and libreadline is rare in old OS versions and can be worked around by setting TERM=dummy.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 31, 2014

    +1 for the patch.

    @Arfrever
    Copy link
    Mannequin

    Arfrever mannequin commented Oct 31, 2014

    Naming it _READLINE_BUILDTIME_VERSION instead of _READLINE_VERSION would avoid ambiguity.

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    I am trying to follow the precedent of zlibmodule with the naming so that users of Python modules have as consistent an experience and user interface as possible.

    @pitrou
    Copy link
    Member

    pitrou commented Oct 31, 2014

    Indeed, I think there's no ambiguity here. Victor, are you ok with the patch?

    @ned-deily
    Copy link
    Member

    The patch doesn't take into account that the readline module may be linked with BSD libedit (as is the default on OS X and is preferred by some third-party distributors) rather than GNU readline. The patch causes the test to be incorrectly skipped on those platforms.

    >>> readline._READLINE_VERSION
    1026

    The documented way of differentiating the two cases is by checking the __doc__ string for "readline" or "libedit":

    >>> readline.__doc__
    'Importing this module enables command line editing using libedit readline.'

    vs.

    >>> readline.__doc__
    'Importing this module enables command line editing using GNU readline.'

    @ned-deily
    Copy link
    Member

    Make that: The documented way of differentiating the two cases is by checking the __doc__ string for "libedit".

    @vstinner
    Copy link
    Member

    In the past, I added sys.thread_info to be able to skip tests on some old platforms. Maybe we need something similar: an object with runtime version, build version, implementation name, etc.

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    @ned.deily Can you offer a suggestion of how you you like the skipIf test modified to check __doc__ for "libedit"?

    @ned-deily
    Copy link
    Member

    Something like:

    @unittest.skipIf(("libedit" not in readline.__doc__)
                         and (readline._READLINE_VERSION < 0x6000),
                     "not supported in this library version")
    

    should work.

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    Okay, grouping was wrong in my initial attempt. New patch attached.

    @ned-deily
    Copy link
    Member

    The test works OK with libedit now, thanks. But are you sure about the readline version test? Testing with a GNU readline 6.3:

    >>> print('%x' % readline._READLINE_RUNTIME_VERSION)
    603

    so the test_readline check:

    readline._READLINE_VERSION < 0x6000

    causes the test to be incorrectly skipped. What does _READLINE_RUNTIME_VERSION look like for an older version of readline?

    @DavidEdelsohn
    Copy link
    Mannequin Author

    DavidEdelsohn mannequin commented Oct 31, 2014

    The test should be 0x0600.

    @DavidEdelsohn DavidEdelsohn mannequin changed the title Export Readline version and expect ANSI sequence for version < 0x6000 Export Readline version and expect ANSI sequence for version < 0x0600 Nov 1, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 4, 2014

    New changeset c4b5a5d44254 by Antoine Pitrou in branch '3.4':
    Issue bpo-22773: fix failing test with old readline versions due to issue bpo-19884.
    https://hg.python.org/cpython/rev/c4b5a5d44254

    New changeset be374b8c40c8 by Antoine Pitrou in branch 'default':
    Issue bpo-22773: fix failing test with old readline versions due to issue bpo-19884.
    https://hg.python.org/cpython/rev/be374b8c40c8

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 4, 2014

    New changeset eba6e68e818c by Antoine Pitrou in branch '2.7':
    Issue bpo-22773: fix failing test with old readline versions due to issue bpo-19884.
    https://hg.python.org/cpython/rev/eba6e68e818c

    @pitrou pitrou closed this as completed Nov 4, 2014
    @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
    extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants