Bug 975339 - Review Request: rubygem-gssapi - A FFI wrapper around the system GSSAPI library
Review Request: rubygem-gssapi - A FFI wrapper around the system GSSAPI library
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Vít Ondruch
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 981119
  Show dependency treegraph
 
Reported: 2013-06-18 03:20 EDT by Jan Pazdziora
Modified: 2013-07-04 01:59 EDT (History)
4 users (show)

See Also:
Fixed In Version: rubygem-gssapi-1.1.2-1.el6
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 981119 (view as bug list)
Environment:
Last Closed: 2013-07-02 21:32:11 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
vondruch: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Jan Pazdziora 2013-06-18 03:20:22 EDT
Spec URL: http://adelton.fedorapeople.org/rubygem-gssapi.spec
SRPM URL: http://kojipkgs.fedoraproject.org//work/tasks/1695/5511695/rubygem-gssapi-1.1.2-1.fc19.src.rpm
Description: A FFI wrapper around the system GSSAPI library. Please make sure and read
the Yard docs or standard GSSAPI documentation if you have any questions.
There is also a class called GSSAPI::Simple that wraps many of the common
features used for GSSAPI.
Fedora Account System Username: adelton
Comment 1 Vít Ondruch 2013-06-19 04:53:49 EDT
I'll take this for a review.
Comment 2 Vít Ondruch 2013-06-19 06:12:06 EDT
* Test suite
  - Is there any feasible way how to run a test suite of this package?

* License should be included in main package
  - Please move "%doc %{gem_instdir}/COPYING" into the main package [1].

* Do not mark everything in -doc subpackage by %doc macro
  - examples, test and Rakefile should not be marked as %doc, since they are
    not documentation IMO

* rpmlint
  - There used to be bug in gem2rpm which results in this warning:

    ./rubygem-gssapi.spec:48: W: macro-in-comment %gem_dir

    Please escape the macro
  - There is useless line commented out which produces following rpmlint
    warnings:

    ./rubygem-gssapi.spec:56: W: macro-in-comment %{buildroot}
    ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_instdir}
    ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_name}
    ./rubygem-gssapi.spec:56: W: macro-in-comment %{buildroot}
    ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_spec}

    Please remove this line.
  
Otherwise, the package look quite OK. Please fix the issues so I can approve it. Thanks.


[1] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text
Comment 3 Jan Pazdziora 2013-06-20 08:07:10 EDT
(In reply to Vít Ondruch from comment #2)
> * Test suite
>   - Is there any feasible way how to run a test suite of this package?

I don't think so. The setup needed would be pretty complex (you need Kerberos server and stuff).

> * License should be included in main package
>   - Please move "%doc %{gem_instdir}/COPYING" into the main package [1].

Fixed.

> * Do not mark everything in -doc subpackage by %doc macro
>   - examples, test and Rakefile should not be marked as %doc, since they are
>     not documentation IMO

Fixed.

> * rpmlint
>   - There used to be bug in gem2rpm which results in this warning:
> 
>     ./rubygem-gssapi.spec:48: W: macro-in-comment %gem_dir
> 
>     Please escape the macro
>   - There is useless line commented out which produces following rpmlint
>     warnings:
> 
>     ./rubygem-gssapi.spec:56: W: macro-in-comment %{buildroot}
>     ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_instdir}
>     ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_name}
>     ./rubygem-gssapi.spec:56: W: macro-in-comment %{buildroot}
>     ./rubygem-gssapi.spec:56: W: macro-in-comment %{gem_spec}
> 
>     Please remove this line.

Fixed.

Patch

--- rubygem-gssapi.spec.2013-06-18	2013-06-18 07:18:10.811000440 +0000
+++ rubygem-gssapi.spec	2013-06-20 12:03:04.267000530 +0000
@@ -45,7 +45,7 @@
 # Create the gem as gem install only works on a gem file
 gem build %{gem_name}.gemspec
 
-# %%gem_install compiles any C extensions and installs the gem into ./%gem_dir
+# %%gem_install compiles any C extensions and installs the gem into ./%%gem_dir
 # by default, so that we can move it into the buildroot in %%install
 %gem_install
 
