FlorianRhiem / pyGLFW

Python bindings for GLFW
MIT License
232 stars 36 forks source link

ENH: Rework library loading on Win to add better conda support #41

Closed hoechenberger closed 4 years ago

hoechenberger commented 4 years ago

This PR reworks the library loading on Windows slightly.

I realized that glfw3.dll could not be loaded in some conda environments. glfw3.dll is installed in sys.prefix/Library/bin/ by conda. So I've added this search path.

I've also changed the library search order to (hopefully?) match the order of Linux and macOS:

Tested to be working on Python 3.6, 3.7, and 3.8.

FlorianRhiem commented 4 years ago

Hello Richard, thank you for the pull request!

I'm not sure about changing the ordering, as the Linux and macOS search directory order is far less important due to the version checking and picking that happens for those platforms. The script checks all of these directories, imports all GLFW libraries it finds, checks their versions and returns the one with the highest version. The ordering only comes into play when two different libraries have the exact same version. On Linux, if a GLFW library is found in one of the system paths it is somewhat likely to be from a package manager that ships an older version, e.g. 3.2.1 on Ubuntu 18.04LTS. This makes the version checking necessary in my eyes.

On Windows, this would instead be a fairly strict order, as the library in the package will be found and used for all installations from wheels from PyPI, the library in the conda path will be found and used for conda installations and only installations from source will fall back to the default search paths. I don't really expect the use of package managers on Windows, so If a GLFW library is found using the default search paths, it is probably because the user wanted it to be found, e.g. because they placed it in the current working directory. Of course this is just me guessing the user's intentions, but I hope it's a reasonable guess. :) As a result of this, the script checks the default search paths first and then falls back to the library shipped with the package if necessary.

So I think using the default search paths first and then falling back to the package directory and conda directory would be better, but I might be missing something or taking the wrong conclusions. What are your thoughts on this?

hoechenberger commented 4 years ago

Thanks for your detailed response! I don't have a strong opinion on this, so will change it as you proposed :)

hoechenberger commented 4 years ago

@FlorianRhiem Changed as requested

hoechenberger commented 4 years ago

Thanks for the quick response, @FlorianRhiem! Are you planning to make a new release anytime soon? :)

FlorianRhiem commented 4 years ago

Thank you, I've merged it and released the updated version as 1.9.1.

hoechenberger commented 4 years ago

Wow, awesome! You're so blazingly fast :D