https://bugzilla.redhat.com/show_bug.cgi?id=2241553 --- Comment #3 from Kalev Lember <klember@xxxxxxxxxx> --- (In reply to Fabio Valentini from comment #2) > Quick comments before I do a full review: > > > I'm slightly unsure how to best handle the aperture crate. It's developed > > in-tree, and referenced as 'path = "./aperture"' in Cargo.toml. Perhaps it > > would make sense to package it separately, but for now I've gone with > > including it in the same rpm. Opinions welcome. > > I see the "aperture" crate is also published separately: > https://crates.io/crates/aperture Right, that's why I brought this up. > There are two options for handling this case: > - use the version bundled in snapshot > - package from crates.io, replace "path" dependency with a "normal" > dependency > > So long as snapshot is the only user of the "aperture" crate, I think both > are OK. > But as soon as more things start depending on it, I would try to unbundle it. I think I'll go with the bundled version for now as I have that ready, and discuss with upstream what they think is the best way to handle it. Another aspect is that it looks like the aperture crate is getting updates more often than the app itself: aperture 0.3.2 was released without a corresponding snapshot release, so unbundling would help us get updates faster. (0.3.2 was a tiny unimportant release, but still.) > > %global tarball_version %%(echo %{version} | tr '~' '.') > > I think you could replace this with "%{version_no_tilde .}": > > """ > $ rpm --define "version 45.0~rc1" -E "%{version_no_tilde .}" > 45.0.rc1 > """ I'd like to stick with the current way for now as that's how it's done in all other GNOME packages, but this definitely could use some overhaul. I've been thinking of adding gnome-srpm-macros which would contain macros for constructing the source url for download.gnome.org and handling the ~ and . translations in version strings. > What is the license of snapshot itself? It might be good to add either > SourceLicense: GPL-3.0-or-later > > or a comment like > # snapshot: GPL-3.0-or-later > # crate dependencies: > # <... list crate licenses> Oh, good idea! I added both :) > > Provides: bundled(rust-aperture) = 0.3.1 > > This should be in a slightly different format to match other Provides for > Rust crates: > > Provides: bundled(crate(aperture)) = 0.3.1 Fixed. > You mentioned on Matrix that you needed gstreamer1-plugin-gtk4 for this > application, but I don't see it mentioned in the spec file? > Should it be a dependency (Requires: gstreamer1-plugin-gtk4), or is it > somehow added automatically? It just uses the rust crate directly and doesn't go through the plugin interface, if I understand it right. I may be wrong of course :) I'm having some trouble reading rust code. So it just needs it as a build dep, which is picked up automatically thanks to the %cargo_generate_buildrequires. Spec URL: https://kalev.fedorapeople.org/snapshot.spec SRPM URL: https://kalev.fedorapeople.org/snapshot-45.0-1.fc40.src.rpm Diff to the the previous version: https://paste.centos.org/view/74fd2061 Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=106960088 -- You are receiving this mail because: You are always notified about changes to this product and component You are on the CC list for the bug. https://bugzilla.redhat.com/show_bug.cgi?id=2241553 Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202241553%23c3 _______________________________________________ package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx 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/package-review@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
