Opened 6 years ago
Closed 6 years ago
#17682 closed defect (fixed)
Generic filename extension for shared libraries
Reported by:  gouezel  Owned by:  

Priority:  major  Milestone:  sage6.7 
Component:  porting: Cygwin  Keywords:  
Cc:  jpflori  Merged in:  
Authors:  Sebastien Gouezel  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  ad478d3 (Commits)  Commit:  ad478d3d9ca70f98316243c3cb0800c7a9210ca7 
Dependencies:  Stopgaps: 
Description (last modified by )
Some places in the code and some doctests assume that the filename extension for shared libraries is .so
, while it is .dll
in cygwin. The attached patch defines once and for all the correct extension depending on the platform, and uses it where necessary.
Change History (21)
comment:1 Changed 6 years ago by
 Component changed from PLEASE CHANGE to porting: Cygwin
 Description modified (diff)
 Summary changed from help to Generic filename extension for shared libraries
comment:2 Changed 6 years ago by
 Branch set to u/gouezel/dll_extension
comment:3 Changed 6 years ago by
 Commit set to 02331047be37ffa47927a0b9db68fed7ef262ae2
comment:4 Changed 6 years ago by
 Status changed from new to needs_review
 Type changed from PLEASE CHANGE to defect
comment:5 Changed 6 years ago by
comment:7 Changed 6 years ago by
 Commit changed from 02331047be37ffa47927a0b9db68fed7ef262ae2 to db7df5c13b7c57272f0cc262c89d80a632c878ef
Branch pushed to git repo; I updated commit sha1. New commits:
db7df5c  Ticket #17682: add generic extension in cython caching

comment:8 followup: ↓ 9 Changed 6 years ago by
Ahem, I do not completely understand what the patch does (i'm new to sage/python);
Question: what are the consequences, e.g. for init_libsingular()
in src/sage/libs/singular/singular.pyx
?
*snip*:
... for extension in ["so", "dylib", "dll"]: lib = os.environ['SAGE_LOCAL']+"/lib/libsingular."+extension if os.path.exists(lib): handle = dlopen(lib, RTLD_GLOBALRTLD_LAZY) break ...
comment:9 in reply to: ↑ 8 ; followup: ↓ 10 Changed 6 years ago by
Replying to jakobkroeker:
Short answer: the code you show is fine, no need to change anything.
Long answer: shared libraries usually have the extension .so
on unixlikes. So, some parts of sage's code assume that this is a universal extension, and use it for loading or doctesting purposes. This leads to problems on cygwin, where the extension is .dll
. The patch fixes this code, replacing the universal .so
with an extension depending on the platform (.dll
on windows, .so
anywhere else). Code that already considered the possibility of different extensions is fine.
There is yet another difference on MacOS, where the extension for shared libraries can be .so
or .dylib
 this means that, in this case, there is no universal possible choice, so I picked .so
since it is the most common in sage. In the case of libsingular, I don't know if it is built using .so
or .dylib
(and I don't have a Mac to test). If it is .so
, one might consider using the patch to predict the correct extension, but in any case the current code works fine.
comment:10 in reply to: ↑ 9 Changed 6 years ago by
Replying to gouezel:
There is yet another difference on MacOS, where the extension for shared libraries can be
.so
or.dylib
 this means that, in this case, there is no universal possible choice
It's more subtle than that. On OS X, both .so
and .dylib
exist but with a different purpose: .so
is meant for "loadable modules" to be opened at runtime by some dlopen()
mechanism, which .dylib
is for dynamic linking of libraries (linking done when a program a started). For example, Cython extensions would use .so
but the libraries in $SAGE_LOCAL/lib
would use .dylib
.
On Linux, this difference simply doesn't exist.
comment:11 Changed 6 years ago by
 Branch changed from u/gouezel/dll_extension to u/gouezel/dll_extension2
 Commit changed from db7df5c13b7c57272f0cc262c89d80a632c878ef to 4286d976916a6a6894e51d50e37e70e8b30736e8
comment:12 Changed 6 years ago by
 Milestone changed from sage6.5 to sage6.7
 Status changed from needs_review to needs_work
I dislike the complete lack of documentation. Given that constants are hard to document, it's probably better to make it a function def ..._extension()
(fill in the ...
to what this thing represents, generic_so
doesn't mean much to me, see also 10).
comment:13 Changed 6 years ago by
Also: can you replace F[len(generic_so_extension):] == generic_so_extension
by F.endswith(extension)
.
comment:14 Changed 6 years ago by
 Commit changed from 4286d976916a6a6894e51d50e37e70e8b30736e8 to 8815433a50b356d25a0be040d29c7224d6f4b1fb
Branch pushed to git repo; I updated commit sha1. New commits:
8815433  #17682: use documented function instead of constant

comment:15 Changed 6 years ago by
 Status changed from needs_work to needs_review
Converted generic_so_extension
to a documented function shared_lib_extension()
. It is a little bit silly to recompute it every time, but given its triviality and the fact that it should never be timecritical, I don't think caching it is useful.
comment:16 Changed 6 years ago by
I think all this
sage: from site import getsitepackages sage: site_packages = getsitepackages()[0] sage: from sage_setup.find import installed_files_by_module sage: files_by_module = installed_files_by_module(site_packages) sage: from sage.misc.sageinspect import shared_lib_extension sage: files_by_module['sage.structure.sage_object'].pop().endswith(shared_lib_extension())
can be replaced by
sage: sage.structure.sage_object.__file__.endswith(shared_lib_extension())
comment:17 Changed 6 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to needs_work
Can you replace the terminology shared_lib
by loadable_module
. As I said, these are different on OS X and Cython modules are "loadable modules" and not "shared libraries" (the latter would have a .dylib
extension on OS X).
comment:18 Changed 6 years ago by
 Commit changed from 8815433a50b356d25a0be040d29c7224d6f4b1fb to ad478d3d9ca70f98316243c3cb0800c7a9210ca7
Branch pushed to git repo; I updated commit sha1. New commits:
ad478d3  #17682: change to loadable_module_extension

comment:19 Changed 6 years ago by
 Status changed from needs_work to needs_review
comment:20 Changed 6 years ago by
 Status changed from needs_review to positive_review
comment:21 Changed 6 years ago by
 Branch changed from u/gouezel/dll_extension2 to ad478d3d9ca70f98316243c3cb0800c7a9210ca7
 Resolution set to fixed
 Status changed from positive_review to closed
I put the definition of the generic filename extension in sage.misc.sageinspect, since its use is relevant for introspection. Tell me if there is a better place!
New commits:
More generic shared library file extension