Bug 978284 - Review Request: rubygem-redis - A Ruby client library for Redis
Review Request: rubygem-redis - A Ruby client library for Redis
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Vít Ondruch
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-26 05:24 EDT by Achilleas Pipinellis
Modified: 2013-10-02 02:38 EDT (History)
2 users (show)

See Also:
Fixed In Version: rubygem-redis-3.0.4-3.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-10-02 02:31:56 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
vondruch: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Achilleas Pipinellis 2013-06-26 05:24:11 EDT
Spec URL: http://axilleas.fedorapeople.org/pkgs/rubygem-redis/rubygem-redis.spec
SRPM URL: http://axilleas.fedorapeople.org/pkgs/rubygem-redis/rubygem-redis-3.0.4-1.fc19.src.rpm

Description: A Ruby client that tries to match Redis' API one-to-one, while still
providing an idiomatic interface. It features thread-safety,
client-side sharding, pipelining, and an obsession for performance.

Fedora Account System Username: axilleas

------

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=5539385
(Although, it fails as it cannot open a netcat connection needed by one of tests, will refer about this on devel)

Other useful logs (rpmlint, mock): http://axilleas.fedorapeople.org/pkgs/rubygem-redis/
Comment 1 Vít Ondruch 2013-06-27 05:57:52 EDT
I'll take this for a review.
Comment 2 Vít Ondruch 2013-06-27 07:03:18 EDT
(In reply to Axilleas Pipinellis from comment #0)
> (Although, it fails as it cannot open a netcat connection needed by one of
> tests, will refer about this on devel)

This can be workarounded by applying:

sed -i "s/localhost/localhost4/" test/publish_subscribe_test.rb

prior the test suite is executed. After discussion with ncat maintainer, we discovered, that the "localhost" resolves to IPv6 address, while Redis does not suppoer IPv6 yet [1]. You might want to try to propose this fix to upstream as well, until Redis is IPv6 ready.

* rpmlint error
  - You should not include dot files in RPM, as is reported by rpmlint:

    rubygem-redis-doc.noarch: E: version-control-internal-file
      /usr/share/gems/gems/redis-3.0.4/test/db/.gitignore

* Remove Requires: Redis
  - This is Redis client library. The server can run on Remote machine.
    Therefore Redis should not be runtime dependency of this package, until
    there will be some kind of soft dependencies supported by YUM/RPM


Otherwise the package looks good. Please fix these two nits.


[1] https://github.com/antirez/redis/pull/61
Comment 3 Vít Ondruch 2013-06-27 07:03:38 EDT
Actually 3 nits ;)
Comment 4 Vít Ondruch 2013-06-27 07:55:23 EDT
Just FYI: https://bugzilla.redhat.com/show_bug.cgi?id=978964
Comment 5 Achilleas Pipinellis 2013-06-27 14:04:14 EDT
(In reply to Vít Ondruch from comment #2)
> (In reply to Axilleas Pipinellis from comment #0)
> > (Although, it fails as it cannot open a netcat connection needed by one of
> > tests, will refer about this on devel)
> 
> This can be workarounded by applying:
> 
> sed -i "s/localhost/localhost4/" test/publish_subscribe_test.rb
> 
> prior the test suite is executed. After discussion with ncat maintainer, we
> discovered, that the "localhost" resolves to IPv6 address, while Redis does
> not suppoer IPv6 yet [1]. You might want to try to propose this fix to
> upstream as well, until Redis is IPv6 ready.
 
Replacing localhost with localhost4 fails with:

[422/485] TestPublishSubscribe#test_subscribe_past_a_timeoutNcat: Could not resolve hostname "localhost4": Name or service not known. QUITTING.

Which is ok, since in my /etc/hosts I have localhost6 for IPv6 and localhost for IPv4. I don't think changing these values should be the way of this fix. How come it plays nice for you? I changed it to 127.0.0.1 in order to pass the test. 

On another note, some distros still use the old gnu-netcat which doesn't fail as with ncat. That might be an issue if I opened a bug report.

Finally, I found out that by explicitly telling ncat to use IPv4, build passes. See this koji build where I passed the -4 switch to nc:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5551282


> * rpmlint error
>   - You should not include dot files in RPM, as is reported by rpmlint:
> 
>     rubygem-redis-doc.noarch: E: version-control-internal-file
>       /usr/share/gems/gems/redis-3.0.4/test/db/.gitignore

Oops, I forgot to run rpmlint on the -doc subpackage... Fixed.

> * Remove Requires: Redis
>   - This is Redis client library. The server can run on Remote machine.
>     Therefore Redis should not be runtime dependency of this package, until
>     there will be some kind of soft dependencies supported by YUM/RPM

Removed.

--------------------

New Spec URL: http://axilleas.fedorapeople.org/pkgs/rubygem-redis/rubygem-redis.spec
New SRPM URL: http://axilleas.fedorapeople.org/pkgs/rubygem-redis/rubygem-redis-3.0.4-2.fc19.src.rpm
Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5551169
Comment 6 Vít Ondruch 2013-09-02 11:31:29 EDT
Hi, sorry for the delay :/

* ncat
  - You might want to reference bug 978964 in your .spec file, since it seems
    there is also some progress with regard to fallback to IPv4 if IPv6 is not
    available. Let's see, which of the two fixes will land sooner ;)

* rpmlint
  - rpmlint still complains:

    rubygem-redis-doc.noarch: E: version-control-internal-file
        /usr/share/gems/gems/redis-3.0.4/test/db/.gitignore

  - You should exclude the file in -doc subpackage, not in the main package.

* Unstable test suite
  - It seems that test suite may fail sometimes: 

    http://koji.fedoraproject.org/koji/taskinfo?taskID=5882957

  - It might be even ARM related. Who knows ... Second build on ARM was just OK
    and it builds on my laptop just fine. So it might be just worth of
    consultation with upstream. It seems to be just floating point precision
    issue anyway.

Otherwise the package looks good => APPROVED. Please fix the rpmlint issue prior importing.
Comment 7 Achilleas Pipinellis 2013-09-03 06:18:00 EDT
Thanks, I've updated the relevant parts, see SPEC in my previous comment.

New Package SCM Request
=======================
Package Name: rubygem-redis
Short Description: A Ruby client library for Redis
Owners: axilleas
Branches: f19 f20
InitialCC:
Comment 8 Gwyn Ciesla 2013-09-03 08:20:09 EDT
Git done (by process-git-requests).
Comment 9 Fedora Update System 2013-09-22 13:37:05 EDT
rubygem-redis-3.0.4-3.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/rubygem-redis-3.0.4-3.fc19
Comment 10 Fedora Update System 2013-09-22 15:28:08 EDT
rubygem-redis-3.0.4-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/rubygem-redis-3.0.4-3.fc20
Comment 11 Fedora Update System 2013-09-23 20:26:31 EDT
rubygem-redis-3.0.4-3.fc20 has been pushed to the Fedora 20 testing repository.
Comment 12 Fedora Update System 2013-10-02 02:31:56 EDT
rubygem-redis-3.0.4-3.fc20 has been pushed to the Fedora 20 stable repository.
Comment 13 Fedora Update System 2013-10-02 02:38:56 EDT
rubygem-redis-3.0.4-3.fc19 has been pushed to the Fedora 19 stable repository.

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