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

ctypes.util.find_library() should return full pathname instead of filename in linux #65241

Open
HernanGrecco mannequin opened this issue Mar 23, 2014 · 24 comments
Open
Labels
topic-ctypes type-feature A feature request or enhancement

Comments

@HernanGrecco
Copy link
Mannequin

HernanGrecco mannequin commented Mar 23, 2014

BPO 21042
Nosy @berkerpeksag, @vadmium
Files
  • find_lib.patch
  • find_lib_v1.patch
  • find_lib_v2.patch
  • find_lib_v3.patch
  • find_lib_v4.patch
  • 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 2014-03-23.22:47:15.768>
    labels = ['ctypes', 'type-feature']
    title = 'ctypes.util.find_library() should return full pathname instead of filename in linux'
    updated_at = <Date 2021-12-09.14:33:36.731>
    user = 'https://bugs.python.org/HernanGrecco'

    bugs.python.org fields:

    activity = <Date 2021-12-09.14:33:36.731>
    actor = 'Charles Coulombe'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['ctypes']
    creation = <Date 2014-03-23.22:47:15.768>
    creator = 'Hernan.Grecco'
    dependencies = []
    files = ['41789', '41835', '41866', '41992', '42003']
    hgrepos = []
    issue_num = 21042
    keywords = ['patch']
    message_count = 21.0
    messages = ['214647', '259470', '259528', '259725', '259776', '259924', '260258', '260579', '260595', '260636', '260650', '260672', '260948', '260951', '261107', '261228', '261393', '261400', '261844', '261858', '408127']
    nosy_count = 6.0
    nosy_names = ['python-dev', 'berker.peksag', 'martin.panter', 'Hernan.Grecco', 'beng94', 'Charles Coulombe']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'needs patch'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21042'
    versions = []

    @HernanGrecco
    Copy link
    Mannequin Author

    HernanGrecco mannequin commented Mar 23, 2014

    In Windows and OSX, find_library returns the full pathname of the library file. But on Linux, it returns just the filename. Is there a reason for this difference?

    For consistency, it would be better to return the full pathname in all cases. It is easy to get the filename from the full pathname, but not the other way around.

    @HernanGrecco HernanGrecco mannequin added the topic-ctypes label Mar 23, 2014
    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 3, 2016

    Added a small patch that solves this issue on Ubuntu 15.10.

    Produces output like:
    /lib/x86_64-linux-gnu/libm.so.6
    /lib/x86_64-linux-gnu/libc.so.6
    /lib/x86_64-linux-gnu/libbz2.so.1.0

    I'd be glad to add some test cases if someone can give me some tips on how to do that.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 4, 2016

    IMO this should be treated as a new feature for the next release. But consistently returning the path sounds good to me if there is no good reason not to.

    Left a question on the review. I think you also need to update the documentation, and since this is changing documented behaviour it probably needs a What’s New entry.

    For tests, I would try doing something like find_library("c"), and ensuring that the result is an absolute path. We may have to end up skipping the test for platforms like Windows where this is not expected to work. Look through the files in /Lib/ctypes/test/ for a good place for it to live (if there isn’t already a test there you can modify).

    @vadmium vadmium added the type-feature A feature request or enhancement label Feb 4, 2016
    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 6, 2016

    Added a new patch, as Martin pointed out, I put back the ABI matching. The regex looks quite ugly, because it has to match \n\t. To be exact, it has to match something like this: "/lib/x86_64-linux-gnu/libc.so.6\n\tlibbz2.so.1.0 (libc6,x86-64)".

    I updated the docs, although I don't know what should I write for the version, please someone help me with that.

    For testing, there's a test function in this module, I updated that.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 7, 2016

    The ABI matching looks wrong to me. If I am looking for a 32-bit library, won’t it incorrectly catch the wrong path in the following “ldconfig -p” output:

    '\tlibm.so.6 (libc6,x86-64, OS ABI: Linux 2.6.32) => /usr/lib/libm.so.6\n'
    '\tlibm.so.6 (libc6, OS ABI: Linux 2.6.32) => /usr/lib32/libm.so.6\n'

    Perhaps the abi_type check needs to be moved in front of the path name extraction.

    For the version, I would put 3.6. Since this changes documented behaviour and has the potential to break compatibilty, it is best not to change it in a bug fix release. (3.5 has already been released.)

    The problem with the test() function in ctypes.util is that it is not run by the main Python regression test suite. The tests under ctypes/test/ are run by the test suite.

    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 9, 2016

    I fixed the ABI matching, it was a stupid mistake, thanks for pointing it out :) I think now it works as expected.

    I really don't find a place for testing. Maybe a new test file could be added, but I think the testing code for find_library wouldn't be more than 10 lines. Do you have any suggestions?

    Thanks Martin for all your patience :)

    @vadmium
    Copy link
    Member

    vadmium commented Feb 14, 2016

    I think the new regular expression will still find the wrong library in my libm example above. In 32-bit mode, it will be only looking to match \(libc6.*\). Since my example has the 64-bit line first, that one will match first. (I haven’t actually tested this, but I think I compiled 32-bit Python once before just by specifying CC="gcc -m32". Sorry to keep poking holes in your regular expression :)

    Do you know if there is documentation for the “ldconfig -p” output format, or do we just have to go on what we see? If so, I would change it to ensure the ABI type string is either followed by a comma and space ", " or a closing bracket ")". A comma on its own, or other letters, is not a match.

    I did a search for “find_library”, and the most likely place is /Lib/ctypes/test/test_find.py. You could probably get away with just adding a new method like Test_OpenGL_libs.test_path(). On my computer I have the GL and GLU libraries (but not gle), so I guess that these libraries are fairly common (plus it already has Windows and OS X versions to test).

    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 20, 2016

    What do you think about this regex?

    '(lib%s\.[^\\s]+\s\(%s(?:\)|,\s.*\))\s=>\s.*)' % (re.escape(name), abi_type))

    It works on 64 bit, just like before, but I could not test it on 32 bit. I'll add tests soon.

    I looked for documentation on ldconfig, but could not find anything useful.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 21, 2016

    Tamás, it might be a good idea for you to sign a contributor agreement <https://www.python.org/psf/contrib/contrib-form/\>.

    I compiled Python in 32-bit mode and tried your v2 patch out, which found the wrong library as I predicted. Then I tried your new regex and it picked out the correct line. I had to edit it to get it to extract just the filename from the line:

    r'lib%s\.[^\\s]+\s\(%s(?:,\s.*)?\)\s=>\s(.*)' % (re.escape(name), abi_type)

    I factored out the closing bracket from the comma + space bit, moved the group brackets to the end to extract the filename, and made it a raw string.

    Without the patch, in 32-bit mode it will find 64-bit-only libraries:

    >>> find_library("m")  # 32- and 64-bit available
    'libm.so.6'
    >>> find_library("tcl8.6")  # Only 64-bit version available!
    'libtcl8.6.so'

    With my edited regex:

    >>> find_library("m")
    '/usr/lib32/libm.so.6'
    >>> find_library("tcl8.6") is None  # No 32-bit version found
    True

    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 21, 2016

    I've added a new method to Test_OpenGL_libs as you suggested. I check whether find_library returns an absolute path. Note that I didn't distinguish different systems, as according to the docs, only Linux systems return the file name, other systems return the absolute path. (https://docs.python.org/3.5/library/ctypes.html#ctypes-reference) An other thing to note, that I introduced some code duplication as I use the same code snippet from setUpClass method to figure out the correct parameters to find_library.

    The patch uses the same regex as you gave.

    By the way, what do I have to do to compile CPython on a 64 bit system in 32 bit mode? I tried ./configure CC="gcc -m32" but it gave me an error. Is it the correct way?

    Also, I signed contributor agreement.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 22, 2016

    I left a suggestion about the duplication in the code review.

    I set CC with “configure” like you said. I also had to run “make clean” to get rid of the old 64-bit stuff. But it might depend on the GCC that you have installed. On Arch Linux, I have gcc-multilib, which supports 64-bit and 32-bit targets.

    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Feb 22, 2016

    Updated the patch to remove the code duplication, now it stores the values that are calculated in the setUpClass method. It was a good and simple idea, I should have come up with it... :)

    I'm pretty sure I got the errors during configuration because of the gcc version I have.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 27, 2016

    Thanks, this looks pretty good to me. I just need to remember to write a What’s New entry.

    @berkerpeksag
    Copy link
    Member

    I don't think we need an entry in whatsnew/3.6.rst for this (we already have documented the change in a versionchanged directive and a Misc/NEWS item)

    @beng94
    Copy link
    Mannequin

    beng94 mannequin commented Mar 2, 2016

    Is there anything else that I can do for this issue?

    @vadmium
    Copy link
    Member

    vadmium commented Mar 6, 2016

    No I think this is ready Tamás. I have been away, but it is on my list of things to catch up on. I won’t add any What’s New entry.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 9, 2016

    New changeset 3092cf163eb4 by Martin Panter in branch 'default':
    Issue bpo-21042: Return full path in ctypes.util.find_library() on Linux
    https://hg.python.org/cpython/rev/3092cf163eb4

    @vadmium
    Copy link
    Member

    vadmium commented Mar 9, 2016

    It looks like the ldconfig parsing isn’t working for some ABIs. See the following buildbot failures:

    I presume there are other flags in the ABI string, perhaps like (libc6,hard-float) on ARM. The code that produces these strings seems to be here: <https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=blob;f=elf/cache.c;h=fbee172#l72\>.

    Looking closer at the find_library() implementation, I also realize it is not correct to say an absolute path is always returned. If the ldconfig check fails, it falls back to _get_soname(). I think we have the following options:

    • Adjust the documentation to say an absolute path is only returned if the ldconfig call works
    • Figure out how to get the right ldconfig flags for ARM and PPC
    • Use the old parsing code on ARM and PPC platforms, and only return a full path on x86 or other platforms
    • Revert the whole change

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 16, 2016

    New changeset 811ec2860dc4 by Martin Panter in branch 'default':
    Issue bpo-21042: Revert Linux find_library() to return just filename
    https://hg.python.org/cpython/rev/811ec2860dc4

    @vadmium
    Copy link
    Member

    vadmium commented Mar 16, 2016

    I reverted the change until we can come up with something more consistent.

    @CharlesCoulombe
    Copy link
    Mannequin

    CharlesCoulombe mannequin commented Dec 9, 2021

    Any update on this issue?

    This would be helpful to HPC systems that don't have libraries installed in standard place, and to standardize find_library as well!

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

    I would like this feature too (possibly optionally, based on an extra parameter require_full_path defaulting to false).

    On MacOS and Windows I can influence where find_library searches by setting sys.environ["DYLD_LIBRARY_PATH"] or "PATH" before calling on find_library, but on Linux this doesn't work: find_library does find the library, but the dynamic loader has cached the value of the LD_LIBRARY_PATH on program startup, so it doesn't see my runtime changes, so it can't find the unqualified library name.

    @mara004
    Copy link
    Contributor

    mara004 commented Dec 20, 2023

    I also think it would be useful to have the full path info available to avoid uncertainty as to which file was loaded.

    @mara004
    Copy link
    Contributor

    mara004 commented Jan 28, 2024

    Re @jackjansen:

    but on Linux this doesn't work: find_library does find the library, but the dynamic loader has cached the value of the LD_LIBRARY_PATH on program startup, so it doesn't see my runtime changes, so it can't find the unqualified library name.

    I can't reproduce this -- at least on Fedora 37 / python 3.11, find_library() does honor prior runtime changes to os.environ["LD_LIBRARY_PATH"]. Or did I misunderstand your problem?

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-ctypes type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants