https://bugzilla.redhat.com/show_bug.cgi?id=1991164 Fabio Valentini <decathorpe(a)gmail.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe(a)gmail.com Doc Type|--- |If docs needed, set a value --- Comment #1 from Fabio Valentini <decathorpe(a)gmail.com&gt; --- 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). -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 --- Comment #3 from Fabio Valentini <decathorpe(a)gmail.com&gt; --- 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 --- Comment #6 from Stuart D Gathman <stuart(a)gathman.org&gt; --- Added BR: libsodium-devel But it doesn't help -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 Fabio Valentini <decathorpe(a)gmail.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody(a)fedoraproject.org |decathorpe(a)gmail.com Flags| |fedora-review? Status|NEW |ASSIGNED --- Comment #8 from Fabio Valentini <decathorpe(a)gmail.com&gt; --- 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. 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. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 Stuart D Gathman <stuart(a)gathman.org&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |2045255 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=2045255 [Bug 2045255] cjdns: FTBFS in Fedora rawhide/f36 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 Fabio Valentini <decathorpe(a)gmail.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+ --- Comment #10 from Fabio Valentini <decathorpe(a)gmail.com&gt; --- 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) 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) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 --- Comment #12 from Fabio Valentini <decathorpe(a)gmail.com&gt; --- 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? -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1991164

https://bugzilla.redhat.com/show_bug.cgi?id=1991164 Miro Hrončok <mhroncok(a)redhat.com&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mhroncok(a)redhat.com Resolution|NOTABUG |RAWHIDE Fixed In Version| |rust-libsodium-sys-0.2.7-1. | |fc38 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=1991164