Bug 2272354 - Review Request: rust-yasna - ASN.1 library for Rust
Summary: Review Request: rust-yasna - ASN.1 library for Rust
Keywords:
Status: CLOSED DUPLICATE of bug 2312030
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2272352 2272355 2296139
TreeView+ depends on / blocked
 
Reported: 2024-03-31 07:25 UTC by Peter Robinson
Modified: 2024-09-13 10:02 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-09-12 20:55:25 UTC
Type: ---
Embargoed:
decathorpe: fedora-review?


Attachments (Terms of Use)

Description Peter Robinson 2024-03-31 07:25:39 UTC
SPEC: https://pbrobinson.fedorapeople.org/rust-yasna.spec
SRPM: https://pbrobinson.fedorapeople.org/rust-yasna-0.5.2-1.fc39.src.rpm

Description:
Generic session middleware for the warp HTTP framework

FAS: pbrobinson

koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=115678137

Comment 1 Fabio Valentini 2024-04-06 12:06:11 UTC
1. unaddressed FIXME:

> # FIXME: no license files detected

This needs to be addressed in one way or another, I see you already filed an issue - thanks!
https://github.com/qnighy/yasna.rs/issues/76

Ideally, upstream should be asked to include "LICENSE-*" files here:
https://github.com/qnighy/yasna.rs/blob/master/Cargo.toml#L14-L17

And publish new versions. Can you submit a PR for this?

Until that is fixed upstream, you will need to include the license files from
https://github.com/qnighy/yasna.rs manually as separate source files.

2. Why do tests fail on 32-bit x86? Is this a real problem in the code or problems with the tests?

It looks like the Debian packagers also found this problem and submitted a PR to fix something:
https://github.com/qnighy/yasna.rs/pull/73

Not sure if this addresses *all* failures on i686, but it would be great if that commit could be included as a patch and the tests enabled unconditionally.

Comment 2 Peter Robinson 2024-04-07 00:35:27 UTC
(In reply to Fabio Valentini from comment #1)
> 1. unaddressed FIXME:
> 
> > # FIXME: no license files detected
> 
> This needs to be addressed in one way or another, I see you already filed an
> issue - thanks!
> https://github.com/qnighy/yasna.rs/issues/76
> 
> Ideally, upstream should be asked to include "LICENSE-*" files here:
> https://github.com/qnighy/yasna.rs/blob/master/Cargo.toml#L14-L17
> 
> And publish new versions. Can you submit a PR for this?

It's on my backlog but I don't see upstream fix should block it, yes I'll update locally to add the LICENSE file.

> Until that is fixed upstream, you will need to include the license files from
> https://github.com/qnighy/yasna.rs manually as separate source files.
> 
> 2. Why do tests fail on 32-bit x86? Is this a real problem in the code or
> problems with the tests?

TBH I don't have time to look into it and I don't personally care about 32 bit

> Not sure if this addresses *all* failures on i686, but it would be great if
> that commit could be included as a patch and the tests enabled
> unconditionally.

I would generally just err on disabling 32 bit in general, can you explain why it matters?

Comment 4 Fabio Valentini 2024-04-15 14:30:23 UTC
Sorry for the delay.

I just checked, and the patch that was proposed by debian packagers indeed fixes the test failures.
Can you include it make tests run unconditionally?

Ideally, the license files would also come from upstream URLs instead of being plain files, to indicate that they are, indeed, from upstream.

Something like this should do it:

"""
Source1:        https://github.com/qnighy/yasna.rs/raw/yasna-0.5.0/LICENSE-APACHE
Source2:        https://github.com/qnighy/yasna.rs/raw/yasna-0.5.0/LICENSE-MIT

# backport upstream patch to fix test failures on 32-bit architectures
Patch:          https://github.com/qnighy/yasna.rs/commit/bf5afbe.patch
"""

Comment 5 Fabio Valentini 2024-07-05 17:06:02 UTC
Are you still interested in getting this crate packaged?

Comment 6 Peter Robinson 2024-07-05 17:10:13 UTC
(In reply to Fabio Valentini from comment #5)
> Are you still interested in getting this crate packaged?

Yes, need it for the newer parsec-tool

Comment 7 Davide Cavalca 2024-07-06 23:04:58 UTC
I also need this packaged. fwiw, it builds fine on i686 for me when using the following rust2rpm.toml

[package]
extra-sources = [
  { number = 2, file = "https://github.com/qnighy/yasna.rs/raw/yasna-0.5.0/LICENSE-APACHE", comments = ["https://github.com/qnighy/yasna.rs/issues/76"]},
  { number = 3, file = "https://github.com/qnighy/yasna.rs/raw/yasna-0.5.0/LICENSE-MIT", comments = ["https://github.com/qnighy/yasna.rs/issues/76"]},
]
extra-patches = [
  { number = 2, file = "https://github.com/qnighy/yasna.rs/pull/73.patch", comments = ["Fix test failure on i686"]},
]

[scripts]
prep.pre = ["cp -p %SOURCE2 %SOURCE3 ."]

Comment 8 Fabio Valentini 2024-07-07 21:44:31 UTC
Turns out I will need rcgen -> yasna too for the rustls / quinn stack soon.
Peter, if you don't have the cycles to continue this package review, would it be OK if Davide or me submitted a new ticket to supersede yours?

Comment 9 Fabio Valentini 2024-09-12 20:55:25 UTC

*** This bug has been marked as a duplicate of bug 2312030 ***

Comment 10 Peter Robinson 2024-09-13 10:02:14 UTC
(In reply to Fabio Valentini from comment #8)
> Turns out I will need rcgen -> yasna too for the rustls / quinn stack soon.
> Peter, if you don't have the cycles to continue this package review, would
> it be OK if Davide or me submitted a new ticket to supersede yours?

Yup, sorry, I completely missed that update.


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