Bug 978284 - Review Request: rubygem-redis - A Ruby client library for Redis
Summary: Review Request: rubygem-redis - A Ruby client library for Redis
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2013-06-26 09:24 UTC by Achilleas Pipinellis
Modified: 2013-10-02 06:38 UTC (History)
2 users (show)

(edit)
Clone Of:
(edit)
Last Closed: 2013-10-02 06:31:56 UTC
vondruch: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Achilleas Pipinellis 2013-06-26 09:24:11 UTC
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 09:57:52 UTC
I'll take this for a review.

Comment 2 Vít Ondruch 2013-06-27 11:03:18 UTC
(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 11:03:38 UTC
Actually 3 nits ;)

Comment 4 Vít Ondruch 2013-06-27 11:55:23 UTC
Just FYI: https://bugzilla.redhat.com/show_bug.cgi?id=978964

Comment 5 Achilleas Pipinellis 2013-06-27 18:04:14 UTC
(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 15:31:29 UTC
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 10:18:00 UTC
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 12:20:09 UTC
Git done (by process-git-requests).

Comment 9 Fedora Update System 2013-09-22 17:37:05 UTC
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 19:28:08 UTC
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-24 00:26:31 UTC
rubygem-redis-3.0.4-3.fc20 has been pushed to the Fedora 20 testing repository.

Comment 12 Fedora Update System 2013-10-02 06:31:56 UTC
rubygem-redis-3.0.4-3.fc20 has been pushed to the Fedora 20 stable repository.

Comment 13 Fedora Update System 2013-10-02 06:38:56 UTC
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.