Merge pull request #119 from pycompression/applyisalpatches

Use a patched version of isa-l 2.30

Author: rhpvorderman

.github/workflows/ci.yml

@@ -120,33 +120,33 @@ jobs:
 
   # Test if the python-isal conda package can be build. Which is linked
   # dynamically to the conda isa-l package.
-#  test-dynamic:
-#    runs-on: ${{ matrix.os }}
-#    defaults:
-#      run:
-#        # This is needed for miniconda, see:
-#        # https://github.com/marketplace/actions/setup-miniconda#important.
-#        shell: bash -l {0}
-#    strategy:
-#      matrix:
-#        os: ["ubuntu-latest", "macos-latest", "windows-latest"]
-#    steps:
-#      - uses: actions/[email protected]
-#        with:
-#          submodules: recursive
-#      - name: Install miniconda.
-#        uses: conda-incubator/[email protected] # https://github.com/conda-incubator/setup-miniconda.
-#        with:
-#          channels: conda-forge,defaults
-#      - name: Install requirements (universal)
-#        run: conda install isa-l python tox
-#      - name: Set MSVC developer prompt
-#        uses: ilammy/[email protected]
-#        if: runner.os == 'Windows'
-#      - name: Run tests (dynamic link)
-#        run: tox
-#        env:
-#          PYTHON_ISAL_LINK_DYNAMIC: True
+  test-dynamic:
+    runs-on: ${{ matrix.os }}
+    defaults:
+      run:
+        # This is needed for miniconda, see:
+        # https://github.com/marketplace/actions/setup-miniconda#important.
+        shell: bash -l {0}
+    strategy:
+      matrix:
+        os: ["ubuntu-latest", "macos-latest", "windows-latest"]
+    steps:
+      - uses: actions/[email protected]
+        with:
+          submodules: recursive
+      - name: Install miniconda.
+        uses: conda-incubator/[email protected] # https://github.com/conda-incubator/setup-miniconda.
+        with:
+          channels: conda-forge,defaults
+      - name: Install requirements (universal)
+        run: conda install isa-l python tox
+      - name: Set MSVC developer prompt
+        uses: ilammy/[email protected]
+        if: runner.os == 'Windows'
+      - name: Run tests (dynamic link)
+        run: tox
+        env:
+          PYTHON_ISAL_LINK_DYNAMIC: True
 
   deploy:
     if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')

.gitmodules

@@ -1,3 +1,3 @@
 [submodule "src/isal/isa-l"]
 	path = src/isal/isa-l
-	url = https://github.com/intel/isa-l.git
+	url = https://github.com/rhpvorderman/isa-l.git

CHANGELOG.rst

@@ -37,14 +37,15 @@ that PyPy is no longer supported.
   IgzipDecompressor.decompress has been updated to allocate ``maxsize`` buffers
   when these are of reasonable size, instead of growing the buffer to maxsize
   on every call. This has improved gzip decompression speeds by 7%.
-+ ISA-L library version variables are now available on windows as well.
-+ Wheels are now always build with nasm for the x86 architecture. Previously
-  yasm was used for Linux and MacOS due to build issues that have since been
-  fixed upstream.
-+ Updated statically included ISA-L to version 2.31.0. This fixes the bug
-  mentioned below in python-isal as well.
-+ Fixed a bug upstream in ISA-L were zlib headers would be created with an
-  incorrect wbits value.
++ Patch statically linked included library (ISA-L 2.30.0) to fix the following:
+
+  + ISA-L library version variables are now available on windows as well,
+    for the statically linked version available on PyPI.
+  + Wheels are now always build with nasm for the x86 architecture.
+    Previously yasm was used for Linux and MacOS due to build issues.
+  + Fixed a bug upstream in ISA-L were zlib headers would be created with an
+    incorrect wbits value.
+
 + Python-isal shows up in Python profiler reports.
 + Support and tests for Python 3.10 were added.
 + Due to a change in the deployment process wheels should work for older

setup.py

@@ -32,6 +32,17 @@
 BUILD_CACHE_FILE = Path(os.environ.get("PYTHON_ISAL_BUILD_CACHE_FILE",
                                        DEFAULT_CACHE_FILE))
 
