https://bugzilla.redhat.com/show_bug.cgi?id=2481458
Srikrishna Veturi <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |[email protected] --- Comment #3 from Srikrishna Veturi <[email protected]> --- Informal review of rust-prefix-trie 0.8.4 Note: I am not yet a member of the packager group, so this is an informal review. I cannot set fedora-review flags. I built the package locally with fedora-review (mock, fedora-rawhide-x86_64) and verified the spec against the Fedora Rust Packaging Guidelines and the Review Guidelines. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= None. The "File listed twice" warnings for LICENSE-APACHE and LICENSE-MIT are a known, harmless pattern in rust2rpm-generated specs: %license marks them and the %{crate_instdir}/ glob also covers them. Not a packaging error. ===== MUST items ===== Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. Note: Built successfully in mock (fedora-rawhide-x86_64). Also built on Koji (task 145916466) and COPR (build 10513403). [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. Note: License is "MIT OR Apache-2.0". Both are approved open-source licenses. [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* Apache License 2.0", "MIT License". 38 files have unknown license. Detailed output of licensecheck in licensecheck.txt. The "unknown" files are .rs source files, configs, and Cargo.toml — these are covered by the top-level LICENSE-APACHE and LICENSE-MIT files. The crate metadata (Cargo.toml) declares "MIT OR Apache-2.0", matching the spec License tag. Per Rust Packaging Guidelines, license metadata for all Rust crates MUST match the package license tag. [x]: License file installed when any subpackage combination is installed. Note: %license %{crate_instdir}/LICENSE-APACHE and LICENSE-MIT are listed in the -devel subpackage %files section. [-]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. Note: This is a dual-license (OR), not a combined license (AND). No breakdown beyond the License tag is needed. [x]: %build honors applicable compiler flags or justifies otherwise. Note: Uses %cargo_build which inherits flags from %cargo_prep. Per Rust Packaging Guidelines, %cargo_prep configures cargo to use the default %build_rustflags. [x]: Package contains no bundled libraries without FPC exception. Note: Pure Rust library crate, no bundled dependencies. [x]: Changelog in prescribed format. Note: Uses %autochangelog. [x]: Sources contain only permissible code or content. Note: Source SHA256 4cf6e3177f0684016a5c209b00882e15f8bdd3f3bb48f049 1df10cd102d0c6e7 matches upstream crates.io download. No problematic content found. [-]: Package contains desktop file if it is a GUI application. Note: Library crate, not a GUI application. [x]: Development files must be in a -devel package. Note: Library source is correctly packaged in rust-prefix-trie-devel. [x]: Package uses nothing in %doc for runtime. Note: Only Readme.md in %doc. [x]: Package consistently uses macros (instead of hard-coded directory names). Note: Uses %{crate_instdir}, %{crates_source}, %cargo_* macros throughout. [x]: Package is named according to the Package Naming Guidelines. Note: Named rust-prefix-trie, following the Rust Packaging Guidelines MUST rule that crate packages use rust-$crate naming. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. Note: Files installed under /usr/share/cargo/registry/ as expected for Rust crate packages. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. Note: Not a rename. [x]: Requires correct, justified where necessary. Note: rust-prefix-trie-devel requires crate(either/default) and crate(num-traits/default) with proper SemVer ranges. Feature subpackages correctly require crate(prefix-trie) = 0.8.4. Per Rust Packaging Guidelines, the main -devel subpackage MUST require the virtual Provides for all non-optional crate dependencies, and feature subpackages MUST require the corresponding optional dependencies. Verified this is the case. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. Note: Library crate, no daemons. [-]: Package is not known to require an ExcludeArch tag. Note: Pure Rust library, all subpackages are noarch. [x]: Package complies to the Packaging Guidelines. Note: See Rust-specific checks below. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). Only spelling-error on "datastructure" — false positive, this is a standard CS term from the upstream crate description. [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. Note: "MIT OR Apache-2.0" is valid SPDX. [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. Note: Via %autorelease. [x]: Package does not contain duplicates in %files. Note: Build log shows "File listed twice" warnings for LICENSE-APACHE and LICENSE-MIT. This is a known pattern in rust2rpm specs where %license and the %{crate_instdir}/ glob both match the license files. The files are not actually duplicated in the RPM — RPM deduplicates them. Not a packaging error. [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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 0 bytes in 0 files. [x]: Packages must not store files under /srv, /opt or /usr/local. ===== Rust-specific checks (Fedora Rust Packaging Guidelines) ===== [x]: BuildRequires: cargo-rpm-macros is present. Note: "BuildRequires: cargo-rpm-macros >= 24". Per guidelines, packages that call %cargo_* macros MUST add this. [x]: %cargo_prep is called in %prep after sources are unpacked. [x]: %cargo_generate_buildrequires is called in %generate_buildrequires. [x]: %bcond check is set. Note: "%bcond check 0". Spec comment explains: "missing dev-dependencies: generic-tests, ip_network_table-deps-treebitmap". Per guidelines, packages MUST set this bcond. [x]: Source is from crates.io via %{crates_source}. Note: Per guidelines, crate packages MUST use %{crates_source}. [x]: Feature subpackages are in sync with crate metadata. Note: After the patch removes the "cidr" feature, remaining features are: default, ipnet, ipnetwork, serde. Subpackages exist for all four. Per guidelines, feature list MUST be kept in sync. [x]: RPM Provides are correct per guidelines. Note: rust-prefix-trie-devel provides crate(prefix-trie) = 0.8.4. Each +feature-devel provides crate(prefix-trie/$feature) = 0.8.4. [x]: RPM Requires are correct per guidelines. Note: Each +feature-devel requires crate(prefix-trie) = 0.8.4. +default-devel requires crate(prefix-trie/ipnet) (the default feature enables ipnet). [x]: Patch removes unavailable optional dependency from crate metadata. Note: prefix-trie-fix-metadata.diff drops the "cidr" feature and its dependency. Per guidelines, unavailable optional dependencies MUST be removed from crate metadata. [x]: License metadata in crate matches package license tag. ===== SHOULD items ===== Generic: [x]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. Note: Upstream includes LICENSE-APACHE and LICENSE-MIT. [x]: Final provides and requires are sane (see attachments). Note: Verified — all crate() provides and requires match expected values from the patched Cargo.toml metadata. [x]: Fully versioned dependency in subpackages if applicable. Note: fedora-review flagged missing "Requires: %{name}%{?_isa} = %{version}-%{release}" in subpackages. However, for Rust crate packages this is handled differently — feature subpackages use "crate(prefix-trie) = %{version}" instead, which is the correct mechanism per Rust Packaging Guidelines. Not an issue. [x]: Package functions as described. Note: Library crate providing prefix trie data structures. Builds successfully and installs crate sources for downstream use. [x]: Latest version is packaged. Note: 0.8.4 is the latest version on crates.io. [x]: Package does not include license text files separate from upstream. Note: LICENSE-APACHE and LICENSE-MIT come from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. Note: Patch comment explains: "drop unused feature and dependencies for CIDR support". The cidr crate is not packaged in Fedora. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: crates.io does not provide GPG signatures. Not applicable. [x]: Package should compile and build into binary rpms on all supported architectures. Note: All subpackages are noarch. Built successfully. [x]: %check is present and all tests pass. Note: %check is present but disabled (%bcond check 0) due to missing dev-dependencies (generic-tests, ip_network_table-deps-treebitmap). This is justified and documented in the spec header comment. Per Rust Packaging Guidelines, if tests are skipped, the package SHOULD include comments explaining why — and it does. [x]: Packages should try to preserve timestamps of original installed files. Note: %cargo_install handles this. [x]: Reviewer should test that the package builds in mock. Note: Built successfully in fedora-rawhide-x86_64 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]: Packager, Vendor, PreReq, Copyright tags should not be in spec file. [x]: Sources can be downloaded from URI in Source: tag. [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: Only spelling-error on "datastructure" — false positive. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: rust-prefix-trie-devel-0.8.4-1.fc45.noarch.rpm rust-prefix-trie+default-devel-0.8.4-1.fc45.noarch.rpm rust-prefix-trie+ipnet-devel-0.8.4-1.fc45.noarch.rpm rust-prefix-trie+ipnetwork-devel-0.8.4-1.fc45.noarch.rpm rust-prefix-trie+serde-devel-0.8.4-1.fc45.noarch.rpm rust-prefix-trie-0.8.4-1.fc45.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.8.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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/tmp8qm_hm8i')] checks: 32, packages: 6 rust-prefix-trie+default-devel.noarch: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') rust-prefix-trie+ipnet-devel.noarch: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') rust-prefix-trie+ipnetwork-devel.noarch: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') rust-prefix-trie+serde-devel.noarch: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') rust-prefix-trie.src: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') rust-prefix-trie-devel.noarch: E: spelling-error ('datastructure', '%description -l en_US datastructure -> data structure, data-structure, structured') 6 packages and 0 specfiles checked; 6 errors, 0 warnings, 31 filtered, 6 badness; has taken 0.5 s Rpmlint (installed packages) ---------------------------- rpmlint: 2.9.0 5 packages and 0 specfiles checked; 5 errors (all spelling-error on "datastructure"), 0 warnings, 27 filtered. Source checksums ---------------- https://crates.io/api/v1/crates/prefix-trie/0.8.4/download#/prefix-trie-0.8.4.crate : CHECKSUM(SHA256) this package : 4cf6e3177f0684016a5c209b00882e15f8bdd3f3bb48f0491df10cd102d0c6e7 CHECKSUM(SHA256) upstream package : 4cf6e3177f0684016a5c209b00882e15f8bdd3f3bb48f0491df10cd102d0c6e7 Requires -------- rust-prefix-trie-devel (rpmlib, GLIBC filtered): (crate(either/default) >= 1.0.0 with crate(either/default) < 2.0.0~) (crate(num-traits/default) >= 0.2.4 with crate(num-traits/default) < 0.3.0~) cargo rust rust-prefix-trie+default-devel (rpmlib, GLIBC filtered): cargo crate(prefix-trie) crate(prefix-trie/ipnet) rust-prefix-trie+ipnet-devel (rpmlib, GLIBC filtered): (crate(ipnet/default) >= 2.3.0 with crate(ipnet/default) < 3.0.0~) cargo crate(prefix-trie) rust-prefix-trie+ipnetwork-devel (rpmlib, GLIBC filtered): (crate(ipnetwork/default) >= 0.20.0 with crate(ipnetwork/default) < 0.21.0~) cargo crate(prefix-trie) rust-prefix-trie+serde-devel (rpmlib, GLIBC filtered): (crate(ipnet/default) >= 2.3.0 with crate(ipnet/default) < 3.0.0~) (crate(ipnet/serde) >= 2.3.0 with crate(ipnet/serde) < 3.0.0~) (crate(ipnetwork/default) >= 0.20.0 with crate(ipnetwork/default) < 0.21.0~) (crate(ipnetwork/serde) >= 0.20.0 with crate(ipnetwork/serde) < 0.21.0~) (crate(serde/default) >= 1.0.0 with crate(serde/default) < 2.0.0~) cargo crate(prefix-trie) Provides -------- rust-prefix-trie-devel: crate(prefix-trie) rust-prefix-trie-devel rust-prefix-trie+default-devel: crate(prefix-trie/default) rust-prefix-trie+default-devel rust-prefix-trie+ipnet-devel: crate(prefix-trie/ipnet) rust-prefix-trie+ipnet-devel rust-prefix-trie+ipnetwork-devel: crate(prefix-trie/ipnetwork) rust-prefix-trie+ipnetwork-devel rust-prefix-trie+serde-devel: crate(prefix-trie/serde) rust-prefix-trie+serde-devel Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2481458 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic Disabled plugins: C/C++, Python, SugarActivity, Perl, R, fonts, Haskell, Ocaml, Java, PHP Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH ===== Conclusion ===== The package looks good. It follows the Fedora Rust Packaging Guidelines correctly. The spec was generated by rust2rpm 28 with a single well-justified patch to remove the unpackaged cidr dependency. Tests are disabled due to missing dev-dependencies, which is properly documented. All MUST items pass. LGTM. -- 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=2481458 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202481458%23c3 -- _______________________________________________ package-review mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://forge.fedoraproject.org/infra/tickets/issues/new
