Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1983745 (rust-ed25519-dalek) - Review Request: rust-ed25519-dalek - Fast and efficient ed25519 EdDSA key generations, signing, and verification
Summary: Review Request: rust-ed25519-dalek - Fast and efficient ed25519 EdDSA key gen...
Keywords:
Status: CLOSED NOTABUG
Alias: rust-ed25519-dalek
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Stuart D Gathman
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: rust-curve25519-dalek rust-ed25519
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2021-07-19 16:07 UTC by Robert-André Mauchin 🐧
Modified: 2023-03-07 15:26 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-03-07 00:45:19 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Robert-André Mauchin 🐧 2021-07-19 16:07:43 UTC
Spec URL: https://eclipseo.fedorapeople.org/for-review/rust-ed25519-dalek.spec
SRPM URL: https://eclipseo.fedorapeople.org/for-review/rust-ed25519-dalek-1.0.1-1.fc35.src.rpm

Description:
Fast and efficient ed25519 EdDSA key generations, signing, and verification in pure Rust.

Fedora Account System Username: eclipseo

Comment 2 Stuart D Gathman 2021-08-07 18:55:35 UTC
< # rust-cpufeatures is only available on these arches
< ExclusiveArch: x86_64 aarch64
---
> ExclusiveArch:  %{rust_arches}

Comment 3 Stuart D Gathman 2021-08-07 19:29:35 UTC
Build requires rust-curve25519-dalek, which does not have a review request that I see.
Looks like ed25519 and ed25519-dalek need to be built together.

Comment 4 Stuart D Gathman 2021-08-07 19:31:32 UTC
I will take this even though I am just starting on rust packages.  At least it is two sets of eyes, and I did find one nit.

Comment 5 Stuart D Gathman 2021-08-07 20:03:10 UTC
Never mind about rust-arches.  You mentioned that this package supports only some rust_arches in bz#1981114

Comment 6 Stuart D Gathman 2021-08-07 22:30:36 UTC
So as soon as rust-curve25519-dalek is on QA, we can move forward with these two.

Comment 7 Fabio Valentini 2021-09-08 08:40:02 UTC
> ExclusiveArch: x86_64 aarch64

This is wrong, use %{rust_arches} instead.

Comment 8 Stuart D Gathman 2021-09-08 12:24:45 UTC
Yes, the cpufeatures package provides *optimizations* for the supported arches. It still builds and runs on other arches. So please use %{rust_arches}

Comment 9 Stuart D Gathman 2021-11-08 03:27:00 UTC
I see rust-curve25519-dalek was added to f34+ 3 months ago, so I'll look at this tomorrow.

Comment 10 Stuart D Gathman 2021-11-09 01:13:35 UTC
I can't download the spec to see if ExclusiveArch was fixed.

Comment 11 Stuart D Gathman 2021-11-09 01:16:41 UTC
I will continue with review as if you had fixed the ExclusiveArch.

Comment 12 Stuart D Gathman 2022-08-09 21:26:15 UTC
Ok, it builds on f35.  Looking up tutorial on what else to check for rust reviews.

Comment 13 Fabio Valentini 2022-08-09 22:08:03 UTC
For new packages, it's more important that they build on rawhide.

As for Rust reviews, it's usually a good idea to compare the spec file with an the original unmodified rust2rpm output, and check if it prints any error messages.
Additionally, you need to ensure that the built packages are actually installable (this is not always the case, for example, non-default optional dependencies are not needed for the build, but only when installing the built packages).

You could take a look at the Rust packaging tutorial / workshop that I held at Nest 2022 last weekend - the recording is still available, and the slides are available here: https://decathorpe.fedorapeople.org/flock2022/

Comment 14 Stuart D Gathman 2022-10-13 01:03:36 UTC
New Spec URL: https://gathman.org/linux/SPECS/rust-ed25519-dalek.spec
New SRPM URL: https://gathman.org/linux/f37/src/rust-ed25519-dalek-1.0.1-4.fc37.src.rpm


 Problem 1: conflicting requests
  - nothing provides (crate(merlin) >= 2.0.0 with crate(merlin) < 3.0.0~) needed by rust-ed25519-dalek+merlin-devel-1.0.1-4.fc37.noarch
 Problem 2: conflicting requests
  - nothing provides (crate(sha2/asm) >= 0.9.0 with crate(sha2/asm) < 0.10.0~) needed by rust-ed25519-dalek+asm-devel-1.0.1-4.fc37.noarch
 Problem 3: package rust-ed25519-dalek+batch-devel-1.0.1-4.fc37.noarch requires crate(ed25519-dalek/merlin) = 1.0.1, but none of the providers can be installed
  - conflicting requests
  - nothing provides (crate(merlin) >= 2.0.0 with crate(merlin) < 3.0.0~) needed by rust-ed25519-dalek+merlin-devel-1.0.1-1.fc37.noarch
  - nothing provides (crate(merlin) >= 2.0.0 with crate(merlin) < 3.0.0~) needed by rust-ed25519-dalek+merlin-devel-1.0.1-4.fc37.noarch
 Problem 4: package rust-ed25519-dalek+batch_deterministic-devel-1.0.1-4.fc37.noarch requires crate(ed25519-dalek/merlin) = 1.0.1, but none of the providers can be installed
  - conflicting requests
  - nothing provides (crate(merlin) >= 2.0.0 with crate(merlin) < 3.0.0~) needed by rust-ed25519-dalek+merlin-devel-1.0.1-1.fc37.noarch
  - nothing provides (crate(merlin) >= 2.0.0 with crate(merlin) < 3.0.0~) needed by rust-ed25519-dalek+merlin-devel-1.0.1-4.fc37.noarch

Comment 15 Fabio Valentini 2022-10-13 09:39:45 UTC
That means two things:

- the "asm" feature needs to be disabled, because it's disabled in our package for the "sha2" crate
- the "merlin" feature needs to be disabled, or the "merlin" crate needs to be packaged (version 2.0.1, not the latest 3.0.0)

Comment 16 Fabio Valentini 2023-02-04 14:49:45 UTC
Please continue with this review if you still need this package.

Comment 17 Package Review 2023-03-07 00:45:19 UTC
This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.

Comment 18 Stuart D Gathman 2023-03-07 02:32:28 UTC
Can there be two versions of merlin packaged?  Should I try to compile with current merlin?

Comment 19 Fabio Valentini 2023-03-07 14:41:22 UTC
Why would we need two versions?
We need either one (the one which is required by ed25519-dalek), or none (if the optional feature is disabled in ed25519-dalek).

Comment 20 Stuart D Gathman 2023-03-07 15:20:49 UTC
The app (cjdns) doesn't work with the feature disabled.  Fedora rust policy says you must use the latest on cargo.io

Comment 21 Fabio Valentini 2023-03-07 15:26:01 UTC
It actually says you SHOULD package the latest version, not MUST:
https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_crate_versions

If whatever you're working on does require an older version and does not work with the latest, then that's a sufficiently good reason for not packaging the latest version.


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