Bug 1955985 - Review Request: rubygem-ffi-yajl - Ruby FFI wrapper around YAJL 2.x [NEEDINFO]
Summary: Review Request: rubygem-ffi-yajl - Ruby FFI wrapper around YAJL 2.x
Keywords:
Status: ASSIGNED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Michel Alexandre Salim
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1114146 (view as bug list)
Depends On:
Blocks: 823344 823352 1133213
TreeView+ depends on / blocked
 
Reported: 2021-05-01 21:42 UTC by Davide Cavalca
Modified: 2022-06-01 00:45 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed:
Type: ---
michel: fedora-review?
package-review: needinfo? (michel)


Attachments (Terms of Use)

Description Davide Cavalca 2021-05-01 21:42:43 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rubygem-ffi-yajl/rubygem-ffi-yajl.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rubygem-ffi-yajl/rubygem-ffi-yajl-2.4.0-1.fc35.src.rpm

Description:
Ruby FFI wrapper around YAJL 2.x.

Fedora Account System Username: dcavalca

Comment 1 Davide Cavalca 2021-05-01 21:42:46 UTC
This package built on koji:  https://koji.fedoraproject.org/koji/taskinfo?taskID=67050984

Comment 2 Davide Cavalca 2021-05-01 21:43:50 UTC
*** Bug 1114146 has been marked as a duplicate of this bug. ***

Comment 3 Michel Alexandre Salim 2021-05-12 21:10:13 UTC
Taking this review.

Some initial issues:

Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://docs.fedoraproject.org/en-US/packaging-guidelines/
- Pure Ruby package must be built as noarch
- Package contains Requires: ruby(release).
- If (and only if) the source package includes the text of the license(s)
  in its own file, then that file, containing the text of the license(s)
  for the package is included in %license.
  Note: License file MIT-LICENSE is not marked as %license
  See: https://docs.fedoraproject.org/en-US/packaging-
  guidelines/LicensingGuidelines/#_license_text

Installation errors
-------------------
Error: 
 Problem: conflicting requests
  - nothing provides libyajl.so.2()(x86-64) needed by rubygem-ffi-yajl-2.4.0-1.fc35.x86_64

Is libyajl being reviewed? It should probably be marked as a blocker

Comment 4 Michel Alexandre Salim 2021-05-12 21:21:10 UTC
Ah, you want this:

$ sudo dnf repoquery --repo rawhide --provides yajl                         
libyajl.so.2
libyajl.so.2()(64bit)
yajl = 2.1.0-16.fc34
yajl(x86-32) = 2.1.0-16.fc34
yajl(x86-64) = 2.1.0-16.fc34

Not sure what's best here. either hardcode whether you're matching agaist 64-bit or 32-bit based on %ifarch, or... just don't specify the architecture at all.

Comment 6 Vít Ondruch 2021-05-27 07:56:17 UTC
(In reply to Vít Ondruch from comment #5)
> This is the way vetted by RPM folks:
> 
> https://src.fedoraproject.org/rpms/rubygem-ruby-vips/blob/rawhide/f/rubygem-
> ruby-vips.spec#_16
> https://bugzilla.redhat.com/show_bug.cgi?id=1870208#c5

And this should be probably used also as a BR instead of the yajl-devel.

Any volunteer to formalize this method via some macro and documenting it in packaging guidelines? ;)

Comment 7 Davide Cavalca 2021-05-27 19:20:36 UTC
Spec URL: https://dcavalca.fedorapeople.org/review/rubygem-ffi-yajl/rubygem-ffi-yajl.spec
SRPM URL: https://dcavalca.fedorapeople.org/review/rubygem-ffi-yajl/rubygem-ffi-yajl-2.4.0-2.fc35.src.rpm

Changelog:
- Fix requires for yail
- Add missing requires on ruby(release)
- Install additional license file

Comment 8 Davide Cavalca 2021-05-27 19:25:37 UTC
Note: /usr/bin/ffi-yajl-bench doesn't actually run as it requires a bunch of other gems that aren't packaged yet (oj, yajl-ruby, and something called "perftools.rb" I can't find anywhere). I'm looking into that now.

Comment 9 Vít Ondruch 2021-05-31 13:53:45 UTC
(In reply to Davide Cavalca from comment #8)
> Note: /usr/bin/ffi-yajl-bench doesn't actually run

This sounds as some development tool. Should it even be installed by default?

Comment 10 Package Review 2022-06-01 00:45:24 UTC
This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time, but it seems
that the review is still being working out by you. If this is right, please
respond to this comment clearing the NEEDINFO flag and try to reach out the
submitter to proceed with the review.

If you're not interested in reviewing this ticket anymore, please clear the
fedora-review flag and reset the assignee, so that a new reviewer can take
this ticket.

Without any reply, this request will shortly be resetted.


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