https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Bug ID: 1935650 Summary: Review Request: rubygem-ffi-rzmq-core - This gem provides only the FFI wrapper for the ZeroMQ (0mq) networking library Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: jprokop@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r... SRPM URL: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r...
Description: This gem provides only the FFI wrapper for the ZeroMQ (0mq) networking library
Fedora Account System Username: jackorp
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=63127361
This package is one of the new dependencies required for cucumber update.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pvalena@redhat.com
--- Comment #1 from Pavel Valena pvalena@redhat.com --- Can we depend on versioned library "so file" instead? (That's the preferred way of specifying dependencies AFAIK.)
Like in this commit: https://github.com/fedora-distgit/rubygem-ffi-rzmq-core/commit/c0729fb1c3a2f... (Yes, we do want that, on purpose.)
As there's no binary extension, ``` BuildArch: noarch ``` we need to specify so arch-specific dependencies with richdeps (if libffi...). On the upside, there's no need for the patch.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #2 from Jarek Prokop jprokop@redhat.com --- (In reply to Pavel Valena from comment #1)
Can we depend on versioned library "so file" instead? (That's the preferred way of specifying dependencies AFAIK.)
Like in this commit: https://github.com/fedora-distgit/rubygem-ffi-rzmq-core/commit/ c0729fb1c3a2f4c5c225addfd3e07bb8de490f1b#diff- 4fe66120347be998c33ea765bccd78806cd3ebf6cc7eafef37bf2841fabbb0ec (Yes, we do want that, on purpose.)
Yes, you are right, depending on libzmq.so.5 like that does what we want.
As there's no binary extension,
BuildArch: noarch
Yes, that is specified correctly.
we need to specify so arch-specific dependencies with richdeps (if libffi...). On the upside, there's no need for the patch.
There is because the library is using hardcoded to search for `libzmq.so`[0] which is only in the `zeromq-devel` and that package pulls in many unnecessary devel dependencies (and libzmq.so is not present not even via symlink in the bare `zeromq` package).
[0] the line gets expanded into `libzmq.so` specifically, so if we require `libzmq.so.5` in spec it would pull in zeromq, but it would not work.
[0] https://github.com/chuckremes/ffi-rzmq-core/blob/master/lib/ffi-rzmq-core/li...
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #3 from Jarek Prokop jprokop@redhat.com --- update spec: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r... updated srpm: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r...
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |denis.arnaud_fedora@m4x.org Flags| |needinfo?(denis.arnaud_fedo | |ra@m4x.org) | |needinfo?(denis.arnaud_fedo | |ra@m4x.org)
--- Comment #4 from Pavel Valena pvalena@redhat.com --- (In reply to Jarek Prokop from comment #2)
(In reply to Pavel Valena from comment #1)
</snip>
we need to specify so arch-specific dependencies with richdeps (if libffi...). On the upside, there's no need for the patch.
There is because the library is using hardcoded to search for `libzmq.so`[0] which is only in the `zeromq-devel` and that package pulls in many unnecessary devel dependencies (and libzmq.so is not present not even via symlink in the bare `zeromq` package).
Ah, sorry, I didn't check the patch and thought it's doing something different. Yes, we don't want zeromq-devel. Isn't that a bug, though?
Denis, could you comment, please?
[0] the line gets expanded into `libzmq.so` specifically, so if we require `libzmq.so.5` in spec it would pull in zeromq, but it would not work.
Jarku, if it's a correct soname, can you create a PR upstream?
[0] https://github.com/chuckremes/ffi-rzmq-core/blob/master/lib/ffi-rzmq-core/ libzmq.rb#L39
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |pvalena@redhat.com Flags| |fedora-review?
--- Comment #5 from Pavel Valena pvalena@redhat.com --- I'm taking this review.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #6 from Jarek Prokop jprokop@redhat.com --- (In reply to Pavel Valena from comment #4)
(In reply to Jarek Prokop from comment #2)
(In reply to Pavel Valena from comment #1)
</snip> > > we need to specify so arch-specific dependencies with richdeps (if > > libffi...). On the upside, there's no need for the patch. > > There is because the library is using hardcoded to search for `libzmq.so`[0] > which is only in the `zeromq-devel` and that package pulls in many > unnecessary devel dependencies (and libzmq.so is not present not even via > symlink in the bare `zeromq` package).
Ah, sorry, I didn't check the patch and thought it's doing something different. Yes, we don't want zeromq-devel. Isn't that a bug, though?
Denis, could you comment, please?
Hmm, looks like at least according to ruby-ffi[1] wiki it does not seem like a bug on our end.
[0] the line gets expanded into `libzmq.so` specifically, so if we require `libzmq.so.5` in spec it would pull in zeromq, but it would not work.
Jarku, if it's a correct soname, can you create a PR upstream?
I think it should look not only for `libzmq.so` (which seems like the preferred upstream approach) but also for `libzmq.so.5`. I'll open a PR upstream and add a comment with a PR link to the spec once I do so.
[0] https://github.com/chuckremes/ffi-rzmq-core/blob/master/lib/ffi-rzmq-core/ libzmq.rb#L39
[1] https://github.com/ffi/ffi/wiki/Loading-Libraries#linux-packages
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Denis Arnaud denis.arnaud_fedora@m4x.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(denis.arnaud_fedo | |ra@m4x.org) | |needinfo?(denis.arnaud_fedo | |ra@m4x.org) |
--- Comment #7 from Denis Arnaud denis.arnaud_fedora@m4x.org --- (In reply to Jarek Prokop from comment #6)
I'll open a PR upstream and add a comment with a PR link to the spec once I do so.
Thanks!
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #8 from Jarek Prokop jprokop@redhat.com --- I opened upstream PR [0] and referenced it in the specfile. There is no functional change in the package, just the reference.
updated spec: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r... updated srpm: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-r...
[0] https://github.com/chuckremes/ffi-rzmq-core/pull/18
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vondruch@redhat.com
--- Comment #9 from Vít Ondruch vondruch@redhat.com --- I just wonder, do we really need this package? As far as I understand, this is required to satisfy Cucumber test requirements. I realize this can be also runtime dependency if chosen, but we don't have this use case, or do we?
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #10 from Jarek Prokop jprokop@redhat.com --- (In reply to Vít Ondruch from comment #9)
I just wonder, do we really need this package? As far as I understand, this is required to satisfy Cucumber test requirements. I realize this can be also runtime dependency if chosen, but we don't have this use case, or do we?
IMHO it comes down to if we want to provide that functionality by default. We can just package protobuf (with disabled zmq support and maybe some message), and if users will want to use the zeromq capability they will have to install the gem and the library.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #11 from Vít Ondruch vondruch@redhat.com --- (In reply to Jarek Prokop from comment #10)
IMHO it comes down to if we want to provide that functionality by default.
We want just enough functionality to be able to update Cucumber. If somobody needs some additional functionality, the zmq can be added later).
We can just package protobuf (with disabled zmq support and maybe some message), and if users will want to use the zeromq capability they will have to install the gem and the library.
But this is the default state, isn't it? zmq is not default runtime dependency of protobug AFAIK.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
--- Comment #12 from Jarek Prokop jprokop@redhat.com --- (In reply to Vít Ondruch from comment #11)
(In reply to Jarek Prokop from comment #10)
</snip>
But this is the default state, isn't it? zmq is not default runtime dependency of protobug AFAIK.
It is not a default runtime. Let's hold on with this review (and the subsequent rubygem-ffi-rzmq rhbz#1935699 [0]), I will investigate how much does rubygem-protobuf actually need to be able to pass cucumber-messages tests.
[0] https://bugzilla.redhat.com/show_bug.cgi?id=1935699
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Jarek Prokop jprokop@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1935699
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1935699 [Bug 1935699] Review Request: rubygem-ffi-rzmq - This gem wraps the ZeroMQ (0mq) networking library using Ruby FFI (foreign function interface)
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Jarek Prokop jprokop@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |WONTFIX Last Closed| |2021-03-23 12:59:45
--- Comment #13 from Jarek Prokop jprokop@redhat.com --- I am closing this request. This package won't be required in the end.
https://bugzilla.redhat.com/show_bug.cgi?id=1935650
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |
package-review@lists.fedoraproject.org