Spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/dorinda/rutabaga_gfx/rust-rutabaga_gfx.git/plain/rust-rutabaga_gfx.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dorinda/rutabaga_gfx/fedora-rawhide-x86_64/08372697-rust-rutabaga_gfx/rust-rutabaga_gfx-0.1.4-1.fc42.src.rpm Description: rutabaga_gfx - Handling virtio-gpu protocols Fedora Account System Username: dorinda
Copr build: https://copr.fedorainfracloud.org/coprs/build/8371965 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2331330-rutabaga_gfx/fedora-rawhide-x86_64/08371965-rust-rutabaga_gfx/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.
Spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/dorinda/rutabaga_gfx/rust-rutabaga_gfx.git/plain/rust-rutabaga_gfx.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dorinda/rutabaga_gfx/fedora-rawhide-x86_64/08368537-rust-rutabaga_gfx/rust-rutabaga_gfx-0.1.4-1.fc42.src.rpm Description: rutabaga_gfx - Handling virtio-gpu protocols Fedora Account System Username: dorinda (fixing spec url to use plain/raw text, should help fedora-review -b)
taking for review
Package was generated with rust2rpm, simplifying the review. ✅ package contains only permissible content ✅ 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 and is acceptable for Fedora ✅ license file is included with %license in %files ✅ package complies with Rust Packaging Guidelines Package APPROVED. === A following task is to update the existing rutabaga-gfx-ffi package, that "bundles" the rust code. Now that it is released and standalone, we can adjust its dependencies. Sadly, ffi was not released with rutabaga-gfx (https://crates.io/search?q=rutabaga). Also upstream lacks the corresponding tags (https://chromium.googlesource.com/crosvm/crosvm/). I will reach the maintainer gurchetansingh. Recommended post-import rust-sig tasks: - set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version (*NOT* pre-release) filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate - add @rust-sig with "commit" access as package co-maintainer (should happen automatically) - set bugzilla assignee overrides to @rust-sig (optional) - track package in koschei for all built branches (should happen automatically once rust-sig is co-maintainer)
I see one problem wit this ticket - the package name in the spec file is rust-rutabaga_gfx, but the ticket is for rutabaga_gfx. They MUST match.
The manual patch for Cargo.toml is also not documented - it needs at least a short comment. Especially since the patch is wrong. :) crates should have *either* a package.license-file *or* a package.license property in Cargo.toml, but not both. The license-file setting is only for non-standard licenses that cannot be expressed in SPDX form. Since this is the case here (assuming that setting the license to "BSD-3-Clause" is correct?), only package.license should be set. c.f. https://doc.rust-lang.org/cargo/reference/manifest.html#the-license-and-license-file-fields
> # current bindgen limitation > ExclusiveArch: x86_64 aarch64 %{ix86} What does this mean? > %license %{crate_instdir}/LICENSE > %exclude %{crate_instdir}/LICENSE What is this supposed to accomplish? It's wrong. The way it's generated by rust2rpm is correct, the warnings from RPM about duplicately listed files is harmless and can be ignored.
Hi Fabio, Thank you for the review, the rust2rpm could not generate a spec file due to this custom license so I patched it with the BSD-3-Clause which I found for Crosvm. I will fix the spec removing that patch then. Thanks!
> Hi Fabio, Thank you for the review, the rust2rpm could not generate a spec file due to this custom license so I patched it with the BSD-3-Clause which I found for Crosvm. I will fix the spec removing that patch then. Thanks! No, that's not what I said. The Cargo.toml should have *only* package.license, and *not* package.license-file. The license text seems to be standard BSD-3-Clause, so that is the correct way to represent this in crate metadata.
> What does this mean? > the virglrenderer generated bindings fail to build for the other architecture not listed, Some compilation problem with the other architecture. I can rebuild it with the other architectures to share the log. > > %license %{crate_instdir}/LICENSE > > %exclude %{crate_instdir}/LICENSE > > What is this supposed to accomplish? It's wrong. > The way it's generated by rust2rpm is correct, the warnings from RPM about > duplicately listed files is harmless and can be ignored. It was meant to fix the warning, but this makes sense. Noted. > The Cargo.toml should have *only* package.license, and *not* package.license-file. ack.
> the virglrenderer generated bindings fail to build for the other architecture not listed, Some compilation problem with the other architecture. I can rebuild it with the other architectures to share the log. Thanks, that information was missing for me. a build log so I can see the failures would be appreciated! Maybe we can fix the issue.
sure, I will appreciate that - see logs at https://download.copr.fedorainfracloud.org/results/dorinda/rutabaga_gfx/fedora-rawhide-s390x/08372210-rust-rutabaga_gfx/builder-live.log.gz and https://download.copr.fedorainfracloud.org/results/dorinda/rutabaga_gfx/fedora-rawhide-ppc64le/08372210-rust-rutabaga_gfx/builder-live.log.gz
Thanks! I was able to find the problem pretty quickly. In the src/generated/virtl_debug_callback_binding.rs file, the `stdio` module is only defined for x86_64, x86, arm, aarch64, and riscv64, but not s390x or powerpc64. So the fact that this package doesn't compile on ppc64le and s390x is a problem in the upstream source code, not with bindgen. Using ExclusiveArch is fine in this case.
Note that the rust-rutabaga_gfx-devel package also has a "Requires: python3" that isn't wanted due to the *.py script in that same folder (src/generated/). They should be excluded from the package to avoid the dependency on python3.
Thanks for review, I have updated the spec file addressing your comments.
Spec URL: https://copr-dist-git.fedorainfracloud.org/cgit/dorinda/rutabaga_gfx/rust-rutabaga_gfx.git/plain/rust-rutabaga_gfx.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/dorinda/rutabaga_gfx/fedora-rawhide-x86_64/08372697-rust-rutabaga_gfx/rust-rutabaga_gfx-0.1.4-1.fc42.src.rpm
Hi @decathorpe I have updated the spec file and SRPM can you give your review? Thanks!
Hi - sorry for taking so long to respond. The package is already approved by Marc-Andre, you needn't have waited for me. That aside, the Spec URL above currently doesn't work because copr-dist-git-cgit has been shut down temporarily. :)
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-rutabaga_gfx
FEDORA-2025-c8dc7bc2bd (rust-rutabaga_gfx-0.1.4-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2025-c8dc7bc2bd
FEDORA-2025-c8dc7bc2bd (rust-rutabaga_gfx-0.1.4-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.