Bug 2231013 - Review Request: rust-zerocopy - Utilities for zero-copy parsing and serialization
Summary: Review Request: rust-zerocopy - Utilities for zero-copy parsing and serializa...
Keywords:
Status: CLOSED RAWHIDE
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/zerocopy
Whiteboard:
Depends On: 2231007
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-08-10 11:18 UTC by Jakub Čajka
Modified: 2023-08-28 13:35 UTC (History)
2 users (show)

Fixed In Version: rust-zerocopy-0.6.3-1.fc40
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-08-28 13:35:00 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

Comment 1 Fedora Review Service 2023-08-10 11:21:21 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6263160
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231013-rust-zerocopy/fedora-rawhide-x86_64/06263160-rust-zerocopy/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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 Jakub Čajka 2023-08-10 11:23:07 UTC
Build failure is due to the issues with GPG in f39 repos and it would fail anyway as it BRs the zerocopy-derive, its review is in the depending bug.

Comment 3 Fabio Valentini 2023-08-26 21:46:56 UTC
SRPM URL is now HTTP 404, please update.

Comment 4 Jakub Čajka 2023-08-28 08:44:32 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/jcajka/zerocopy-derive/fedora-rawhide-x86_64/06304635-rust-zerocopy/rust-zerocopy.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/jcajka/zerocopy-derive/fedora-rawhide-x86_64/06304635-rust-zerocopy/rust-zerocopy-0.6.3-1.fc40.src.rpm

Sorry, I have forgotten to update the links after the fixes similar to the one for zerocopy-derive. In same way I would like to keep the unpinning patch, just to keep the record finding re. failing tests and possible staring point for proper fix.

Comment 5 Fedora Review Service 2023-08-28 08:52:01 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6348640
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231013-rust-zerocopy/fedora-rawhide-x86_64/06348640-rust-zerocopy/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 2023-08-28 11:22:18 UTC
Package was generated with rust2rpm, simplifying the review.

- package builds and installs without errors on rawhide
? test suite is run and all unit tests pass (tests are disabled, reason is documented)
? latest version of the crate is packaged (I assume you need v0.6, so this is fine)
- license matches upstream specification (BSD-2-Clause) and is acceptable for Fedora
- license file is included with %license in %files
- package complies with Rust Packaging Guidelines

Package APPROVED.

===

Recommended post-import rust-sig tasks:

- set up package on release-monitoring.org:
  project: $crate
  homepage: https://crates.io/crates/$crate
  backend: crates.io
  version scheme: semantic
  version filter: alpha;beta;rc;pre
  distro: Fedora
  Package: rust-$crate

- add @rust-sig with "commit" access as package co-maintainer
  (should happen automatically)

- set bugzilla assignee overrides to @rust-sig (optional)

- track package in koschei for all built branches
  (should happen automatically once rust-sig is co-maintainer)

===

Three non-blocking comments:

1. You need to exclude generate-readme.sh from the installed files. Otherwise the package will have an unnecessary generated dependency on /usr/bin/bash.
You can either patch Cargo.toml to add an "exclude" setting, or use "%exclude %{crate_instdir}/generate-readme.sh" in the %files list of the -devel subpackage.

2. If you patch Cargo.toml, you MUST do that by passing the "-p" flag to "rust2rpm".
Patches to Cargo.toml can affect spec file generation, and only changes made by generating a patch with "rust2rpm -p" can be taken into account (obviously).

3. Note that you messed up the spec file for rust-zerocopy-derive on import.
You need to import the *unmodified* spec file (i.e. not the one from inside the SRPM / not processed by rpmautospec).
Please take care of that when importing this package.

I fixed the rust-zerocopy-derive issue:
https://src.fedoraproject.org/rpms/rust-zerocopy-derive/c/507dd2022c6e91fd1748e1862ab6ad2ef8e212d9?branch=rawhide

===

