Bug 1981114 (rust-curve25519-dalek) - Review Request: rust-curve25519-dalek - Pure-Rust implementation of group operations on ristretto255 and Curve25519
Summary: Review Request: rust-curve25519-dalek - Pure-Rust implementation of group ope...
Keywords:
Status: CLOSED RAWHIDE
Alias: rust-curve25519-dalek
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jens Petersen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: rust-ed25519-dalek
TreeView+ depends on / blocked
 
Reported: 2021-07-11 14:19 UTC by Robert-André Mauchin 🐧
Modified: 2021-09-13 18:15 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-09-08 09:05:48 UTC
Type: ---
Embargoed:
petersen: fedora-review+


Attachments (Terms of Use)

Description Robert-André Mauchin 🐧 2021-07-11 14:19:31 UTC
Spec URL: https://eclipseo.fedorapeople.org/for-review/rust-curve25519-dalek.spec
SRPM URL: https://eclipseo.fedorapeople.org/for-review/rust-curve25519-dalek-3.1.0-1.fc35.src.rpm

Description:
Pure-Rust implementation of group operations on ristretto255 and Curve25519.

Fedora Account System Username: eclipseo

Comment 2 Jens Petersen 2021-07-31 14:22:10 UTC
I assume:

%if %{__cargo_skip_build}
BuildArch:      noarchspe
%endif                ^^^

is a typo.

LICENSE and src headers in
https://github.com/dalek-cryptography/curve25519-dalek/tree/3.1.0
looks correct as BSD.

No surprising divergences from vanilla rust2rpm spec file.

Koji scratch: https://koji.fedoraproject.org/koji/taskinfo?taskID=73028583
does seems to build on all archs.

Looks like dalek.rs 's webcert might have expired.

Package is APPROVED

Comment 3 Jens Petersen 2021-08-02 07:15:37 UTC
One please tweak: revisiting the cpufeatures review,
I think should package can and should be enabled for all rust_arches?

Comment 4 Robert-André Mauchin 🐧 2021-08-02 08:16:11 UTC
Thanks for the review(!

In reply to Jens Petersen from comment #3)
> One please tweak: revisiting the cpufeatures review,
> I think should package can and should be enabled for all rust_arches?

The readme specifies that it is only working for aarch64 and x86_64: https://crates.io/crates/cpufeatures

Comment 5 Stuart D Gathman 2021-08-07 20:01:33 UTC
I missed that.

Comment 6 Gwyn Ciesla 2021-08-09 20:32:47 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-curve25519-dalek

Comment 7 Fabio Valentini 2021-08-10 21:09:25 UTC
(In reply to Jens Petersen from comment #3)
> One please tweak: revisiting the cpufeatures review,
> I think should package can and should be enabled for all rust_arches?

Hi Jens, what do you mean here?

cpufeatures is an aarch64 and x86_64 specific dependency, but it is also only a dependency on those architectures ...
It should always be scoped to those two targets and not be a dependency otherwise, like here, in sha2:
https://github.com/RustCrypto/hashes/blob/master/sha2/Cargo.toml#L24

Comment 8 Stuart D Gathman 2021-08-11 02:19:26 UTC
I did a scratch build with all %{rust_arches}.  https://koji.fedoraproject.org/koji/taskinfo?taskID=73652213
It all but armv7hl have finished and unit tests run with no errors as I write.  I think there might be a misunderstanding about what cpufeatures means.  Apparently, the library can use CPU features to accelerate operations - but only on the supported CPUs.  The other arches work, but are slower.  I believe now that you should use %{rust_arches}.

Comment 9 Stuart D Gathman 2021-08-21 11:41:39 UTC
Several packages waiting on this.  Eagerly looking for files to be imported.

Comment 10 Fabio Valentini 2021-09-08 09:05:48 UTC
Looks like Robert-André imported the version from Comment#1 without addressing any issues.

I pushed a follow-up commit and builds to update the package to the latest version (3.1.0 -> 3.2.0) and fix the remaining issues ...

So this is now in rawhide, f35, and f34, and I have dropped the buildroot overrides for the faulty 3.1.0 version and made ones for 3.2.0.

Robert-André, if you're reading this, please add @rust-sig to this package (and your other Rust packages where you forgot to do this, for example, rust-cpufeatures). I already added it to anitya / release-monitoring and to koschei.

Comment 11 Robert-André Mauchin 🐧 2021-09-13 18:15:40 UTC
Sorry I must have missed it.


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