FlorianRhiem / pyGLFW

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

visual studio redistributable #36

Closed christopherhesse closed 4 years ago

christopherhesse commented 4 years ago

Currently, the windows version of glfw needs to have the VC redistributable installed or else it fails with an error when you attempt to use it. Would you be willing to redistribute the DLL inside the windows wheel?

For the x64 version this looks like copying msvcr110.dll from c:\windows\system32 to the package directory, then modifying this code: https://github.com/FlorianRhiem/pyGLFW/blob/master/glfw/library.py#L161 to look something like this:

elif sys.platform == 'win32':
    # try glfw3.dll using windows search paths
    try:
        glfw = ctypes.CDLL('glfw3.dll')
    except OSError:
        # try glfw3.dll in package directory
        try:
            glfw = ctypes.CDLL(os.path.join(os.path.abspath(os.path.dirname(__file__)), 'glfw3.dll'))
        except OSError:
            # try glfw3.dll with included VC redistributable
            os.environ["PATH"] = os.environ["PATH"] + ";" + os.path.abspath(os.path.dirname(__file__))
            try:
                glfw = ctypes.CDLL('glfw3.dll')
            except OSError:
                glfw = None

It looks like you are explicitly allowed to redistribute the redistributable DLLs: https://docs.microsoft.com/en-us/cpp/windows/determining-which-dlls-to-redistribute?view=vs-2019

christopherhesse commented 4 years ago

Possibly you could just load the msvcr110.dll first, rather than messing with PATH.

FlorianRhiem commented 4 years ago

I'll have to look into this. I was under the impression that I'd only be allowed to distribute the redistributable installer and I don't want a Python package installation (possible in a virtual environment) to trigger something that impacts the whole system. If I can just ship the required DLL as part of the wheel, that might help some users.

christopherhesse commented 4 years ago

Yeah, that would be great! I would like to use this library more but I need it to be pip install-able in a cross-platform setting.

This is a separate issue, but do you have any interest in including a linux .so file as a backup if the user does not have glfw installed? I think I could make a dockerfile that would produce the desired shared library since glfw does not publish one.

FlorianRhiem commented 4 years ago

I've added the Visual C++ runtime libraries to the repo on a branch and include them in the Windows wheels, so that the library loader can try loading the required runtime before loading the GLFW shared library. Could you please check that these wheels work for your use case? I've tried them on my Windows system and I'm fairly confident that they work, but even after uninstalling the redistributable packages some runtime libraries still seemed to remain. dist.zip

My experience with distributing binaries for Linux is that as soon as you have dependencies beyond the standard library, the various distributions become less and less compatible. For a fallback library this isn't as crucial, as a library that works on some or most systems is still much better than not having a library at all, but there are some use cases which can't really be covered this way, unless I'd include a wide variety of libraries for Linux. I think a CentOS 6 / manylinux1 based library for X11 window creation without Vulkan support would be best there, as it'd mean that X11 will be the only big dependency. What are your thoughts on that?

christopherhesse commented 4 years ago

Thanks! I tried it out and it seemed fine. If you do this route instead of modifying the PATH variable, I think it will prefer the glfw dll over the system one if it exists, but I'm not sure if this matters in practice.

manylinux1 or manylinux2010 seems like the way to go, I forgot that this depends on X11, which is unfortunate.

Here's the dependencies for the conda-forge glfw binary (from https://anaconda.org/conda-forge/glfw/files) for reference:

ldd /opt/conda/lib/libglfw.so
        linux-vdso.so.1 =>  (0x00007ffdaa9f1000)
        librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007f8b9f818000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f8b9f50f000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f8b9f30b000)
        libX11.so.6 => /opt/conda/lib/./libX11.so.6 (0x00007f8b9fa8d000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f8b9f0ee000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f8b9ed24000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f8b9fa20000)
        libxcb.so.1 => /opt/conda/lib/././libxcb.so.1 (0x00007f8b9fa60000)
        libXau.so.6 => /opt/conda/lib/./././libXau.so.6 (0x00007f8b9fa5b000)
        libXdmcp.so.6 => /opt/conda/lib/./././libXdmcp.so.6 (0x00007f8b9fa52000)
christopherhesse commented 4 years ago

Out of those dependencies, it looks like auditwheel would likely want to vendor the following:

        libxcb.so.1 => /opt/conda/lib/././libxcb.so.1 (0x00007f8b9fa60000)
        libXau.so.6 => /opt/conda/lib/./././libXau.so.6 (0x00007f8b9fa5b000)
        libXdmcp.so.6 => /opt/conda/lib/./././libXdmcp.so.6 (0x00007f8b9fa52000)

based on https://github.com/pypa/auditwheel/blob/8b255b5cf09365d2fd4ab770996005df05a62b63/auditwheel/policy/policy.json#L19

It's not clear to me if those should be vendored or not.

FlorianRhiem commented 4 years ago

If you do this route instead of modifying the PATH variable, I think it will prefer the glfw dll over the system one if it exists, but I'm not sure if this matters in practice.

The bundled runtime DLL will only be loaded if loading a system GLFW has failed (raised an OSError), so that should be fine. I've released this as 1.8.6.

FlorianRhiem commented 4 years ago

Those libraries you mention aren't in the PEP 571 whitelist, but unlike packages which truly require a shared library to work, for glfw having a shared library that works for most people would probably still be preferable to the current status where users absolutely have to install glfw themselves. So I think it will still be worth a try. I'll run a few tests with GLFW libraries built on manylinux1 and manylinux2010.

christopherhesse commented 4 years ago

The bundled runtime DLL will only be loaded if loading a system GLFW has failed (raised an OSError), so that should be fine. I've released this as 1.8.6.

Sorry, I meant it will prefer the glfw msvcr110.dll instead of c:\windows\system32\msvcr110.dll, it's possible these are two different versions. Like I said though, I'm not certain it matters. I remember issues with loading two copies of Qt shared librares into a process before, but not sure how windows handles this case.

christopherhesse commented 4 years ago

Those libraries you mention aren't in the PEP 571 whitelist, but unlike packages which truly require a shared library to work, for glfw having a shared library that works for most people would probably still be preferable to the current status where users absolutely have to install glfw themselves. So I think it will still be worth a try. I'll run a few tests with GLFW libraries built on manylinux1 and manylinux2010.

Awesome, thanks! Should I file a new issue rather than keep this one open?

FlorianRhiem commented 4 years ago

I would expect it to only prefer the bundled runtime if it got loaded explicitly or if its directory somehow ended up in the path, and I don't see how that would happen.

Awesome, thanks! Should I file a new issue rather than keep this one open?

I'm fine with either.

christopherhesse commented 4 years ago

I think it would only happen if the user loaded some other python extension that depended on msvcr110.dll before loading glfw.

FlorianRhiem commented 4 years ago

If you have the time, please check out the feature-manylinux2010-wheels branch, which adds the creation of wheels for manylinux2010, and the wheels built from it. The glfw libraries shipped with these wheels depend on X11 libraries which aren't part of manylinux2010, so they're not "proper" wheels, but they should still be useful and better than nothing.

christopherhesse commented 4 years ago

I tested this on a headless server and it seemed to work fine. I don't have a desktop linux machine currently.

Thanks for doing this! Being able to depend on glfw without adding per-platform install instructions is great.

FlorianRhiem commented 4 years ago

Thanks for your help! I've just uploaded v1.9.0 with wheels for linux, windows and macOS.