Bug 2227475 - Review Request: rust-librsvg - Render SVG documents with Cairo
Summary: Review Request: rust-librsvg - Render SVG documents with Cairo
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/librsvg
Whiteboard:
Depends On:
Blocks: 2224807 2224817 2227479
TreeView+ depends on / blocked
 
Reported: 2023-07-30 01:34 UTC by Davide Cavalca
Modified: 2023-08-07 01:26 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-07-30 20:57:09 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6223017 to 6223682 (1.97 KB, patch)
2023-07-30 16:41 UTC, Fedora Review Service
no flags Details | Diff

Description Davide Cavalca 2023-07-30 01:34:29 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rust-librsvg/rust-librsvg.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rust-librsvg/rust-librsvg-2.56.0-1.fc39.src.rpm

Description:
Render SVG documents with Cairo.

Fedora Account System Username: dcavalca

Comment 1 Davide Cavalca 2023-07-30 01:34:31 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=104118329

Comment 2 Fedora Review Service 2023-07-30 05:48:56 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6223017
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2227475-rust-librsvg/fedora-rawhide-x86_64/06223017-rust-librsvg/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.

Comment 3 Fabio Valentini 2023-07-30 09:22:53 UTC
Don't do this:

> %global __cargo_is_bin() false

The %__cargo_is_bin macro is an internal implementation detail (hence the __ prefix), which can change or be removed at any time (in fact, it *did* change in rust-packaging >= 24). Setting it to false / 0 is an incomplete and hacky way to disable binaries that doesn't correctly affect spec file generation.

Instead, patch Cargo.toml to remove binaries:

- remove all [[bin]] sections (if present)
- add "autobins = false" to the [package] table (if necessary)

This way rust2rpm will correctly detect that the package has no binaries and will write a library-only spec file accordingly.

===

You also don't need to manually list all the skipped tests if they share a common prefix, the "--skip" flag does substring matching unless the "--exact" flag is passed to the test harness as well.

If you indeed want to avoid matching on tests that you *don't* want to skip, then add the "--exact" flag. If you don't mind skipping some more tests than necessary, you could simplify the %cargo_test call by using the substring matching.

===

Note that if you want to package this crate for older Fedora releases as well, you will likely need separate branches / patches, since some dependencies that are only used by librsvg2 currently are at different versions in different branches (f38: lopdf 0.29, f37: lopdf 0.27).

I have started providing the same versions of libraries across all Fedora branches to avoid situations like these, but since lopdf is currently only a dependency of librsvg2, it didn't make sense to maintain multiple versions of it.

Comment 4 Davide Cavalca 2023-07-30 16:25:58 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rust-librsvg/rust-librsvg.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rust-librsvg/rust-librsvg-2.56.0-1.fc39.src.rpm

Changelog:
- patch metadata to disable binaries
- use substring matching for tests

Comment 5 Fedora Review Service 2023-07-30 16:41:06 UTC
Created attachment 1980747 [details]
The .spec file difference from Copr build 6223017 to 6223682

Comment 6 Fedora Review Service 2023-07-30 16:41:08 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6223682
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2227475-rust-librsvg/fedora-rawhide-x86_64/06223682-rust-librsvg/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.

Comment 7 Fabio Valentini 2023-07-30 18:27:25 UTC
Thanks, looks good to me now (with one exception, noted below).
This package is even running tests, which the librsvg2 package apparently doesn't 😅

===

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 (LGPL-2.1-or-later) 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:

- 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

- 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)

===

This crate tarball contains a large amount of stuff that's unused and should be dropped.
For example, there's a lot of data files (graphics and fonts with separate licenses, etc.) that should be excluded since autotools, docs, CI files, etc. are all unwanted in a pure-Rust library package.

Adding something like this to the [package] table in Cargo.toml should work
(with "%doc %{crate_instdir}/rsvg-convert.rst" removed from the file list):

"""
exclude = [
    "/benchmarks/",
    "/ci/",
    "/devel-docs/",
    "/doc/",
    "/include",
    "/m4/",
    "/tests/",
    "/tools/",
    "/win32",
    "/acinclude.m4",
    "/appveyor.yml",
    "/autogen.sh",
    "/configure.ac",
    "/deny.toml",
    "/example.svg",
    "/glib-tap.mk",
    "/INSTALL",
    "/librsvg.doap",
    "/librsvg.pc.in",
    "/librsvg-zip.in",
    "/Makefile.am",
    "/Rsvg-2.0.metadata",
    "/Rsvg-2.0-custom.vala",
    "/rsvg-convert.rst",
    "/rsvg-c-srcs.mk",
    "/tap-driver.sh",
    "/tap-test",
]
"""

Comment 8 Fedora Admin user for bugzilla script actions 2023-07-30 20:21:38 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-librsvg

Comment 9 Fedora Update System 2023-07-30 20:54:04 UTC
FEDORA-2023-bb211d2f48 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-bb211d2f48

Comment 10 Fedora Update System 2023-07-30 20:57:09 UTC
FEDORA-2023-bb211d2f48 has been pushed to the Fedora 39 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 11 Fedora Update System 2023-08-06 21:01:15 UTC
FEDORA-2023-c2d333f341 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-c2d333f341

Comment 12 Fedora Update System 2023-08-07 01:26:55 UTC
FEDORA-2023-c2d333f341 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.