search

aliles / filemagic

File type identification using libmagic
http://pypi.python.org/pypi/filemagic
Other
47 stars 10 forks source link

patch for filemagic on Solaris #2

Closed erhudy closed 11 years ago

erhudy commented 11 years ago

I was working on getting filemagic set up on some Solaris servers and found that ctypes.util.find_library does not appear to work on Solaris, even on the most recent version of Python 2.7 (doing something like ctypes.util.find_library('c') returns None when you'd obviously expect it to return something there), and thusly filemagic won't work since it doesn't know that the library does exist. I made a little patch to magic/api.py that checks the operating system and uses dl.open to test for the presence of libmagic.so if the OS is SunOS.

aliles commented 11 years ago

Oh my, this appears to be issue 5289. I've added myself to the nosey list on that particular issue.

Unfortunately, as you may have noticed, your pull request failed to buid on Travis CI. This is because the dl module has been deprecated so doesn't exist on Python 3.2 and PyPy.

What do you think about instead special casing for 'SunOS' when find_library returns None. 

libname = ctypes.util.find_library('magic')                                     
if not libname:                                                                 
    if platform.system() == 'SunOS':                                            
        libname = 'libmagic.so'                                                 
    else:                                                                       
        raise ImportError('Unable to find magic library')

One downside is that this would raise ImportError with a "Loading libmagic.so failed" rather than "Unable to find magic library" when libmagic can not be found. Perhaps a warning should be issued during the special casing?

erhudy commented 11 years ago

Shamefully, I should have noticed that dl was removed in Python 3. I think your counteroffer would be fine, since Solaris is probably not in real common use (I wish I didn't have to use it), and I will turn my attention to 5289 to see if I can do something about the underlying problem with ctypes.

aliles commented 11 years ago

Sorry, back online now. Would you like me to go ahead and make those changes or would you like test them on Solaris first and update the pull request? You might like to also create a CONTRIBUTORS file with you name in it. :)

erhudy commented 11 years ago

On Dec 17, 2012, at 6:17 AM, Aaron Iles notifications@github.com wrote:

Sorry, back online now. Would you like me to go ahead and make those changes or would you like test them on Solaris first and update the pull request?

I'll test the changes and update the pull request. You might like to also create a CONTRIBUTORS file with you name in it. :)

Haha, I didn't really do anything, but okay. — Reply to this email directly or view it on GitHub.

erhudy commented 11 years ago

I closed this pull request so I could do a new one the Proper GitHub Way (in a topic branch instead of on master).