Spec URL: https://gathman.org/linux/SPECS/rust-libsodium-sys.spec SRPM URL: https://gathman.org/linux/f35/src/rust-libsodium-sys-0.2.7-1.fc35.src.rpm Description: This package contains library source intended for building other packages which use "libsodium-sys" crate. Fedora Account System Username: sdgathman
Are you still interested in packaging this crate? If yes, please adapt the packaging to drop the vendored copy of libsodium (i.e. "rm -rf libsodium" in %prep) and make it link against the shared library from the system (i.e. patch build.rs to enable code gated behind "use-pkg-config" feature enabled unconditionally).
Thanks for the response! Yes, ultimately I need to package sodiumoxide, which has a number of dependencies including this. I am new to rust packaging, so your feedback is very welcome. Let me try it ...
The changes that are required to make this work well as a distro package don't look too bad: 1) Patch the build.rs file according to these rules: - remove all blocks that are gated behind #[cfg(not(feature = "use-pkg-config"))] - remove #[cfg(feature = "use-pkg-config)] attributes to make those blocks compile unconditionally 2) Do "rm -r libsodium" between %prep and %cargo_prep. This should ensure that the crate always dynamically links against the system copy of libsodium, whether during its own build or while it's being compiled as dependency for another crate.
Haven't succeeded yet, but trying again carefully.
https://gathman.org/linux/SPECS/rust-libsodium-sys.spec Removing the libsodium breaks the build script. I'm reposting for any help. thread 'main' panicked at 'Failed to find configure script! Did you clone the submodule at `libsodium-sys/libsodium`?: Os { code: 2, kind: NotFound, message: "No such file or directory" }', build.rs:232:89
Added BR: libsodium-devel But it doesn't help
Spec URL: https://gathman.org/linux/SPECS/rust-libsodium-sys.spec SRPM URL: https://gathman.org/linux/f37/src/rust-libsodium-sys-0.2.7-1.fc37.src.rpm
Ok, starting the formal review process. 1) All new Rust packages should be using rpmautospec. Did you intentionally opt out? 2) You should use rust2rpm's "-p" flag when you want to patch Cargo.toml, it automates the "edit + create patch + add patch to generated spec file" steps. 3) Don't mix tabs and spaces (in the Patch0 line). 4) You need to add "Requires: libsodium-devel" to the "-devel" sub-package, in addition to the BuildRequires. Otherwise dependent packages will fail to build, as libsodim-devel will not be pulled in as a transitive dependency. 5) Remove the commented-out "#license %{crate_instdir}/libsodium/LICENSE" line. It doesn't provide any information, and creates warnings about expanded macros inside comments. 6) Add some comments why you're removing a bunch of stuff in %prep, for example: # remove pre-built libsodium objects for MSVC / MinGW targets rm -r mingw/ msvc/ # remove bundled libsodium sources rm -r libsodium/ 7) Either explain what the sed script does to src/gen.sh, or do it differently. > sed -i -e '1 i#!/bin/sh' src/gen.sh I have never seen this sed expression syntax before, and I wouldn't know what to do with it when I need to touch this package. I assume you want to delete the shebang from the script, in order to prevent a generated dependency on /bin/sh? 8) The bindings are machine-generated code, and we *SHOULD* be re-generating them during the build process. We do have all the tools that are necessary to do this packaged for Fedora, and it would be pretty easy to adapt the build.rs script to do automatically (by using the bindgen library interface) what that "src/gen.sh" script does manually (by using the bindgen CLI interface). 9) Remove "export RUST_BACKTRACE=1" from %build. I don't know why it's there, but it should never be needed in %build (though sometimes it's needed in %check to make some tests work as expected). 10) Rather than patch Cargo.toml to make the "pkg-config" feature a default feature, we should patch the build.rs to use pkg-config unconditionally. This is much safer, especially since dependent crates can work around the current patch by using "no-default-features = true" for their libsodium dependency, in which case, they will break.
Spec URL: https://gathman.org/linux/SPECS/rust-libsodium-sys.spec SRPM URL: https://gathman.org/linux/f37/src/rust-libsodium-sys-0.2.7-5.fc37.src.rpm I switched to rpmautospec (hopefully ok). You prefer patching build.rs, so I didn't address 2. Added FIXME comments for 8 and 10
ad 2) This is independent of the other point. If you patch Cargo.toml, then using "rust2rpm -p" is the standard way to do that - it opens an Cargo.toml in an editor for you, generates the patch file once you save changes, and includes that patch file in the generated spec file automatically. It's not a blocker though. I can clean this up with a follow-up commit. ad 7) > # Add shebang to top of gen.sh as build tries to run it directly > # FIXME: patch build.rs to use the bindgen library interface instead > sed -i -e '1 i#!/bin/sh' src/gen.sh I don't understand this. "src/gen.sh" is not run during the build at all. === Either way, these are now small problems (points 2, 7, 8, 10) and the package looks good (other than a missing %changelog tag before %autochangelog). If you convert package to rpmautospec, you can use "rpmautospec convert" command, which automates this. Please fix this before importing the package. I'll then push a follow-up commit to address the remaining non-blocking issues, so please hold off launching a koji build after importing the package. Package APPROVED. === Package was generated with rust2rpm, simplifying the review. - package builds and installs without errors on rawhide - test suite is run and all unit tests pass - latest version of the crate is packaged - license matches upstream specification (MIT OR Apache-2.0) and is acceptable for Fedora - license file is included with %license in %files - package complies with Rust Packaging Guidelines Package APPROVED. === Recommended post-import rust-sig tasks: - add @rust-sig with "commit" access as package co-maintainer - set bugzilla assignee overrides to @rust-sig (optional) - set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate - track package in koschei for all built branches === You can proceed with requesting the dist-git repo (and branches) now: $ fedpkg request-repo rust-libsodium-sys 1991164 $ fedpkg request-branch --repo rust-libsodium-sys f37 (same command for f36 and f35, if you need this package there)
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-libsodium-sys
Looks like you ignored my request not to launch a koji build yet? https://koji.fedoraproject.org/koji/buildinfo?buildID=2074817 Either way, I set up release-monitoring and koschei for the package, since that wasn't done yet, either ... So this bug can now probably be closed?
I ended up manually copying files for import - did not know the trick with rpmbuild -bs. I did a build in rawhide, thinking you were concerned about f37. In hindsight, I should have done a scratch build. I apologize. My only excuse is working too late on the project.