Bug 728088

Taking this one 

* rpmlint looks good

* koji build is green:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3407485

* The license refers to ASL and MIT but I was only able to find ASL in the project, which components are licensed under MIT? The files you pointed out are all licensed under the ASL 

http://www.apache.org/licenses/LICENSE-2.0

* Also the NOTICE file states that 

"This software contains code derived from the Net::XMPP for Ruby project by Yuki Mitsui (http://netxmpp-ruby.jabberstudio.org/)"

This exact url does not seem to be accessible but I found the project's website which indicates that it is licensed under the LGPL:

http://jabberstudio.org/projects/netxmpp/project/view.php

I'm not sure what they mean their code is "derived" from net-xmpp, but it might be good to clarify (I believe licensing the project under ASL v2 AND LGPL would be acceptable)

* rm -rf %{buildroot} is no longer needed at the beginning of the %install section

* since you are packaging this gem for non-gem use, that non-gem subpackage needs to own the ruby_sitelib symlinks according to these guidelines:

"- All the toplevel library files of the Gem must be symlinked into ruby_sitelib.
 - The subpackage must own these symbolic links."

Other than these, package looks good

I will fix however, Ruby is going to 1.9.3 in Fedora 17 which is going to break a lot of things (compatibility?). I will fix these issues. They also have dropped the non-gem use for rubygems (it's not updated in packaging guidelines!).

Thanks for reviewing further updates to this bug pending.

Sounds good. Good to know that the packaging rubygems for non-gem use guideline is going away, feel free to remove the non-gem subpackage from this altogether if you want (this was the first package that I've seen do this in a very long time).

Also how definite is it that Ruby 1.9.3 is going into Fedora 17?

Thank you.

The Ruby SIG team would need to be engaged they have a mailing list. Their changes will cause changes in these packages.

(In reply to comment #2)
> They also have dropped the non-gem use for rubygems (it's not updated in
> packaging guidelines!).

I proposed this change, since the non-gem use is bad designed and I can't see any benefit of such packages. However, there was not much done WRT update of packaging guidelines, but I'll try to collect some ideas in [1]. Going to add this proposal right now.


(In reply to comment #3)
> Sounds good. Good to know that the packaging rubygems for non-gem use guideline
> is going away, feel free to remove the non-gem subpackage from this altogether
> if you want (this was the first package that I've seen do this in a very long
> time).
> 
> Also how definite is it that Ruby 1.9.3 is going into Fedora 17?
> 
> Thank you.

I am highly interested in R 1.9.3 in F17 and I did not heard any strong objections, so it is highly probable that I will push them forward. Of course, you can check Ruby-SIG ML as was mentioned by Shawn.


[1] https://fedoraproject.org/wiki/Packaging_talk:Ruby

ping? could you fix the issues described in my review. rubygem-addressable needs this to work, which in return is required by rubygem-webmock, which in return is required by aeolus-conductor.

Thank you

Yes, I've just been swamped with RL stuff, I hope to look at this and the other this weekend. These are pre-req's for OpenNebula 3.x also which will depend on how Ruby SIG will have for backwards compatibility or that will kill this.

Hi Shawn, same as rubygem-addressable ... I'm happy to add review feedback to help this review move forward.  Let me know if I can help.

Stripped out non-gem, since we're not doing this any more (as mentioned before by Vit, it makes no sense)

Updates can be found here:

http://www.sh0n.net/spstarr/fedora/rubygem-idn/rubygem-idn.spec
http://www.sh0n.net/spstarr/fedora/rubygem-idn/rubygem-idn-0.0.2-2.fc17.src.rpm

Additionally, this does not use RSPec 2.x and as mentioned with rubygem-addressable any help to make these compliant would be greatly appreciated for Ruby 1.9.x and 1.8 compatibility.

This gem doesn't use RSpec, so this issue is irrelevant in this case. Please don't worry too much about Ruby 1.9 compatibility. If there will be necessary some changes, it will be just one of 300 gems, so no big deal.

However, you are currently executing the test suite using Rake. I would suggest to replace the "rake test -v --trac" with something like "testrb -Ilib test/tc_*.rb". That allows you to remove the dependency on RDoc and Rake and the patch, which simplifies the maintenance of the library IMO.

Moreover:

* I would suggest to use %exclude macro instead of deleting files in install section, i.e:

%files
%exclude %{geminstdir}/ext
%exclude %{geminstdir}/.yardoc
%exclude %{gemname}-%{version}.gem

* I suggest to use:
%exclude %{gemdir}/cache/%{gemname}-%{version}.gem
since the gem cache has no meaning with RPM.

* I suggest to remove the -devel package, because I doubt that it has some meaning in this case. Will somebody except Ruby users base their work on the gem?

* I am not exactly sure about the meaning of the "cp %{SOURCE0} %{gemname}-%{version}.gem" command when you can simple call the gem install command with the %{SOURCE0} macro.

OK thanks for the update

- looks like you addressed all my feedback

- rpmlint / koji builds are still green

- as Vit mentioned the cp command and devel packages can probably be removed unless there is good reason to keep those there. I feel it might be useful to keep the cached gem in place though as it's good for reference and in order to keep the files provided by rpm and gem installs fairly consistent. None of this is a big deal either way though

- Vit's other feedback is also good but not blockers as the package is now in compliance w/ Fedora guidelines. Feel free to implement his suggestions pre/post push.

APPROVED

Vit feedback is added:

using:testrb -Ilib test/tc_*.rb

Removed copy %{SOURCE0}. I originally wanted to keep the .gem in the SRPM, What would happen if the gem upstream was compromised somehow? 

Removed -devel package.

use %exclude for files above + .h file.

Please mark package review + so I can submit package into git.

(In reply to comment #12)
> Removed copy %{SOURCE0}. I originally wanted to keep the .gem in the SRPM, What
> would happen if the gem upstream was compromised somehow? 

The %{SROURCE0} is always kept in SRPM. You cannot build the SRPM without the source gem. Moreover there is use --local flag during gem installation. The copying of %{SOURCE0} did not have any meaning, did not prevent anything.

(In reply to comment #12)
> Removed copy %{SOURCE0}. I originally wanted to keep the .gem in the SRPM, What
> would happen if the gem upstream was compromised somehow? 

There is even no internet connection on Koji during build, so there is no way how the gem could be accidentally downloaded from some compromised source.

For some reason the '+' isn't available as an option for the fedora-review flag. Does anyone know the reason for this? It has worked in the past for me.

I've set fedora_review+ as requested

I will be pushing this today was blocked due to fedora filer issue and being sick ;/

New Package SCM Request
=======================
Package Name: rubygem-idn
Short Description: Ruby Bindings for the GNU LibIDN library
Owners: spstarr

Can I get an update from the SCM team on this approval?

n/m fedora-cvs is still used ;p

Git done (by process-git-requests).

This is now in

Could this be pushed into Fedora 16 as well. rubygem-webmock will be broken there until it is.

I've attached the following SRPM / SPEC and diff to the idn spec adding ruby 1.8 / Fedora 16 support. I've also requested commit rights to the rawhide so that I can request the new branch / do the push if you don't have the cycles.

Thanks.

http://mo.morsi.org/files/rpms/rubygem-idn-0.0.2-7.fc18.src.rpm
http://mo.morsi.org/files/rpms/rubygem-idn.spec
http://mo.morsi.org/files/rpms/idn-0.0.2.spec.diff

Please do request commit, never can have enough maintainers :) even for older versions

Package Change Request
======================
Package Name: rubygem-idn
New Branches: f16
Owners: mmorsi
InitialCC: 

F16 branch for rubygem-idn

Git done (by process-git-requests).

rubygem-idn-0.0.2-7.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/rubygem-idn-0.0.2-7.fc16

rubygem-idn-0.0.2-7.fc16 has been pushed to the Fedora 16 stable repository.

Package Change Request
======================
Package Name: rubygem-idn
New Branches: epel7
Owners: tdawson

Git done (by process-git-requests).