#26439: ctypes.util.find_library fails when ldconfig/glibc not available (e.g., AIX)
|
Created:
3 years, 9 months ago by aixtools Modified:
3 years, 2 months ago Reviewers:
vadmium+py CC:
haypo, ned.deily, michael.haubenwallner_ssi-schaefer.com, devnull_psf.upfronthosting.co.za, Martin Panter, dje.gcc_gmail.com, Michael.Felt, aixtools_gmail.com, Mariatta Visibility:
Public. |
Patch Set 1 #
Total comments: 28
Patch Set 2 #
Total comments: 28
Patch Set 3 #
Total comments: 29
Patch Set 4 #
Total comments: 17
Patch Set 5 #
Total comments: 2
Created: 3 years, 2 months ago
MessagesTotal messages: 20
https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode87 Lib/ctypes/util.py:87: if sys.platform.startswith('aix'): Should this use “elif”? Otherwise I think it would cancel out the find_library() definition for platform Darwin above. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode92 Lib/ctypes/util.py:92: if (sys.maxsize < 2**32): Brackets not needed https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: f = os.popen(_cmd) Would it be better to use subprocess.Popen or check_output()? That would cut out the shell middle-man, and it seems unwise to insert arbitrary file names into a shell command line. (See also the sys.executable version below, and Issue 22636.) https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode110 Lib/ctypes/util.py:110: expr = r'\[lib%s[_]*64\.so\]' % name Does name need converting to RE syntax, using re.escape()? Why the special “[_]*” syntax? Is this the same as “_*”? https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode114 Lib/ctypes/util.py:114: expr = r'\[shr4_64.o\]' Does the full stop (.) need escaping? Do you even need to use a regular expression to search for a fixed string? It just seems to add unnecessary escaping and errors. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode124 Lib/ctypes/util.py:124: return '%s' % res[0][1:-1] %s just calls str(); isn’t that redundant? https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode128 Lib/ctypes/util.py:128: # sort function, borrowed from below Please pull it out into a single common private function, unless there is a valid reason to fork it https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode164 Lib/ctypes/util.py:164: return(_get_member64(archive, name, lines)) Brackets for return() look odd. Return is not a function. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode184 Lib/ctypes/util.py:184: # my preference is that sys.prefix would be included automatically by cdll.LoadLibrary Is this a bug in LoadLibrary(), or just your expression of the APIs not being designed to your taste? https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode198 Lib/ctypes/util.py:198: libpaths = "%s:%s" % (libpaths, x) Why do you join everything with colons if you just split it again below? https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode208 Lib/ctypes/util.py:208: # and whole path with .so - unsure on python behavior elsewhere On Linux, it tends to return the soname, but is documented to return the filename without path (see Issue 21622). But other platforms return the full path, and I would be happy to change Linux to return the full path too (Issue 21042). So if it makes sense to return a full path on AIX I think that would be okay. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode214 Lib/ctypes/util.py:214: print 'checking:' + _arfile Is this meant to be there?
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode440 Lib/ctypes/util.py:440: print 'aix:find_library("c")' Perhaps you can merge some of this with the general case above
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:365: mode |= RTLD_MEMBER Indentation is a bit excessive here :)
Sign in to reply to this message.
I expected I would make beginner errors, and I thank you for your patience. Do not be shy to point out weaknesses and/or changes in common practice compared to what is available as examples (I found/find). I shall review it all again (I have made changes locally since the last patch). What is the best way to submit a new patch? delta from what/where? https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode87 Lib/ctypes/util.py:87: if sys.platform.startswith('aix'): On 2016/04/28 02:43:26, vadmium wrote: > Should this use “elif”? Otherwise I think it would cancel out the find_library() > definition for platform Darwin above. I choose 'if', following the current structure. The function is defined several times (I added one as well) +6 # find_library(name) returns the pathname of a library, or None. +49 def find_library(name): +71 def find_library(name): +76 def find_library(name): +151 def find_library(name): +236 def find_library(name): +276 def find_library(name, is64 = False): +308 def find_library(name): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode92 Lib/ctypes/util.py:92: if (sys.maxsize < 2**32): On 2016/04/28 02:43:26, vadmium wrote: > Brackets not needed https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode99 Lib/ctypes/util.py:99: f = os.popen(_cmd) On 2016/04/28 02:43:26, vadmium wrote: > Would it be better to use subprocess.Popen or check_output()? That would cut out > the shell middle-man, and it seems unwise to insert arbitrary file names into a > shell command line. (See also the sys.executable version below, and Issue > 22636.) Will look into this. More learning! https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode110 Lib/ctypes/util.py:110: expr = r'\[lib%s[_]*64\.so\]' % name On 2016/04/28 02:43:26, vadmium wrote: > Does name need converting to RE syntax, using re.escape()? > > Why the special “[_]*” syntax? Is this the same as “_*”? I suspect, initially, I was looking at more than just one character (as this is looking for AIX legacy member names). _* may be better. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode114 Lib/ctypes/util.py:114: expr = r'\[shr4_64.o\]' On 2016/04/28 02:43:26, vadmium wrote: > Does the full stop (.) need escaping? Do you even need to use a regular > expression to search for a fixed string? It just seems to add unnecessary > escaping and errors. Yes, to distinguish from ".o$" from ".so$" Do not recall what failed to work. Will verify need. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode128 Lib/ctypes/util.py:128: # sort function, borrowed from below On 2016/04/28 02:43:26, vadmium wrote: > Please pull it out into a single common private function, unless there is a > valid reason to fork it Will try. I did not want to break something established by moving it. My valid reason - was - caution. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode164 Lib/ctypes/util.py:164: return(_get_member64(archive, name, lines)) On 2016/04/28 02:43:26, vadmium wrote: > Brackets for return() look odd. Return is not a function. I 'learned' using C and Pascal too many years ago. - Looks odd without them (but I need to acquire a new taste!) Thx. for heads-up! https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode184 Lib/ctypes/util.py:184: # my preference is that sys.prefix would be included automatically by cdll.LoadLibrary On 2016/04/28 02:43:26, vadmium wrote: > Is this a bug in LoadLibrary(), or just your expression of the APIs not being > designed to your taste? More learning. I expect it is working as designed and not a bug. Simply not what I expected. Taste can be acquired. As "learning" goes, this is a question to "you"/reminder to "me" to verify that I was not doing something wrong. Further, as packaging may not use /usr I wanted to be sure that packager prefix (e.g., I use /opt/* and AIXToolBox (IBM, BULL and Perzl) use /opt/freeware/* is searched). Looking everywhere is how I interpreted the scan/search of ldconfig output. My intent is to look in expected locations. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode198 Lib/ctypes/util.py:198: libpaths = "%s:%s" % (libpaths, x) On 2016/04/28 02:43:26, vadmium wrote: > Why do you join everything with colons if you just split it again below? cut/paste and learning-curve with python. This 'works'. However I welcome your more efficient way! https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode208 Lib/ctypes/util.py:208: # and whole path with .so - unsure on python behavior elsewhere On 2016/04/28 02:43:26, vadmium wrote: > On Linux, it tends to return the soname, but is documented to return the > filename without path (see Issue 21622). But other platforms return the full > path, and I would be happy to change Linux to return the full path too (Issue > 21042). So if it makes sense to return a full path on AIX I think that would be > okay. Want to check default AIX behavior - whether filename only finds the same object or not - as it may be in multiple locations. In any case, I was testing here, I believe I am now returning only filename: Modified tests (from python -m ctypes/util) returns: root@x064:[/opt/lib/python2.7]python -m ctypes/util libm.a libc.a libbz2.a <CDLL 'libiconv.a(libiconv.so.2)', handle 6 at 70000000013ff28> <CDLL 'libcrypto.a(libcrypto64.so)', handle 7 at 70000000013ff28> <CDLL 'libc.a(shr_64.o)', handle 8 at 70000000013ff28> <CDLL 'None', handle 9 at 70000000013ff28> <CDLL 'None', handle a at 70000000013ff28> <CDLL 'libcrypt.a(shr_64.o)', handle b at 70000000013ff28> libcrypt.a <CDLL 'libcrypt.a(shr_64.o)', handle c at 70000000013ff60> And now, after looking at the comment on line 6, not sure what I should be doing. +6 # find_library(name) returns the pathname of a library, or None. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode214 Lib/ctypes/util.py:214: print 'checking:' + _arfile On 2016/04/28 02:43:26, vadmium wrote: > Is this meant to be there? No. This was for debugging. Forgot to remove before submitting. (have already removed it locally) https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode440 Lib/ctypes/util.py:440: print 'aix:find_library("c")' On 2016/04/28 03:46:55, vadmium wrote: > Perhaps you can merge some of this with the general case above Yes. Will do. In any case, less verbose. "Learning-curve".
Sign in to reply to this message.
It is probably best to make a new patch that replaces the original, like you have been doing. If possible, generate it with Mercurial or based off Python 3 code (to help the review system), but if not possible, based off whatever you have. https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py File Lib/ctypes/util.py (right): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode87 Lib/ctypes/util.py:87: if sys.platform.startswith('aix'): On 2016/04/29 11:57:44, Michael.Felt wrote: > On 2016/04/28 02:43:26, vadmium wrote: > > Should this use “elif”? Otherwise I think it would cancel out the > find_library() > > definition for platform Darwin above. > I choose 'if', following the current structure. I have added the if/elif logic to your structure. Consider the case where posix and darwin are both true: if nt: > +49 def find_library(name): if ce: > +71 def find_library(name): if posix and darwin: > +76 def find_library(name): if aix: # Darwin is not AIX . . . > +151 def find_library(name): elif posix: # . . . but is Posix, so execute this block if freebsd or openbsd or dragonfly: > +236 def find_library(name): elif sunos5: > +276 def find_library(name, is64 = False): else: # Overrides Darwin-specific definition > +308 def find_library(name): https://bugs.python.org/review/26439/diff/17096/Lib/ctypes/util.py#newcode208 Lib/ctypes/util.py:208: # and whole path with .so - unsure on python behavior elsewhere On 2016/04/29 11:57:44, Michael.Felt wrote: > And now, after looking at the comment on line 6, not sure what I should be > doing. > > +6 # find_library(name) returns the pathname of a library, or None. One option might be to correct the comment to reflect reality :)
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcode17 Lib/ctypes/__init__.py:17: # RTLD_MEMBER is not in _ctypes (yet), or whereever it needs to be defined See Modules/_ctypes/_ctypes.c where RTLD_LOCAL/GLOBAL are added to _ctypes. I guess it would need an AIX conditional around it. They should also be added to the documentation if you intend to make them part of the public API. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:364: if name is None: It is undocumented, but I think CDLL(None) should call dlopen(NULL, ...) and return a handle for the running Python interpreter program. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:369: mode |= RTLD_NOW In Modules/_ctypes/callproc.c it looks like RTLD_NOW is always forced on. Do you really need this code as well? https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:371: if _name[-1] != ')': if not _name.endswith(')') https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py File Lib/ctypes/aixutil.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode39 Lib/ctypes/aixutil.py:39: # stderr=_os.devnull, Unfortunately for Python 2, I think you have to manually open os.devnull, pass the file object or file descriptor, and close it. In Python 3, there is subprocess.DEVNULL. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode43 Lib/ctypes/aixutil.py:43: universal_newlines=False, This is already the default. In Python 3, it means to read bytes rather than text, so you might find it works better set to True. Or it might be better to use os.fsdecode(), as proposed in Issue 22636. But that function does not exist in Python 2. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode45 Lib/ctypes/aixutil.py:45: p.wait() This is not good; in theory it could deadlock if the child process tries to write more output than can fit into the OS’s buffers. The solution is to either: 1. Return the Popen object to e.g. _getLibPath_aix(), which will have to call wait() after it has finished reading the output (and closed p.stdout!). 2. Read all the output here. The simplest way would be to call p.communicate(). This is probably the better solution, unless there would be such a large amount of output to make it inefficient. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode155 Lib/ctypes/aixutil.py:155: return libpaths This will cause KeyboardInterrupt etc to be ignored. Better to be explicit about what exception you are catching, ignore that exception, and then return after the exception handler. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode172 Lib/ctypes/aixutil.py:172: if _stem.find("lib") < 0: How does this work with names like find_library("glib-2.0")? (Returns "libglib-2.0.so.0" for me.) https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode181 Lib/ctypes/aixutil.py:181: _os.environ['LIBPATH'] = "%s/lib" % _sys.prefix Doesn’t seem wise to modify the environment. Hope this is just a temporary hack? :)
Sign in to reply to this message.
Will test the changes and update patch. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcode17 Lib/ctypes/__init__.py:17: # RTLD_MEMBER is not in _ctypes (yet), or whereever it needs to be defined On 2016/05/08 08:08:44, vadmium wrote: > See Modules/_ctypes/_ctypes.c where RTLD_LOCAL/GLOBAL are added to _ctypes. I > guess it would need an AIX conditional around it. > Not sure about the AIX conditional - wouldn't that break things when it is not AIX, or does the interpreter only read after conditionals pass? In any case, probably better to have two import statements - the first asis, and the second in the "aix" block where they are used. > They should also be added to the documentation if you intend to make them part > of the public API. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:364: if name is None: On 2016/05/08 08:08:44, vadmium wrote: > It is undocumented, but I think CDLL(None) should call dlopen(NULL, ...) and > return a handle for the running Python interpreter program. The problem was with if _name[-1] when name (so _name) == None. if _name.endswith(")") works when _name is None - then all is good! https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:369: mode |= RTLD_NOW On 2016/05/08 08:08:44, vadmium wrote: > In Modules/_ctypes/callproc.c it looks like RTLD_NOW is always forced on. Do you > really need this code as well? If it is already forced on, no not needed. But I could not see that from the code above. Would still want to leave it as a comment that it needs to be set, per AIX documentation (or, for self-documentation, set DEFAULT= (RTLD_NOW | RTLD_GLOBAL) rather than just RTLD_GLOBAL (as an assignment is being made anyway).
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py File Lib/ctypes/aixutil.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode39 Lib/ctypes/aixutil.py:39: # stderr=_os.devnull, On 2016/05/08 08:08:44, vadmium wrote: > Unfortunately for Python 2, I think you have to manually open os.devnull, pass > the file object or file descriptor, and close it. In Python 3, there is > subprocess.DEVNULL. Maybe it is better to see there is an error with a library when there are no objects in the right size (32 or 64 bits). So, I shall remove the comments. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode43 Lib/ctypes/aixutil.py:43: universal_newlines=False, On 2016/05/08 08:08:44, vadmium wrote: > This is already the default. In Python 3, it means to read bytes rather than > text, so you might find it works better set to True. Or it might be better to > use os.fsdecode(), as proposed in Issue 22636. But that function does not exist > in Python 2. Yes, it works better set to True (with False the code fails on Python3, but it works with True in both Python2 and Python3(.5.1)) https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode155 Lib/ctypes/aixutil.py:155: return libpaths On 2016/05/08 08:08:44, vadmium wrote: > This will cause KeyboardInterrupt etc to be ignored. Better to be explicit about > what exception you are catching, ignore that exception, and then return after > the exception handler. I was having a lot of trouble with subprocess. I'll look at this again. Simple is better. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode172 Lib/ctypes/aixutil.py:172: if _stem.find("lib") < 0: On 2016/05/08 08:08:44, vadmium wrote: > How does this work with names like find_library("glib-2.0")? (Returns > "libglib-2.0.so.0" for me.) What I would expect is: a) use "stem" to look for the archive (would pass -lglib-2.0 to a linker, of -lglib-2)? b) look in libglib-2.a for member libglib-2.0.so; if libglib-2.0.so is not found try to find libglib-2.0.so."versioning" c) if b) fails, look for a file named libglib-2.0 (so I see I need to add looking specifically for a .so file as well! What (I believe) AIX ld does (i.e., AIX ld expands -lFOO-X to libFOO-X.a, looks for a suitable member to resolve the name sought, if .a archive is not found, looks for libFOO-X.so and this is the behavior I am trying to match. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode181 Lib/ctypes/aixutil.py:181: _os.environ['LIBPATH'] = "%s/lib" % _sys.prefix On 2016/05/08 08:08:44, vadmium wrote: > Doesn’t seem wise to modify the environment. Hope this is just a temporary hack? > :) Well, LIBPATH is a legal, and documented environment variable - AND, I am only modifying it when it is not set already. However, I have been thinking about adding -L$prefix/lib to the linker so that it is in the default path already. By default, python does not need any special libraries so it is not taking it in on it's own. Without changing one of these two dlopen() will not look there - and things will fail because even though find_library finds something, cdll (aka dlopen()) might not. Considering your concern, "documenting" the need to add -L$prefix/lib (in my case, that is -L/opt/lib which then adds /opt/lib to the linker blibpath) will satisfy both find_library and dlopen()
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcode17 Lib/ctypes/__init__.py:17: # RTLD_MEMBER is not in _ctypes (yet), or whereever it needs to be defined On 2016/05/08 22:19:01, Michael.Felt wrote: > On 2016/05/08 08:08:44, vadmium wrote: > > See Modules/_ctypes/_ctypes.c where RTLD_LOCAL/GLOBAL are added to _ctypes. I > > guess it would need an AIX conditional around it. > > > Not sure about the AIX conditional - wouldn't that break things when it is not > AIX, or does the interpreter only read after conditionals pass? > In any case, probably better to have two import statements - the first asis, and > the second in the "aix" block where they are used. Well on a non-AIX platform, what should _ctypes.RTLD_MEMBER be? Often in this situation, such attributes don’t exist. (E.g. OSError.winerror only exists on Windows.) Maybe you could only import RTLD_MEMBER in the AIX block in CDLL(). https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:364: if name is None: On 2016/05/08 22:19:01, Michael.Felt wrote: > On 2016/05/08 08:08:44, vadmium wrote: > > It is undocumented, but I think CDLL(None) should call dlopen(NULL, ...) and > > return a handle for the running Python interpreter program. > > The problem was with if _name[-1] when name (so _name) == None. if > _name.endswith(")") works when _name is None - then all is good! No it won’t work. Maybe you need to write ‘if _name is not None and _name.endswith(")")’, or use _name[-1] if you prefer that way. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:369: mode |= RTLD_NOW On 2016/05/08 22:19:01, Michael.Felt wrote: > On 2016/05/08 08:08:44, vadmium wrote: > > In Modules/_ctypes/callproc.c it looks like RTLD_NOW is always forced on. Do > you > > really need this code as well? > > If it is already forced on, no not needed. But I could not see that from the > code above. It is far from obvious, but see <https://hg.python.org/cpython/file/v2.7.11/Modules/_ctypes/callproc.c#l1428>. The way I see it is the Python-level CDLL() always uses RTLD_NOW mode, so it doesn’t have to be specified at the Python level. > Would still want to leave it as a comment that it needs to be set, > per AIX documentation (or, for self-documentation, set DEFAULT= (RTLD_NOW | > RTLD_GLOBAL) rather than just RTLD_GLOBAL (as an assignment is being made > anyway). I agree a comment would be good. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py File Lib/ctypes/aixutil.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode172 Lib/ctypes/aixutil.py:172: if _stem.find("lib") < 0: > On 2016/05/08 08:08:44, vadmium wrote: > > How does this work with names like find_library("glib-2.0")? (Returns > > "libglib-2.0.so.0" for me.) The original reason I asked was because this would (incorrectly?) find "lib" in the middle of the string, and then remove the first three characters, leaving "b-2" (or should that be "b-2.0"?). Another example: find_library("Imlib2") -> "libImlib2.so.1". Now I realize glib-2.0 raises a second issue, because the library stem name contains a full stop. The 2.0 is part of the library name, not the ABI version tacked onto the end. My Linux computer has a few of these cases, mostly associated with GTK/gobject/Gnome libraries. Another example: find_library("SDL-1.2") -> "libSDL-1.2.so.0". Maybe libraries on AIX don’t use these kinds of stem names, but I am a bit suspicious. On 2016/05/08 22:44:52, Michael.Felt wrote: > What I would expect is: > a) use "stem" to look for the archive (would pass -lglib-2.0 to a linker, of > -lglib-2)? -lglib-2.0, judging by a quick Google search https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode181 Lib/ctypes/aixutil.py:181: _os.environ['LIBPATH'] = "%s/lib" % _sys.prefix On 2016/05/08 22:44:52, Michael.Felt wrote: > Well, LIBPATH is a legal, and documented environment variable - AND, I am only > modifying it when it is not set already. However, I have been thinking about > adding -L$prefix/lib to the linker so that it is in the default path already. By > default, python does not need any special libraries so it is not taking it in on > it's own. > Without changing one of these two dlopen() will not look there - and things will > fail because even though find_library finds something, cdll (aka dlopen()) might > not. This sounds similar to <https://bugs.python.org/issue18502>, where someone has Linux set up so that /usr/local/lib is searched at compile time and by find_library(), but not at runtime or by CDLL(). IMO this is an argument for having find_library() return the full path, rather than just the file name or soname.
Sign in to reply to this message.
still testing, but getting surprising comment from subprocess (better Popen()) via aixutil.py - more comments there. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:364: if name is None: On 2016/05/09 02:19:38, vadmium wrote: > On 2016/05/08 22:19:01, Michael.Felt wrote: > > On 2016/05/08 08:08:44, vadmium wrote: > > > It is undocumented, but I think CDLL(None) should call dlopen(NULL, ...) and > > > return a handle for the running Python interpreter program. > > > > The problem was with if _name[-1] when name (so _name) == None. if > > _name.endswith(")") works when _name is None - then all is good! > > No it won’t work. Maybe you need to write ‘if _name is not None and > _name.endswith(")")’, or use _name[-1] if you prefer that way. So, now with your explanation as to why __init__ is called with None - I want to be sure that the API is already open, as expected. However, as find_library may also return None, still need to be testing for "not name" before checking for ")" as last character. imho - the code is stronger with: if name and not name.endswith(")"): and later if name and name.endswith(")"): Question: does the output: <CDLL 'None', handle a at 0x300f7090> imply that the Python API has been found? Searching IBM Docs on dlopen(), that seems to be a correct assumption: Quote: If the value of FilePath is NULL, a value for the main application is returned. This allows dynamically loaded objects to look up symbols in the main executable, or for an application to examine symbols available within itself. 2nd question: As find_library returns None when nothing can be found, and NULL returns "the application" tests such as: ctypes/test/test_loading.py:42: self.assertRaises(OSError, cdll.LoadLibrary, "libc.so.9") ctypes/test/test_loading.py:43: self.assertRaises(OSError, cdll.LoadLibrary, self.unknowndll) always fail. Do you have a suggestion on how to let dlopen() be called such that OSError is raised?, e.g., if name and not name.endswith(")"): import ctypes.aixutil as aix name = aix.find_library(name) if name: self._name = name FYI: with that change I get OSError, but also more (I do not yet understand) root@x064:[/data/prj/aixtools/python/python-3.5.1/Lib/ctypes/test]../../../python test_loading.py /data/prj/aixtools/python/python-3.5.1/Lib/ctypes/aixutil.py:182: ResourceWarning: unclosed file <_io.TextIOWrapper name=4 encoding='ISO8859-1'> paths = _getLibPath_aix() /data/prj/aixtools/python/python-3.5.1/Lib/ctypes/aixutil.py:189: ResourceWarning: unclosed file <_io.TextIOWrapper name=4 encoding='ISO8859-1'> lines = _get_dumpH(_arfile).read() libc_name is libc.a(shr.o) ss..sss ---------------------------------------------------------------------- Ran 7 tests in 0.210s OK (skipped=5)
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py File Lib/ctypes/aixutil.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode45 Lib/ctypes/aixutil.py:45: p.wait() On 2016/05/08 08:08:44, vadmium wrote: > This is not good; in theory it could deadlock if the child process tries to > write more output than can fit into the OS’s buffers. The solution is to either: > > 1. Return the Popen object to e.g. _getLibPath_aix(), which will have to call > wait() after it has finished reading the output (and closed p.stdout!). > > 2. Read all the output here. The simplest way would be to call p.communicate(). > This is probably the better solution, unless there would be such a large amount > of output to make it inefficient. I think I better understand your comments - now that I get some new errors... And, guess what option 1 fixes! :) Now to see if I can make it prettier (i.e., use option 2)
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:364: if name is None: On 2016/05/09 08:51:12, Michael.Felt wrote: > Question: > does the output: > <CDLL 'None', handle a at 0x300f7090> > imply that the Python API has been found? Something like that. I am mainly going off the Posix specification <http://pubs.opengroup.org/onlinepubs/9699919799/functions/dlopen.html> which says “If _file_ is a null pointer, dlopen() shall return a global symbol table handle for the currently running process image.” E.g.: >>> main = CDLL(None) >>> main.PyBytes_FromString # Part of the Python API <_FuncPtr object at 0x7f28414822a0> >>> main.printf # Works because Python is linked to the C library <_FuncPtr object at 0x7f28414821d8> > 2nd question: > As find_library returns None when nothing can be found, and NULL returns "the > application" tests such as: > ctypes/test/test_loading.py:42: self.assertRaises(OSError, > cdll.LoadLibrary, "libc.so.9") > ctypes/test/test_loading.py:43: self.assertRaises(OSError, > cdll.LoadLibrary, self.unknowndll) > > always fail. > > Do you have a suggestion on how to let dlopen() be called such that OSError is > raised?, e.g., > if name and not name.endswith(")"): > import ctypes.aixutil as aix > name = aix.find_library(name) > if name: > self._name = name That could improve things, but I suspect you shouldn’t be calling find_library() here at all.
Sign in to reply to this message.
Thought this was already published... One click too few. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/__init__.py#newcode17 Lib/ctypes/__init__.py:17: # RTLD_MEMBER is not in _ctypes (yet), or whereever it needs to be defined On 2016/05/09 02:19:38, vadmium wrote: > On 2016/05/08 22:19:01, Michael.Felt wrote: > > On 2016/05/08 08:08:44, vadmium wrote: > > > See Modules/_ctypes/_ctypes.c where RTLD_LOCAL/GLOBAL are added to _ctypes. > I > > > guess it would need an AIX conditional around it. > > > > > Not sure about the AIX conditional - wouldn't that break things when it is not > > AIX, or does the interpreter only read after conditionals pass? > > In any case, probably better to have two import statements - the first asis, > and > > the second in the "aix" block where they are used. > > Well on a non-AIX platform, what should _ctypes.RTLD_MEMBER be? Often in this > situation, such attributes don’t exist. (E.g. OSError.winerror only exists on > Windows.) Maybe you could only import RTLD_MEMBER in the AIX block in CDLL(). Done.
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:359: # adding RTLD_NOW is already 'forced' in Modules/_ctypes/callproc.c I think this would be better off in the documentation of the “mode” parameter (i.e. <https://docs.python.org/3.5/library/ctypes.html#loading-shared-libraries>). It is not specific to AIX, and applies to people using the module, not just reading the source code. https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py File Lib/ctypes/_aixutil.py (right): https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcode21 Lib/ctypes/_aixutil.py:21: # Much code is written as: CDLL(find_library("libFOO")) Can you point to an example? This seems like an AIX-specific quirk, so I doubt there would be much code using this form. https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcode95 Lib/ctypes/_aixutil.py:95: cpy=input This seems redundant https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:101: next = cpy.pop(idx) This hurts my head. Perhaps rewrite it with a loop iterating by index: idx = 0 while idx < len(input): line = input[idx] idx += 1 ... next = input[idx] idx += 1 or use a shared iterator: i = iter(input) for line in i: ... next_line = next(i) https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:105: if not "not" in next: "not" not in next https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:106: list.append(line[line.find("["):line.rfind(":")]) (r)index would be more robust than (r)find, which can return -1 https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:107: return(list) Don’t use brackets on return statements https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:111: matches = [m.group(0) for line in lines for m in [re.search(expr, line)] if m] Please rewrite this without the confusing second “for loop”. I guess it is equivalent to something like matches = list() for line in lines: m = re.search(expr, line) if m: matches.append(m.group(0)) Also see the get_version() function https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:115: member = match[match.find("[")+1:match.find("]")] Since you’re already using a regular expression, you may as well use it to extract this string. Something like expr = r"\[(%s\.so)\]" m = re.search(expr, line) member = m.group(1) https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:132: # '\[%s_*64\.so\]' % name, -> has either _64 or 64 added to name Question mark for optional single underscore: %s_?64\.so https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:134: # '\[%s.so.[0-9]+.*\]' % name, -> needs versioning (when no version was specified) This has new unescaped dots (.), in addition to the ones in previous versions. Why are they needed here as well? https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:136: # '\[shr4*_*64.o\]']: -> legacy AIX names: shr_64.o, shr_64.o, shr64.o Please explain if the first 4 (and underscore) are to be repeated https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:138: '\[%s_*64\.so\]' % name, Please go back to using raw strings: r'\[. . .\]'. There should be no practical differences the string is just used as a regular expression, but it is clearer to read (because Python has no string escape sequence \[ etc). https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:141: '\[shr4*_*64.o\]']: IMO it would be easier to read with the list of REs not interfering with the indentation so much. It looks like the loop body is hanging off the last shr4* expression. Some options that come to mind: exprs = [ ..., r'. . .'] for expr in exprs: member = ... for expr in [ ..., r'. . .', ]: member = ... for expr in [ ..., r'. . .']: member = ... Also, why not move the comments closer to the actual strings, rather than duplicating the strings and encouraging errors? https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:161: expr = '%s[_64]*.so.[0-9]+[0-9\.]*' % name This looks suspicious. At a minimum it needs clarifying what it is trying to do. I think the end bit could be simplified to [0-9][0-9\.]* https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:178: '\[%s\.so\]' % name]: Redundant “for” loop with only one iteration https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:196: return None Would make more sense indented under “else:” https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:203: skipping = True; semicolons like this are not needed (there is another instance too) https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:230: paths = parts[0] Just write this as paths = name[0:pathe] etc https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:234: if name.find("lib") != 0: if not name.startswith("lib") https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:241: _name = parts[1] Please find more distinctive names for the “name” variables https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:256: if name.rfind("/") > 0: I presume you are just trying to check for the same condition as find_parts(). Maybe set a flag? Otherwise it looks like you are checking for find_library("/NAME"), but I suspect this is irrelevant. https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:258: shlib = "%s/%s.a(%s)" % (parts[0], _base, member) I’m confused, is parts[0] a list of search paths, or a single path? https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:282: if _name.find(".so") > 0: exact = _name.find(".so") > 0 https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcod... Lib/ctypes/_aixutil.py:291: if exact == 1: if exact: https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/test/test_loading.py File Lib/ctypes/test/test_loading.py (right): https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/test/test_loading.... Lib/ctypes/test/test_loading.py:30: @unittest.skipUnless(sys.platform.startswith ("aix"), Drop the space between startswith ("aix") https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/test/test_loading.... Lib/ctypes/test/test_loading.py:38: self.assertRaises(OSError, CDLL, self.unknowndll) This duplicates test_load(). If that test is skipped for you, perhaps either update the code for libc_name, or pull it out into a new method, test_load_unknown() or something.
Sign in to reply to this message.
Sigh, again had some old comments that stayed as Draft - and a new comment I hope goes out now. https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py File Lib/ctypes/aixutil.py (right): https://bugs.python.org/review/26439/diff/17194/Lib/ctypes/aixutil.py#newcode82 Lib/ctypes/aixutil.py:82: expr = r'\[lib%s\.so\.[0-9]+.*\]' % stem This line should be: expr = 'lib%s.so.[0-9]+[0-9\.]*' % stem https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:359: # adding RTLD_NOW is already 'forced' in Modules/_ctypes/callproc.c On 2016/06/20 08:43:57, vadmium wrote: > I think this would be better off in the documentation of the “mode” parameter > (i.e. > <https://docs.python.org/3.5/library/ctypes.html#loading-shared-libraries>). It > is not specific to AIX, and applies to people using the module, not just reading > the source code. No idea - yet - of how to edit documentation. Sorry. https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py File Lib/ctypes/_aixutil.py (right): https://bugs.python.org/review/26439/diff/17689/Lib/ctypes/_aixutil.py#newcode21 Lib/ctypes/_aixutil.py:21: # Much code is written as: CDLL(find_library("libFOO")) On 2016/06/20 08:43:57, vadmium wrote: > Can you point to an example? This seems like an AIX-specific quirk, so I doubt > there would be much code using this form. I wrote these comments "early", and misread code. So, this is "what I thought I saw" - after review, months later, this is not the case - it is not an AIXism - is my being cross-eyed. I have - for the 2.7 patch also submitted, largely re-written this file to address as much as I was able - your other comments in this file. I shall copy that file to a new Python3.6 and test, and submit. I apologize in advance that I will still not have learned how to use Mercurial.
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/__init__.py File Lib/ctypes/__init__.py (right): https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:342: When the name contains ".a(" and ends with ")", asin "libFOO.a(libFOO.so)" as in [two words] https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/__init__.py#newcod... Lib/ctypes/__init__.py:346: if name and name.endswith(")") and name.rfind(".a(") > 0: You can write the last term as ... and ".a(" in name https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py File Lib/ctypes/_aix.py (right): https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode41 Lib/ctypes/_aix.py:41: lines=[line.strip() for line in p.stdout.readlines() if line.strip()] Iterating over a file is a more efficient version of readlines() anyway (it reads line by line rather than storing all lines in memory at once). You could write ... for line in p.stdout if ... https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode44 Lib/ctypes/_aix.py:44: # p.wait() ## comment rather than risk blocking while waiting It is actually better to enable this in your current code. Otherwise, when the child becomes a zombie it will not necessarily be cleaned up. Originally my complaint was that you waited for the child to exit, and _then_ read its output. Now, you read the output first, so there is no deadlock. However if you are happy to have this code only work with Python 3, the close() and wait() calls could be simplified by using a “with” statement if you like: with subprocess.Popen(...) as p: lines = ... return lines https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode49 Lib/ctypes/_aix.py:49: # i.e., is striping information re: static archive members stripping [needs double P] https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode125 Lib/ctypes/_aix.py:125: This routine addresses specific issues for 64-bit leagacy issues legacy [drop the extra A] https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode147 Lib/ctypes/_aix.py:147: # CHECKME - superfluous due to find_library() convention? As I understand it, this may be relevant for find_library(). On Linux, I can have two versions of a library installed, say libXXX.so.1 and libXXX.so.2. When I install a package for compiling against this library, I would normally get a symbolic link libXXX.so -> libXXX.so.2 The compiler, and find_library(), should follow the plain symbolic link libXXX.so to determine which version to use. Not sure how that corresponds to AIX though. https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode174 Lib/ctypes/_aix.py:174: # member names in in the dumpH output are between square brackets doubled word: in in https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode178 Lib/ctypes/_aix.py:178: Given an list of members find and return the most appropriate result Given [a] list . . . https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode208 Lib/ctypes/_aix.py:208: Looking at LIBPATH is an enhancement, and as issue9998 is still undecided, it is commented out I think it is reasonable to enable this in the next version of Python (3.6 or whatever). And Issue 9998 for the Linux equivalent was committed for 3.6. https://bugs.python.org/review/26439/diff/18236/Modules/_ctypes/_ctypes.c File Modules/_ctypes/_ctypes.c (right): https://bugs.python.org/review/26439/diff/18236/Modules/_ctypes/_ctypes.c#new... Modules/_ctypes/_ctypes.c:5507: * maybe this should be added in the interface as well? I don’t see why it should. It is always added by the ctypes implementation anyway. As I understand it, the complement to RTLD_NOW is RTLD_LAZY, which ctypes does not support. Anyway, I fail to see why this is specific to AIX.
Sign in to reply to this message.
Will update patch asap. Thank you for your careful (proof) reading! https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py File Lib/ctypes/_aix.py (right): https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode41 Lib/ctypes/_aix.py:41: lines=[line.strip() for line in p.stdout.readlines() if line.strip()] On 2016/08/27 09:11:16, vadmium wrote: > Iterating over a file is a more efficient version of readlines() anyway (it > reads line by line rather than storing all lines in memory at once). You could > write > > ... for line in p.stdout if ... I'll review. Originally, I was unsure whether I would need the output more than once and I did not want to repeat the subprocess unnecessarily. Further. since the action on "this" line sometimes depends on the "next" line having it in memory was easier for me. I would be surprised if there is not a more elegant solution. https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode44 Lib/ctypes/_aix.py:44: # p.wait() ## comment rather than risk blocking while waiting On 2016/08/27 09:11:16, vadmium wrote: > It is actually better to enable this in your current code. Otherwise, when the > child becomes a zombie it will not necessarily be cleaned up. > Comment is easily removed :). And my personal preference is one file for both Python2 and Python3. But if the changes are minor, and functionally better (Python3 subprocess e.g., does have some advantages, e.g., stderr redirection) - I may reconsider. > Originally my complaint was that you waited for the child to exit, and _then_ > read its output. Now, you read the output first, so there is no deadlock. > > However if you are happy to have this code only work with Python 3, the close() > and wait() calls could be simplified by using a “with” statement if you like: > > with subprocess.Popen(...) as p: > lines = ... > return lines https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode49 Lib/ctypes/_aix.py:49: # i.e., is striping information re: static archive members On 2016/08/27 09:11:16, vadmium wrote: > stripping [needs double P] thx. https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode147 Lib/ctypes/_aix.py:147: # CHECKME - superfluous due to find_library() convention? On 2016/08/27 09:11:16, vadmium wrote: > As I understand it, this may be relevant for find_library(). On Linux, I can > have two versions of a library installed, say libXXX.so.1 and libXXX.so.2. When > I install a package for compiling against this library, I would normally get a > symbolic link > > libXXX.so -> libXXX.so.2 > > The compiler, and find_library(), should follow the plain symbolic link > libXXX.so to determine which version to use. > > Not sure how that corresponds to AIX though. Using openssl libraries as a template: the convention WAS to have a member that ended without a version number that was a copy of the versioned one. For years, libssl.so was a copy of libssl.so.0.9.8 while libssl.1.0.0 was also available. So, this find_library looks first for a member ending in .so and uses that if available. If not, it looks for the latest version - as usually, the latest version is what a symbolic link is pointing at. Faced with dilemma I had added "things" that did not fit in the find_library("xxx") mode. So now, just as for any other platform, if the default is not going to work for "your code" then you must hardcode the string dlopen is expecting, e.g., "libssl.a(libssl.so.0.9.8)" https://bugs.python.org/review/26439/diff/18236/Lib/ctypes/_aix.py#newcode208 Lib/ctypes/_aix.py:208: Looking at LIBPATH is an enhancement, and as issue9998 is still undecided, it is commented out On 2016/08/27 09:11:16, vadmium wrote: > I think it is reasonable to enable this in the next version of Python (3.6 or > whatever). And Issue 9998 for the Linux equivalent was committed for 3.6. Another reason for two versions. I am convinced! :)
Sign in to reply to this message.
re: RTLD_NOW definition. If you feel it is unneeded, remove it. My simple thought is that adding it prevent any future issues of it being documented in include files, but not in _ctypes - just as RTLD_MEMBER has been defined as far back as I could look (no AIX 4.3 easily available), so just year 2000 and AIX 5.1 include files. https://bugs.python.org/review/26439/diff/18236/Modules/_ctypes/_ctypes.c File Modules/_ctypes/_ctypes.c (right): https://bugs.python.org/review/26439/diff/18236/Modules/_ctypes/_ctypes.c#new... Modules/_ctypes/_ctypes.c:5507: * maybe this should be added in the interface as well? On 2016/08/27 09:11:16, vadmium wrote: > I don’t see why it should. It is always added by the ctypes implementation > anyway. As I understand it, the complement to RTLD_NOW is RTLD_LAZY, which > ctypes does not support. Anyway, I fail to see why this is specific to AIX. I commented - maybe it should be added - not because it is specific to AIX - more because it is hard-coded into 'mode' elsewhere (if I remember a earlier comment correctly). However, iirc, AIX expects RTLD_NOW to be set, and ignores RTLD_LAZY if both are set. In any case, on AIX they are different bits. Bottom line: if you feel there is no need, either leave as comment only, or perhaps better - remove.
Sign in to reply to this message.
Mostly working, one import too many had me chasing ghosts! http://bugs.python.org/review/26439/diff/18714/Lib/ctypes/_aix.py File Lib/ctypes/_aix.py (right): http://bugs.python.org/review/26439/diff/18714/Lib/ctypes/_aix.py#newcode15 Lib/ctypes/_aix.py:15: from . import util This line causes the module to lose it's way, needs to be deleted. ../../python util.py ends with: Traceback (most recent call last): File "util.py", line 102, in <module> import ctypes._aix as aix File "/data/prj/python/python-3.6.0.177/Lib/ctypes/_aix.py", line 15, in <module> from . import util File "/data/prj/python/python-3.6.0.177/Lib/ctypes/util.py", line 102, in <module> import ctypes._aix as aix AttributeError: module 'ctypes' has no attribute '_aix'
Sign in to reply to this message.
https://bugs.python.org/review/26439/diff/18714/Lib/ctypes/_aix.py File Lib/ctypes/_aix.py (right): https://bugs.python.org/review/26439/diff/18714/Lib/ctypes/_aix.py#newcode15 Lib/ctypes/_aix.py:15: from . import util On 2016/10/04 22:02:10, Michael.Felt wrote: > This line causes the module to lose it's way, needs to be deleted. > > ../../python util.py ends with: > Traceback (most recent call last): > File "util.py", line 102, in <module> > import ctypes._aix as aix > File "/data/prj/python/python-3.6.0.177/Lib/ctypes/_aix.py", line 15, in > <module> > from . import util > File "/data/prj/python/python-3.6.0.177/Lib/ctypes/util.py", line 102, in > <module> > import ctypes._aix as aix > AttributeError: module 'ctypes' has no attribute '_aix' Sorry about that. Your fix of moving _last_version() into a separate common file is probably as good as any.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
