Spec URL: https://sicherha.fedorapeople.org/blake3.spec SRPM URL: https://sicherha.fedorapeople.org/blake3-1.5.0-1.fc40.src.rpm Description: BLAKE3 is a cryptographic hash function that is: - Much faster than MD5, SHA-1, SHA-2, SHA-3, and BLAKE2. - Secure, unlike MD5 and SHA-1. And secure against length extension, unlike SHA-2. - Highly parallelizable across any number of threads and SIMD lanes, because it's a Merkle tree on the inside. - Capable of verified streaming and incremental updates, again because it's a Merkle tree. - A PRF, MAC, KDF, and XOF, as well as a regular hash. - One algorithm with no variants, which is fast on x86-64 and also on smaller architectures. Fedora Account System Username: sicherha Getting blake3 into Fedora is a prerequisite for updating the mold linker to 2.2.0 - see https://bugzilla.redhat.com/show_bug.cgi?id=2240671#c1. This C library is built from the same upstream GitHub repository as the already-existing rust-blake3 package (https://src.fedoraproject.org/rpms/rust-blake3), but the latter focuses purely on Rust crates.
Initial comment is that CC0-1.0 is not an allowed license for code. May wish to just use Apache-2.0 CC-PDDC is allowed though: https://docs.fedoraproject.org/en-US/legal/allowed-licenses/ The rust crate offers a variety of build options: https://src.fedoraproject.org/rpms/rust-blake3/blob/rawhide/f/rust-blake3.spec Fedora typically allows use of SSE and AVX2 intrinsics, though maybe an additional package with AVX512 might be helpful for some users. Can the test be run? https://github.com/BLAKE3-team/BLAKE3/blob/master/c/test.py
Thanks a lot for the prompt sanity check! (In reply to Benson Muite from comment #1) > Initial comment is that CC0-1.0 is not an allowed license for code. May wish > to just use Apache-2.0 > > CC-PDDC is allowed though: > https://docs.fedoraproject.org/en-US/legal/allowed-licenses/ Thanks, I missed that. I suggest we just stick to Apache-2.0 then. > The rust crate offers a variety of build options: > https://src.fedoraproject.org/rpms/rust-blake3/blob/rawhide/f/rust-blake3. > spec Hmm, so should we rather augment that SPEC file instead of adding a separate one for the C library? > Fedora typically allows use of SSE and AVX2 intrinsics, though maybe an > additional package with AVX512 > might be helpful for some users. The library dynamically selects the best SIMD variant based on the capabilities of the CPU it is being run on - see https://github.com/BLAKE3-team/BLAKE3/blob/master/c/blake3_dispatch.c. Does that make it okay to include the AVX512 code path in the default build? > Can the test be run? > https://github.com/BLAKE3-team/BLAKE3/blob/master/c/test.py Actually, that script is already called indirectly via `Makefile.testing`.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Package contains no static executables. [x]: Rpath absent or only used for internal libs. [x]: Development (unversioned) .so files in -devel subpackage, if present. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* Creative Commons CC0 1.0 Apache License 2.0", "Apache License 2.0". 85 files have unknown license. Detailed output of licensecheck in /home/FedoraPackaging/reviews/blake3/review-blake3/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [ ]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [ ]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [ ]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 28226 bytes in 2 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: The License field must be a valid SPDX expression. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [ ]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [ ]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [!]: Spec use %global instead of %define unless justified. Note: %define requiring justification: %define x86_flags BLAKE3_NO_SSE2=1 BLAKE3_NO_SSE41=1 BLAKE3_NO_AVX2=1 BLAKE3_NO_AVX512=1, %define arm_flags %{?flags} BLAKE3_USE_NEON=1 [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. ===== EXTRA items ===== Generic: [!]: Spec file according to URL is the same as in SRPM. Note: Spec file as given by url is not the same as in SRPM (see attached diff). See: (this test has no URL) [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: blake3-1.5.0-1.fc38.x86_64.rpm blake3-devel-1.5.0-1.fc38.x86_64.rpm blake3-debuginfo-1.5.0-1.fc38.x86_64.rpm blake3-debugsource-1.5.0-1.fc38.x86_64.rpm blake3-1.5.0-1.fc38.src.rpm =================================== rpmlint session starts ================================== rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmp5mrfhgb7')] checks: 31, packages: 5 blake3-devel.x86_64: E: summary-too-long Official C implementation of the BLAKE3 cryptographic hash function - development files blake3.src: W: strange-permission blake3.spec 600 blake3.src: W: name-repeated-in-summary BLAKE3 blake3.x86_64: W: name-repeated-in-summary BLAKE3 blake3-devel.x86_64: W: description-shorter-than-summary ==== 5 packages and 0 specfiles checked; 1 errors, 4 warnings, 1 badness; has taken 3.3 s === Rpmlint (debuginfo) ------------------- Checking: blake3-debuginfo-1.5.0-1.fc38.x86_64.rpm =================================== rpmlint session starts ================================== rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpsdqsxvil')] checks: 31, packages: 1 ==== 1 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.8 s === Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 4 blake3-devel.x86_64: E: summary-too-long Official C implementation of the BLAKE3 cryptographic hash function - development files blake3.x86_64: W: name-repeated-in-summary BLAKE3 blake3-devel.x86_64: W: description-shorter-than-summary 4 packages and 0 specfiles checked; 1 errors, 2 warnings, 1 badness; has taken 3.8 s Source checksums ---------------- https://github.com/BLAKE3-team/BLAKE3//archive/1.5.0/blake3-1.5.0.tar.gz : CHECKSUM(SHA256) this package : f506140bc3af41d3432a4ce18b3b83b08eaa240e94ef161eb72b2e57cdc94c69 CHECKSUM(SHA256) upstream package : f506140bc3af41d3432a4ce18b3b83b08eaa240e94ef161eb72b2e57cdc94c69 Requires -------- blake3 (rpmlib, GLIBC filtered): libc.so.6()(64bit) rtld(GNU_HASH) blake3-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config blake3(x86-64) cmake-filesystem(x86-64) libblake3.so.0()(64bit) blake3-debuginfo (rpmlib, GLIBC filtered): blake3-debugsource (rpmlib, GLIBC filtered): Provides -------- blake3: blake3 blake3(x86-64) libblake3.so.0()(64bit) blake3-devel: blake3-devel blake3-devel(x86-64) cmake(blake3) pkgconfig(libblake3) blake3-debuginfo: blake3-debuginfo blake3-debuginfo(x86-64) debuginfo(build-id) libblake3.so.1.5.0-1.5.0-1.fc38.x86_64.debug()(64bit) blake3-debugsource: blake3-debugsource blake3-debugsource(x86-64) Diff spec file in url and in SRPM --------------------------------- --- /home/benson/Projects/FedoraPackaging/reviews/blake3/blake3.spec 2023-09-27 23:50:33.000000000 +0300 +++ /home/benson/Projects/FedoraPackaging/reviews/blake3/review-blake3/srpm-unpacked/blake3.spec 2023-09-27 03:00:00.000000000 +0300 @@ -1,2 +1,7 @@ +## START: Set by rpmautospec +## (rpmautospec version 0.3.5) +## RPMAUTOSPEC: autochangelog +## END: Set by rpmautospec + Name: blake3 Version: 1.5.0 @@ -86,3 +91,4 @@ %changelog -%autochangelog +* Wed Sep 27 2023 John Doe <packager> - 1.5.0-1 +- Uncommitted changes Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -n blake3 -m fedora-38-x86_64 Buildroot used: fedora-38-x86_64 Active plugins: C/C++, Shell-api, Generic Disabled plugins: fonts, Haskell, Ocaml, Java, Python, Perl, R, SugarActivity, PHP, Ruby Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH Comments: a) Use %global instead of %define b) It is ok to have this as a separate package. The rust sources at: https://crates.io/api/v1/crates/blake3/1.5.0/download do also contain the C implementation so updating that seems reasonable, but it maybe hard to find as the c package will be part of rust-blake3, rather than just having blake3. Maybe the rust package could become a sub package of this one as it contains the Rust sources? Rust package can probably be obsoleted once this is in Fedora. c) The sonames are not consistent, libblake3.so.0 libblake3.so.1.5.0 Would have expected the first one to be libblake3.so.1 See https://github.com/BLAKE3-team/BLAKE3/pull/355 d) There seem to be problems with builds on i686 and x84_64: https://koji.fedoraproject.org/koji/taskinfo?taskID=107011376 e) Maybe upstream would consider adding a C binding to the Rust library: https://developers.redhat.com/articles/2022/09/05/how-create-c-binding-rust-library
(In reply to Benson Muite from comment #3) > a) Use %global instead of %define I have added an explanatory comment that these macros are used only locally within `%check`. That's why I refrained from making them global. > c) The sonames are not consistent, > libblake3.so.0 > libblake3.so.1.5.0 > Would have expected the first one to be > libblake3.so.1 > See https://github.com/BLAKE3-team/BLAKE3/pull/355 I have added your patch to the `.spec` file. > d) There seem to be problems with builds on i686 and x84_64: > https://koji.fedoraproject.org/koji/taskinfo?taskID=107011376 Okay, this looks strange. AddressSanitizer reports a segfault within the AVX-512 code. Using the power of Podman, I can reproduce the segfault ... * on an AVX-512-capable machine under Fedora 37, 38, 39 and Rawhide. * on an AVX-512-capable machine under Ubuntu 23.04 and 23.10. However, I cannot reproduce it ... * on an AVX-512-capable machine under Ubuntu 22.04. * on a machine that does not support AVX-512. I'm wondering whether this might be a regression in AddressSanitizer.
The segfault in the unit tests does appear to be a regression in GCC's AddressSanitizer. I am in the process of bisecting GCC's Git history to find the commit that introduced the breakage and file a bug report upstream. For the time being, I suggest we build the blake3 package with Clang instead of GCC. I have updated the `.spec` file accordingly.
Upstream GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110027 Reproduction and analysis: https://github.com/sicherha/gcc-asan-stack-misalign In a nutshell, it's a bug in GCC where one specific AddressSanitizer check (stack-use-after-return) causes troubles. We could: a) build blake3 with GCC but disable this particular check, b) build blake3 with Clang. Which is the preferred choice?
It seems reasonable to use Clang. Maybe also add a link to the bug report. Upstream does not seem to be very responsive on the soname. Is it reasonable to assume semantic versioning?
Link to bug report added to `.spec` file. I'm a bit undecided when it comes to the SONAME topic. Can semantic versioning even be applied properly when two independent libraries - one in Rust, one in C - are hosted within the same Git repository? For instance, an interface-breaking change in the Rust code would have to trigger a new major version (and thus a new SONAME) even if the C interface remains the same. That would mean unnecessary extra packaging effort for downstream distributions. Right now I'm leaning slightly towards the 'increment whenever the ABI breaks' approach. For a simple cryptographic-hash library such as blake3 I don't expect frequent interface changes anyway.
The sonames should be consistent. So would suggest either: libblake3.so.0 libblake3.so.0.1.5.0 or libblake3.so.1 libblake3.so.1.5.0 The first option allows for an easier increment compared to the second. Maybe upstream would consider having 2 repositories? The C repository could be made a sub repository for the Rust repository.
I'd go with option 2 then, since it appears more intuitive to me. This is also the way it's being done in the source RPM under review.
Are there any further road blocks that have to be cleared in order to get this package in?
Would suggest using libblake3.so.0 libblake3.so.0.1.5.0 as it is better to increment once upstream decides what to use. This would follow the guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/#_downstream_so_name_versioning
I'm unsure whether that guideline ('In cases where upstream ships unversioned .so library') applies to our case, where upstream ships a library that is versioned but confusingly so. Out of curiosity I have done a quick survey of other occurrences for libraries installed on my F38 system: ``` libaccounts-glib.so.0 -> libaccounts-glib.so.1.25 libIMECore.so.0 -> libIMECore.so.1.1.2 libIMEPinyin.so.0 -> libIMEPinyin.so.1.1.2 libIMETable.so.0 -> libIMETable.so.1.1.2 libnextcloud_csync.so.0 -> libnextcloud_csync.so.3.10.0 libnextcloudsync.so.0 -> libnextcloudsync.so.3.10.0 libopenblaso.so.0 -> libopenblaso-r0.3.21.so libopenblasp64_.so.0 -> libopenblasp64_-r0.3.21.so libopenblasp.so.0 -> libopenblasp-r0.3.21.so libopenblas.so.0 -> libopenblas-r0.3.21.so libSDL-1.2.so.0 -> libSDL-1.2.so.1.2.68 libserf-1.so.0 -> libserf-1.so.1.3.9 libutempter.so.0 -> libutempter.so.1.2.1 ``` So this phenomenon actually doesn't seem all that uncommon. With that in mind, how confident do you feel about your recommendation? Should we consult the mailing list to be sure?
There do seem to be alternative versioning schemes: https://autotools.info/libtool/version.html https://www.gnu.org/software/libtool/manual/html_node/Libtool-versioning.html Perhaps check on mailing list, may be helpful to check that can use any versioning scheme upstream has, provided it has one even if it is not semantic versioning.
Based on discussion in Fedora packaging list: https://lists.fedoraproject.org/archives/list/packaging@lists.fedoraproject.org/message/FFKA5ZOZDT7RT6TUUYGQAVVVU4NEWJ42/ soname is ok. Approved.
The Pagure repository was created at https://src.fedoraproject.org/rpms/blake3
FEDORA-2023-6fa112c096 has been submitted as an update to Fedora 40. https://bodhi.fedoraproject.org/updates/FEDORA-2023-6fa112c096
FEDORA-2023-6fa112c096 has been pushed to the Fedora 40 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2023-50e9b4fe35 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-50e9b4fe35
FEDORA-2023-094944dca2 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-094944dca2
FEDORA-EPEL-2023-db7b259576 has been submitted as an update to Fedora EPEL 9. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-db7b259576
FEDORA-EPEL-2023-99c4e2d65f has been submitted as an update to Fedora EPEL 8. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-99c4e2d65f
FEDORA-2023-094944dca2 has been pushed to the Fedora 38 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-094944dca2 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-094944dca2 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2023-99c4e2d65f has been pushed to the Fedora EPEL 8 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-99c4e2d65f See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-EPEL-2023-db7b259576 has been pushed to the Fedora EPEL 9 testing repository. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2023-db7b259576 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-50e9b4fe35 has been pushed to the Fedora 39 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-50e9b4fe35 \*` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-50e9b4fe35 See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
FEDORA-2023-50e9b4fe35 has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-2023-094944dca2 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2023-99c4e2d65f has been pushed to the Fedora EPEL 8 stable repository. If problem still persists, please make note of it in this bug report.
FEDORA-EPEL-2023-db7b259576 has been pushed to the Fedora EPEL 9 stable repository. If problem still persists, please make note of it in this bug report.