Package SDL2 with ale-py

This commit introduces the necesarry changes to package SDL with ale-py.
Most notably we no longer package the native shared library as the entire
package, but now have a Python entry point so we can setup some infrastructure
around search paths for dynamic libraries.

We also introduce the necesarry changes for our CI infrastructure to
build / test / bundle SDL with ale-py releases.

Author: JesseFarebro

.github/workflows/build-ale-py.yml

@@ -14,9 +14,9 @@ jobs:
     strategy:
       matrix:
         config: [
-          {"os": "ubuntu-latest", "deps": ["zlib"], "triplet": "x64-linux"},
-          {"os": "windows-latest", "deps": ["zlib"], "triplet": "x64-windows"},
-          {"os": "macos-latest", "deps": ["zlib"], "triplet": "x64-osx"}
+          {"os": "ubuntu-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-linux-mixed"},
+          {"os": "windows-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-windows"},
+          {"os": "macos-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-osx-mixed"}
         ]
     runs-on: ${{ matrix.config.os }}
     steps:
@@ -34,17 +34,19 @@ jobs:
     - name: Build wheels
       uses: pypa/[email protected]
       env:
-        CIBW_MANYLINUX_X86_64_IMAGE: "ghcr.io/mgbellemare/manylinux2014-vcpkg"
-        CIBW_SKIP: "*i686* *win32*"
+        CIBW_MANYLINUX_X86_64_IMAGE: "ghcr.io/jessefarebro/manylinux2014-vcpkg"
+        CIBW_SKIP: "*i686* *win32* pp*"
+        CIBW_ARCHS_MACOS: "x86_64"
         CIBW_TEST_REQUIRES: "pytest"
         CIBW_TEST_COMMAND: "pytest {project}"
-        CIBW_ENVIRONMENT: "ALE_BUILD_VERSION=${{ env.ALE_BUILD_VERSION }} VCPKG_DEFAULT_TRIPLET=${{ matrix.config.triplet }}"
-        CIBW_BEFORE_ALL: "vcpkg install ${{ join(matrix.config.deps, ' ') }} --triplet ${{ matrix.config.triplet }}"
+        CIBW_ENVIRONMENT: "ALE_BUILD_VERSION=${{ env.ALE_BUILD_VERSION }} MACOSX_DEPLOYMENT_TARGET='10.15' VCPKG_DEFAULT_TRIPLET=${{ matrix.config.triplet }}"
+        CIBW_BEFORE_ALL: "vcpkg install ${{ join(matrix.config.deps, ' ') }} --triplet ${{ matrix.config.triplet }} --overlay-triplets cmake/custom-triplets --feature-flags=-compilertracking"
 
-    - uses: actions/upload-artifact@v1
+    - name: Upload wheels
+      uses: actions/upload-artifact@v2
       with:
         name: wheels
