Bug 786636

The package doesn't seems to build in rawhide :
ruby(abi) = 1.8 
cannot be found.

Guillermo: Not sure if you're still working on this package or not, but I made a few changes to clean it up and upgrade to 4.5.0.

http://jdunn.fedorapeople.org/rubygem-unicorn/rubygem-unicorn.spec

Unfortunately, tests are failing at the moment. I've contacted upstream; if you can think of any reason why this would not work, let me know. The build log is at

http://jdunn.fedorapeople.org/rubygem-unicorn/build.log

(In reply to comment #2)
> Guillermo: Not sure if you're still working on this package or not, but I
> made a few changes to clean it up and upgrade to 4.5.0.
> 
> http://jdunn.fedorapeople.org/rubygem-unicorn/rubygem-unicorn.spec
> 
> Unfortunately, tests are failing at the moment. I've contacted upstream; if
> you can think of any reason why this would not work, let me know. The build
> log is at
> 
> http://jdunn.fedorapeople.org/rubygem-unicorn/build.log

Thanks Julian, im planning to keep working with it, just looking for the time.

I'll take a look at your spec, again thanks. But mostly as i remember, i need to clear the deps.

Cool, no problem.

I managed to work around the build problem. It's a namespace conflict when running under testrb, so I applied a patch to rename the test class. This SRPM and spec build cleanly in a Fedora 19 mock:

http://jdunn.fedorapeople.org/rubygem-unicorn/rubygem-unicorn.spec
http://jdunn.fedorapeople.org/rubygem-unicorn/rubygem-unicorn-4.5.0-2.fc19.src.rpm

Let me know what you think.

(In reply to comment #4)
Not sure what is the issue with the test suite, but if you run the tests independently, there is no point to execute them using testrb. You should be able to run them by ruby itself.

Also, you can always omit testrb by using something like:

  $ ruby -e "Dir.glob('./test/**/*_test.rb').each {|t| require t}"

to execute the whole test suite at once.

Thank you for carrying on this review. I'll need Unicorn for Gitorious.

Unfortunately, Ruby on RHEL 6 does not load binary gems from (what is defined on Fedora as) %{gem_extdir}, so we have to use %{ruby_sitearch} instead. Here's what I use for RHEL 6 compatibility in my arch-specific rubygem packages.

At the top, I have a list of macros like you have, and I add ruby_sitearch to the bottom of the list:

%{!?ruby_sitearch: %global ruby_sitearch %(ruby -rrbconfig -e 'puts Config::CONFIG["sitearchdir"]')}

Then, in %install, instead of copying the shared lib to gem_extdir, I use a dist tag conditional:

%if 0%{?el6}
# gem_extdir doesn't really work on EL 6
mkdir -p %{buildroot}%{ruby_sitearch}
# move the extension to ruby_sitearch
mv %{buildroot}%{gem_libdir}/unicorn_http.so %{buildroot}%{ruby_sitearch}
%else
mkdir -p %{buildroot}%{gem_extdir}/lib
# move the extension to gem_extdir
mv %{buildroot}%{gem_libdir}/unicorn_http.so %{buildroot}%{gem_extdir}/lib/
%endif

And lastly, in %files, I use a second dist tag conditional:

%if 0%{?el6}
%{ruby_sitearch}/unicorn_http.so
%else
%{gem_extdir}
%endif

Hey Guillermo, are you still on to this? If not I'd like to take over, thanks.

Actually i was working on it yesterday (casually). Im planning to upload new f19 spec/srpm today.

Spec URL: http://gomix.fedorapeople.org/rubygem-unicorn/rubygem-unicorn.spec
SRPM URL: http://gomix.fedorapeople.org/rubygem-unicorn/rubygem-unicorn-4.6.3-1.fc19.src.rpm

$ rpmlint -v SPECS/rubygem-unicorn.spec 
SPECS/rubygem-unicorn.spec: I: checking-url http://rubygems.org/downloads/unicorn-4.6.3.gem (timeout 10 seconds)
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint -v /var/lib/mock/fedora-19-x86_64/result/rubygem-unicorn-4.6.3-1.fc19.x86_64.rpm 
rubygem-unicorn.x86_64: I: checking
rubygem-unicorn.x86_64: I: checking-url http://unicorn.bogomips.org/ (timeout 10 seconds)
rubygem-unicorn.x86_64: W: no-soname /usr/lib64/gems/ruby/unicorn-4.6.3/lib/unicorn_http.so
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

no-soname can be safely ignored afaik.

$ rpmlint -v /var/lib/mock/fedora-19-x86_64/result/rubygem-unicorn-doc-4.6.3-1.fc19.x86_64.rpm 
rubygem-unicorn-doc.x86_64: I: checking
rubygem-unicorn-doc.x86_64: W: spelling-error Summary(en_US) rdoc -> doc, r doc, Doctor
rubygem-unicorn-doc.x86_64: I: checking-url http://unicorn.bogomips.org/ (timeout 10 seconds)
rubygem-unicorn-doc.x86_64: W: doc-file-dependency /usr/share/gems/gems/unicorn-4.6.3/test/aggregate.rb /usr/bin/ruby
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

We can safely ignore those two warnings too.

Hi Guillermo,I've made some adjustments to the spec file and pushed the changes to a Git repo, here:
http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-unicorn.git/log/

I've broken each change into a separate Git commit so you can review each one. I'll also try to explain each suggested change below.

1. use HTTPS in URL: This change changes the URL from using plaintext HTTP to secured HTTPS.

2. rm BuildRoot, %clean, and default defattr. There were a couple of older bits in the RPM that are not needed on EL6 and above.

3. adjust dist conditionals post F17. Since Fedora 17 is EOL, this commit removes references to it in the dist conditionals.

4. adjust whitespace. The Requires: ruby(release) field wasn't properly indented to align with the others, so this commit fixes that.

5. remove trailing whitespace. There was a lot of trailing whitespace in the spec file, so this commit fixes that.

6. update to 4.7.0. The upstream Unicorn project has released a new version this month, so I've updated the spec for this.

7. rm spurious dist tag conditional. There was an empty Fedora 18 dist tag conditional, and this commit removes it.

8. fix tests. This commit is a big one, and it might be a little controversial. It has two main changes: I've moved the test modifications to %prep, and I've changed the test invocation in %check to be simply "make test".

9. translate comment to english. This commit simply translates the comment about Unix permissions to english.

10. add comment about upstream fix for doc permissions. As it turns out the permissions problem was a bug in the wrongdoc gem. It was fixed upstream, so I contacted the Unicorn author and asked him to update to the lastest wrongdoc version. As it turns out, Eric was happy to do so, and the next version of Unicorn shouldn't have this permissions problem. I've added a link to the mailinglist discussion so that future maintainers will know "maybe this is fixed upstream and I can remove the chmod statement."

11. require rubygems in test helper. Since each of the tests load test_helper, it's simpler to require rubygems in the test helper rather than adding the rubygems line to each test file individually.

12. use simpler sed commands to fix shebang. So, this one might be a little controversial as well, but I think that it's simpler/cleaner to avoid grepping for upstream's strange shebang line and just use these sed functions instead.

13. build doc subpackage as noarch. The -doc subpackage should be set to "noarch" since there's no architecture-specific files within it.

14. filter test/doc files from auto-requires. If you look at the --requires on the -doc subpackage, you'll see that some weird things like /usr/bin/ruby, /bin/sh, /usr/bin/env get set. When we use the __requires_exclude_from macro, we can skip scanning the files that we ship in -doc.


What do you think?

Hi Guillermo, I've pushed a couple more patches to Git:

15. use gem_docdir macro. This commit uses the gem_docdir macro in the %files list instead of manually specifying the same thing.

16. add missing dependency on rack. When testing this on an EL6 box, I found that I needed to have rubygem-rack installed. This commit adds the rack gem to Requires.

17. require rubygems during binary invocation. I found that I needed to have "require 'rubygems'" to make unicorn work properly on EL6.

18. add macros for el6. On RHEL 6 prior to 6.5, many of the modern Ruby and Gem RPM macros are not available. This commit conditionally defines these macros if they happen to be missing.

19. add EL6 conditionals. This commit allows the RPM to be built for EL6. It uses the ruby_sitearch technique I described in comment 6. With this commit, I can successfully run /usr/bin/unicorn on EL6.

Git repo:
http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-unicorn.git/log/

Thanks Ken for your contributions :), i will make changes an update my spec for f20 and look for a reviewer (probably today). Package yet needs to be approved.

(In reply to Vít Ondruch from comment #5)
> (In reply to comment #4)
> Not sure what is the issue with the test suite, but if you run the tests
> independently, there is no point to execute them using testrb. You should be
> able to run them by ruby itself.
> 
> Also, you can always omit testrb by using something like:
> 
>   $ ruby -e "Dir.glob('./test/**/*_test.rb').each {|t| require t}"
> 
> to execute the whole test suite at once.

Well yes, that's ideal, but the point of running them independently is to be able to avoid a particular test and make the packaging succeed, no need sometimes to force every single test to pass for packaging needs, perhaps there are non sense test for our environments. This is also a result instead of failing tests previously, so also worked for me to isolate things. BTW thanks for the great tips :)

I keep using testrb just as a good practice and consistency and readability, hope this is not a blocker.

Well, have many repeating lines has nothing to do with readability. This applies to the sed lines as well as the testrb lines.

If you want to disable single file, it is better to disable the one file then run them separately.

Moreover, since you run them one by one, you might miss newly introduced file when package gets updated.

Actually, I am afraid that although the first test may fail, the build will successful, which is another unexpected result of running the tests file by file.

Well, i'm not just repeating exactly the same line over and over (just for people not viewing the code in which case that would just stupid, basically the reason for me to comment) , that's not repeating. Using a block like you suggested is great for repeating the same action over many files, and i'm working on improving the actual spec.

And yes, thanks, i'm also aware about the weakness of the actual testing scenario im using. Again, will improve asap (im on it).

I'm reviewing the make test option Ken mentioned and merging many changes he has proposed. I'm not planning on EPEL releases yet.

Hi Guillermo,

I noticed something else that can be changed in the packaging today.

20. skip .gitkeep removals. This commit removes the "find" + "rm" command that deletes the .gitkeep files. Since upstream no longer ships .gitkeep files in 4.7.0, there's no need to search and remove them in the RPM packaging.

Git repo:
http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-unicorn.git/log/

Hi Guillermo, just checking to see if you've had a chance to review those patches and merge them into your unicorn package? It would be great to get unicorn into Fedora.

I've updated for the latest Unicorn release (4.8.2):

http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-unicorn.git/commit/?id=480d0473ce4b40bca0137d50f50df9170f6f1abf

I've updated the package to drop the Fedora 18 conditionals, since that release is EOL now.

http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-unicorn.git/commit/?id=a9edc7c4b789758834f3c0fd285fab438b720ea5

New package review request at bug 1065685

*** This bug has been marked as a duplicate of bug 1065685 ***

I was about today to work on this. Nobody reviewed the bug during the history of it, it was always an open discussion. Closing it this way is just rude. That's not the Fedora way to do things.

If Ken was so interested on getting Unicorn on Fedora, he could just take it for review and work with me for getting it done.

I will pass on it and let it go, but please, Ken, just take note that's not proper way to proceed.

Please be this comment just to note that im ok with the new review and Ken and I talked about it off line and we are ok and in good terms. :)