Red Hat Bugzilla – Bug 601160
Review Request: rubygem-rdiscount - Fast Implementation of Gruber's Markdown in C
Last modified: 2010-06-21 17:43:41 EDT
Spec URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec SRPM URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount-1.6.3.2-1.fc12.src.rpm Description: Discount is an implementation of John Gruber's Markdown markup language in C. It implements all of the language described in the markdown syntax document and passes the Markdown 1.0 test suite. Successful scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2235208
Some initial notes: * %define -> %global - Now we prefer to use %global instead of %define: https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define * debuginfo rpm - Please don't set "%define debug_package %{nil}" and create debuginfo rpm correctly * License - As far as I checked the source codes, the license tag should be "ASL 1.1". * ruby(abi) dependency - For ruby module packages, writing "R: ruby(abi) = 1.8" is mandatory on Fedora: https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_Packaging_Guidelines * BuildRoot no longer needed - For Fedora (not for EPEL), BuildRoot tag is no longer needed: https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag * redundant "exit 0" - You needed not redundant "exit 0" at the end of %prep, %build, %install (not: %prep, %build, %install stage execute shell script with "/bin/sh -e") * Some generic packaging issue for rubygems containing C extension modules - Arch-dependent files (like rdiscount.so) must be moved to under %ruby_sitearch: https://fedoraproject.org/wiki/Packaging/Ruby#Ruby_packages_with_binary_content.2Fshared_libraries - Files under ext/ directory (and ext/ directory itself) are needed to generate C extension module file and should not be needed on runtime. i.e. ext/ directory should not be in generated binary rpm. * Marking files as %doc - Please mark document files as %doc properly - (COPYING and) README.markdown Rakefile should be marked as %doc ( note that Rakefile is something like makefiles in autotool based packages ) - man/ test/ directories should also be marked as %doc - Also I usually suggest to create -doc subpackage and move * Rakefile * man/ test/ directories * %{gemdir}/doc/%{gemname}-%{version} to -doc subpackage - By the way * I think rdiscount.1 man file should be moved to %_mandir/man1 * I am not sure if markfile.7 should also be moved to %_mandir/man7 or not. * Duplicate %files entry - Please make it sure that every file/directory/etc is listed only once in %files entry. Currently build.log shows: ------------------------------------------------------------------------- 164 Processing files: rubygem-rdiscount-1.6.3.2-1.fc14.i686 165 warning: File listed twice: /usr/lib/ruby/gems/1.8/gems/rdiscount-1.6.3.2/COPYING ------------------------------------------------------------------------- * Enabling test - As this gem contains test/ directory, please add %check section and execute some test program (like $ rake test) there.
Thank you for the first look. Would you please make a second look? I did not increment the revision (release) number. I only overwrite the files at Spec URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec and SRPM URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount-1.6.3.2-1.fc12.src.rpm The URL from the scratch build of the current files is: http://koji.fedoraproject.org/koji/taskinfo?taskID=2238352 I do the best I can for "Enabling tests". May be you know a better solution for that.
fedora-review flag must be set by reviewer (assignee), once resetting. Also would you bump release number to avoid confusion? (and bumping release number is helpful when using diff)
I am sorry for the mistake. I was in a hurry because I had an appointment and my last review request is some time ago. I increment the release number in the spec file, it is still stored under the URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec The new SRPM is at: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount-1.6.3.2-2.fc12.src.rpm Scratch build URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=2238903
Well, * ruby(abi) dependency - must be strict equality (i.e. R: ruby(abi) = 1.8) * rubygem module related dependency - For rubygem module related dependency, please use "(Build)Requires: rubygem(foo)" style. ref: (althogh this for perl guideline:) https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides * Dependency for -doc subpackage - -doc subpackage should depend on main package for directory ownership issue (also see below) * Directory for non-arch-dependent ruby scripts - As I said, arch-dependent files (like C extentsion modules) must be moved to %ruby_sitearch. On the other hand, non-arch-dependent ruby scripts must not be moved from %geminstdir. * Movement of files under %_builddir - Currently build.log says in %check section: -------------------------------------------------------------- 199 + rake test 200 rm -f 201 touch ext/ruby-54f732744bed984d9d0d133997bf4128 202 cd ext 203 (in /builddir/build/BUILD/rubygem-rdiscount-1.6.3.2/usr/lib/ruby/gems/1.8/gems/rdiscount-1.6.3.2) 204 /usr/bin/ruby extconf.rb 205 checking for random()... yes 206 checking for srandom()... yes .... .... 211 cd ext && make clean && make 212 gcc -I. -I. -I/usr/lib/ruby/1.8/i386-linux -I. -DHAVE_RANDOM -DHAVE_SRANDOM -DHAVE_RAND -DHAVE_SRAND -D_FILE_OFFSET_BITS=64 -fPIC -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -fno-strict-aliasing -fPIC -c Csio.c .... 264 cp -p ext/rdiscount.so lib/ 265 /usr/lib/ruby/site_ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require': no such file to load -- markdown (LoadError) -------------------------------------------------------------- So currently "rake test" - tries to compile rdiscont.so again which was actually built in %build - and finally fails because lib/markdown.rb is not found (under %_builddir/%gemname-%version/%geminstdir). This is because rdiscount.so is moved to %ruby_sitearch before the tree is copied into %buildroot. You must - first copy the whole tree to %buildroot - then move files under %buildroot if needed. ! Note that as said above lib/markdown should not be moved to %ruby_sitearch. * "rake test" - For this gem "rake test" fails because the needed test/MarkdownTest_1.0.3 is missing. Please replace this with "rake test:unit" * Issues in %files - As %geminstdir is explicitly defined, please use the macro also in %files - The directory %geminstdir itself is not owned by any packages: https://fedoraproject.org/wiki/Packaging/UnownedDirectories#Common_Mistakes * %doc tag in -doc subpackage - Usually I think %doc attribute in -doc subpackage is just redundant because the name of the rpm already says that this is for documentation. * %exclude'ing ext/ directory - Leaves ext/rdiscount.so.debug files in -debuginfo rpm, which is not needed. Please "remove" ext/ directory from %buildroot completely (not from %_builddir) instead of using %exclude to avoid this.
Thank you again for the competent comments and good help. The old SPEC file is: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec.2 The URL of the current SPEC file is: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec new SRPM-URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount-1.6.3.2-3.fc12.src.rpm Scratch build URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=2244251
For -3: * Unneeded macro - %ruby_sitelib macro is used nowhere. * Dependency for -doc subpackage - As I said in the previous comment, for directory (%geminstdir) ownership issue, -doc subpackage should have the dependency for main package ( i.e. -doc subpackage should have "R: %{name} = %{version}-%{release} ) * .gemspec - Move %{geminstdir}/rdiscount.gemspec to -doc subpackage. This one is needed only for tests. ( %{gemdir}/specifications/%{gemname}-%{version}.gemspec should stay in main package ).
I done the three items from the last list. Only the dependency in the doc subpackage to the main package I do not like so much. I would prefer the possibility to install the doc package und be able to read the documentation without the need to have the main package installed. But I looked to the spec file of an other rubygem package. That has the same dependency included. The URL for the old SPEC file is: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec.3 The URL of the current SPEC file is: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount.spec new SRPM-URL: ftp://ftp.uni-siegen.de/pub/review/rubygem-rdiscount-1.6.3.2-4.fc12.src.rpm new scratch build URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=2246202
Apologize again, after a deeper look in the documentation I understand that setting the dependency is the only way to resolve the ownership of the directory "%geminstdir".
For -4: * Directory ownership issue - Oops... I missed one thing. Currently the directory %{geminstdir}/bin itself is not owned by any package. Please fix this. (i.e. change to %{geminstdir}/bin/ in %files or explicitly add the directory by %dir) ! %doc in -doc - As I said in the above, I think %doc in -doc subpackage is redundant, however not a blocker. Please fix the above issue(s) before importing this package into Fedora. ---------------------------------------------------------------- This package (rubygem-rdiscount) is APPROVED by mtasaka -----------------------------------------------------------------
New Package CVS Request ======================= Package Name: rubygem-rdiscount Short Description: Fast Implementation of Gruber's Markdown in C Owners: gerd Branches: F-12 F-13 InitialCC: gerd
Please set fedora-cvs flag to ?
CVS done (by process-cvs-requests.py).
rubygem-rdiscount-1.6.3.2-4.fc13 has been submitted as an update for Fedora 13. http://admin.fedoraproject.org/updates/rubygem-rdiscount-1.6.3.2-4.fc13
rubygem-rdiscount-1.6.3.2-4.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/rubygem-rdiscount-1.6.3.2-4.fc12
rubygem-rdiscount-1.6.3.2-4.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.
rubygem-rdiscount-1.6.3.2-4.fc13 has been pushed to the Fedora 13 stable repository. If problems still persist, please make note of it in this bug report.