-        path: wheelhouse/*.whl
+        path: ./wheelhouse/*.whl
 
 
   pypi:

.github/workflows/build-ale.yml

@@ -27,9 +27,9 @@ jobs:
     strategy:
       matrix:
         config: [
-          {"os": "ubuntu-latest", "deps": ["zlib"], "triplet": "x64-linux"},
-          {"os": "windows-latest", "deps": ["zlib"], "triplet": "x64-windows"},
-          {"os": "macos-latest", "deps": ["zlib"], "triplet": "x64-osx"}
+          {"os": "ubuntu-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-linux-mixed"},
+          {"os": "windows-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-windows"},
+          {"os": "macos-latest", "deps": ["zlib", "sdl2"], "triplet": "x64-osx-mixed"}
         ]
     env:
       VCPKG_DEFAULT_TRIPLET: ${{ matrix.config.triplet }}
@@ -61,7 +61,7 @@ jobs:
     - name: Install vcpkg dependencies
       if: steps.vcpkg-cache.outputs.cache-hit != 'true'
       run: |
-        vcpkg install ${{ join(matrix.config.deps, ' ') }} --triplet ${{ matrix.config.triplet }}
+        vcpkg install ${{ join(matrix.config.deps, ' ') }} --overlay-triplets cmake/custom-triplets --triplet ${{ matrix.config.triplet }}
 
     - name: Get pip cache info
       id: pip-cache-info
@@ -82,8 +82,9 @@ jobs:
 
     - name: Configure ALE
       run: |
+        cmake --version
         mkdir build; cd build
-        cmake ../ -DUSE_SDL=OFF -DPython3_EXECUTABLE=$(which python)
+        cmake ../ -DSDL_SUPPORT=ON -DPython3_EXECUTABLE=$(which python)
 
     - name: Build ALE
       working-directory: ./build

cmake/custom-triplets/x64-linux-mixed.cmake

@@ -0,0 +1,10 @@
+set(VCPKG_TARGET_ARCHITECTURE x64)
+if(PORT MATCHES "sdl")
+  set(VCPKG_LIBRARY_LINKAGE dynamic)
+  set(VCPKG_CRT_LINKAGE dynamic)
+else()
+  set(VCPKG_LIBRARY_LINKAGE static)
+  set(VCPKG_CRT_LINKAGE static)
+endif()
+
+set(VCPKG_CMAKE_SYSTEM_NAME Linux)

cmake/custom-triplets/x64-osx-mixed.cmake

@@ -0,0 +1,14 @@
+set(VCPKG_TARGET_ARCHITECTURE x64)
+if(PORT MATCHES "sdl")
+  set(VCPKG_LIBRARY_LINKAGE dynamic)
+  set(VCPKG_CRT_LINKAGE dynamic)
+else()
+  set(VCPKG_LIBRARY_LINKAGE static)
+  set(VCPKG_CRT_LINKAGE static)
+endif()
+
+set(VCPKG_CMAKE_SYSTEM_NAME Darwin)
+
+if(DEFINED ENV{MACOSX_DEPLOYMENT_TARGET})
+  set(VCPKG_OSX_DEPLOYMENT_TARGET "$ENV{MACOSX_DEPLOYMENT_TARGET}")
+endif()

setup.cfg

@@ -26,7 +26,12 @@ project_urls =
 [options]
 python_requires = >=3.6
 zip_safe = False
+packages =
+    ale_py
+package_dir =
+    ale_py = src/python
 install_requires =
     numpy
+    importlib_metadata;python_version<'3.8'
 tests_require =
     pytest

setup.py

@@ -49,6 +49,8 @@ def build_extension(self, ext):
             f"-DCMAKE_BUILD_TYPE={config}",
             f"-DPython3_EXECUTABLE={sys.executable}",
             f"-DCMAKE_LIBRARY_OUTPUT_DIRECTORY={extdir}",
+            "-DSDL_SUPPORT=ON",
+            "-DSDL_DYNLOAD=ON",
             "-DBUILD_CPP_LIB=OFF",
             "-DBUILD_PYTHON_LIB=ON"
         ]
@@ -137,6 +139,6 @@ def parse_version(version_file):
     setup(
         version=parse_version('version.txt'),
         distclass=CMakeDistribution,
-        ext_modules=[CMakeExtension("ale_py")],
+        ext_modules=[CMakeExtension("ale_py._ale_py")],
         cmdclass={"build_ext": CMakeBuild}
     )

src/CMakeLists.txt

@@ -78,7 +78,7 @@ if (BUILD_PYTHON_LIB)
   add_library(ale-py MODULE ale_interface.cpp ale_python_interface.cpp)
   target_link_libraries(ale-py PUBLIC ale)
   target_link_libraries(ale-py PRIVATE pybind11::module)
-  set_target_properties(ale-py PROPERTIES OUTPUT_NAME ale_py
+  set_target_properties(ale-py PROPERTIES OUTPUT_NAME _ale_py
                                           PREFIX "${PYTHON_MODULE_PREFIX}"
                                           SUFFIX "${PYTHON_MODULE_EXTENSION}")
   # If we're dynamically loading SDL with Python we'll be building a wheel

src/ale_python_interface.hpp

@@ -68,7 +68,7 @@ class ALEPythonInterface : public ALEInterface {
 
 } // namespace ale
 
-PYBIND11_MODULE(ale_py, m) {
+PYBIND11_MODULE(_ale_py, m) {
   m.attr("__version__") = py::str(ALE_VERSION);
 #ifdef ALE_SDL_SUPPORT
   m.attr("SDL_SUPPORT") = py::bool_(true);

src/python/__init__.py

@@ -0,0 +1,41 @@
+import platform
+import sys
+import os
+
+packagedir = os.path.abspath(os.path.dirname(__file__))
+
+if platform.system() == "Windows":
+    try:
+        import ctypes
+        ctypes.CDLL('vcruntime140.dll')
+        ctypes.CDLL('msvcp140.dll')
+    except OSError:
+        raise OSError("""Microsoft Visual C++ Redistribution Pack is not installed.
+It can be downloaded from https://aka.ms/vs/16/release/vc_redist.x64.exe.""")
+
+    # Loading DLLs on Windows is kind of a disaster
+    # The best approach seems to be using LoadLibraryEx
+    # with user defined search paths. This kind of acts like
+    # $ORIGIN or @loader_path on Unix / macOS.
+    # This way we guarantee we load OUR DLLs.
+    if sys.version_info.major == 3 and sys.version_info.minor >= 8:
+        os.add_dll_directory(packagedir)
+    else:
+        # TODO: Py38: Remove AddDllDirectory
+        kernel32 = ctypes.WinDLL('kernel32.dll', use_last_error=True)
+        if hasattr(kernel32, 'AddDllDirectory'):
+            kernel32.AddDllDirectory(packagedir)
+
+# TODO Py38: Once 3.7 is deprecated use importlib.metadata to parse
+# version string from package.
+try:
+    import importlib.metadata as importlib_metadata
+except ImportError:
+    import importlib_metadata
+try:
+    __version__ = importlib_metadata.version(__package__)
+except importlib_metadata.PackageNotFoundError:
+    __version__ = "unknown"
+
+# Import native shared library
+from ._ale_py import *

tests/fixtures.py

@@ -1,6 +1,10 @@
 import pytest
 
-import ale_py
+# Try to import native library before attempting ale_py
+try:
+    import _ale_py as ale_py
+except ImportError:
+    import ale_py
 
 
 @pytest.fixture

tests/test_python_interface.py

@@ -3,7 +3,12 @@
 import pickle
 import tempfile
 import numpy as np
-import ale_py
+
+# Try to import native library before attempting ale_py
+try:
+    import _ale_py as ale_py
+except ImportError:
+    import ale_py
 
 
 def test_ale_version():