Spec URL: https://decathorpe.fedorapeople.org/rust-exr.spec SRPM URL: https://decathorpe.fedorapeople.org/rust-exr-1.72.0-1.fc40.src.rpm Description: Read and write OpenEXR files without any unsafe code. Fedora Account System Username: decathorpe
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=120391533
Copr build: https://copr.fedorainfracloud.org/coprs/build/7731112 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2297646-rust-exr/fedora-rawhide-x86_64/07731112-rust-exr/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
This looks great except for CRLF line terminators. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated The spec file is almost exactly as generated by rust2rpm with no configuration file, greatly simplifying the review. The only difference is that certain tests are skipped; this is done nicely/correctly, and is properly commented. --- rust-exr.spec 2024-07-18 12:46:52.995599270 -0400 +++ ../srpm-unpacked/rust-exr.spec 2024-06-15 15:38:15.000000000 -0400 @@ -64,7 +64,14 @@ %if %{with check} %check -%cargo_test +%ifarch s390x +# * skip tests which require test data that is not included in published crates +# * skip a test that is an expected failure on big-endian architectures +%cargo_test -- -- --skip compression::b44::test::border_on_multiview --skip image::validate_results::test_value_result::test_error --skip compare_compression_contents_ --skip compare_png_to_ --skip damaged --skip fuzzed --skip roundtrip_all_files_in_repository_x4 --skip pxr24_expect_error_on_big_endian +%else +# * skip tests which require test data that is not included in published crates +%cargo_test -- -- --skip compression::b44::test::border_on_multiview --skip image::validate_results::test_value_result::test_error --skip compare_compression_contents_ --skip compare_png_to_ --skip damaged --skip fuzzed --skip roundtrip_all_files_in_repository_x4 +%endif %endif Issues: ======= - Package does not contain duplicates in %files. Note: warning: File listed twice: /usr/share/cargo/registry/exr-1.72.0/CONTRIBUTORS.md See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_duplicate_files This is not a serious problem; if it should be fixed, then it should be fixed in rust2rpm. - All of the packaged Markdown files have the wrong line terminations: $ file exr-1.72.0/*.md exr-1.72.0/CONTRIBUTORS.md: ASCII text, with CRLF line terminators exr-1.72.0/GUIDE.md: C source, ASCII text, with CRLF line terminators exr-1.72.0/LICENSE.md: ASCII text, with very long lines (755), with CRLF line terminators exr-1.72.0/README.md: C source, ASCII text, with very long lines (350), with CRLF line terminators exr-1.72.0/releasing.md: ASCII text, with CRLF line terminators This showed up in the rpmlint output: rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/CONTRIBUTORS.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/GUIDE.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/README.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/releasing.md Furthermore, all of the packaged Rust sources also have Windows/DOS style CRLF line endings. While the Rust compiler still accepts them, it’s probably better to convert them. There are many ways to do this; a straightforward approach is to add a BuildRequires on dos2unix, and use the following one-liner in %prep: find . -type f -execdir dos2unix --keepdate '{}' '+' If you are worried about this possibly mishandling future non-text files that might appear in the crate, you can do something like find . -type f \( -name '*.md' -o -name '*.rs' \) -execdir dos2unix --keepdate '{}' '+' noting that the only files this “misses" are those that already have the correct line terminators (Cargo.toml.orig) or are not installed. ./.cargo_vcs_info.json: JSON text data ./.github/workflows/rust.yml: ASCII text, with CRLF line terminators ./.gitignore: ASCII text, with CRLF line terminators ./Cargo.lock: ASCII text ./Cargo.toml: ASCII text ./Cargo.toml.orig: ASCII text, with CRLF line terminators ===== MUST items ===== 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", "BSD 3-Clause License". 79 files have unknown license. Detailed output of licensecheck in /home/ben/fedora/review/2297646-rust-exr/licensecheck.txt [x]: License file installed when any subpackage combination is installed. $ rpm -qL -p results/rust-exr-devel-1.72.0-1.fc41.noarch.rpm /usr/share/cargo/registry/exr-1.72.0/LICENSE.md [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. [x]: 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]: Package is not known to require an ExcludeArch tag. [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. This is basically true: LICENSE.md contains two separate BSD-3-Clause license texts, plus some descriptive text, which is not quite as nice as one-license-per-file, but should be sufficient to satisfy packaging guidelines and legal/license requirements. [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]: 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 ===== 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). [-]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rust- exr-devel , rust-exr+default-devel [x]: 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. [x]: Package should compile and build into binary rpms on all supported architectures. https://koji.fedoraproject.org/koji/taskinfo?taskID=120391533 [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [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]: 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: There are rpmlint messages (see attachment). [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: rust-exr-devel-1.72.0-1.fc41.noarch.rpm rust-exr+default-devel-1.72.0-1.fc41.noarch.rpm rust-exr-1.72.0-1.fc41.src.rpm ============================================================================================ rpmlint session starts ============================================================================================ rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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/tmpw3a_sk0y')] checks: 32, packages: 3 rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/CONTRIBUTORS.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/GUIDE.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/README.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/releasing.md rust-exr+default-devel.noarch: W: no-documentation ====================================================== 3 packages and 0 specfiles checked; 0 errors, 5 warnings, 12 filtered, 0 badness; has taken 0.3 s ======================================================= Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.13/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: 32, packages: 2 rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/CONTRIBUTORS.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/GUIDE.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/README.md rust-exr-devel.noarch: W: wrong-file-end-of-line-encoding /usr/share/cargo/registry/exr-1.72.0/releasing.md rust-exr+default-devel.noarch: W: no-documentation 2 packages and 0 specfiles checked; 0 errors, 5 warnings, 8 filtered, 0 badness; has taken 0.2 s Source checksums ---------------- https://crates.io/api/v1/crates/exr/1.72.0/download#/exr-1.72.0.crate : CHECKSUM(SHA256) this package : 887d93f60543e9a9362ef8a21beedd0a833c5d9610e18c67abe15a5963dcb1a4 CHECKSUM(SHA256) upstream package : 887d93f60543e9a9362ef8a21beedd0a833c5d9610e18c67abe15a5963dcb1a4 Requires -------- rust-exr-devel (rpmlib, GLIBC filtered): (crate(bit_field/default) >= 0.10.1 with crate(bit_field/default) < 0.11.0~) (crate(flume) >= 0.11.0 with crate(flume) < 0.12.0~) (crate(half/default) >= 2.1.0 with crate(half/default) < 3.0.0~) (crate(lebe/default) >= 0.5.2 with crate(lebe/default) < 0.6.0~) (crate(miniz_oxide/default) >= 0.7.1 with crate(miniz_oxide/default) < 0.8.0~) (crate(rayon-core/default) >= 1.11.0 with crate(rayon-core/default) < 2.0.0~) (crate(smallvec/default) >= 1.7.0 with crate(smallvec/default) < 2.0.0~) (crate(zune-inflate) >= 0.2.3 with crate(zune-inflate) < 0.3.0~) (crate(zune-inflate/zlib) >= 0.2.3 with crate(zune-inflate/zlib) < 0.3.0~) cargo rust rust-exr+default-devel (rpmlib, GLIBC filtered): cargo crate(exr) Provides -------- rust-exr-devel: crate(exr) rust-exr-devel rust-exr+default-devel: crate(exr/default) rust-exr+default-devel Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2297646 Buildroot used: fedora-rawhide-x86_64 Active plugins: Shell-api, Generic Disabled plugins: Java, Perl, Ocaml, PHP, R, Python, fonts, C/C++, Haskell, SugarActivity Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
Thank you for the review! I've added the second suggestion (only processing .md and .rs files) and uploaded new spec and SRPM files at the same URLs. [fedora-review-service-build]
Created attachment 2040222 [details] The .spec file difference from Copr build 7731112 to 7777482
Copr build: https://copr.fedorainfracloud.org/coprs/build/7777482 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2297646-rust-exr/fedora-rawhide-x86_64/07777482-rust-exr/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Fabio Valentini from comment #4) > Thank you for the review! > I've added the second suggestion (only processing .md and .rs files) and > uploaded new spec and SRPM files at the same URLs. > > [fedora-review-service-build] I’m on a phone, but the spec-file diff shows that you are running dos2unix on all files, without filtering on extension, i.e. the first suggestion. That should be OK, as there are no binary files in the crate and dos2unix itself does some kind of heuristic filtering to avoid mangling binary files, but it’s not what your comment said you intended to do.
Copr build: https://copr.fedorainfracloud.org/coprs/build/7777581 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2297646-rust-exr/fedora-rawhide-x86_64/07777581-rust-exr/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Ben Beasley from comment #7) > (In reply to Fabio Valentini from comment #4) > > Thank you for the review! > > I've added the second suggestion (only processing .md and .rs files) and > > uploaded new spec and SRPM files at the same URLs. > > > > [fedora-review-service-build] > > I’m on a phone, but the spec-file diff shows that you are running dos2unix > on all files, without filtering on extension, i.e. the first suggestion. > > That should be OK, as there are no binary files in the crate and dos2unix > itself does some kind of heuristic filtering to avoid mangling binary files, > but it’s not what your comment said you intended to do. I’m going to go ahead and approve the package, because it is acceptable as-is, but please double-check this to make sure you’re doing exactly what you wanted to.
Thanks! You are correct. I pasted the wrong line. I will use the one that filters for *.rs and *.md files only.
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-exr
Imported and built: https://bodhi.fedoraproject.org/updates/FEDORA-2024-f16adf9339 And I also opened an issue upstream about all files having CRLF line endings.