Spec URL: https://raw.github.com/jcourteau/rubygems-rpms/master/fc17/rubygem-yajl-ruby/rubygem-yajl-ruby.spec SRPM URL: http://rpms.courteau.org/fedora/rubygem-yajl-ruby-1.1.0-1.fc17.src.rpm Description: Ruby C bindings to the excellent Yajl JSON stream-based parser library This is part of a set of dependencies for rubygem-chef. I've got about 14 packages to add, all ruby gems, and am looking for a sponsor. Several of the packages were previously in Fedora (F11 and F12), but were removed due to lack of a maintainer.
I'll take it for a review, since it is dependency of rubygem-chef. Not sure if Jonas is active though.
* Keep Gemfile and %{gem_name}.gemspec - Please consider to keep above mentioned files in -doc subpackage. Although they are not needed for runtime, they were originally shipped by upstream, so lets try to keep the modifications limited. * Move files not needed for runtime into -doc subpackage - Please consider to move following files/directories into -doc subpackage, since they are not needed for runtime IMO: %doc %{gem_instdir}/CHANGELOG.md %doc %{gem_instdir}/README.md %{gem_instdir}/benchmark %{gem_instdir}/spec %{gem_instdir}/tasks * Package does not own the binary extension directories - The "%{gem_extdir}/lib/yajl/yajl.so" means that only the yajl.so file is owned by the package. Please use "%{gem_extdir}" instead, to own the whole directory structure including the .so file. * Exclude %{gem_cache} - Please exclude %{gem_cache}. This file is not needed on Fedora. * Bundled library - This gem contains bundled YAJL library. Please work with upstream to allow to use the YAJL available on system. - I already submitted issue upstream: https://github.com/brianmario/yajl-ruby/issues/111
I have corrected all the issues identified and rebuilt the SRPM. http://fedorapeople.org/~jdunn/rubygem-yajl-ruby/rubygem-yajl-ruby-1.1.0-2.fc19.src.rpm http://fedorapeople.org/~jdunn/rubygem-yajl-ruby/rubygem-yajl-ruby.spec I take it the bundled YAJL is a showstopper and that we can't proceed until upstream fixes this?
Yep, the bundled is a showstopper. But we can proceed on the other review in the mean time.
Josef Stribny is trying to make yajl work with Fedora's yajl. Hopefully he will provide use with some update soon.
As Vit has already mentioned, I am looking into yajl-ruby to find a way to make it work with system yajl (which is one version ahead, but with patches from the patched lib included with yajl-ruby).
I made two patches for yajl-ruby; first is an update to Yajl 2 (to match the current system version on Fedora) and second is a switch to the system version of the library. I am discussing the progress with upstream on the GitHub project page [1]. [1] https://github.com/brianmario/yajl-ruby/issues/111#issuecomment-12542826
Just FYI, I just sent pull request [1] to upstream allowing yajl-ruby to build against system yajl library. [1] https://github.com/brianmario/yajl-ruby/issues/113
I think, once the upstream merges the changes to allow to use system's yajl libraries, the name of this package should be changed to 'rubygem-yajl' as per Fedora rubygem naming guidelines.
(In reply to comment #9) > I think, once the upstream merges the changes to allow to use system's yajl > libraries, the name of this package should be changed to 'rubygem-yajl' as > per Fedora rubygem naming guidelines. I disagree. This guidelines applies to "ruby" packages, not to rubygems. Moreover, we already met packages which were named foo and foo-ruby, while their sourcecode and upstreams were different.
Can we go ahead with the "devendorized" patches locally in this package, until upstream responds?
Adding myself to the CC, since this blocks Gitorious in Fedora as well. (yajl-ruby -> pygments.rb -> makeup -> libdolt -> Gitorious)
Hi Julian, 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-yajl-ruby.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. (60bdf07) remove gem2rpm autogenerated comment. The gem2rpm tool inserts this comment into the packages, but it is just cruft so we can remove it. 2. (233f9b0) use HTTPS in URLs. This commit changes the URL from using plaintext HTTP to secured HTTPS. 3. (f054c1c) remove trailing whitespace. There were two lines with trailing whitespace in the spec file, so this commit fixes that. 4. (27bddfb) simplify el6/rubyabi conditionals. The last Fedora version that supported ruby(abi) is Fedora 18, and this is going end-of-life in a few days. We only need to support ruby(abi) on EL6. This commit simplifies the RPM dist conditionals to reflect this. 5. (a9e8c12) build doc subpackage as noarch. The -doc subpackage should be set to "noarch" since there's no architecture-specific files within it. 6. (c6cb71d) move build process into %build. The Ruby guidelines indicate that the gem should be unpacked in %prep and built in %build. This commit runs "gem unpack" and "gem spec" in %prep, and "gem build" in %build. 7. (2abbc85) use %gem_install macro. On platforms that are not EL6, the %gem_install macro is available and preferable to manually compiling the gem. 8. (2b748f8) use %{ruby_sitearch} and %{gem_extdir_mri}. The %{gem_extdir} macro is deprecated. Use %{ruby_sitearch} on EL6, and use %{gem_extdir_mri} elsewhere. 9. (a2de3a8) BuildRequire ruby(rubygems). I needed to add this to get the package to build on EL6. 10. (031261f) mark README.md and CHANGELOG.md as %doc. These files are text, not code, so I marked them as such in this commit. 11. (8f5c851) move README.md to main package. I thought it would be a good idea to ship this in the main package because so that it's more accessible to users. 12. (1e3db58) delete unnecessary gemspec during %install. It's not necessary to ship two gemspecs for this package. This commit deletes the duplicate during %install. 13. (128ca61) remove developer-only files during %prep. Some files were deleted during %install, some files were %exclude'd, and some were shipped in -doc. This commit just deletes them all during %prep (and removes them from the gemspec) for cleanliness and consistency. I've tested this version on EL6 and F19, and simple JSON tests work on both platforms.
Hi Julian, just checking to see if you've had a chance to review those patches and merge them into your yajl-ruby package? It would be great to get yajl-ruby into Fedora. I've updated for the latest yajl-ruby release (1.2.0): http://fedorapeople.org/cgit/ktdreyer/public_git/rubygem-yajl-ruby.git/commit/?id=40e3e04fa5fb421ffda3b2112eb0570257413fb5 F21 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=6533809
Hi Ken, I'm ok with the build patches, but we still need to solve the big issue which is whether to integrate the de-vendorization patch into Fedora yajl. That would potentially change the behavior of yajl from upstream, since the patch is not 100% finished pending input from (unresponsive) upstream we would need to make a call as to what to do. Specifically: the de-vendorization patch (https://github.com/brianmario/yajl-ruby/pull/113) causes some tests to fail, and I'm unsure whether this is a problem.
Just for the record, it seems tha yajl-ruby might be renamed to yajl: https://github.com/brianmario/yajl-ruby/pull/128
Since Brian Lopez seems to not want the devendorization patches against yajl-ruby, we (Chef Software, Inc.) have rewritten our own FFI wrapper around libyajl2 to get around this vendoring issue. Future versions of Chef will use ffi-yajl instead of both yajl-ruby and the JSON gem, so I have submitted bz#1114146 as a precursor to getting Chef into Fedora.
*** Bug 1373005 has been marked as a duplicate of this bug. ***
Bumped to 1.3.1. Spec URL: http://copr-dist-git.fedorainfracloud.org/cgit/sbonazzo/ovirt-fluentd/rubygem-yajl-ruby.git/tree/rubygem-yajl-ruby.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/sbonazzo/ovirt-fluentd/fedora-rawhide-x86_64/00678767-rubygem-yajl-ruby/rubygem-yajl-ruby-1.3.1-1.fc28.src.rpm
Matthias can you please review? Also pushed to https://review.rdoproject.org/r/#/c/10681/
Sandro, you -2'ed the RDO review. I guess, this is not necessary anymore?
(In reply to Matthias Runge from comment #21) > Sandro, you -2'ed the RDO review. I guess, this is not necessary anymore? It's still needed but the RDO review patch is not working: Fails functional testing on fedora. I get: Ignoring yajl-ruby-1.3.1 because its extensions are not built. Try: gem pristine yajl-ruby --version 1.3.1
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
Resetting the reviewer. I think this will get eventually autoclosed unless somebody have enough energy to move this forward.