Bug 831749

I forgot to say.
This is my first review and I need a sponsor.

Troy,

I'll sponsor you.

To join the packager group you need to be able to do
the following things:
1. provide competent reviews of other people's packages
2. produce high quality packaging that passes the guidelines prior to review
3. help coach packagers on trouble points in their packaging

Read:

http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

A package should follow the packaging guidelines:

http://fedoraproject.org/wiki/Packaging:Guidelines

Since you have submitted a package, I will ask you in the bugzilla to review a
couple other people's packages.  While you are not a packager, you can
still provide reviews to demonstrate you are capable of providing a
review of a new package.  To execute a review, you would follow the
review guidelines:

http://fedoraproject.org/wiki/Packaging:ReviewGuidelines

Some example reviews I have done are here:

https://bugzilla.redhat.com/buglist.cgi?f1=flagtypes.name&list_id=79500&o1=equals&classification=Fedora&emailtype1=substring&query_format=advanced&emailassigned_to1=1&token=1338582948-9534ec43e4e74cdb0393ec72859aedfe&bug_status=NEW&bug_status=ASSIGNED&bug_status=MODIFIED&bug_status=ON_DEV&bug_status=ON_QA&bug_status=VERIFIED&bug_status=RELEASE_PENDING&bug_status=POST&bug_status=CLOSED&email1=sdake%40redhat.com&v1=fedora-review%2B&component=Package%20Review

Once you have given a couple high quality reviews of other's packages,
I'll review your package submission and we will get it beat into
submission for Fedora.

Let me know what your ready to give a review of a few packages.

To fix the rpmlint warning:

rubygem-sshkey-doc.noarch: I: enchant-dictionary-not-found en_US


run

fedora% sudo yum install hunspell-en

You will want to do this before providing reviews.

Regards
-steve

Troy,

Could you please check the Packaging guidelines and especially Ruby packaging guidelines and update yours .spec file according to them. I will just quickly point out a few keywords of interest: defattr, BuildRoot, macros from rubygems-devel.

Vit,
Thank you for the pointers.  It took a lot of years to learn it the one way, might take some time to learn it the other.  I went through my spec file with both the normal updated Fedora packaging guide, and the updated ruby packaging guide.
I think I got everything, but it's possible I missed a thing or two.

I have updated my spec and src.rpm
http://www.yortnoswad.org/packages/f17/rubygem-sshkey.spec
http://www.yortnoswad.org/packages/f17/rubygem-sshkey-1.3.0-3.fc17.src.rpm

[me@vm-64f17]$ rpmlint rubygem-sshkey.spec /home/quake/rpmbuild/SRPMS/rubygem-sshkey-1.3.0-3.fc17.src.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-1.3.0-3.fc17.noarch.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-doc-1.3.0-3.fc17.noarch.rpm
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.0/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 1 specfiles checked; 0 errors, 2 warnings.

Troy,

Further review of this package is blocked on providing some package reviews of other people's package review requests.  See Comment #2.

Troy, while waiting on an official review of your package I can give you an unofficial one. I didn't find much to say though:

You need to exclude the cached gem in the files section:
%exclude %{gemdir}/cache/%{gemname}-%{version}.gem

It looks like the doc warnings from rpmlint are ok.

I have done two informal reviews.  
One on rubygem-slim and rubygem-stickshift-common.

https://bugzilla.redhat.com/show_bug.cgi?id=839653
https://bugzilla.redhat.com/show_bug.cgi?id=839064

I can and will do more.  But any comment on my reviews would be nice.

Thanks for linking the reviews.  In the past we followed a template like:
https://bugzilla.redhat.com/show_bug.cgi?id=706565

Your reviews look like a good first start, but since the Fedora community has automated some of the review process, I'd recommend taking a look at the tool:

https://fedorahosted.org/FedoraReview/

This will help you cover all the bases when doing a review for a new package request.

Regards
-steve

