jedisct1 / libsodium-sys-stable

Sodiumoxide's libsodium-sys crate, but that installs stable versions of libsodium.
MIT License
35 stars 6 forks source link

Fix Windows builds #5

Closed tom25519 closed 2 years ago

tom25519 commented 2 years ago

Hi, Windows builds of this crate currently appear to be broken. The build.rs file makes reference to mingw and msvc directories which existed in the original libsodium-sys crate, and contained pre-built binaries for these platforms, but have been removed here. This means the crate itself can't be built on its own, and also that it doesn't quite function as a drop-in replacement for the older library, which did support Windows.

Steps to Reproduce

On Windows (MSVC or MinGW):

git clone https://github.com/jedisct1/libsodium-sys-stable
cd libsodium-sys-stable
cargo test

An error occurs indicating the Sodium library is missing.

What This PR Does

This PR is intended to fix Windows builds by providing pre-built binaries for MSVC & MinGW. I have added the tarball/zip for the binaries to the repo, together with their signatures from libsodium.org, and updated build.rs to use the binaries from the archives when building on Windows. I have also updated the STABLE tarball to the latest stable release from libsodium.org, and made some minor tweaks to simplify the build.rs file.

Merging this PR should hopefully ensure the crate builds as expected on Windows.

It should be noted that this PR adds the zip crate as a build dependency for MSVC, used to extract the zip containing the binaries for this platform.

Other Issues

Currently enabling the fetch-latest feature breaks Windows builds, apparently due to ring failing to compile? I'm not massively familiar with the Windows ecosystem/Rust on Windows so this isn't something I'm immediately sure how to fix, but for the time being it might make sense to just indicate this feature doesn't work on Windows. This PR didn't introduce this bug, it's present in the current master.

The location for the MSVC library in this PR is hardcoded as being "v143" (see build.rs, ~line 383). Obviously this is something that is likely to change in the future, so we might want to find a better way of specifying the lib directory?

Thanks!

jedisct1 commented 2 years ago

This is great, thank you!

tom25519 commented 2 years ago

No worries, thanks so much for the quick merge! Would you be able to push a new release?