@@ -53,7 +53,6 @@
 mkdir -p %{buildroot}%{gem_dir}
 cp -pa .%{gem_dir}/* \
         %{buildroot}%{gem_dir}/
-# mv %{buildroot}%{gem_instdir}/%{gem_name}.gemspec %{buildroot}%{gem_spec}
 rm -f %{buildroot}%{gem_instdir}/%{gem_name}.gemspec 
 
 
@@ -62,17 +61,17 @@
 %{gem_libdir}
 %exclude %{gem_cache}
 %{gem_spec}
+%doc %{gem_instdir}/COPYING
 
 %files doc
 %doc %{gem_docdir}
 %doc %{gem_instdir}/README.textile
-%doc %{gem_instdir}/COPYING
-%doc %{gem_instdir}/VERSION
-%doc %{gem_instdir}/examples
-%doc %{gem_instdir}/test
-%doc %{gem_instdir}/Rakefile
+%{gem_instdir}/VERSION
+%{gem_instdir}/examples
+%{gem_instdir}/test
+%{gem_instdir}/Rakefile
 %doc %{gem_instdir}/preamble
 
 %changelog
-* Mon Jun 17 2013 root - 1.1.2-1
+* Mon Jun 20 2013 Jan Pazdziora - 1.1.2-1
 - Initial package

applied to the .spec file. New .spec file is at

   http://adelton.fedorapeople.org/rubygem-gssapi.spec

and the new .src.rpm is at

   http://kojipkgs.fedoraproject.org//work/tasks/3696/5523696/rubygem-gssapi-1.1.2-1.fc19.src.rpm

> Otherwise, the package look quite OK. Please fix the issues so I can approve
> it. Thanks.

Thank you.
Comment 4 Vít Ondruch 2013-06-21 07:52:57 EDT
(In reply to Jan Pazdziora from comment #3)
> (In reply to Vít Ondruch from comment #2)
> > * Test suite
> >   - Is there any feasible way how to run a test suite of this package?
> 
> I don't think so. The setup needed would be pretty complex (you need
> Kerberos server and stuff).

OK, np ... but it would be nice if you could somehow document it in the .spec file, for future reference.

> applied to the .spec file. New .spec file is at

Thanks. I have two additional remarks:

* Wrong changelog format
  - According to [1], I am missing your email in changelog.

* Release bump
  - Although not mandatory, it is nice to bump release for each review cycle.
    It makes easier to check the differences between SRPM of each iteration.

Since these are just minor nits, I APPROVE the package. Nevertheless, please fix the changelog prior importing the package into Fedora (of course no point in bumping release now, but you can make me happier next time ;)


[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
Comment 5 Jan Pazdziora 2013-06-21 08:07:43 EDT
(In reply to Vít Ondruch from comment #4)
> 
> * Wrong changelog format
>   - According to [1], I am missing your email in changelog.

Ah, that sad email changelog rule. When will that get dropped?

> * Release bump
>   - Although not mandatory, it is nice to bump release for each review cycle.
>     It makes easier to check the differences between SRPM of each iteration.

Oh, sorry about that.

> Since these are just minor nits, I APPROVE the package. Nevertheless, please
> fix the changelog prior importing the package into Fedora (of course no
> point in bumping release now, but you can make me happier next time ;)

So do you want that changelog changed and retain -1 release or -2 bumped?
Comment 6 Jan Pazdziora 2013-06-21 08:11:33 EDT
New Package SCM Request
=======================
Package Name: rubygem-gssapi
Short Description: A FFI wrapper around the system GSSAPI library
Owners: adelton
Branches: f19 el6
InitialCC:
Comment 7 Vít Ondruch 2013-06-21 08:13:18 EDT
(In reply to Jan Pazdziora from comment #5)
> So do you want that changelog changed and retain -1 release or -2 bumped?

I hope you don't need a review for the changelog change, so no reason to bump the release.
Comment 8 Vít Ondruch 2013-06-21 08:13:48 EDT
IOW -1 will be just fine.
Comment 9 Gwyn Ciesla 2013-06-21 08:52:01 EDT
Git done (by process-git-requests).
Comment 10 Fedora Update System 2013-06-24 05:46:23 EDT
rubygem-gssapi-1.1.2-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/rubygem-gssapi-1.1.2-1.fc19
Comment 11 Fedora Update System 2013-06-24 05:48:14 EDT
rubygem-gssapi-1.1.2-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/rubygem-gssapi-1.1.2-1.el6
Comment 12 Fedora Update System 2013-06-24 14:49:29 EDT
rubygem-gssapi-1.1.2-1.fc19 has been pushed to the Fedora 19 testing repository.
Comment 13 Vít Ondruch 2013-06-28 11:24:39 EDT
Now I realized, that I did not noticed any reference to some gssapi library. Shouldn't be there added "Requires: /usr/lib64/libgssapi_krb5.so.2"? The gem probably cannot work properly without it.
Comment 14 Vít Ondruch 2013-06-28 11:30:40 EDT
Or may be libgssapi.so, but libgssapi seems to be deprecated, as of today: "Removed package:  libgssapi-0.11-12.fc19", that is how I realized the missing dependencies.
Comment 15 Jan Pazdziora 2013-07-01 05:27:57 EDT
The gem uses libgssapi_krb5.so.2.

The question then is -- is it OK to add

   Requires: krb5-libs

or do I need to add Requires for a particular .so/lib path, in which case the rubygem package would probably need to change from noarch to arch one?
Comment 16 Vít Ondruch 2013-07-01 05:37:57 EDT
(In reply to Jan Pazdziora from comment #15)
> The gem uses libgssapi_krb5.so.2.
> 
> The question then is -- is it OK to add
> 
>    Requires: krb5-libs

Yes, that is fine.

> or do I need to add Requires for a particular .so/lib path, in which case
> the rubygem package would probably need to change from noarch to arch one?

You are right. There is probably no better was the add dependency on library directly. But I'll ask on fedora-devel just out of curiosity.
Comment 17 Vít Ondruch 2013-07-01 05:59:11 EDT
Just for a referrence: https://lists.fedoraproject.org/pipermail/devel/2013-July/184690.html
Comment 18 Fedora Update System 2013-07-02 21:32:11 EDT
rubygem-gssapi-1.1.2-1.fc19 has been pushed to the Fedora 19 stable repository.
Comment 19 Fedora Update System 2013-07-03 16:53:12 EDT
rubygem-gssapi-1.1.2-1.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 20 Jan Pazdziora 2013-07-04 01:59:57 EDT
(In reply to Vít Ondruch from comment #16)
> (In reply to Jan Pazdziora from comment #15)
> > The gem uses libgssapi_krb5.so.2.
> > 
> > The question then is -- is it OK to add
> > 
> >    Requires: krb5-libs
> 
> Yes, that is fine.

Tracking this in bug 981119 now.

Note You need to log in before you can comment on or make changes to this bug.