Repository navigation
Keep isal_zlib importable with ISA-L 2.30, which Read the Docs uses - #273
Merged
Merged
Conversation
isal_zlib_header_init was added in ISA-L 2.31. Read the Docs builds on Ubuntu 22.04, which ships libisal 2.30, so isal_zlib failed to import there and the API documentation for isal_zlib, igzip and igzip_threaded was empty. The function only zeroes the four fields of the struct, and all four are set right after the call, so it is not needed. Run the docs check on the same Ubuntu release as Read the Docs so CI catches this kind of breakage.
ptweezy
marked this pull request as ready for review
October 18, 2026 16:35
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For the "Check latest documentation looks fine" step in #267.
On https://python-isal.readthedocs.io/en/latest/, the API documentation for
isal_zlib,igzipandigzip_threadedis empty. Read the Docs builds on Ubuntu 22.04 (build.osin.readthedocs.yml), which installs libisal 2.30.0. The log of the latest build shows:isal_zlib_header_initwas added in ISA-L 2.31. It is called by the preset dictionary fix from #257, so this started after 1.8.0. Anyone who links dynamically against ISA-L 2.30, for example on Debian bookworm or Ubuntu 22.04, gets the sameImportError. The 1.9.0 tag would have the same empty API pages.This PR:
isal_zlib_header_initcall. The function only sets the four fields ofstruct isal_zlib_headerto zero, and the next four lines set all of them. ISA-L 2.30 already has the struct andisal_write_zlib_header.docsandtwine_checkjobs onubuntu-22.04, the same release asbuild.os. Thedocsjob runssphinx-build -W, so it fails when a module cannot be imported on Read the Docs. Onubuntu-latestit got libisal 2.31, which is why CI did not catch this.Results with each build linked dynamically against the distribution's libisal, in containers:
sphinx-build -Wdevelop, Debian bookworm, CPython 3.14.8undefined symbolThe 216 failures are the same on 1.8.0 and on this PR. They are all in
test_compat.py::test_compress_compressobjwithwbits9 to 14. ISA-L 2.30 always writes a window size of 32 KiB (CINFO 7) in the zlib header, and CPython'szlib.decompressrejects that whenwbitsis smaller. So this PR does not make ISA-L 2.30 work fully. It restores the 1.8.0 behavior, so the modules import and the documentation builds.If you would rather require ISA-L 2.31, setting
build.ostoubuntu-24.04(libisal 2.31.0) also fixes Read the Docs, but dynamic builds against 2.30 would still fail at import.There is no CHANGELOG entry, because no release has this problem.
Checklist