PS: I will likely request stable dist-git branches of zerocopy / zerocopy-derive at some point, since it will allow us to enable currently disabled optional features of a handful of other crates.

PPS: I wonder what you are packaging this crate for?

Comment 7 Jakub Čajka 2023-08-28 11:45:59 UTC
Thank you very much for quick review. For rather exotic package of s390utils ;). I will try to make update as soon as possible to 0.70, but it really hangs on the s390utils.

Comment 8 Fedora Admin user for bugzilla script actions 2023-08-28 11:51:47 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-zerocopy

Comment 9 Jakub Čajka 2023-08-28 11:53:44 UTC
Nitpick on the rpmautospec, its seems that the packaging tooling it not really working well with it, i.e. fedpkg import just cryptically fails. I don't have atm that error handy. Is there some plan to add support or more clear error message(pointing to how to do the import properly)? Should I open an bug for it?

Comment 10 Fabio Valentini 2023-08-28 12:09:36 UTC
(In reply to Jakub Čajka from comment #7)
> Thank you very much for quick review. For rather exotic package of s390utils
> ;). I will try to make update as soon as possible to 0.70, but it really
> hangs on the s390utils.

Ah, interesting. Another project that started using Rust for some of its code :)

Sure. There's no need to update yet. As far as I can tell, almost all users of the zerocopy crate still depend on v0.6, so updating to v0.7 and adding a compat package for v0.6 would be counter-productive.

(In reply to Jakub Čajka from comment #9)
> Nitpick on the rpmautospec, its seems that the packaging tooling it not
> really working well with it, i.e. fedpkg import just cryptically fails. I
> don't have atm that error handy. Is there some plan to add support or more
> clear error message(pointing to how to do the import properly)? Should I
> open an bug for it?

Yeah, "fedpkg import" is one of the few remaining things that don't work well with rpmautospec.
For now, just copy-paste and "git add" the original / unmodified (!) spec, source file(s) and patch(es) (if any) manually.

The error message for "fedpkg import" was added because importing an SRPM that was already processed will lead to a package that no longer uses rpmautospec at all ...
The issue about how to handle rpmautospec support in "fedpkg import" is here:
https://pagure.io/rpkg/issue/641 / https://pagure.io/fedora-infra/rpmautospec/pull-request/292

Comment 11 Jakub Čajka 2023-08-28 12:29:06 UTC
(In reply to Fabio Valentini from comment #10)
> (In reply to Jakub Čajka from comment #7)
> > Thank you very much for quick review. For rather exotic package of s390utils
> > ;). I will try to make update as soon as possible to 0.70, but it really
> > hangs on the s390utils.
> 
> Ah, interesting. Another project that started using Rust for some of its
> code :)
> 
> Sure. There's no need to update yet. As far as I can tell, almost all users
> of the zerocopy crate still depend on v0.6, so updating to v0.7 and adding a
> compat package for v0.6 would be counter-productive.
> 
> (In reply to Jakub Čajka from comment #9)
> > Nitpick on the rpmautospec, its seems that the packaging tooling it not
> > really working well with it, i.e. fedpkg import just cryptically fails. I
> > don't have atm that error handy. Is there some plan to add support or more
> > clear error message(pointing to how to do the import properly)? Should I
> > open an bug for it?
> 
> Yeah, "fedpkg import" is one of the few remaining things that don't work
> well with rpmautospec.
> For now, just copy-paste and "git add" the original / unmodified (!) spec,
> source file(s) and patch(es) (if any) manually.
> 

Ack, I have tried, but I guess I have missed the autochangelog. I will fix it with next build.

> The error message for "fedpkg import" was added because importing an SRPM
> that was already processed will lead to a package that no longer uses
> rpmautospec at all ...
> The issue about how to handle rpmautospec support in "fedpkg import" is here:
> https://pagure.io/rpkg/issue/641 /
> https://pagure.io/fedora-infra/rpmautospec/pull-request/292

Thanks.


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