+EXTENSIONS = [
+        Extension("isal.isal_zlib", ["src/isal/isal_zlibmodule.c"]),
+        Extension("isal.igzip_lib", ["src/isal/igzip_libmodule.c"]),
+    ]
+
+# This does not add the extension on windows for dynamic linking. The required
+# header file might be missing.
+if not (SYSTEM_IS_WINDOWS and
+        os.getenv("PYTHON_ISAL_LINK_DYNAMIC") is not None):
+    EXTENSIONS.append(Extension("isal._isal", ["src/isal/_isalmodule.c"]))
+
 
 class BuildIsalExt(build_ext):
     def build_extension(self, ext):
@@ -186,9 +197,5 @@ def build_isa_l(compiler_command: str, compiler_options: str):
         "Operating System :: Microsoft :: Windows",
     ],
     python_requires=">=3.7",  # We use METH_FASTCALL
-    ext_modules=[
-        Extension("isal.isal_zlib", ["src/isal/isal_zlibmodule.c"]),
-        Extension("isal.igzip_lib", ["src/isal/igzip_libmodule.c"]),
-        Extension("isal._isal", ["src/isal/_isalmodule.c"])
-    ]
+    ext_modules=EXTENSIONS
 )

src/isal/__init__.py

@@ -5,13 +5,19 @@
 # This file is part of python-isal which is distributed under the
 # PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2.
 
-from . import _isal
-
-ISAL_MAJOR_VERSION: int = _isal.ISAL_MAJOR_VERSION
-ISAL_MINOR_VERSION: int = _isal.ISAL_MINOR_VERSION
-ISAL_PATCH_VERSION: int = _isal.ISAL_PATCH_VERSION
-ISAL_VERSION: str = _isal.ISAL_VERSION
+from typing import Optional
 
+try:
+    from . import _isal
+    ISAL_MAJOR_VERSION: Optional[int] = _isal.ISAL_MAJOR_VERSION
+    ISAL_MINOR_VERSION: Optional[int] = _isal.ISAL_MINOR_VERSION
+    ISAL_PATCH_VERSION: Optional[int] = _isal.ISAL_PATCH_VERSION
+    ISAL_VERSION: Optional[str] = _isal.ISAL_VERSION
+except ImportError:
+    ISAL_MAJOR_VERSION = None
+    ISAL_MINOR_VERSION = None
+    ISAL_PATCH_VERSION = None
+    ISAL_VERSION = None
 
 __all__ = [
     "ISAL_MAJOR_VERSION",

src/isal/isa-l

@@ -11,6 +11,7 @@
 
 import gzip
 import itertools
+import os
 import zlib
 from pathlib import Path
 
@@ -34,6 +35,8 @@
 # Wbits for ZLIB compression, GZIP compression, and RAW compressed streams
 WBITS_RANGE = list(range(9, 16)) + list(range(25, 32)) + list(range(-15, -8))
 
+DYNAMICALLY_LINKED = os.getenv("PYTHON_ISAL_LINK_DYNAMIC") is not None
+
 
 @pytest.mark.parametrize(["data_size", "value"],
                          itertools.product(DATA_SIZES, SEEDS))
@@ -90,6 +93,8 @@ def test_decompress_isal_zlib(data_size, level):
 @pytest.mark.parametrize(["data_size", "level", "wbits", "memLevel"],
                          itertools.product([128 * 1024], range(4),
                                            WBITS_RANGE, range(1, 10)))
+@pytest.mark.xfail(condition=DYNAMICALLY_LINKED,
+                   reason="Dynamically linked version may not have patch.")
 def test_compress_compressobj(data_size, level, wbits, memLevel):
     data = DATA[:data_size]
     compressobj = isal_zlib.compressobj(level=level,

tests/test_compat.py

@@ -41,6 +41,9 @@
 
 class VersionTestCase(unittest.TestCase):
 
+    @unittest.skipIf(os.getenv("PYTHON_ISAL_LINK_DYNAMIC") is not None and
+                     sys.platform.startswith("win"),
+                     "Header file missing on windows")
     def test_library_version(self):
         # Test that the major version of the actual library in use matches the
         # major version that we were compiled against. We can't guarantee that