Spec URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core-0.1.2-1.fc14.src.rpm Description: I would like to add BoxGrinder Build (http://www.jboss.org/boxgrinder) to Fedora. BoxGrinder is a command line tool for building appliances (virtual machines) to various platforms (KVM, Xen, VMware, EC2). This package (boxgrinder-core) contains core files required by BoxGrinder family of projects.
Some notes * Licensing - It seems that openhash/openhash.rb comes from hashery gem ( the latest version of hashery gem is 1.3.0). Also the license of lib/hashery/openhash.rb in hashery 1.3.0 is currently under ASL 2.0. - Would you tell me where openhash/openhash.rb (in boxgrinder-core gem) came from? - Also would you show us if the license of openhash.rb is really under MIT? - And. bundling files in other projects is generally forbidden and needs FPC's approval. Would you consider to package rubygem-hashery seperately? * BuildRoot line / %clean section - BuildRoot line is no longer needed on Fedora and EPEL6 - %clean section is no longer needed on Fedora 12+ and EPEL6. * Unneeded version specific dependency - " >= 1.2" part on "R: rubygems" or so are all unneeded ( Only that "= %{abi}" part on "R: ruby(abi)" is needed ) https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires ( Please see the last 2 sentences in this section ) * Output file on %check - If %check section produces some extra files, it is preferable that %check section won't touch %buildroot tree to avoid unneeded confusion on %files and will touch files under %_builddir ( i.e. In this case, it is preferable that gem is once installed under %_builddir and then copy the whole tree to %buildroot at %install, then execute %check under %_builddir) * Ability to build - Your srpm does not build on koji for dist-rawhide (at least). At least, "BR: rubygem(rake) rubygem(open4)" is needed (for %check) (and perhaps also rubygem(rspec))
(In reply to comment #1) > * Licensing > - It seems that openhash/openhash.rb comes from hashery gem > ( the latest version of hashery gem is 1.3.0). Also the license > of lib/hashery/openhash.rb in hashery 1.3.0 is currently under > ASL 2.0. Yes, this is the current state of OpenHash. > - Would you tell me where openhash/openhash.rb (in boxgrinder-core > gem) came from? As you pointed out, it was used from hashery: https://github.com/rubyworks/hashery/blob/master/lib/hashery/openhash.rb I added small modification in line 88 and license header. > - Also would you show us if the license of openhash.rb is really > under MIT? It seems that they moved to Apache License at some point: https://github.com/rubyworks/hashery/commit/0934b591ee716235a17d4ee691257bed291e36fb I used the code before the license switch. > - And. bundling files in other projects is generally forbidden > and needs FPC's approval. Would you consider to package > rubygem-hashery seperately? Ouch. Yes, this sounds like a good idea but in my case still I would need to override the method_missing (which is ~50% of the code :)) code to meet my needs. > * BuildRoot line / %clean section > - BuildRoot line is no longer needed on Fedora and EPEL6 I added BuildRoot tag because rpmlint highlighted it as a warning. Removing. > - %clean section is no longer needed on Fedora 12+ and EPEL6. OK, good to know – this is my first package. > * Unneeded version specific dependency > - " >= 1.2" part on "R: rubygems" or so are all unneeded > ( Only that "= %{abi}" part on "R: ruby(abi)" is needed ) > https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires > ( Please see the last 2 sentences in this section ) OK, I'll remove unnecessary requires. > * Output file on %check > - If %check section produces some extra files, it is preferable > that %check section won't touch %buildroot tree to avoid > unneeded confusion on %files and will touch files under > %_builddir > ( i.e. In this case, it is preferable that gem is once installed > under %_builddir and then copy the whole tree to %buildroot > at %install, then execute %check under %_builddir) Right, I didn't know how to deal with that. Should it look like this? %install rm -rf %{buildroot} rm -rf %{_builddir}%{gemdir} mkdir -p %{_builddir}%{gemdir} gem install --local --install-dir %{_builddir}%{gemdir} \ --force --rdoc %{SOURCE0} mkdir -p %{buildroot}/%{gemdir} cp -r %{_builddir}%{gemdir}/* %{buildroot}/%{gemdir} %check pushd %{_builddir}/%{geminstdir} rake spec popd > * Ability to build > - Your srpm does not build on koji for dist-rawhide (at least). > At least, "BR: rubygem(rake) rubygem(open4)" is needed (for %check) > (and perhaps also rubygem(rspec)) OK, I'll take a look at this issue. Thanks for the first review, I'll fix my issues and get back to you.
I'll move to Hashery gem, therefore I created a new review request for rubygem-hashery: BZ#653407.
I updated the spec file and rebuild the SRPM: Spec URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core-0.1.3-1.fc14.src.rpm
Will review.
I found unnecessary "Requires: rubygems" – I'll remove it in the next version with all your comments for this spec file.
Created attachment 461153 [details] mock build log for rawhide For 0.1.3-1: Well, the spec file in the srpm and the spec file in your comment 4 differ, however: * BR - Still your srpm does not build (build log attached). At least "BR: rubygem(echoe)" is needed. ! Cleaning up %buildroot at %install ------------------------------------------- %install rm -rf %{buildroot} rm -rf %{_builddir}%{gemdir} ------------------------------------------- - Well, on Fedora these lines are no longer needed (%buildroot is anyway cleaned at the beginning of %install) * About output file in %check > > * Output file on %check > > - If %check section produces some extra files, it is preferable > > that %check section won't touch %buildroot tree to avoid > > unneeded confusion on %files and will touch files under > > %_builddir > > ( i.e. In this case, it is preferable that gem is once installed > > under %_builddir and then copy the whole tree to %buildroot > > at %install, then execute %check under %_builddir) > > Right, I didn't know how to deal with that. Should it look like this? - Please use "%setup -q -T". Usually we do like --------------------------------------------- %prep %setup -q -c -T mkdir -p .%{gemdir} gem install -V --local --install-dir .%{gemdir} \ --force --rdoc %{SOURCE0} %build %install mkdir -p %{buildroot}%{gemdir} cp -a .%{gemdir}/* %{buildroot}%{gemdir}/ --------------------------------------------- * Dependency - I think installed .gemspec (under %{gemdir}/specifications/) should have runtime dependency for "hashery".
(In reply to comment #7) > Created attachment 461153 [details] > mock build log for rawhide > > For 0.1.3-1: > > Well, the spec file in the srpm and the spec file in > your comment 4 differ, however: > > * BR > - Still your srpm does not build (build log attached). > At least "BR: rubygem(echoe)" is needed. Added! Sorry for missing that... > ! Cleaning up %buildroot at %install > ------------------------------------------- > %install > rm -rf %{buildroot} > rm -rf %{_builddir}%{gemdir} > ------------------------------------------- > - Well, on Fedora these lines are no longer needed > (%buildroot is anyway cleaned at the beginning of %install) Removed. > > Right, I didn't know how to deal with that. Should it look like this? > - Please use "%setup -q -T". Usually we do like > --------------------------------------------- > %prep > %setup -q -c -T > mkdir -p .%{gemdir} > > gem install -V --local --install-dir .%{gemdir} \ > --force --rdoc %{SOURCE0} > > %build > > %install > mkdir -p %{buildroot}%{gemdir} > cp -a .%{gemdir}/* %{buildroot}%{gemdir}/ > --------------------------------------------- Ah, OK, adjusted. > * Dependency > - I think installed .gemspec (under %{gemdir}/specifications/) > should have runtime dependency for "hashery". True, added! Spec URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core.spec SRPM URL: http://goldmann.fedorapeople.org/package_review/boxgrinder/rubygem-boxgrinder-core-0.1.3-3.fc14.src.rpm Thank you for your help!
Okay. ------------------------------------------------------- This package (rubygem-boxgrinder-core) is APPROVED by mtasaka -------------------------------------------------------
By the way I would appreciate it if you have some time to review my rubygem-related review request (bug 639636)
Thanks for review! Will try to review your package shortly! New Package SCM Request ======================= Package Name: rubygem-boxgrinder-core Short Description: Core files required by BoxGrinder Owners: goldmann Branches: f13 f14
Git done (by process-git-requests).
For F-15, you can build this package using chain-build (or just wait a few hours after building rubygem-hashery on rawhide) For F-14/13, if you want to build this package now (i.e. without waiting one week before rubygem-hashery being pushed into stable), you can ask rel-eng team to get rubygem-hashery tagged as dist-fXX-override to get rubygem-hashery pulled in buildroot.
Rawhide build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2613414
rubygem-hashery tagged dist-f13-override and dist-f14-override
Umm.. for F-14/13, it seems that you have to wait for rubygem-echoe (owned by mfojtik) to get pushed into stable... (It seems that today 7 testing days delay will be completed so I expected that F-14/13 rubygem-echoe will appear on stable within 1-2 days)
(In reply to comment #16) > Umm.. for F-14/13, it seems that you have to wait for rubygem-echoe > (owned by mfojtik) to get pushed into stable... Yes, just saw it too (after my build failed). BTW, shouldn't rubygem-hashery be available now automatically for my build? Or do I need to wait some time too? http://koji.fedoraproject.org/koji/taskinfo?taskID=2617437 From root.log: DEBUG util.py:260: No Package Found for rubygem(echoe) DEBUG util.py:260: No Package Found for rubygem(hashery)
(In reply to comment #17) > (In reply to comment #16) > > Umm.. for F-14/13, it seems that you have to wait for rubygem-echoe > > (owned by mfojtik) to get pushed into stable... > > Yes, just saw it too (after my build failed). BTW, shouldn't rubygem-hashery be > available now automatically for my build? Or do I need to wait some time too? > > http://koji.fedoraproject.org/koji/taskinfo?taskID=2617437 > > From root.log: > > DEBUG util.py:260: No Package Found for rubygem(echoe) > DEBUG util.py:260: No Package Found for rubygem(hashery) For rubygem-hashery, perhaps now okay (it seems that your build was about 10 min earlier than override-tagged rubygem-hashery came into F-13 buildroot)
Now waiting for rubygem-echoe only to build the package, which was pushed to stable. Should be there today.
rubygem-boxgrinder-core-0.1.3-3.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/rubygem-boxgrinder-core-0.1.3-3.fc14
rubygem-boxgrinder-core-0.1.3-3.fc13 has been submitted as an update for Fedora 13. https://admin.fedoraproject.org/updates/rubygem-boxgrinder-core-0.1.3-3.fc13
rubygem-boxgrinder-core-0.1.3-3.fc14 has been pushed to the Fedora 14 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update rubygem-boxgrinder-core'. You can provide feedback for this update here: https://admin.fedoraproject.org/updates/rubygem-boxgrinder-core-0.1.3-3.fc14
rubygem-boxgrinder-core-0.1.3-3.fc14 has been pushed to the Fedora 14 stable repository. If problems still persist, please make note of it in this bug report.
rubygem-boxgrinder-core-0.1.3-3.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: rubygem-boxgrinder-core New Branches: el5 el6 Owners: goldmann