Bug 719854
Summary: | Review Request: rubygem-xmlparser-0.6.81-1 - Ruby bindings to the Expat XML parsing library | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ulrich Schwickerath <ulrich.schwickerath> |
Component: | Package Review | Assignee: | Steve Traylen <steve.traylen> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | 16 | CC: | bkabrda, notting, package-review, shawn.starr, steve.traylen, vondruch |
Target Milestone: | --- | Flags: | steve.traylen:
fedora-review+
ulrich.schwickerath: rhel-rawhide+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | opennebula-dep | ||
Fixed In Version: | rubygem-xmlparser-0.6.81-9.fc15 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2012-05-08 04:20:16 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Ulrich Schwickerath
2011-07-08 08:23:02 UTC
Hi Ulrich, Thanks for submitting the review, A quick glance at the .spec file I notice the following things: 1) Your %build section references %{SOURCE0} which of course refers directly to your source. You need to prepare you source in %prep into a new directory and then build what you have prepared. 2) The .spec file contains some commented out lines, e.g #rm -f %{buildroot}%{gemdir}/gems/xmlparser-0.6.81/ext/xmlparser/*.so #rm -f %{buildroot}%{gemdir}/gems/xmlparser-0.6.81/ext/xmlparser/*.o for no apparent reason, at the very least it should be commented to as to why but probably it should just go. 3) You use %define and %global at the top, should almost always be %global. http://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define Now actually trying the .src.rpm. 4) Building on F15 results in: ERROR: Error installing /home/steve/rpmbuild/SOURCES/xmlparser-0.6.81.gem: ERROR: Failed to build gem native extension. Since it does not build I've added "BuildFails" to the whiteboard above, be sure to remove that once the package builds. I confess I don't know the ruby guidelines well so if I contradict anything in those I may well be wrong. I've updated the sources. The cause of the building problem eventually needs further investigation, I've reviewed the build dependencies to address this (possibly missing ruby-devel). Can you provide the link to the new .src.rpm and .spec file. You should increase the release number at each iteration of the review. Yes, of course. http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-2.el6.src.rpm Hi, 1) I'm afraid the build still fails: 1xmlparser.c:50:24: fatal error: xmlparse.h: No such file or directory to check the dependencies are correct you should definitely be checking with mock: mock -r fedora-rawhide-x86_64 \ --rebuild ../SRPMS/rubygem-xmlparser-0.6.81-2.fc15.src.rpm will produce the above error. 2) Concerning the "Your %build section references %{SOURCE0}" I'll try to explain better. Source0: http://whatever.org/%{gemname}-%{version}.gem was correct, the URL before the file name is all only informational anyway but of course very useful. What I did not explain well is that during %prep you prepare all your sources into a directory ready for building. So %prep %setup -q -c -T copies the file over to a fresh directory and then %build should be the following to operate on those files. gem install -V --local --install-dir $(pwd)/%{gemdir} \ --no-rdoc --no-ri --force %{gemname}-%{version}.gem The problem with %{SOURCE0} is that this expands to the full source location, probably $HOME/rpmbuilds/SOURCES/%{gemname}-%{version}.gem 3) I expect BuildRequires: ruby-devel, rubygem-rake, rubygem-mkrf is wrong and reubygems should be specified as BuildRequires: rubygem(mkrf), rubygem(rake) Check some existing rubygem packages to see what they do. e.g. http://pkgs.fedoraproject.org/gitweb/?p=rubygem-rake.git;a=blob;f=rubygem-rake.spec;h=9accf911a5cd25db4d4dbe392341f35a6f4d8dde;hb=HEAD Hi, Steve, thanks for the comments. ad 1/ Obviously, expat is needed. I've added a a build dependency on expat-devel, and an install dependency on expat. That should solve this problem. ad 2/ I think you are wrong here ... Nothing is being downloaded, and if I follow what you suggest, it bombs in fact out: -bash-4.1$ rpmbuild -bi rubygem-xmlparser.spec Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.Fp4pk7 + umask 022 + cd /afs/cern.ch/user/u/uschwick/rpm/BUILD + LANG=C + export LANG + unset DISPLAY + cd /afs/cern.ch/user/u/uschwick/rpm/BUILD + rm -rf rubygem-xmlparser-0.6.81 + /bin/mkdir -p rubygem-xmlparser-0.6.81 + cd rubygem-xmlparser-0.6.81 + /bin/chmod -Rf a+rX,u+w,g-w,o-w . + exit 0 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.gbzH0c + umask 022 + cd /afs/cern.ch/user/u/uschwick/rpm/BUILD + cd rubygem-xmlparser-0.6.81 + LANG=C + export LANG + unset DISPLAY + mkdir -p ./usr/lib/ruby/gems/1.8 ++ pwd + gem install -V --local --install-dir /afs/cern.ch/user/u/uschwick/rpm/BUILD/rubygem-xmlparser-0.6.81//usr/lib/ruby/gems/1.8 --no-rdoc --no-ri --force xmlparser-0.6.81.gem ERROR: Could not find a valid gem 'xmlparser-0.6.81.gem' (>= 0) in any repository error: Bad exit status from /var/tmp/rpm-tmp.gbzH0c (%build) What I have been doing is strictly following the procedures, and if you check the link you sent in the previous comment you can see that they are actually using SOURCE0 in the install step of the gem, just as I did and as it is described in the instructions. I've checked at least 10 other gems as well, and they all do it this way. AFAIK I roll back to what I had in my first rpm. ad 3) From the dependencies of the required packages (checking them with -q --provides) both ways seem to be technically possible. So it's not wrong, it's rather a matter of taste or policy. I've changed it anyway as you requested. So, here's the result of the mock run: -bash-4.1$ mock -r fedora-rawhide-x86_64 --rebuild ./rubygem-xmlparser-0.6.81-3.el6.src.rpm INFO: mock.py version 1.1.11 starting... State Changed: init plugins INFO: selinux enabled State Changed: start INFO: Start(./rubygem-xmlparser-0.6.81-3.el6.src.rpm) Config(fedora-rawhide-x86_64) State Changed: lock buildroot State Changed: clean INFO: chroot (/var/lib/mock/fedora-rawhide-x86_64) unlocked and deleted State Changed: unlock buildroot State Changed: init State Changed: lock buildroot Mock Version: 1.1.11 INFO: Mock Version: 1.1.11 INFO: enabled root cache State Changed: unpacking root cache INFO: enabled yum cache State Changed: cleaning yum metadata INFO: enabled ccache State Changed: running yum State Changed: unlock buildroot State Changed: setup State Changed: build INFO: Done(./rubygem-xmlparser-0.6.81-3.el6.src.rpm) Config(fedora-rawhide-x86_64) 0 minutes 22 seconds INFO: Results and/or logs in: /var/lib/mock/fedora-rawhide-x86_64/result State Changed: end Created packages: -rw-rw-r--. 1 uschwick c3 12658 Jul 9 22:51 build.log -rw-rw-r--. 1 uschwick c3 18756 Jul 9 22:51 root.log -rw-rw-r--. 1 uschwick mock 92331 Jul 9 22:51 rubygem-xmlparser-0.6.81-3.fc16.src.rpm -rw-rw-r--. 1 uschwick mock 169108 Jul 9 22:51 rubygem-xmlparser-0.6.81-3.fc16.x86_64.rpm -rw-rw-r--. 1 uschwick mock 46288 Jul 9 22:51 rubygem-xmlparser-debuginfo-0.6.81-3.fc16.x86_64.rpm -rw-rw-r--. 1 uschwick c3 422 Jul 9 22:51 state.log New packages: http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-3.el6.src.rpm Concerning the %{SOURCE} item the following is I think better and does work. %prep %setup -q -c -T cp -p %{SOURCE0} %{gemname}-%{version}.gem %build mkdir -p .%{gemdir} #gem install -V --local --install-dir $(pwd)/%{gemdir} --no-rdoc --no-ri --force %{gemname}-%{version}.gem gem install -V --local --install-dir $(pwd)/%{gemdir} --no-rdoc --no-ri --force %{gemname}-%{version}.gem I appreciate it makes no difference in reality but is consistant with the flow of other packages. I also see my earlier example rubygem-rake uses %{SOURCE0} in %build but I believe this to be wrong. Anyway other things: 1) Requires: expat is not needed, see: http://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires %global ruby_sitearch %(ruby -rrbconfig -e "puts Config::CONFIG['sitearchdir']") %global ruby_sitelib %(ruby -rrbconfig -e "puts Config::CONFIG['sitelibdir']") is defined conditionally, http://fedoraproject.org/wiki/Packaging:Ruby %{!?ruby_sitelib: %global rub.... The rational being that in some future fedora version these macros will be defined like e.g. the python ones. 3) Your changelog is not actually being updated between these increments. Genrally this package is looking better now, in order for me to sponsor you which I am willing to do can you maybe submit another package for review possibly but certainly provide an informal review of some other packages. Choose a package from : http://fedoraproject.org/PackageReviewStatus/NEW.html I would avoid the shaded "sponsor needed ones" when you have made a review of package there which you mark as informal review provide a link here to it. The other thing you can do now is submit this .src.rpm to koji as a --scratch build. For the sake of openness I declare also now that I work with and know you. Steve. As discussed offline, I have changed the package doing a cp of the sources instead of using {SOURCES0} during the build step. ad 1/ indeed, the explicit requirement on expat is not needed, removed it. For build it is kept of course, else it won't build ad 2/ config paths updated. Hope that is what you mean... Please let me know ad 3/ changelog updated. new version: http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-4.el6.src.rpm Hi Ulrich, I am not sponsor so I cannot help with that, but I have several comments to your package: 1) First of all, the gem seems to be rather old. It seems that upstream released new version [1] but the update was never released in gem form. Also, the expat upstream doesn't seems to be extremely vital. So are you really sure that you like to import the gem into Fedora, instead of considering to switching to Nokogiri for example? 2) License The license should be probably "GPLv2+ or Ruby or MIT" and according to readme, there are also some files licensed under the same terms as Perl. 3) You don't need the ruby_sitelib at all (btw you have type there). 4) BuildRoot is not required anymore 5) %clean section is not required 6) defattr is obsolete 7) You should submit standard build flags for building the C extension: export CONFIGURE_ARGS="--with-cflags='%{optflags}'" 8) Since the gem command itself merges all tree steps which are usually required, i.e. %prep, %build, %install, it is preferred to install the gem in %build section and then copy it in %install section into final place. 9) There has to be spaces between each changelog entry, otherwise "fedpkg clog" fails to extract the log entries correctly. For example * Wed Jul 06 2011 Ulrich Schwickerath <uschwick.ch> - 0.6.81-4 - cleaner way to treat SOURCE - remove explicit dependency on expat - make globals conditional * Wed Jul 06 2011 Ulrich Schwickerath <uschwick.ch> - 0.6.81-3 - fix build problems [1] http://www.yoshidam.net/Ruby.html#xmlparser
> 4) BuildRoot is not required anymore
>
> 5) %clean section is not required
>
> 6) defattr is obsolete
But you can leave them in if you are targeting older platforms as well, e.g RHEL6 as well.
The other comments look to all be valid.
Once you provide some informal review examples I will complete the formal review of the package.
Steve.
Hi, thanks for the additional comments! ad 1/ for the time being it is still needed. For a specific application (OpenNebula) a huge speedup is archived by using this package (in addition to nokogiri). I've contacted the developers, the problem seems to come from dependencies on a third package. So, yes, although this package is old it would be good to have it in Fedora. It'll be needed in EPEL for RHES5 and RHES6 ad 2/ should be ruby, from the README. Fixed. ad 3/ true. Removed it. ad 4/5/6 : I need to keep them as I need to build the package also on RHES5 and RHES6. Getting it for these distributions is the goal of this submission ad 7/ done ad 8/ This is what I'm doing in this spec. The copy in the prep step was requested by Steve. ad 9/ done http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-5.el6.src.rpm (In reply to comment #9) > 2) License > The license should be probably "GPLv2+ or Ruby or MIT" and according to readme, > there are also some files licensed under the same terms as Perl. > [1] http://www.yoshidam.net/Ruby.html#xmlparser Just to be clear I had not done the full review yet and had not checked the licensing. The licensing is impressively confused in fact. http://www.yoshidam.net/xmlparser_en.txt and the README gives part Ruby or "license of expat" and also bizarrely partly there is a Perl license on encoding.h In addition: xpath.rb is GPLv2+ so this all resolves to License: GPLv2+ and ( Ruby or GPLv2+ ) and ( GPLv2+ or Artistic ) which would definitely need some comments to explain which file is which. (In reply to comment #12) Since Expat is MIT licensed, there should be mentioned also MIT, otherwise I agree, the licensing is a mess :/ So, License: GPLv2+ and ( Ruby or GPLv2+ ) and ( GPLv2+ or Artistic ) and MIT + add something in the description ? How about ... License: GPLv2+ and ( Ruby or GPLv2+ or MIT ) and ( GPLv2+ or Artistic ) ... %description Ruby bindings to the Expat XML parsing library. For details about the license conditions of individual files in this package please refer to the README. Look at an example for how to add comments explaining the license, the comment is to describe the License: in the spec file. http://pkgs.fedoraproject.org/gitweb/?p=gridsite.git;a=blob;f=gridsite.spec;h=9ee371d19a1c50cfabe0a2b89c880cc4c6e4df41;hb=HEAD In particular read http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Dual_Licensing_Scenarios I've updated the spec file and bumped up the release to 6 http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-5.el6.src.rpm Alright, you are also trying to package xmlparser for OpenNebula 2.2+? I also was trying to package this, and other dependencies needed. See bug #722364 Ulrich, if you want to help with OpenNebula packaging please come on IRC in the #opennebula channel and chat with jmelis and me spstarr, let's coordinate :) Shawn, I can't use IRC due to security restrictions, sorry. I've been discussing this with Ruben. If you want to take over on this one that's fine with me. Please go ahead with your submission, I will focus on dm-sqlite/mysql-adapter rubygems. I will close my bug. *** Bug 722364 has been marked as a duplicate of this bug. *** Package Review ============== rubygem-xmlparser-0.6.81-5.fc15.src.rpm Builds in F15 x86_64 mock. $ rpmlint rubygem-xmlparser.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/*.rpm 0 packages and 0 specfiles checked; 0 errors, 0 warnings. Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Spec file is legible and written in American English. [x] Spec file lacks Packager, Vendor, PreReq tags. [x] Spec uses macros instead of hard-coded directory names. [x] Package consistently uses macros. [x] Macros in Summary, %description expandable at SRPM build time. [x] PreReq is not used. [x] Requires correct, justified where necessary. [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)). [x] Package run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) and the beginning of %install. [x] Package use %makeinstall only when ``make install DESTDIR=...'' doesn't work. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [-] The spec file handles locales properly. [x] Changelog in prescribed format. [x] Rpmlint output is silent. [x] License field in the package spec file matches the actual license. [-] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc. [-] License file installed when any subpackage combination is installed. [x] Sources contain only permissible code or content. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. MD5SUM this package : dc494768a4ea0c7bedd3444a112ac3ae MD5SUM upstream package : dc494768a4ea0c7bedd3444a112ac3ae [x] Compiler flags are appropriate. Yes %{optflags} is being used. [-] ldconfig called in %post and %postun if required. [!] Package must own all directories that it creates. [x] Package does not own files or directories owned by other packages. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Each %files section contains %defattr. [x] No %config files under /usr. [-] %config files are marked noreplace or the reason is justified. [-] Package contains a properly installed %{name}.desktop using desktop-file-install file if it is a GUI application. [-] Package contains a valid .desktop file. [x] Package contains code, or permissable content. [-] Package contains a SysV-style init script if in need of one. [x] File names are valid UTF-8. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [x] Package contains no bundled libraries. [-] Header files in -devel subpackage, if present. [-] Static libraries in -static subpackage, if present. [x] Package contains no static executables. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [-] Package does not contain any libtool archives (.la). [x] Useful -debuginfo package or justification otherwise. [x] Rpath absent or only used for internal libs. [x] Package does not genrate any conflict. [x] Package does not contains kernel modules. [x] Package is not relocatable. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. [x] Package is not known to require ExcludeArch. [x] Package installs properly. [x] Package obeys FHS, except libexecdir and /usr/target. [x] Package meets the Packaging Guidelines. === Issues === 1. Not all directories you created are owned or belong to packages you pulled in, in particular: /usr/lib/ruby/gems/1.8/gems/xmlparser-0.6.81 is not owned but you created it. See: http://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership 2. While your .spec file includes the extra licensing comments the .spec file in the .src.rpm does. Be sure to bring this into with with the next iteration. === Final Notes === The package looks good now other than the two items above. Please do some informal reviews of other packages and provide a reference here so I can find them. This is needed before I can sponsor you. Additional comments: the DSO library should be in %{ruby_sitearch}/%{gemname}/*.so Same as rubygem-cairo, although rubygem-json uses %{ruby_sitearch}/%{gemname}/etc/ for the DSO location. I guess that doesn't matter, there doesn't seem to be a clear guideline, but I've been following %{ruby_sitearch}/%{gemname}/*.so Can this package be approved for submission? I can sponser ulrich and approve package, but i still need to see an informal review of some other package. Steve. (In reply to comment #26) > I guess that doesn't matter, there doesn't seem to be a clear guideline, but > I've been following %{ruby_sitearch}/%{gemname}/*.so The *.so file has to follow the original gem folder structure. E.g: 1) %{geminstdir}/lib/foo.so => %{ruby_sitearch}/foo.so 2) %{geminstdir}/lib/%{gemname}/foo.so => %{ruby_sitearch}/%{gemname}/foo.so 2) %{geminstdir}/ext/foo.so => %{ruby_sitearch}/ext/foo.so If you do not follow this rule, then you would need to patch the require of *.so file somewhere in gem ruby code, because the *.so file wouldn't be possible to find using Ruby $LOAD_PATH. Hi,sorry for the long silence on this, I was away for a while on holiday. I'll get in touch with Steve to see how to unblock this gem. I've done an informal review done on https://bugzilla.redhat.com/show_bug.cgi?id=760357 I've tried to follow the rule for moving the .so files. If I do so, i.e. if I move the library to %{ruby_sitearch}/lib/, the package no longer works, at least not on my test machine. Adding a link solved this problem. Concerning %{gemdir}/gems/%{gemname}-%{version} which was created but not owned by the package, I have added it as %dir in the new version. Here's a new version of the package: http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-7.el6.src.rpm The spec file in the src.rpm and the one given are identical (I've checked) but maybe I misinterpreted this comment from Steve. (In reply to comment #31) > I've tried to follow the rule for moving the .so files. If I do so, i.e. if I > move the library to %{ruby_sitearch}/lib/, the package no longer works, at > least not on my test machine. Adding a link solved this problem. You should first understand how Ruby together with RubyGems are treating load paths. I'll try to shortly explain: 1) If you do "require 'foo'", Ruby searches all the load paths and if finds the file on some of them, it loads it. Ruby has some set of default load paths and %{buildroot}%{ruby_sitearch} is one of them. I.e. if you place file foo.rb (or foo.so) into that path, then you can do later "require 'foo'". 2) If the file is not found in load paths, then the RubyGems loading mechanism kicks in. RubyGems goes through all installed gems and looks for suitable file to load. When it is found, RubyGems will insert its paths (which are specified by #require_paths method in .gemspec file) into Ruby's load paths and issues again regular Ruby's require. So if you place the library into the %{buildroot}%{ruby_sitearch}/xmlparser.so location, then there is no need to do any link, since Ruby can find the file on its load path. And that is the way how it should be done currently. And I have some additional notes: * It seems that the author of the gem is somebody else then the original upstream. However, it would be nice if the link to homepage was somehow useful, may be http://www.yoshidam.net/Ruby.html#xmlparser is the correct one? * It might make sense to ask the author of the gem for update to the latest upstream version of the library? Hi, Vít, thanks for the explanation. I admit that I'm not a ruby geek ;-) I did the change in this latest version because of comment 28. What you suggest now is what I did before. Personally I have a slight preference for the original approach because it is simpler, I have no problem to roll back. Let me know. About upgrading: this was discussed earlier in this thread already. I agree that it is a good idea to ping the original author to get an updated package. For the time being though this one seems to be the best we can do. It's needed for OpenNebula. (In reply to comment #33) > Hi, Vít, > > thanks for the explanation. I admit that I'm not a ruby geek ;-) I did the > change in this latest version because of comment 28. What you suggest now is > what I did before. Personally I have a slight preference for the original > approach because it is simpler, I have no problem to roll back. Let me know. Sorry for the confusion. In comment 28, I just tried to point out that Shawn was not right, that the guideline is clear, but I apparently achieved the opposite effect ;) > About upgrading: this was discussed earlier in this thread already. I agree > that it is a good idea to ping the original author to get an updated package. > For the time being though this one seems to be the best we can do. It's needed > for OpenNebula. Not an issue. It was just note. ok, no problem. I rolled back this link thing. http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-8.el6.src.rpm Are we good for submission? Can someone approve the review if this is acceptable? also, has Ulrich been mentored for packager/provenpackager? who is maintainer of this once it's approved? Vít, I'd like to sponsor Ulrich as I know he knows what he is doing but while my packaging is generally good my ruby is less so. This package looks almost good to me but if you have further comment I would be grateful, this turns out to be a non-trivial ruby package. The alternative is I release this as being assigned to me. Steve. Problems I still see however: (1) Requires: ruby-libs is almost certainly not needed and is you get anyway from both libruby.so.1.8()(64bit) ruby(abi) = 1.8 the first one of which in particular is autogenerate, basically you should not added which is determined automatically anyway. (2) The fact the package contains the origional gem seems bad. /usr/lib/ruby/gems/1.8/cache/xmlparser-0.6.81.gem it should not. (In reply to comment #38) > Vít, > > I'd like to sponsor Ulrich as I know he knows what he is doing but while my > packaging is generally good my ruby is less so. > > This package looks almost good to me but if you have further comment I would be > grateful, this turns out to be a non-trivial ruby package. > > The alternative is I release this as being assigned to me. > > Steve. > > Problems I still see however: > > (1) > Requires: ruby-libs > > is almost certainly not needed and is you get anyway from both > libruby.so.1.8()(64bit) > ruby(abi) = 1.8 > > the first one of which in particular is autogenerate, basically you should > not added which is determined automatically anyway. Actaully the 'ruby-libs' should be replaced with 'ruby', as is stated in Ruby packaging guidelines. > (2) > The fact the package contains the origional gem seems bad. > > /usr/lib/ruby/gems/1.8/cache/xmlparser-0.6.81.gem > > it should not. Although there is a lot of gems which keeps the original gem in the RPM and it is not against packaging guidelines, I also recommend to use %exclude for the cached gem. Otherwise I am fine with the package. Feel free to approve it and sponsor Ulrich any time. APPROVED You can now proceed to request an SCM area for this package: http://fedoraproject.org/wiki/Package_SCM_admin_requests Ulrich you make the changes that VIt mentions in comment #39 first of course. Can I get an update please? This is now urgent for OpenNebula and Fedora 17 timeframes. Sorry for the delay on this. I've fixed the ruby-lib dependency in http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec http://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.6.81-9.el6.src.rpm New Package SCM Request ======================= Package Name: rubygem-xmlparser Short Description: Ruby bindings to the Expat XML parsing library Owners: schwicke Branches: f15 f16 el6 InitialCC: Git done (by process-git-requests). Don't include the version in future summaries. Thanks! The build for el6 currently fails because rubygem-mkrf is missing for el6. Can that be included in el6 ? (In reply to comment #46) Well if you become maintainer for epel6, I guess it should be no problem. Please ask in the mkrf review ticket #508416 The original package does not build with ruby >= 1.9. I've contacted the original developer who provided an updated gemspec from which I built a newer version of the gem. It's available here: https://uschwick.web.cern.ch/uschwick/software/xmlparser-0.7.2.gem Updated spec file: https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec Updated source rpm: https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.7.2-1.el6.src.rpm Please note, that you need to update to 0.7.2.1. The previous release had buggy require_paths and was removed. Looks like I'm doomed. The new version compiles fine on fed17 and fed18 but the package was removed from there because it did not build, and I cannot upgrade. On fed16 the old package builds but the new one fails in the installation step. This might be related to the version of rubygem-mkrf which is older than on fed17. It builds fine locally on an el6 box. I'm not sure if I understand you problem correctly, but wouldn't placing the 0.6.81 into F16 and 0.7.2.1 into F17 and above solve the problem? Yes, that was my original intention. I probably misunderstood you then. I was trying to upgrade f16 as well. Sorry about that, will have to roll that back. I lost access to f17 and the development branch, cannot commit any longer. Ticket stays with me as record of who approved the package. I've opened a new review request (805911) for f17 and rawhide for the new version 0.7.2.1 This version here is good for up to f16 (included). Will open a new SCM request for the update, as suggested in 805911 Package Change Request ====================== Package Name: rubygem-xmlparser New Branches: f17 Owners: schwicke You need to set the cvs flag (I did that for you, hope it doesn't matter who did that :) Not sure why this was retired, I unretired f17 and devel, take ownership in pkgdb. rubygem-xmlparser-0.7.2-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/rubygem-xmlparser-0.7.2-1.fc17 rubygem-xmlparser-0.6.81-10.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/rubygem-xmlparser-0.6.81-10.fc16 rubygem-xmlparser-0.6.81-9.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/rubygem-xmlparser-0.6.81-9.fc15 rubygem-xmlparser-0.7.2-1.fc17 has been pushed to the Fedora 17 testing repository. As pointed out by vondruch, the package for >=fed17 should apply the new packaging guidelines. Here's a new spec file https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.7.2-2.fc17.src.rpm It still requires a bit of functional testing. Please note that you can/should now use macros provided by rubygems-devel to ease the packaging. Please refer to macros section of Ruby packaging guidelines [1]. In other words, your files section should looks like: %files %{gem_extdir}/lib/xmlparser.so %dir %{gem_instdir}/ %doc %{gem_instdir}/[A-Z]* %{gem_instdir}/[a-z]* %exclude %{gem_cache} %{gem_spec} [1] https://fedoraproject.org/wiki/Packaging:Ruby#Macros Hi Vit, thanks a lot for your help on this! I have another iteration on this ready now, as I also realised that since I built this updated version there is finally an updated gem available from rubyforge while the previous version was using what I directly got from the author. https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser.spec https://uschwick.web.cern.ch/uschwick/software/rubygem-xmlparser-0.7.2.1-3.fc17.src.rpm rpmlint output: srpm: 1 packages and 0 specfiles checked; 0 errors, 0 warnings. binary (on fed17): rubygem-xmlparser.x86_64: W: no-soname /usr/lib64/gems/exts/xmlparser-0.7.2.1/lib/xmlparser.so 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Hopefully, this will be the final version. Any feedback on it is very welcome before I proceed. I did not tested it but it seems it should be OK. Thank you. rubygem-xmlparser-0.7.2.1-3.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/rubygem-xmlparser-0.7.2.1-3.fc17 Package rubygem-xmlparser-0.7.2.1-3.fc17: * should fix your issue, * was pushed to the Fedora 17 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing rubygem-xmlparser-0.7.2.1-3.fc17' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2012-6709/rubygem-xmlparser-0.7.2.1-3.fc17 then log in and leave karma (feedback). rubygem-xmlparser-0.7.2.1-3.fc17 has been pushed to the Fedora 17 stable repository. If problems still persist, please make note of it in this bug report. rubygem-xmlparser-0.6.81-10.fc16 has been pushed to the Fedora 16 stable repository. rubygem-xmlparser-0.6.81-9.fc15 has been pushed to the Fedora 15 stable repository. rubygem-xmlparser-0.6.81-10.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/rubygem-xmlparser-0.6.81-10.el6 rubygem-xmlparser-0.6.81-10.el6 has been pushed to the Fedora EPEL 6 stable repository. |