(In reply to comment #5)
* You can use %{gem_spec}, %{gem_cache} and %{gem_docdir} macros instead of paths such as %{gem_dir}/cache/%{gem_name}-%{version}.gem, %{gem_dir}/specifications/%{gem_name}-%{version}.gemspec and %{gem_dir}/doc/%{gem_name}-%{version}

* One showstopper is that you do not own the gem directory, you must place "%dir %{gem_instdir}" on the top of your %files section.

Should probably make summary and description match (use %{name} in both places instead of once using ${gem_name}).

${gem_dir} does not need a leading '/'

%{gem_libdir} = %{gem_instdir}/lib

Previous comment about cache still applies, but a better way:
%exclude ${gem_cache}

%{gem_spec} = %{gem_dir}/specifications/%{gem_name}-%{version}.gemspec

%{gem_docdir} = %{gem_dir}/doc/%{gem_name}-%{version}

As Vit pointed out, files section should start with "%dir %{gem_instdir}"

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[X]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST 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.
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package installs properly.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[X]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[X]: SHOULD Latest version is packaged.
[X]: SHOULD Package does not include license text files separate from
     upstream.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0 (sshkey-1.3.0.gem)
[x]: SHOULD SourceX is a working URL.
[x]: SHOULD Spec use %global instead of %define.


==== Language ====
[x]: MUST Gem package must not define a non-gem subpackage
[!]: MUST Gem package must exclude cached Gem.
[X]: MUST Platform dependent files must all go under %{gem_extdir}, platform
     independent under %{gem_dir}.
[x]: MUST Gem package is named rubygem-%{gem_name}
[x]: MUST Package contains BuildRequires: rubygems-devel.
[x]: MUST Gem package must define %{gem_name} macro.
[x]: MUST Pure Ruby package must be built as noarch
[x]: MUST Package contains Requires: ruby(abi).
[!]: SHOULD Specfile should utilize macros from rubygem-devel package.
     Note: The specfile doesn't use these macros: %exclude %{gem_cache},
     %{gem_libdir}, %{gem_spec}, %doc %{gem_docdir}
[x]: SHOULD Test suite should not be run by rake.
[x]: SHOULD Test suite of the library should be run.

Issues:
[!]: MUST Gem package must exclude cached Gem.
See: http://fedoraproject.org/wiki/Packaging:Ruby

Rpmlint
-------
Checking: rubygem-sshkey-doc-1.3.0-3.fc17.noarch.rpm
          rubygem-sshkey-1.3.0-3.fc17.noarch.rpm
          rubygem-sshkey-1.3.0-3.fc17.src.rpm
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.0/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 0 specfiles checked; 0 errors, 2 warnings.


Rpmlint (installed packages)
----------------------------
# rpmlint rubygem-sshkey
# echo 'rpmlint-done:'

Requires
--------
rubygem-sshkey-doc-1.3.0-3.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    rubygem-sshkey = 1.3.0-3.fc17

rubygem-sshkey-1.3.0-3.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    ruby(abi) = 1.9.1
    rubygems  

Provides
--------
rubygem-sshkey-doc-1.3.0-3.fc17.noarch.rpm:
    
    rubygem-sshkey-doc = 1.3.0-3.fc17

rubygem-sshkey-1.3.0-3.fc17.noarch.rpm:
    
    rubygem(sshkey) = 1.3.0
    rubygem-sshkey = 1.3.0-3.fc17

MD5-sum check
-------------
http://rubygems.org/downloads/sshkey-1.3.0.gem :
  MD5SUM this package     : 40e9b0859fb9a1fab3f9b6b8d418d788
  MD5SUM upstream package : 40e9b0859fb9a1fab3f9b6b8d418d788


Generated by fedora-review 0.2.0 (53cc903) last change: 2012-07-09

(In reply to comment #11)
> Should probably make summary and description match (use %{name} in both
> places instead of once using ${gem_name}).

If you are referring to %doc description, then usage of %{name} is quite common pattern for Ruby packages.

So, I think I have my specfile fixed up with all of your (both Jeff and Vit's) recommendations.  Those macro's do make it look much better.

But I am not able to figure out this one recommendation

${gem_dir} does not need a leading '/'

Where am I doing that?

(In reply to comment #13)
> But I am not able to figure out this one recommendation
> 
> ${gem_dir} does not need a leading '/'
> 
> Where am I doing that?

You are using ./%{gem_dir} on several places, while .%{gem_dir} should be enough.

OK, I see that now.
Those sections I took straight out of the ruby packaging guidelines.
http://fedoraproject.org/wiki/Packaging/Ruby#Building_gems

Maybe we should update that section so it is more consistant.  So they are either all one way or another. Not part doing .%{gem_dir} and part doing ./%{gem_dir}

But, either way, I'll update my spec.

(In reply to comment #15)
> Those sections I took straight out of the ruby packaging guidelines.
> http://fedoraproject.org/wiki/Packaging/Ruby#Building_gems
> 
> Maybe we should update that section so it is more consistant.  So they are
> either all one way or another. Not part doing .%{gem_dir} and part doing
> ./%{gem_dir}

Ups, good catch. I'll discuss it with FPC. Sorry for confusion and thank you for pointing it out :)

I have updated my spec and src.rpm file with the changes suggested.

Spec URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey.spec
SRPM URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey-1.3.0-4.fc17.src.rpm

Rpmlint output:
[me@vm-64f17]$ rpmlint rubygem-sshkey.spec /home/quake/rpmbuild/SRPMS/rubygem-sshkey-1.3.0-4.fc17.src.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-1.3.0-4.fc17.noarch.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-doc-1.3.0-4.fc17.noarch.rpm
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 1 specfiles checked; 0 errors, 1 warnings.

[me@vm-64f17]$ rpmlint rubygem-sshkey rubygem-sshkey-doc
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
2 packages and 0 specfiles checked; 0 errors, 1 warnings.

Just FYI: https://fedorahosted.org/fpc/ticket/197

Running fedora-review on my own packages I found one last bug (hopefull).  I didn't have %doc on my %{gem_docdir}.  Updated spec and package at

Spec URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey.spec
SRPM URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey-1.3.0-5.fc17.src.rpm

I have updated my informal reviews including findings from fedora-review ... or actually failures from fedora-review.

https://bugzilla.redhat.com/show_bug.cgi?id=839653
https://bugzilla.redhat.com/show_bug.cgi?id=839064

The only thing I see left is to escape the %doc macro in your latest changelog entry.

Actually I did find one more thing when going through this, 1.3.1 is the current release. Should probably bump the version.

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[X]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[-]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[X]: MUST Package contains no bundled libraries.
[X]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[X]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[-]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package contains desktop file if it is a GUI application.
[-]: MUST Development files must be in a -devel package
[X]: MUST Package requires other packages for directories it uses.
[X]: MUST Package uses nothing in %doc for runtime.
[X]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[X]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[-]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST 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.
[X]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "*No copyright* UNKNOWN" For detailed output of licensecheck see file:
     /home/jpeeler/reviews/831749-rubygem-sshkey/licensecheck.txt
[X]: MUST License file installed when any subpackage combination is installed.
[X]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[X]: MUST Package is named according to the Package Naming Guidelines.
[X]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[X]: MUST Package obeys FHS, except libexecdir and /usr/target.
[X]: MUST Package must own all directories that it creates.
[X]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[X]: MUST Package is not relocatable.
[X]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[X]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[-]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[X]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: SHOULD Package functions as described.
[!]: SHOULD Latest version is packaged. NOTE: 1.3.1 released
[X]: SHOULD Package does not include license text files separate from
     upstream.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0 (sshkey-1.3.0.gem)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[X]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[X]: SHOULD %check is present and all tests pass.
[?]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.


==== Language ====
[x]: MUST Gem package must not define a non-gem subpackage
[x]: MUST Gem package must exclude cached Gem.
[X]: MUST Platform dependent files must all go under %{gem_extdir}, platform
     independent under %{gem_dir}.
[x]: MUST Gem package is named rubygem-%{gem_name}
[x]: MUST Package contains BuildRequires: rubygems-devel.
[x]: MUST Gem package must define %{gem_name} macro.
[x]: MUST Pure Ruby package must be built as noarch
[x]: MUST Package contains Requires: ruby(abi).
[x]: SHOULD Specfile should utilize macros from rubygem-devel package.
[x]: SHOULD Test suite should not be run by rake.
[x]: SHOULD Test suite of the library should be run.

Rpmlint
-------
Checking: rubygem-sshkey-1.3.0-5.fc17.noarch.rpm
          rubygem-sshkey-1.3.0-5.fc17.src.rpm
          rubygem-sshkey-doc-1.3.0-5.fc17.noarch.rpm
rubygem-sshkey.src:84: W: macro-in-%changelog %doc
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.0/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 0 specfiles checked; 0 errors, 3 warnings.


Rpmlint (installed packages)
----------------------------
# rpmlint rubygem-sshkey-doc
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.0/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.0/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
1 packages and 0 specfiles checked; 0 errors, 2 warnings.
# echo 'rpmlint-done:'

Requires
--------
rubygem-sshkey-1.3.0-5.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    ruby(abi) = 1.9.1
    rubygems  

rubygem-sshkey-doc-1.3.0-5.fc17.noarch.rpm (rpmlib, GLIBC filtered):
    
    rubygem-sshkey = 1.3.0-5.fc17

Provides
--------
rubygem-sshkey-1.3.0-5.fc17.noarch.rpm:
    
    rubygem(sshkey) = 1.3.0
    rubygem-sshkey = 1.3.0-5.fc17

rubygem-sshkey-doc-1.3.0-5.fc17.noarch.rpm:
    
    rubygem-sshkey-doc = 1.3.0-5.fc17

MD5-sum check
-------------
http://rubygems.org/downloads/sshkey-1.3.0.gem :
  MD5SUM this package     : 40e9b0859fb9a1fab3f9b6b8d418d788
  MD5SUM upstream package : 40e9b0859fb9a1fab3f9b6b8d418d788


Generated by fedora-review 0.2.0 (53cc903) last change: 2012-07-09
Command line :/usr/bin/fedora-review -b 831749
External plugins:

Version bumped.  I believe everything else is in place.

Spec URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey.spec
SRPM URL: http://www.yortnoswad.org/packages/f17/rubygem-sshkey-1.3.1-1.fc17.src.rpm

rpmlint output:
[me@vm-64f17]$ rpmlint rubygem-sshkey.spec /home/quake/rpmbuild/SRPMS/rubygem-sshkey-1.3.1-1.fc17.src.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-1.3.1-1.fc17.noarch.rpm /home/quake/rpmbuild/RPMS/noarch/rubygem-sshkey-doc-1.3.1-1.fc17.noarch.rpm
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.1/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.1/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 1 specfiles checked; 0 errors, 2 warnings.

I have done two more informal reviews.

https://bugzilla.redhat.com/show_bug.cgi?id=829038
https://bugzilla.redhat.com/show_bug.cgi?id=829030

I believe I am getting better at them.

Troy,

Your reviews look great, although missing rpmlint output and it looks like they were not run through mock.  Still, I think you have a handle on reviewing.  I will review your package in the next few days.  Once the review is finished you will be added to packagers group and can submit an SCM request.

rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.1/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f

Using ? in an API name doesn't seem to make much sense (the decoded %3f).  Is there some rationale for upstream using this in the documentation name?  I'd generally say this is not a valid character for a filename.

Please work with upstream to fix this problem.  If this is a common issue with ruby libraries, please file an rpmlint bug in bugzilla with the idea that % encoding in RI doc files should be ignored.

rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.1/ri/cache.ri

Something about this file is wrong - it may be that the file is encoded in DOS format when it should be encoded in non-dos format.  See:
https://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-encoding

Please work with upstream to resolve these issues - they will annoy all distributions, not just Fedora.

Still these issues are not blockers, since the ? is escaped in the filename and passes UTF-8 encoding.  And the other issue really looks like an upstream issue.  If this is not an upstream issue, please file an rpmlint bug if this is standard operation in Ruby applications so that rpmlint can be improved.

Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: No rpmlint messages.
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package contains desktop file if it is a GUI application.
[-]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[-]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST 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.

[x]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "*No copyright* UNKNOWN" For detailed output of licensecheck see file:
     /root/831749-rubygem-sshkey/licensecheck.txt
[!]: MUST License file installed when any subpackage combination is installed.
the doc package does not contain the license contents

[x]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[!]: MUST Package must own all directories that it creates.

The following directories are unowned in rubygem-sshkey package:
/usr/share/gems/gems/sshkey-1.3.1/lib/sshkey
/usr/share/gems/specifications

[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[-]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[-]: SHOULD Package functions as described.
[x]: SHOULD Latest version is packaged.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0 (sshkey-1.3.1.gem)
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[-]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[x]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.

==== Language ====
[x]: MUST Gem package must not define a non-gem subpackage
[x]: MUST Gem package must exclude cached Gem.
[x]: MUST Platform dependent files must all go under %{gem_extdir}, platform
     independent under %{gem_dir}.
[x]: MUST Gem package is named rubygem-%{gem_name}
[x]: MUST Package contains BuildRequires: rubygems-devel.
[x]: MUST Gem package must define %{gem_name} macro.
[x]: MUST Pure Ruby package must be built as noarch
[x]: MUST Package contains Requires: ruby(abi).
[x]: SHOULD Specfile should utilize macros from rubygem-devel package.
[x]: SHOULD Test suite should not be run by rake.
[x]: SHOULD Test suite of the library should be run.

Rpmlint
-------
Checking: rubygem-sshkey-1.3.1-1.fc17.src.rpm
          rubygem-sshkey-doc-1.3.1-1.fc17.noarch.rpm
          rubygem-sshkey-1.3.1-1.fc17.noarch.rpm
rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding /usr/share/gems/doc/sshkey-1.3.1/ri/cache.ri
rubygem-sshkey-doc.noarch: W: unexpanded-macro /usr/share/gems/doc/sshkey-1.3.1/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
3 packages and 0 specfiles checked; 0 errors, 2 warnings.

Rpmlint (installed packages)
----------------------------
# rpmlint rubygem-sshkey
rubygem-sshkey.noarch: I: enchant-dictionary-not-found en_US
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'

Requires
--------
rubygem-sshkey-doc-1.3.1-1.fc17.noarch.rpm (rpmlib, GLIBC filtered):

    rubygem-sshkey = 1.3.1-1.fc17

rubygem-sshkey-1.3.1-1.fc17.noarch.rpm (rpmlib, GLIBC filtered):

    ruby(abi) = 1.9.1
    rubygems

Provides
--------
rubygem-sshkey-doc-1.3.1-1.fc17.noarch.rpm:

    rubygem-sshkey-doc = 1.3.1-1.fc17

rubygem-sshkey-1.3.1-1.fc17.noarch.rpm:

    rubygem(sshkey) = 1.3.1
    rubygem-sshkey = 1.3.1-1.fc17

MD5-sum check
-------------
http://rubygems.org/downloads/sshkey-1.3.1.gem :
  MD5SUM this package     : 7270d0fbb1297eb2379c34ec5d68afbd
  MD5SUM upstream package : 7270d0fbb1297eb2379c34ec5d68afbd


Generated by fedora-review 0.2.0 (53cc903) last change: 2012-07-09
Command line :/usr/bin/fedora-review -b 831749
External plugins:

blocking issues - please correct at your earliest opportunity:
[!]: MUST License file installed when any subpackage combination is installed.
the doc package does not contain the license contents

[!]: MUST Package must own all directories that it creates.

The following directories are unowned in rubygem-sshkey package:
/usr/share/gems/gems/sshkey-1.3.1/lib/sshkey
/usr/share/gems/specifications

blocking issues
- License in subpackages - fixed, will be in next package release

- own all directorys
-- I'm not understanding why you say these directories are unowned.
# rpm -qf /usr/share/gems/gems/sshkey-1.3.1/lib/sshkey
rubygem-sshkey-1.3.1-1.fc18.noarch
# rpm -qf /usr/share/gems/specifications
rubygems-1.8.24-1.fc18.noarch
(rubygems is a requirement for rubygem-sshkey)
-- am I missing something here?

rpmlint issues:
- seeing what I can do.

Sorry the unowned dirs report was an error on my part.  Please ignore.  The other issues remain.

Hi,

I have step in :)

(In reply to comment #26)
> rubygem-sshkey-doc.noarch: W: unexpanded-macro
> /usr/share/gems/doc/sshkey-1.3.1/ri/SSHKey/valid_ssh_public_key%3f-c.ri %3f
> 
> Using ? in an API name doesn't seem to make much sense (the decoded %3f). 
> Is there some rationale for upstream using this in the documentation name? 
> I'd generally say this is not a valid character for a filename.
> 
> Please work with upstream to fix this problem.  If this is a common issue
> with ruby libraries, please file an rpmlint bug in bugzilla with the idea
> that % encoding in RI doc files should be ignored.

This is common issue and I don't think upstream is going to change it, since they would need to change whole concept and it would break backward compatibility.

> rubygem-sshkey-doc.noarch: W: wrong-file-end-of-line-encoding
> /usr/share/gems/doc/sshkey-1.3.1/ri/cache.ri
> 
> Something about this file is wrong - it may be that the file is encoded in
> DOS format when it should be encoded in non-dos format.  See:
> https://fedoraproject.org/wiki/Common_Rpmlint_issues#wrong-file-end-of-line-
> encoding
> 
> Please work with upstream to resolve these issues - they will annoy all
> distributions, not just Fedora.

The RI files are actually binary files, created using Ruby's Marshal.dump. So this is false positive. 

> Still these issues are not blockers, since the ? is escaped in the filename
> and passes UTF-8 encoding.  And the other issue really looks like an
> upstream issue.  If this is not an upstream issue, please file an rpmlint
> bug if this is standard operation in Ruby applications so that rpmlint can
> be improved.

Both checks should be fixed in rpmlint and they are on my TODO list.

(In reply to comment #28)
> blocking issues - please correct at your earliest opportunity:
> [!]: MUST License file installed when any subpackage combination is
> installed.
> the doc package does not contain the license contents

Actually this is not true and Troy has it right. Please see http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Subpackage_Licensing

Spec URL: http://tdawson.fedorapeople.org/rubygem-sshkey/rubygem-sshkey.spec
SRPM URL: http://tdawson.fedorapeople.org/rubygem-sshkey/rubygem-sshkey-1.3.1-2.fc18.src.rpm

- License is in both packages
-- Regular %doc area for rubygem-sshkey
-- in %{gem_instdir} for rubygem-sshkey-doc
--- I figured that rpm contained all the other documentation that was in that directory, so putting the License in there was sort of natural.  Even if it wasn't totaly necessary.
--- Thank you Vic for pointing out the subpackage section on Licenses.

- rpmlint issues
-- rpmlint is at fault and will be updated at some point.

Vit,

Thanks for notes about rubygem docs.

PACKAGE APPROVED FOR FEDORA.

Troy,

Congratulations - welcome to the packager group!  The next step is to create an SCM request.  See:
http://fedoraproject.org/wiki/Package_SCM_admin_requests

Regards
-steve

New Package SCM Request
=======================
Package Name: rubygem-sshkey
Short Description: Generate ssh key-pairs using ruby
Owners: tdawson
Branches: f17 f18
InitialCC: tdawson

Git done (by process-git-requests).

f18 not yet branched.

rubygem-sshkey-1.3.1-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/rubygem-sshkey-1.3.1-2.fc17

rubygem-sshkey-1.3.1-2.fc17 has been pushed to the Fedora 17 testing repository.

rubygem-sshkey-1.3.1-2.fc17 has been pushed to the Fedora 17 stable repository.