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

(Possibly hacky) adjustment to support file-magic (Possibly Fixes #3292) #3320

Merged
merged 1 commit into from Nov 27, 2020
Merged

(Possibly hacky) adjustment to support file-magic (Possibly Fixes #3292) #3320

merged 1 commit into from Nov 27, 2020

Conversation

przerull
Copy link
Contributor

So I had installed lutris using pacman on arch (so it was on 0.5.8), I hadn't realized this at first but I was experiencing issue #3281.

I was getting the "your version of python-magic is too old." message and am pretty sure that python-magic is the file-magic package in pacman.

>>> pacman -Si python-magic
Repository      : community
Name            : python-magic
Version         : 5.39-1
Description     : Python bindings to the magic library
Architecture    : any
URL             : https://darwinsys.com/file/
Licenses        : custom
Groups          : None
Provides        : None
Depends On      : python
Optional Deps   : None
Conflicts With  : None
Replaces        : None
Download Size   : 9.46 KiB
Installed Size  : 28.25 KiB
Packager        : Felix Yan <felixonmars@archlinux.org>
Build Date      : Sat 20 Jun 2020 03:55:17 AM EDT
Validated By    : MD5 Sum  SHA-256 Sum  Signature

(the URL is what leads me to believe this).

Importing magic in a python shell confirmed that from_file was not an attribute of the magic module (because I'm pretty sure I had file-magic installed). I noticed that it had a similar attribute detect_form_filename.

Please note that this is my first time interacting with libmagic.

YOLOing a bit here I thought I'd try monkey-patching it and give it a shot.

Much to my pleasure it appeared to solve my issue. (then I switched back to master and tested again to see that my shortcuts still worked, then read the changelog and saw that the real problem I was experiencing was #3281 - DOH!)

I do believe that this monkey-patch works though. In separate python shells I used both python-magic's from_file and file-magic's detect_from_filename and then did a few string in result on the results from the respective methods and noticed they behaved similarly.

Granted, this might confuse a reader of this module because from_file returns a string whereas detect_from_filename returns a magic.FileMagic object. Fortunately, magic.FileMagic objects do support the __contains__ method so lines 48 through 56 end up behaving the same (or at least similarly) in both scenarios.

I didn't see any unit tests for the find_linux_game_executable. I imagine it would be difficult to write tests against both python-magic and file-magic.

I'd love to hear what y'all think. I'm really not sure if this should be merged or not, but thought this was worthy of some discussion at least.

Thanks for your time everyone!

Copy link
Contributor

@AlexanderRavenheart AlexanderRavenheart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems okay.

MAGIC_AVAILABLE = False
if hasattr(magic, "detect_from_filename"):
magic.from_file = magic.detect_from_filename
MATIC_AVAILABLE = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo? (MATIC)

@przerull
Copy link
Contributor Author

Yes it was a typo (facepalm). Sorry about that. I made the fix, rebased, then forced push so there's only one commit.

Not sure if I was supposed to do it that way... This is my first time actually using git rebase.

@strycore strycore merged commit 116dac7 into lutris:master Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants