Bug 782560

I'll take this one for a review.

- You don't need the macro definitions, as they are already defined in file macros.rubygems included in rubygems-devel package, that you are requiring in build time.
- shadow.so file should be placed under %gem_extdir, see [1] for instructions on how to use it.
- You need to obsolete the ruby-shadow package properly. According to [2], you should have:

Provides: ruby(shadow) = %{version}-%{release}
Obsoletes: ruby(shadow) < 2.1.1-1

- Could you please clarify the license? Your spec says its GPLv2+ or Ruby, the readme says it's "Free for any use with your own risk!", which is probably Public Domain, but you should query the author to state it clearly.
- You don't need to BR: ruby(rubygems), since it gets drawn in by rubygems-devel.
- %{gem_dir}/specifications/%{gem_name}-%{version}.gemspec can be replaced by %{gem_spec}.
- %{gem_dir}/doc/%{gem_name}-%{version} can be replaced by %{gem_docdir}.
- %{gem_instdir}/lib can be replaced by %{gem_libdir}

[1] https://fedoraproject.org/wiki/PackagingDrafts/Ruby#RubyGem_with_extension_libraries_written_in_C
[2] http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

Wouldn't Obsoletes: ruby(shadow) < 1.4.1-16 be better?  1.4.1-15.fc18 was the last version of the old package in Fedora and EPEL AFAICT.

Similarly, I think:

  Obsoletes:      ruby-shadow >= 1.4.1
  Provides:       ruby-shadow >= 1.4.1

should be more like:

  Obsoletes:      ruby-shadow < 1.4.1-16
  Provides:       ruby-shadow = %{version}-%{release}

Is the '*' in the summary going to make life interesting for folks searching via yum?  Maybe 'Ruby shadow password module' is good?

(Just some comments from the peanut gallery. Thanks for packaging this Mike, and thanks for the review Bohuslav. :)

(In reply to comment #3)
> Wouldn't Obsoletes: ruby(shadow) < 1.4.1-16 be better?  1.4.1-15.fc18 was the
> last version of the old package in Fedora and EPEL AFAICT.
> 
> Similarly, I think:
> 
>   Obsoletes:      ruby-shadow >= 1.4.1
>   Provides:       ruby-shadow >= 1.4.1
> 
> should be more like:
> 
>   Obsoletes:      ruby-shadow < 1.4.1-16
>   Provides:       ruby-shadow = %{version}-%{release}
> 

Well, the guidelines for obsoleting are pretty clear, so it should really be the way I mentioned in comment2.

> Is the '*' in the summary going to make life interesting for folks searching
> via yum?  Maybe 'Ruby shadow password module' is good?
> 

That's probably a good point :) Michael, I think that you should fix this (but not a blocker on my side).

> (Just some comments from the peanut gallery. Thanks for packaging this Mike,
> and thanks for the review Bohuslav. :)

(In reply to comment #4)
> Well, the guidelines for obsoleting are pretty clear, so it should really be
> the way I mentioned in comment2.

I'm not sure I see where that would be the case.  The guidelines I am reading are from http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages and state, in part:

  Example: foo being renamed to bar, bar is compatible with foo, and the last foo package release being foo-1.0-3%{?dist} with Epoch: 2; add to bar (and similarly for all subpackages as applicable):

    Version: 1.0
    Release: 4%{?dist}
    Provides: foo = 2:%{version}-%{release}
    Obsoletes: foo < 2:1.0-4

Following that, the last ruby-shadow release was 1.4.1-15%{?dist}, with no epoch.  I still believe that the proper obsoletes/provides entry is:

    Obsoletes:  ruby-shadow < 1.4.1-16
    Provides:   ruby-shadow = %{version}-%{release}

After looking more, obsoletes for ruby(shadow) are not needed at all, as a newer ruby(shadow) provides will already be added as a matter of normal ruby packaging process.  That name isn't changing, so the obsoletes is not required.

The important part that requires Obsoletes/Provides here is ruby-shadow, as that is the old package we're replacing.

Am I missing something?

A very minor thing, but if this package is intended to work on EL5, making the doc subpackage noarch only on Fedora and EL >= 6 would be good.  E.g.:

  %package doc
  Summary: Documentation for %{name}
  Group: Documentation
  Requires: %{name} = %{version}-%{release}
  %if 0%{?fedora} || 0%{?rhel} >= 6
  BuildArch: noarch
  %endif

I wasn't sure if I was going to put this package into EPEL 5, but since you did this nice work for me, maybe I will.   I hope to get to this in the next 24 hours, but that's not a promise.

We should be sure that the questions on obsoletes and provides are resolved. :)

(In reply to comment #5)
> (In reply to comment #4)
> > Well, the guidelines for obsoleting are pretty clear, so it should really be
> > the way I mentioned in comment2.
> 
> I'm not sure I see where that would be the case.  The guidelines I am reading
> are from
> http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages
> and state, in part:
> 
>   Example: foo being renamed to bar, bar is compatible with foo, and the last
> foo package release being foo-1.0-3%{?dist} with Epoch: 2; add to bar (and
> similarly for all subpackages as applicable):
> 
>     Version: 1.0
>     Release: 4%{?dist}
>     Provides: foo = 2:%{version}-%{release}
>     Obsoletes: foo < 2:1.0-4
> 
> Following that, the last ruby-shadow release was 1.4.1-15%{?dist}, with no
> epoch.  I still believe that the proper obsoletes/provides entry is:
> 
>     Obsoletes:  ruby-shadow < 1.4.1-16
>     Provides:   ruby-shadow = %{version}-%{release}
> 
> After looking more, obsoletes for ruby(shadow) are not needed at all, as a
> newer ruby(shadow) provides will already be added as a matter of normal ruby
> packaging process.  That name isn't changing, so the obsoletes is not required.
> 
> The important part that requires Obsoletes/Provides here is ruby-shadow, as
> that is the old package we're replacing.
> 
> Am I missing something?

Hmm, I see your point, but I understand this guideline differently:
(For the Provides:, we obviously have the same, so let's just talk about the Obsoletes:)
In the example you mentioned, foo is being replaced by bar, bar's EVR is 2:1.0-4. I understand it the way, that this should come out of the EVR of bar, not the old foo (that's why the Version and Release tags are written in the example, I think). But I see your point that you want to obsolete "bumped foo". Nevertheless, I still think that my solution is better according to common sense, because if ruby-shadow of version 1.5 would be packaged, than it wouldn't be obsoleted, but still would be of no use, as the provides of ruby-shadow the rubygem is higher.

I just can't see how manufacturing a version that never existed makes sense.  There was never a ruby-shadow-2.1.1.  If someone does resume development of ruby-shadow and it is desirable to put it back in fedora, that's why we have epoch.  If someone puts it back in without coordinating with the maintainers of rubygem-ruby-shadow, then we'll have problems no matter what -- and they are not problems that guidelines can solve. :)

I don't see where the guidelines suggest (literally or in spirit) that a non-existent version (2.1.1) should be used.  They clearly use the last version of foo, the package being replaced.  All that is incremented is the the release field.

I believe that using 2.1.1 is exactly the sort of thing that is discouraged when the text says "$obsEVR is an (Epoch-)Version-Release tuple arranged so that there is a clean upgrade path, **but without gratuitously polluting the version space upwards**" (emphasis mine).

If we were to use the EVR from the new package, then it would be < 2.1.2-1, if anything.  But I don't believe that's what the guidelines suggest or imply.

Ok, agreed.

Hey guys,

I made a few changes to the spec file to reflect manyof the changes already
suggested.  It also makes the spec file work for older Fedora and EPEL.

One difference from the provides is that I used:

Provides: ruby(shadow) = %{version}

instead of %{version}-%{release}.  The draft guidelines¹ say to use:

Provides: ruby(RUBYLIBRARY) = VERSION
Provides: rubygem(%{gem_name}) = %{version}

I don't know if the guidelines are just incorrect or not there.  It's an easy
change if either of you know what it should properly be.  The current
ruby-shadow rpm provides ruby(shadow) = %{version}-%{release}.

I made scratch builds for f1{6..8} and el{5,6}, but haven't done more than
poke at the file lists and provides/requires/obsoletes (i.e. I haven't
installed and used them at all yet).

f18: http://koji.fedoraproject.org/koji/taskinfo?taskID=3796055
f17: http://koji.fedoraproject.org/koji/taskinfo?taskID=3796089
f16: http://koji.fedoraproject.org/koji/taskinfo?taskID=3796039
el6: http://koji.fedoraproject.org/koji/taskinfo?taskID=3796083
el5: http://koji.fedoraproject.org/koji/taskinfo?taskID=3796071

Spec file: http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

I did make these changes to the spec *after* I did the scratch builds:

%{gem_dir}/specifications/%{gem_name}-%{version}.gemspec -> %{gem_spec}
%{gem_dir}/doc/%{gem_name}-%{version} -> %{gem_docdir}
%{gem_instdir}/lib -> %{gem_libdir}

I also left the BR: ruby(rubygems) because rubygems-devel is only available on f17 and above.

This still leaves the question of the license open.

Hopefully this helps a little.  I'll try to find time to install and test this
later today, but no promises there.

¹ http://fedoraproject.org/wiki/PackagingDrafts/Ruby#RubyGems

Something else I just noticed is that the build is being done in %prep.  Can that be changed now or does that require something from ruby-1.9 or newer rubygems?

The ruby draft guidelines seem to suggest that if can be done via the normal %prep, %build, and %install sections.

I updated the spec file to do prep/build/install more in line with the draft guidelines (minus the typos in the draft that make it fail).

I gave up on el5, as rubygems there is a bit too old.  but this should work on el6 and any supported fedora releases.  If we could update rubygems just a little on el5, it'd work there too.

Spec file: http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

(In reply to comment #14)
> I updated the spec file to do prep/build/install more in line with the draft
> guidelines (minus the typos in the draft that make it fail).
> 
> I gave up on el5, as rubygems there is a bit too old.  but this should work on
> el6 and any supported fedora releases.  If we could update rubygems just a
> little on el5, it'd work there too.
> 
> Spec file: http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

The process of creating the new guidelines is still going on. The ruby-sig people are mostly arguing against the unpacking and then repacking the gem, as it is an unnecessary payload. If you don't need to patch the C extension, this really just makes the spec longer and harder to read. I suggest getting rid of it.

Todd, I'm happy to continue to have you get this through review.  We can co-maintain.  I obviously didn't get to it as quickly as you did :)

BTW the rubygems-devel package is available on all supported Fedoras, so if you could move the gem_* macros definition into the %{rhel5} section, it would be nice. Also the BR: rubygems-devel should be adjusted appropriately. I requested these macros also for RHEL6 [1], so we'll see it that happens.

Another question, what is the purpose of the "ruby-shadow" subpackage for RHEL? Why you would like to have it?


[1] https://bugzilla.redhat.com/show_bug.cgi?id=788001

Hi Vit,

Sure, we can move the macros into the %if %{?rhel} section -- though they don't get defined if they exist, so they don't hurt anything where they are.  At the time I worked on this, I don't believe that all Fedora systems had the updated rubygems-devel.

The ruby-shadow subpackage is highly desirable so that folks who were ensuring it was installed in puppet manifests, kickstarts, etc, are able to easily update their systems.  This is important to EPEL if we ever hope to push this newer gem packaging there.

I've been holding off while the ruby guidelines are hashed out on the packaging list.  There is a bit of debate on the draft and the gem unpack/repack bits.  That and I've been on vacation for a week or so and haven't spent much time on packaging. :)

FWIW, I am in favor of the unpack/repack if only to make rpmbuild -bp work as it does in other software.  I don't like the idea of not having anything done in %prep nor do I like calling gem install in %prep.  That gems are designed to work this way is a defect in gem in my opinion.  Whether that extra work turns off some ruby people that might otherwise become Fedora packagers isn't a concern to me.  I'd rather attract packagers who understand why we have these things in separate steps. I'll keep my eye on the packaging list for the results of this discussion.

(In reply to comment #18)
> Hi Vit,
> 
> Sure, we can move the macros into the %if %{?rhel} section -- though they don't
> get defined if they exist, so they don't hurt anything where they are.  At the
> time I worked on this, I don't believe that all Fedora systems had the updated
> rubygems-devel.

Yes, I know they don't hurt, but may confuse. At the end, you never know where the value of the macro is coming from.

> The ruby-shadow subpackage is highly desirable so that folks who were ensuring
> it was installed in puppet manifests, kickstarts, etc, are able to easily
> update their systems.  This is important to EPEL if we ever hope to push this
> newer gem packaging there.

Hmm, the Obsoletes/Provides are not enough? I am not familiar with Puppet.

> I've been holding off while the ruby guidelines are hashed out on the packaging
> list.  There is a bit of debate on the draft and the gem unpack/repack bits. 

I understand. On the other hand, we rebuild all other stuff by the proposed draft, so one package which might become wrong doesn't hurt that much. Finished review would save us from nagging by broken ruby-shadow.

> That and I've been on vacation for a week or so and haven't spent much time on
> packaging. :)

Glad you are relaxed now ;) Yes, I am going for vacation next three weeks, that's why I'm trying to put things into order.

The rest is OT here, arguments were/can be made on the packaging list.

> FWIW, I am in favor of the unpack/repack if only to make rpmbuild -bp work as
> it does in other software.  I don't like the idea of not having anything done
> in %prep nor do I like calling gem install in %prep.  That gems are designed to
> work this way is a defect in gem in my opinion.  Whether that extra work turns
> off some ruby people that might otherwise become Fedora packagers isn't a
> concern to me.  I'd rather attract packagers who understand why we have these
> things in separate steps. I'll keep my eye on the packaging list for the
> results of this discussion.

anything new on this?  I'm not a fedora packager (yet) but I'd be happy to help in any way.

Apologies for the long delay.

I've updated the spec file to hopefully address the remaining issues.  It should follow the current ruby guidelines now.  It also builds for el5 through rawhide.

I've put this in a temporary repository along with puppet-2.7.12 for fedora 17/18 if anyone would like to test.  (Note that puppet-2.7 does not appear to talk to 2.6 puppet masters, this may not be something we can resolve.¹)

To enable this repository:

$ sudo curl -o /etc/yum.repos.d/tmz-puppet-ruby-shadow.repo http://tmz.fedorapeople.org/repo/puppet-ruby-shadow/fedora/tmz-puppet-ruby-shadow.repo

The updated spec file is here:

    http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

¹ http://projects.puppetlabs.com/issues/13622

Ok, here are my comments:
- Are you sure about the license? The readme file says "License: Free for any use with your own risk!". You should probably ask the author what exactly he means by that (I'm guessing public domain, but we must clarify this).
- In F17 and above, the *.so files have to live under %{gem_extdir}, because they are platform dependent and %{gem_instdir} points to /usr/share/gems, which has to contain only platform independent code (also, rpmlint complains about this).
- %{gem_instdir}/README* should be marked as %doc
- Consider updating to the latest upstream version (2.1.3).
- Please remove zero-length files, as reported by rpmlint:
rubygem-ruby-shadow-doc.noarch: E: zero-length /usr/share/gems/doc/ruby-shadow-2.1.2/rdoc/created.rid
rubygem-ruby-shadow-doc.noarch: E: zero-length /usr/share/gems/doc/ruby-shadow-2.1.2/ri/created.rid

Please fix these issues and then the package can be approved.

(In reply to comment #22)
> - Please remove zero-length files, as reported by rpmlint:
> rubygem-ruby-shadow-doc.noarch: E: zero-length
> /usr/share/gems/doc/ruby-shadow-2.1.2/rdoc/created.rid
> rubygem-ruby-shadow-doc.noarch: E: zero-length
> /usr/share/gems/doc/ruby-shadow-2.1.2/ri/created.rid

Slavek, this is not good idea IMO, it should be considered as a false positive. Do you know what these files are for?

Ah sorry, I take this back. Will have to properly look what rpmlint is telling me next time...

Bohuslav,

Thanks.  I'll update to 2.1.3 now and will mail upstream about the license.

Where can I find more details about gem_extdir?  I'd like to read more about that so I can properly document and add the right bits so the spec file works cleanly across all Fedora and EPEL releases.

Actually, the license tag simply never got adjusted after the gem2spec was run initially.  The ruby-shadow package is Public Domain and always has been.  As this code is copied from that with no changes to any of the licensing, I'm going to adjust the license tag here.

Hi Todd,
to answer your questions:
- gem_extdir is part of the new guidelines, which are in somehow inconsistent state with how we have actually packaged Ruby and Rubygems. To make a long story short, gem_extdir translates to %{_libdir}/gems/exts/%{gem_name}-%{version} and from the example specfile:
"
# If there are C extensions, mv them to the extdir.
# You should replace REQUIRE_PATHS with the first value of the require_paths field in
# the gemspec file.  It will typically be either "lib" or "ext".  For instance:
#  s.require_paths = ["lib"]
mkdir -p %{buildroot}%{gem_extdir}/REQUIRE_PATHS
mv %{buildroot}%{gem_instdir}/REQUIRE_PATHS/shared_object.so %{buildroot}%{gem_extdir}/REQUIRE_PATHS/
"
- Could you please add a comment to specfile, that would point to a place, where the author explicitly claims that this library is available under public domain?

Thanks!

Hey was just wondering if there has been any update with this. We are currently in the process of pushing updates to Aeolus to get it working w/ Ruby 1.9 / fix it in Fedora and we depend on a working puppet / rubygem-shadow to do so.

If you didn't have the cycles, I could finish up the package review inorder to get this and the updated puppet in next week.

Thanks alot

I spent time over the past few days adding support for gem_extdir on Fedora >= 17.  An update spec file is now in place:

    http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

As far as adding a comment about the licensing, would it suffice to link to the original ruby-shadow review?  This code has not changed since then as far as the licensing, so I don't think there's any reason to try and chase down the original authors for a statement.

With respect to Fedora 15/16 support, I thought that they were supposed to have backported support in ruby/irb/rubygems so that installing to the gemdir would allow the module to be used from ruby?  This does not work for me:

[root@f16-32 /]# cat /etc/redhat-release 
Fedora release 16 (Verne)
[root@f16-32 /]# rpm -qa ruby\*
rubygems-1.8.11-1.fc16.1.noarch
ruby-libs-1.8.7.358-1.fc16.i686
ruby-irb-1.8.7.358-1.fc16.noarch
rubygem-ruby-shadow-2.1.3-2.fc16.i686
ruby-1.8.7.358-1.fc16.i686
ruby-rdoc-1.8.7.358-1.fc16.noarch
[root@f16-32 /]# irb
irb(main):001:0> require 'shadow'
LoadError: no such file to load -- shadow
        from (irb):1:in `require'
        from (irb):1
        from :0

Hi Todd,

(In reply to comment #29)
> I spent time over the past few days adding support for gem_extdir on Fedora >=
> 17.  An update spec file is now in place:
> 
>     http://tmz.fedorapeople.org/specs/rubygem-ruby-shadow.spec

Thank you Todd. I have a few notes:

* The EPEL section is wrong. It has to follow the old guidelines (hm, they are lost somewhere :/ [1]), so you should update the %{gem_dir} macro:

%global	gem_dir		%(ruby -rubygems -e 'puts Gem::dir' 2>/dev/null)

The rest is OK.

* Detecting version of Ruby is not good practice IMO. What if you have installed by a chance older version of Ruby on your system and later you will produce unexpected packages? I know, it will never happen on build system, but might bite somebody when rebuilding locally.

* The linking of *.so for Fedoras < 17 and EPEL is wrong. Actually there should be no linking at all. You should move the library into the sitearch dir:

mv %{buildroot}%{gem_libdir}/shadow.so %{buildroot}%{ruby_sitearch}/shadow.so

The approach is the same for all mentioned versions.
 
> As far as adding a comment about the licensing, would it suffice to link to the
> original ruby-shadow review?  This code has not changed since then as far as
> the licensing, so I don't think there's any reason to try and chase down the
> original authors for a statement.

Well, as far as I am looking into the original review, there were also uncertainties about licensing which were never clarified. Also, the reviewer guidelines contains this line:

SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.

So I would appreciate if you can ask upstream about the license.

> 
> With respect to Fedora 15/16 support, I thought that they were supposed to have
> backported support in ruby/irb/rubygems so that installing to the gemdir would
> allow the module to be used from ruby?  This does not work for me:
> 
> [root@f16-32 /]# cat /etc/redhat-release 
> Fedora release 16 (Verne)
> [root@f16-32 /]# rpm -qa ruby\*
> rubygems-1.8.11-1.fc16.1.noarch
> ruby-libs-1.8.7.358-1.fc16.i686
> ruby-irb-1.8.7.358-1.fc16.noarch
> rubygem-ruby-shadow-2.1.3-2.fc16.i686
> ruby-1.8.7.358-1.fc16.i686
> ruby-rdoc-1.8.7.358-1.fc16.noarch
> [root@f16-32 /]# irb
> irb(main):001:0> require 'shadow'
> LoadError: no such file to load -- shadow
>         from (irb):1:in `require'
>         from (irb):1
>         from :0

You have forgotten one important thing: require 'rubygems' , since rubygems were not loaded by default in Ruby 1.8. Also 'gem list' is useful command for basic check if gem is installed and recognized by RubyGems.


[1] https://fedorahosted.org/fpc/ticket/165

Hi Vit,

(In reply to comment #30)
> * The EPEL section is wrong. It has to follow the old guidelines (hm, they are
> lost somewhere :/ [1]), so you should update the %{gem_dir} macro:
> 
> %global gem_dir  %(ruby -rubygems -e 'puts Gem::dir' 2>/dev/null)
> 
> The rest is OK.

Hmm, it sounds like it's more hassle than it's worth to try and support Fedora < 17 and EPEL with the same spec file as we'll need in Fedora >= 17.

What I've done for now [since I'm tired of getting broken dep reports for the existing ruby-shadow package -- and it prevented me from moving forward with puppet on Fedora >= 17 :)] is update ruby-shadow with the fixes required to support ruby-1.9.3¹.

This removes the pressure to get this review finished up, from my perspective anyway.  I don't have a lot of time to work on this, so if someone else has a real need for the rubygem version of the shadow module, please feel free to continue this review.

One thing I would ask is that anyone who pushes this into older Fedora or RHEL take great care to ensure that non-gem use remain functional.  Anything else would be a regression.

> * Detecting version of Ruby is not good practice IMO. What if you have
> installed by a chance older version of Ruby on your system and later you will
> produce unexpected packages? I know, it will never happen on build system, but
> might bite somebody when rebuilding locally.

I understand that many ruby developers might have multiple versions of ruby
installed.  This is not a supported build configuration and those who have this
are on their own if they want to build these packages.  I don't see any reason
to go out of our way and make it more difficult for maintainers by hard-coding
the ruby version in the spec file.  That makes merging the spec file into older
branches far more error-prone than it needs to be, IMO.

> > With respect to Fedora 15/16 support, I thought that they were supposed to have
> > backported support in ruby/irb/rubygems so that installing to the gemdir would
> > allow the module to be used from ruby?  This does not work for me:
> > 
> > [root@f16-32 /]# cat /etc/redhat-release 
> > Fedora release 16 (Verne)
> > [root@f16-32 /]# rpm -qa ruby\*
> > rubygems-1.8.11-1.fc16.1.noarch
> > ruby-libs-1.8.7.358-1.fc16.i686
> > ruby-irb-1.8.7.358-1.fc16.noarch
> > rubygem-ruby-shadow-2.1.3-2.fc16.i686
> > ruby-1.8.7.358-1.fc16.i686
> > ruby-rdoc-1.8.7.358-1.fc16.noarch
> > [root@f16-32 /]# irb
> > irb(main):001:0> require 'shadow'
> > LoadError: no such file to load -- shadow
> >         from (irb):1:in `require'
> >         from (irb):1
> >         from :0
> 
> You have forgotten one important thing: require 'rubygems' , since rubygems
> were not loaded by default in Ruby 1.8. Also 'gem list' is useful command for
> basic check if gem is installed and recognized by RubyGems.

Ahh.  I didn't forget this, I just thought that the gemdir was added to the search path on older Fedora releases.  I see now in the guidelines that it is not.  It simply means that we'd need to also provide the ruby-shadow package if this was to be built for Fedora < 17.  Otherwise, current users of the ruby-shadow package would be broken by an update to the rubygem version.

Thanks for all the help with this.  If I get some free time, I will try to work on this.  But it's not nearly as high on my list now that I have ruby-shadow supporting ruby-1.9 in Fedora >= 17¹.

¹ https://admin.fedoraproject.org/updates/FEDORA-2012-6448/ruby-shadow-1.4.1-16.fc17

Hi all, I thought I might and try help out on this one..

I updated the spec with what I could gather were the most recent requested changes, which were to use %{ruby_sitearch} for arch specific files for EPEL and Fedora < 17, remove the symlinking, and to set %gem_dir via %(ruby -rubygems -e 'puts Gem::dir' 2>/dev/null). I also updated for upstream 2.1.4.

Regarding the license difficulty, I contacted the upstream maintainer, and he now is considering a change to split out the license into a separate license file, and add an explicit "OR the Public Domain license". I'll update when I hear more, he said he's get back to me soon.

One thing I wasn't certain of, given how the .so files are moved based on the target, is how best to compose the *.so's in the %files section. Currently I set it up in a nested conditional, but I'm not sure that's good style, so feedback much appreciated.

srpm:
https://s3.amazonaws.com/rubygemshadow/rubygem-ruby-shadow-2.1.4-1.fc16.src.rpm
spec:
https://s3.amazonaws.com/rubygemshadow/rubygem-ruby-shadow.spec

Just an update on this, the upstream maintainer has updated the license to include Public Domain see (https://github.com/apalmblad/ruby-shadow) and this should be in the next release to rubygems, I imagine.

Since this appears to be a stalled review, can I go ahead and do the review? It's a little weird since I submitted the original request, but since then Moses Mendoza has decided to take it over. It would also be his first package in Fedora, which is cool. 

Thoughts?

(In reply to comment #34)
Slavek is on holidays until next week, but I see no problem if you will send your comments here and we can push this review forward. The important think is to put more eyes on the spec file then who does what.

(In reply to comment #33)
I'd love to see comment referencing the upstream commit with the license change, as long as new version carrying this change is not released.

And several other comments:

* %{ruby_version}
  - I already expressed my concerns about %{ruby_version} above and they still
    apply.
  - Moreover, as of this discussion [1] on packaging list, we are likely going
    to drop the mandatory versioning of ruby(abi), which speaks more in favor of
    hardcoding the ruby(abi)

* %install section
  - Wouldn't be better to install just files of interest instead of copying
    everything and then doing clean up?

* You should own all your directories and files
  - For F17+, you do not own the %{gem_extdir}/lib, so I would suggest you to
    change the line to:

    %{gem_extdir}

    which will own its whole content, including the .so file

* shadow.so should be owned by only one package
  - In ruby-shadow subpackage you own the shadow.so file, but that file is
    already owned by the main package, which is correct. Please drop the .so
    file ownership from ruby-shadow subpackage
  - Since the package will not own any file, it could be noarch at the end?
    This is weird :)

* Drop the %{gem_instdir}/*.so
  - This is build leftover and will be never used. Please drop the file.

* Move *gemspec into -doc subpackage
  - The %{gem_instdir}/*gemspec file is not needed by runtime, please move it
    into -doc subpackage

on one additional hint, the package, as of now will not work with {RHEL,EPEL}7 since the Ruby will be similar to F17+ in its layout. But this could be neglected for this review :)



[1] http://lists.fedoraproject.org/pipermail/packaging/2012-December/008798.html

Hi Vit,

I've updated per your comments, here:
https://s3.amazonaws.com/rubygemshadow/2.2.0/rubygem-ruby-shadow-2.2.0-1.fc18.src.rpm
https://s3.amazonaws.com/rubygemshadow/2.2.0/rubygem-ruby-shadow.spec

with the exception of:

> * %{ruby_version}
>  - I already expressed my concerns about %{ruby_version} above and they still
>    apply.

I agree with Tim regarding the maintainability/difficulty of back-porting statically assigned ruby pathing.

> * %install section
>  - Wouldn't be better to install just files of interest instead of copying
>    everything and then doing clean up?

I did this, but it seemed less clean than it was before. Either way is fine with me, and I'm welcome to better ways to accomplish it than I used.

> - Since the package will not own any file, it could be noarch at the end?
>   This is weird :)

I agree, it does seem strange. I left it as is, for now, but am fine with updating to noarch if this is a requirement.

> I'd love to see comment referencing the upstream commit with the license change, as long as new version carrying this change is not released.

Also, I went ahead and updated for upstream 2.2.0. 2.2.0 includes official licensing as Public Domain and is compatible with Ruby 2.0.0, per https://github.com/apalmblad/ruby-shadow/commit/fa317a925f6880d62ce5157792d9901be33ea57f.

Hi,

Could you please update it to follow the latest Ruby packaging guidelines? You can try to use this [1] migration script. However, for such conditionalized .spec file, it does not work very well. Thank you.


[1] https://github.com/voxik/fermig/blob/master/f19.rb

Moses - ping?

Since there seems to be no progress here, I'm closing this bug. Feel free to reopen if you wish to restart the review.

The needinfo request[s] on this closed bug have been removed as they have been unresolved for 1000 days