Bug 2183860 - Review Request: rust-sctk-adwaita - Adwaita-like SCTK Frame
Summary: Review Request: rust-sctk-adwaita - Adwaita-like SCTK Frame
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/sctk-adwaita
Whiteboard:
Depends On: 2098370 2183866 2183874
Blocks: 2111281
TreeView+ depends on / blocked
 
Reported: 2023-04-02 19:18 UTC by Aleksei Bavshin
Modified: 2023-05-19 15:53 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-05-19 15:53:46 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Comment 1 Fabio Valentini 2023-05-01 20:34:02 UTC
Package looks good to me, with one exception: The crate seems to bundle "Cantarell Regular" font (at "/src/title/Cantarell-Regular.ttf").

After a quick look at the code, it looks like this file is only embedded as a fallback font if the system font can't be loaded ... either way, it should be possible to un-bundle this file at build-time
(i.e. add something like `BuildRequires: font(cantarell)`, and replace the path in the include_bytes! macro (in src/title/ab_glyph_render.rs:12) with the path to the system copy).

Additionally, v0.6.0 of this crate has been released - but if you need v0.5.* for alacritty, then keeping this package at <0.6.0 for now is fine.

Comment 2 Aleksei Bavshin 2023-05-02 05:32:27 UTC
Spec URL: https://alebastr.fedorapeople.org/rust/alacritty-0.12.0/rust-sctk-adwaita/rust-sctk-adwaita.spec
SRPM URL: https://alebastr.fedorapeople.org/rust/alacritty-0.12.0/rust-sctk-adwaita/rust-sctk-adwaita-0.5.4-1.fc38.src.rpm

Changes:
 - Updated to 0.5.4
 - Removed bundled font

> (i.e. add something like `BuildRequires: font(cantarell)`, and replace the path in the include_bytes! macro (in src/title/ab_glyph_render.rs:12) with the path to the system copy).

`font(cantarell)` is ambiguous and one of the providers does not have the file we want, so I had to use `abattis-cantarell-fonts`. File dependency could be better, but AFAIR we don't want to have these outside of blessed prefixes (/bin, /usr/bin, /usr/libexec, etc...).

> Additionally, v0.6.0 of this crate has been released - but if you need v0.5.* for alacritty, then keeping this package at <0.6.0 for now is fine.

0.6.0 switches to wayland-rs 0.30/sctk 0.17. The current stable version of `winit` is not compatible with these and there won't be a compatible release for the next few months (https://github.com/rust-windowing/winit/issues/2686).

Comment 3 Fedora Review Service 2023-05-02 05:36:15 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5866247
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2183860-rust-sctk-adwaita/fedora-rawhide-x86_64/05866247-rust-sctk-adwaita/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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 4 Fabio Valentini 2023-05-02 12:21:35 UTC
Hum, is it OK to just replace a font in TTF format with an OTF file?
Other than that, looks good to me, thanks!

Comment 5 Aleksei Bavshin 2023-05-02 13:06:15 UTC
(In reply to Fabio Valentini from comment #4)
> Hum, is it OK to just replace a font in TTF format with an OTF file?
> Other than that, looks good to me, thanks!

In this case - yes. The font data is passed to https://docs.rs/ab_glyph/0.2.21/ab_glyph/struct.FontRef.html#method.try_from_slice and then parsed with owned_ttf_parser. The ttf_parser crate supports both TrueType and OpenType formats.

Comment 6 Fabio Valentini 2023-05-02 13:19:26 UTC
Perfect. Thanks for checking!

===

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 (latest version not compatible)
- license matches upstream specification (MIT) and is acceptable for Fedora
- bundled fallback font with different license replaced by system copy
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- add @rust-sig with "commit" access as package co-maintainer

- set bugzilla assignee overrides to @rust-sig (optional)

- 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

- track package in koschei for all built branches

===

The font is still included verbatim in built binaries, but since this package doesn't ship any binaries, it's not affected.
I'm not sure how to best handle this so that packages that use this crate (i.e. alacritty) will need to include the font's license in their license tag ...

Comment 7 Fedora Admin user for bugzilla script actions 2023-05-02 13:41:28 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-sctk-adwaita

Comment 8 Fabio Valentini 2023-05-19 15:53:46 UTC
This package has been imported and built:
https://bodhi.fedoraproject.org/updates/?packages=rust-sctk-adwaita


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