Repository navigation
Conversation
Since 1.8 replaced PyNaCl, segment encryption and decryption hold the GIL, so threads run one at a time. Release it around the libsodium calls. The Py_buffer views keep the buffers alive and unresizable meanwhile. Also reject keys that are not 32 bytes, and return NULL when the key exchange fails or kx_client gets bad arguments. Before, these paths returned a value with an exception set, which Python reported as SystemError.
The testsuite only ran on pushes, so pull requests from forks got no CI. Run it on pull requests and on demand too, with a read-only token and without persisting credentials in the checkout. A newer push cancels the older runs of the same branch or pull request.
The testsuite encrypts and decrypts with the same code. A bug that both directions share, like a wrong hash in the key exchange, passes every test but breaks all existing files. Decrypt two files that the released 1.8.6 wrote, one sent with a crypt4gh key and one with an ssh key. Checking the sender also covers the conversion of ssh public keys.
--enable-opt adds -march=native -mtune=native. The release wheels are built on GitHub runners, so their generic code paths (BLAKE2b, SHA-512, the curve25519 field arithmetic) require the runner's CPU features, e.g. BMI2 and AVX2 in the 1.8.6 manylinux x86_64 wheel. libsodium still selects its SIMD implementations at runtime.
The extension declared libsodium only for the bundled build. System builds relied on -lsodium in LDFLAGS, which setuptools puts before the object files. Linkers with --as-needed (the Ubuntu default, and part of conda's LDFLAGS) then drop libsodium, and the import fails with undefined symbols. The -Wl,--no-as-needed workaround in the docs and in CI is no longer needed.
configure lists test/Makefile and test/default/Makefile in AC_CONFIG_FILES, so building a wheel from the sdist failed with "cannot find input file: 'test/default/Makefile.in'".
Metadata moves to [project], with an SPDX license expression. The extension is declared in [tool.setuptools.ext-modules]. The bundled libsodium build moves to build_libsodium.py, which setuptools loads through [tool.setuptools.cmdclass]. setuptools becomes a build requirement instead of an entry in requirements.txt, and the release workflow builds the sdist with python -m build. The clean command goes away with setup.py.
pip install . builds from the checkout, so a broken sdist went unnoticed. Build the sdist and install that, which is what pip does wherever no wheel matches.
This was referenced Sep 15, 2026
Hoeze
marked this pull request as ready for review
September 15, 2026 23:12
This branch has not been deployed
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.
tl;dr
-march=native, so the published wheels only run on CPUs like the CI runner's. This PR removes--enable-opt.SODIUM_INSTALL=system, the module could end up without a link to libsodium on Linux. This is the-Wl,--no-as-neededproblem fromdocs/setup.rst. The extension now declares libsodium itself.MANIFEST.inpruned Makefiles that libsodium's configure needs.setup.pyis replaced bypyproject.toml. The libsodium build step moves unchanged intobuild_libsodium.py.Each point is a separate commit, so the first three can be taken without the others.
1. No
-march=nativein the bundled libsodium--enable-optadds-march=native -mtune=native(libsodium'sconfigure.ac: "faster but not portable"). The release wheels are built by cibuildwheel on GitHub runners, so the wheels carry the runner's instruction set.In the 1.8.6 cp312 manylinux x86_64 wheel, BMI2 instructions appear in
blake2b_compress_ref,SHA512_Transformandfe25519_mul. AVX2 instructions appear inge25519_*andcrypto_scalarmult_curve25519_ref10. These are generic code paths, not runtime-dispatched ones. So the key exchange needs at least a Haswell-class CPU, and each wheel's requirement depends on which runner built it.Without
--enable-opt, libsodium still picks its SSSE3/AVX2/AVX-512 implementations at runtime.2. Link libsodium with
SODIUM_INSTALL=systemIn system mode the extension did not declare libsodium, so users had to add
-lsodiumtoLDFLAGS. setuptools putsLDFLAGSbefore the object files on the link line. A linker with--as-neededthen drops libsodium, because nothing needs it yet at that point.--as-neededis the default on Ubuntu and is part of conda'sLDFLAGS. The module then fails at import withundefined symbol: crypto_sign_ed25519_pk_to_curve25519.Declaring
libraries=['sodium']in both modes puts-lsodiumafter the object files, so-Wl,--no-as-neededis no longer needed. The docs drop that advice. The Ubuntu CI job now uses the sameLDFLAGSas macOS, so it exercises this.3. Buildable sdist
MANIFEST.inpruneslibsodium-stable/test, but configure liststest/Makefileandtest/default/MakefileinAC_CONFIG_FILES. So building a wheel from the sdist failed with:cibuildwheel builds from the checkout, so CI never noticed. It affects
pip installwherever no wheel matches, andpython -m build, which builds the wheel from the sdist.MANIFEST.innow keeps the fourMakefile.am/Makefile.infiles oftest/andtest/default/, and still prunes the tests themselves.4.
pyproject.tomlinstead ofsetup.py[project], with the SPDX license expressionApache-2.0(PEP 639). Hencesetuptools>=77.[tool.setuptools.ext-modules]. setuptools still labels that table experimental and prints a warning. Withoutsetup.py, it's the only way to declare an extension with setuptools.BuildLibsodiummoves tobuild_libsodium.py, which setuptools loads through[tool.setuptools.cmdclass]. Its logic is unchanged apart from commits 1 and 2.setuptoolsis now a build requirement instead of a line inrequirements.txt.python -m build --sdist. PEP 517 sdist builds can't pass--owner=root --group=root, so the tarball entries now carry the runner's user. pip ignores them.cleancommand is gone. It neededsetup.py, and it didn't work anyway:CleanLibsodiumdidn't derive fromCommand. To clean, removelibsodium-build/and runmake distcleaninlibsodium-stable/.Stack
This PR is part 2 of 3. A PR from a fork can only target
master, so this PR also contains part 1. Please review only the last five commits. Once part 1 is merged with a merge commit, as usual in this repo, the diff shrinks to those five.Part 3, refactor: replace libsodium with cryptography (#59), builds on this PR.
Stacked on #57.