Bug 975339 - Review Request: rubygem-gssapi - A FFI wrapper around the system GSSAPI library
Summary: Review Request: rubygem-gssapi - A FFI wrapper around the system GSSAPI library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Vít Ondruch
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 981119
TreeView+ depends on / blocked
 
Reported: 2013-06-18 07:20 UTC by Jan Pazdziora
Modified: 2013-07-04 05:59 UTC (History)
4 users (show)

Fixed In Version: rubygem-gssapi-1.1.2-1.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
: 981119 (view as bug list)
Environment:
Last Closed: 2013-07-03 01:32:11 UTC
Type: ---
Embargoed:
vondruch: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jan Pazdziora 2013-06-18 07:20:22 UTC
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 08:53:49 UTC
I'll take this for a review.

Comment 2 Vít Ondruch 2013-06-19 10:12:06 UTC
* 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 12:07:10 UTC
(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 11:52:57 UTC
(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 12:07:43 UTC
(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 12:11:33 UTC
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 12:13:18 UTC
(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 12:13:48 UTC
IOW -1 will be just fine.

Comment 9 Gwyn Ciesla 2013-06-21 12:52:01 UTC
Git done (by process-git-requests).

Comment 10 Fedora Update System 2013-06-24 09:46:23 UTC
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 09:48:14 UTC
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 18:49:29 UTC
rubygem-gssapi-1.1.2-1.fc19 has been pushed to the Fedora 19 testing repository.

Comment 13 Vít Ondruch 2013-06-28 15:24:39 UTC
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 15:30:40 UTC
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 09:27:57 UTC
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 09:37:57 UTC
(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 09:59:11 UTC
Just for a referrence: https://lists.fedoraproject.org/pipermail/devel/2013-July/184690.html

Comment 18 Fedora Update System 2013-07-03 01:32:11 UTC
rubygem-gssapi-1.1.2-1.fc19 has been pushed to the Fedora 19 stable repository.

Comment 19 Fedora Update System 2013-07-03 20:53:12 UTC
rubygem-gssapi-1.1.2-1.el6 has been pushed to the Fedora EPEL 6 stable repository.

Comment 20 Jan Pazdziora 2013-07-04 05:59:57 UTC
(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.