https://bugzilla.redhat.com/show_bug.cgi?id=2033294 --- Comment #8 from Daiki Ueno <dueno@xxxxxxxxxx> --- Thanks for the prompt feedback. I've updated spec and SRPM at: Spec file: https://ueno.fedorapeople.org/keylime-agent-rust/keylime-agent-rust.spec SRPM: https://ueno.fedorapeople.org/keylime-agent-rust/keylime-agent-rust-0.1.0~20211110gitd5a3191-1.fc36.src.rpm (In reply to Fabio Valentini from comment #7) > Great, package looks almost ready for approval now, just a few minor > problems left: > > > Version: %{crate_version}~%{commitdate}g%{shortcommit} > > This looks like a typo, I think you meant to use "git" instead of literal > "g"? Fixed; the versioning guideline says: "The <scm> string may be abbreviated to a single letter", but "git" seems to be clearer. > > Release: 0.1%{?dist} > > This is incorrect when you use the tilde-based versioning for pre-releases. > You can drop the leading "0." (which is used to identify pre-releases in the > old versioning scheme) because using the "~" based Version already expresses > the pre-release sorting. > > c.f. examples in the Versioning Guidelines: > https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/ > #_complex_versioning_with_a_reasonable_upstream Thanks, updated. > > # The source tarball is downloaded using the following commands: > > # wget https://github.com/keylime/rust-keylime/archive/%%{commit}/rust-keylime-%%{version}.tar.gz > > #Source0: %%{crates_source} > > Source0: rust-keylime-%{version}.tar.gz > > You could use something like this directly: > > Source0: %{url}/archive/%{commit}/rust-keylime-%{version}.tar.gz > %autosetup -n rust-keylime-%{commit} -p1 > > This will make it work with spectool -g. > c.f. > https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/ > #_commit_revision > > If that's not something that's important to you, feel free to keep the > current setup, since spectool won't be able to download your second source > tarball anyway. I didn't know spectool; fixed the Source0 line and the comment. > > #Source0: %%{crates_source} > And you can drop this line entirely, it's not used and does not carry any > information that is relevant for this package. Removed. > > The reason was that %check requires wiremock and its dependencies (which are quite a few): > > Would this be an acceptable justification or should we try hard to bring in those dependencies? > > This is perfectly acceptable. We only enable running test suites for Rust > packages if there's not an unreasonable amount of additional dependencies. > Just add "# missing dev-dependencies: wiremock" or something like that above > the "%bcond_with check" statement, so the reason why tests are disabled is > documented. Added the comment. > > %if 0%{?rhel} && !0%{?eln} > > # RHEL: Use bundled deps as it doesn't ship Rust libraries > > %global bundled_rust_deps 1 > > %else > > # Fedora: Use only system Rust libraries > > %global bundled_rust_deps 0 > > %endif > > I am unsure about this. As far as I know, Rust packages in ELN are supposed > to use bundled dependencies (because otherwise this pulls in almost the > entire Rust stack into ELN). If keylime-agent-rust is going to be included > in ELN, please verify with ELN folks if you should use bundled dependencies > there or not. I've just removed !0%{?eln} conditional for now; will change accordingly after I hear back from the ELN folks. -- 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=2033294 _______________________________________________ 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 on the list, report it: https://pagure.io/fedora-infrastructure
