https://bugzilla.redhat.com/show_bug.cgi?id=2033294



--- Comment #7 from Fabio Valentini <[email protected]> ---
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"?

> 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

> # 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.

> #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.

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

> %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.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2033294
_______________________________________________
package-review mailing list -- [email protected]
To unsubscribe send an email to [email protected]
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/[email protected]
Do not reply to spam on the list, report it: 
https://pagure.io/fedora-infrastructure

Reply via email to