Bug 2314124 - Review Request: rust-k256 - Secp256k1 elliptic curve library written in pure Rust
Summary: Review Request: rust-k256 - Secp256k1 elliptic curve library written in pure ...
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/k256
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2024-09-22 20:57 UTC by Kai A. Hiller
Modified: 2024-11-18 10:01 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
Embargoed:
decathorpe: fedora-review?


Attachments (Terms of Use)
The .spec file difference from Copr build 8060891 to 8069679 (1.23 KB, patch)
2024-09-25 13:53 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8069679 to 8159977 (1.19 KB, patch)
2024-10-20 18:50 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 8159977 to 8274697 (1.18 KB, patch)
2024-11-18 10:01 UTC, Fedora Review Service
no flags Details | Diff

Description Kai A. Hiller 2024-09-22 20:57:48 UTC
Spec URL: https://v02460.fedorapeople.org/rust-k256.spec
SRPM URL: https://v02460.fedorapeople.org/rust-k256-0.13.4-1.fc42.src.rpm
Description: Secp256k1 elliptic curve library written in pure Rust
Fedora Account System Username: v02460

Comment 1 Fedora Review Service 2024-09-23 09:35:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8060891
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314124-rust-k256/fedora-rawhide-x86_64/08060891-rust-k256/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 2 Fabio Valentini 2024-09-23 13:29:27 UTC
Please only ever use `rust2rpm -p` for patching Cargo.toml.

And please add a short comment why patching out criterion is OK / necessary.

Comment 3 Kai A. Hiller 2024-09-25 13:33:22 UTC
Spec URL: https://v02460.fedorapeople.org/rust-k256.spec
SRPM URL: https://v02460.fedorapeople.org/rust-k256-0.13.4-2.fc42.src.rpm

I hope this one is better. TIL about rust2rpm -p :D

Comment 4 Fedora Review Service 2024-09-25 13:53:02 UTC
Created attachment 2048665 [details]
The .spec file difference from Copr build 8060891 to 8069679

Comment 5 Fedora Review Service 2024-09-25 13:53:05 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8069679
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314124-rust-k256/fedora-rawhide-x86_64/08069679-rust-k256/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 6 Fabio Valentini 2024-10-03 11:47:31 UTC
-# * Remove criterion dev-dependency. The criterion crate is used for
-#   micro-benchmarking, which adds no value to the packaging process.
-Patch2:        Strip-criterion-dependency.diff
+# Manually created patch for downstream crate metadata changes
+Patch:          k256-fix-metadata.diff

Please don't rename this manually, it will only cause you more work long-term since you'd need to re-apply this for every new version.
The file name for the Cargo.toml metadata patch and the accompanying comment are standardized across all Rust crates.

Comment 7 Kai A. Hiller 2024-10-20 16:40:17 UTC
I don’t think I fully get it:

1. Where should I add the comment that you requested then, if not in the rust2rpm.toml?
2. Regarding further updates: I tried re-running rust2rpm and rust2rpm -p, both removed the existing Patch line from the spec, so I’ll have to perform the same change again for every update.

Of course I can solve both cases with manual edits after spec re-generation, but I think you implied it’s possible to not do that manual work.

Comment 8 Kai A. Hiller 2024-10-20 16:49:48 UTC
Spec URL: https://v02460.fedorapeople.org/rust-k256.spec
SRPM URL: https://v02460.fedorapeople.org/rust-k256-0.13.4-3.fc42.src.rpm

New version with standard name for diff file, still missing a comment explaining the patch.

Comment 9 Fedora Review Service 2024-10-20 18:50:43 UTC
Created attachment 2052915 [details]
The .spec file difference from Copr build 8069679 to 8159977

Comment 10 Fedora Review Service 2024-10-20 18:50:45 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8159977
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314124-rust-k256/fedora-rawhide-x86_64/08159977-rust-k256/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.

Comment 11 Fabio Valentini 2024-10-28 13:12:53 UTC
> 1. Where should I add the comment that you requested then, if not in the rust2rpm.toml?

Not sure I understand what the problem is? You can do both to preserve those changes - 
That's what the "package.cargo-toml-patch-comments" setting is for.
The values set there are added as comments associated with the patch generated by "rust2rpm -p".

> 2. Regarding further updates: I tried re-running rust2rpm and rust2rpm -p, both removed the existing Patch line from the spec, so I’ll have to perform the same change again for every update.

> Of course I can solve both cases with manual edits after spec re-generation, but I think you implied it’s possible to not do that manual work.

It's possible to avoid *almost all* manual work when doing patches for Cargo.toml, but not *all*. Everything but re-applying Cargo.toml patch changes when running "rust2rpm -p" can be automated with rust2rpm.toml settings, and there's an RFE for automatically attempting to re-apply existing patches on re-runs too:

https://pagure.io/fedora-rust/rust2rpm/issue/83
https://pagure.io/fedora-rust/rust2rpm/issue/278

I hope I'll be able to implement something like this for rust2rpm v27.

Comment 12 Kai A. Hiller 2024-11-18 09:43:27 UTC
Spec URL: https://v02460.fedorapeople.org/rust-k256.spec
SRPM URL: https://v02460.fedorapeople.org/rust-k256-0.13.4-4.fc42.src.rpm

Thanks for taking the time to clarify :)

> That's what the "package.cargo-toml-patch-comments" setting is for.

This was the missing puzzle piece for me.

> I hope I'll be able to implement something like this for rust2rpm v27.

Nice, you’re doing great work on rust2rpm!

Comment 13 Fedora Review Service 2024-11-18 10:01:16 UTC
Created attachment 2058462 [details]
The .spec file difference from Copr build 8159977 to 8274697

Comment 14 Fedora Review Service 2024-11-18 10:01:18 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8274697
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2314124-rust-k256/fedora-rawhide-x86_64/08274697-rust-k256/fedora-review/review.txt

Please take a look if any issues were found.


---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

If you want to trigger a new Copr build, add a comment containing new
Spec and SRPM URLs or [fedora-review-service-build] string.


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