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

Improve termcap detection in setup.py #67473

Closed
pooryorick mannequin opened this issue Jan 20, 2015 · 13 comments
Closed

Improve termcap detection in setup.py #67473

pooryorick mannequin opened this issue Jan 20, 2015 · 13 comments
Labels
build The build process and cross-build

Comments

@pooryorick
Copy link
Mannequin

pooryorick mannequin commented Jan 20, 2015

BPO 23284
Nosy @bitdancer, @skrah, @koobs, @mjeveritt
Files
  • 34d54cc5ecfd.diff
  • 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 2015-01-20.22:51:23.645>
    labels = ['build']
    title = 'Improve termcap detection in setup.py'
    updated_at = <Date 2019-09-30.01:35:11.922>
    user = 'https://bugs.python.org/pooryorick'

    bugs.python.org fields:

    activity = <Date 2019-09-30.01:35:11.922>
    actor = 'veremitz'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Build']
    creation = <Date 2015-01-20.22:51:23.645>
    creator = 'pooryorick'
    dependencies = []
    files = ['37796']
    hgrepos = ['292']
    issue_num = 23284
    keywords = ['patch']
    message_count = 12.0
    messages = ['234399', '234699', '234981', '234982', '234983', '234987', '234988', '234995', '235005', '235007', '235035', '235036']
    nosy_count = 5.0
    nosy_names = ['pooryorick', 'r.david.murray', 'skrah', 'koobs', 'veremitz']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = None
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue23284'
    versions = ['Python 2.7']

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 20, 2015

    Building Python-2.7.9 using --prefix, with an ncurses that's linked to libtinfo
    and a readline that isn't linked to any termcap library, I ran into the trouble
    that the curses module wan't buing build with the needed -L and -l flags for
    the libtinfo shared object. I dug into setup.py, and ended up overhauling the
    readline, curses, and also dbm handling (more on that in another ticket). I
    also made changes to allow setup.py to pick up options like -isystem from
    $CPPFLAGS, replacing in the process the "optparse" module with the "argparse"
    module. Since argparse requires collections.OrderedDict, which isn't yet built
    at this stage, I added Lib_boot/argparse_boot.py, which uses the pure-Python
    OrderedDict from

    https://pypi.python.org/pypi/ordereddict
    

    and had setup.py use "argparse_boot" module instead.

    The build also ran into trouble with system directories that setup.py
    explicitly adds to inc_dirs and lib_dirs ahead of those of the alternate
    prefix.

    The attached files fixed all these issues in my scenario, allowing a succesful
    build and install of Python-2.7.9.

    @pooryorick pooryorick mannequin added the build The build process and cross-build label Jan 20, 2015
    @bitdancer
    Copy link
    Member

    At a quick glance this does not seem like a reasonable patch...introducing copies of two stdlib modules is basically a non-starter. Nor do I understand from the description what the problem is that it is trying to solve.

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 29, 2015

    An attempt to build python-2.7.9 resulted in this error:

    renaming "readline" since importing it failed ... undefined symbol: UP
    

    This happened because setup.py chose "-lncursesw" as the readline terminal
    library. Although libncursesw.so itself links to libtinfow.so, in order to
    succeed with the "--as-needed" linker flag, setup.py would have to directly
    choose libtinfow.so. The part of the patch that deals with that does not rely
    on the other changes that were made to accomodate -isystem in CPPFLAGS.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 29, 2015

    What system are you using? The patch is quite large, and the existing libtinfo logic should work on most systems. Is the main problem that ldd is not found?

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 29, 2015

    ldd was found. That patch does abstract and generalize ldd usage into a
    function for reuse. I'm building Python as part of a rather large software
    collection that lives in an alternate prefix. The main issue with the termcap
    library is that with the --as-needed linker flag, which more modern build
    systems prefer, linking to libncurses.so isn't sufficient, even when
    libncurses.so itself links to libtinfo.so, but that's what setup.py decides to
    do.

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 29, 2015

    The wholesale inclusion of two additional files makes the patch look larger
    than it is. The modifications to setup.py address three different issues. The
    modifications for each issue don't overlap much, so they could be cherry-picked
    fairly easily. For the readline issue, all that's needed is the new function,
    "ldd_find_library_file" and then the changes from line 704 to about line 773
    that deal directly with readline and ncurses.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 29, 2015

    I think even the curses changes alone are rather large, especially
    for 2.7. Perhaps you could give us exact instructions how to
    reproduce the issue (OS, version, build command line).

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 29, 2015

    Larger changes represent more work on the part of the submitter ;)
    The following should reproduce the readline problem:

    Have the current setup.py detect as the readline termcap library a
    libncurses.so that itself links against libtinfo.so. Without the --as-needed
    flag, this is sufficient, since libtinfo ends up being indirectly linked to
    readline.so, through libncurses.so. Not that it was ever the right approach,
    but absent --as-needed, it happens to work.

    Then, make sure the --as-needed flag is passed to the link command for the
    readline.so module. In that case, even though there is a "-lncurses" argument,
    the linke will not link libncurses.so to readline.so because readline.so
    doesn't directly need any symbols provided by libncurses.so. The readline
    module will then fail to load because terminal library symbols like UP will be
    missing.

    Without --as-needed, superfluous links between shared objects proliferate
    through a software collection. --as-needed is becoming more common, so it
    would be good if setup.py didn't break because of it.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 29, 2015

    I still don't understand why libncurses is linked against libtinfo but
    libreadline is not. Again, what is your OS?

    Example Ubuntu (a modern system):

    $ ldd /lib/x86_64-linux-gnu/libncurses.so.5
            linux-vdso.so.1 =>  (0x00007fff5bfed000)
            libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fcb8b43a000)
            libtinfo.so.5 => /lib/x86_64-linux-gnu/libtinfo.so.5 (0x00007fcb8b211000)
            libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fcb8ae4a000)
            /lib64/ld-linux-x86-64.so.2 (0x00007fcb8b87d000)
    
    $ ldd /lib/x86_64-linux-gnu/libreadline.so.6
            linux-vdso.so.1 =>  (0x00007fff8c5fe000)
            libtinfo.so.5 => /lib/x86_64-linux-gnu/libtinfo.so.5 (0x00007f43055a0000)
            libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f43051da000)
            /lib64/ld-linux-x86-64.so.2 (0x00007f4305a2b000)

    @pooryorick
    Copy link
    Mannequin Author

    pooryorick mannequin commented Jan 30, 2015

    Ths OS is RHEL 6, but that's not so important because the entire collection,
    including readline, is built from source and installed into an alternate
    location. With the exception of a few essential shared objects from the
    system, everything else is contained in the alternate location. Here's what
    the readline INSTALL document says:

    Readline uses the termcap functions, but does not link with the termcap or
    curses library itself, allowing applications which link with readline 
    to choose an appropriate library.
    

    So if you don't monkey with the readline distribution, but build and install it
    as-is, you get a libreadline.so that's not linked to a termcap library. That's
    the situation for our software collection.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 30, 2015

    Ok thanks, I understand the issue now. I'm focusing this issue
    on the termcap detection. For the other parts of the patch
    you'd have to open separate issues.

    As an aside, the ncurses maintainer seems to endorse a distro
    enforced choice for libreadline's termcap:

    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=602720

    @skrah skrah mannequin changed the title curses, readline, tinfo, and also --prefix, dbm, CPPFLAGS Improve termcap detection in setup.py Jan 30, 2015
    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 30, 2015

    FWIW, even http://www.linuxfromscratch.org/lfs/view/stable/chapter06/readline.html
    enforces -ncurses linking of readline.

    [They should be compiling ncurses with tinfo split out though and
    use -tinfo instead.]

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @serhiy-storchaka
    Copy link
    Member

    setup.py no longer used to build CPython since 3.12 (see #94474).

    And Python 2.7 no longer supported. Is this issue actual in newer Python versions?

    @serhiy-storchaka serhiy-storchaka added the pending The issue will be closed if no feedback is provided label Dec 26, 2023
    @hauntsaninja hauntsaninja closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2024
    @erlend-aasland erlend-aasland removed the pending The issue will be closed if no feedback is provided label Jan 7, 2024
    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
    Projects
    Status: Done
    Development

    No branches or pull requests

    4 participants