Bug 286691

Summary: Review Request: libgssglue - renaming libgssapi to libgssglue
Product: [Fedora] Fedora Reporter: Steve Dickson <steved>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, mtasaka, notting
Target Milestone: ---Flags: steved: fedora_requires_release_note?
panemade: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-10-20 11:57:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Steve Dickson 2007-09-11 19:02:40 UTC
Spec URL: http://people.redhat.com/steved/libgssglue/libgssglue.spec
SRPM URL: http://people.redhat.com/steved/libgssglue/libgssglue-0.1-1.fc8.src.rpm
Description: Rename library from libgssapi to libgssglue to resolve 
conflicts with Heimdal and MIT libraries named libgssapi.

Comment 1 Parag AN(पराग) 2007-09-12 05:03:21 UTC
Preliminary review:-
1) Change Buildroot tag mentioned in
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473

2) Chane License tag

3) Check rpmlint output on all rpms.

4) drop prefix in configure command.

5) should not use makeinstall macro
http://fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002





Comment 2 Parag AN(पराग) 2007-09-12 05:06:59 UTC
Also,
  you can add license file to %doc. Remove .a and .la files. Check
http://fedoraproject.org/wiki/Packaging/Guidelines#head-2302ec1e1f44202c9cc4bcce24cb711266557ad7

Comment 3 Mamoru TASAKA 2007-09-12 06:29:32 UTC
Also:

* Remove static archives or move them to new subpackage
* Don't use PreReq
* Requires(postun/pre) /sbin/ldconfig can be removed.
* Support parallel make when possible, otherwise comment in the
  spec file that this package cannot support parallel make.
* Add "-p" option to "install" commant to keep timestamps
* Use macros. /etc should be %_sysconfdir, for example.

Comment 4 Steve Dickson 2007-09-17 14:21:52 UTC
> 1) Change Buildroot tag mentioned in
Why? How have it requires the 'MUST' of the
requirements? Plus this format is used in 
a number of other related so I would like to
keep the all the same.

> 2) Chane License tag
Done.

> 3) Check rpmlint output on all rpms.
Done.

> 4) drop prefix in configure command.
Done.

> 5) should not use makeinstall macro.
Done.

> Remove static archives or move them to new subpackage
Done.

> * Don't use PreReq
Done.

> * Requires(postun/pre) /sbin/ldconfig can be removed.
Done.

> * Add "-p" option to "install" commant to keep timestamps
Done.

> * Use macros. /etc should be %_sysconfdir, for example.
Done.

> * Support parallel make when possible, otherwise comment in the
>   spec file that this package cannot support parallel make.
Not clear as to how this is achieved.

rpm and spec file on my people page have been updated.
 

Comment 5 Parag AN(पराग) 2007-09-18 10:28:11 UTC
(In reply to comment #4)
> > 1) Change Buildroot tag mentioned in
> Why? How have it requires the 'MUST' of the
> requirements? Plus this format is used in 
> a number of other related so I would like to
> keep the all the same.
> 
  I will consider this as a BLOCKER as packaging guidelines clearly said 
MUST use %{name}, %{version} and %{release} in buildroot.

Comment 6 Steve Dickson 2007-09-18 11:03:06 UTC
> I will consider this as a BLOCKER as packaging guidelines clearly said 
> MUST use %{name}, %{version} and %{release} in buildroot.
Ok... Here is the diff.

--- /tmp/libgssglue.spec.orig   2007-09-17 10:21:19.000000000 -0400
+++ libgssglue.spec     2007-09-18 06:53:43.000000000 -0400
@@ -6,7 +6,7 @@ URL: http://www.citi.umich.edu/projects/
 License: GPL+
 Source0:http://www.citi.umich.edu/projects/nfsv4/linux/libgssglue/libgssglue-0.1.tar.gz
 Group: System Environment/Libraries
-BuildRoot: %{_tmppath}/%{name}-%{version}-root
+BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
 BuildRequires: pkgconfig

The srpm and spec file on my people have been updated.


Comment 7 Parag AN(पराग) 2007-09-18 12:03:01 UTC
Few more suggestion I got 
1) change %defattr(-,root,root) to %defattr(-.root.root,-)
Though nothing specific written in guidelines but SPEC given under section 
http://fedoraproject.org/wiki/Packaging/Guidelines#head-8c605ebf8330f6d505f384e671986fa99a8f72ee
said like that.

2) Usage of parallel make
http://fedoraproject.org/wiki/Packaging/Guidelines#head-525c7d76890cb22df33b759c65c35c82bf434d2e

3) rpmlint on binary rpm gave me
libgssglue.i386: E: library-without-ldconfig-postin /usr/lib/libgssglue.so.1.0.0
This package contains a library and provides no %post scriptlet containing
a call to ldconfig.

libgssglue.i386: E: library-without-ldconfig-postun /usr/lib/libgssglue.so.1.0.0
This package contains a library and provides no %postun scriptlet containing
a call to ldconfig.

check
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?action=show&redirect=ScriptletSnippets#head-d0dbcb7eec27622a21df280009c5b089b02f5bef

4) Mix of 3 licenses summarizes to GPL+ which I take it as correct usage for
License tag.


Comment 8 Parag AN(पराग) 2007-09-18 12:11:54 UTC
last thing I forgot to mention-> remove 
BuildRequires: pkgconfig
mock build went successful without it.

Comment 9 Steve Dickson 2007-09-18 21:31:20 UTC
> 1) change %defattr(-,root,root) to %defattr(-.root.root,-)
This '%defattr(-.root.root,-)' caused 'Bad syntax: %defattr(-.root.root)'
so I changed it to '%defattr(-,root,root,-)'

> 2) Usage of parallel make
Done.

> 3) rpmlint on binary rpm gave me libgssglue.i386: E: 
>    library-without-ldconfig-postin /usr/lib/libgssglue.so.1.0.0
Fixed.

> remove BuildRequires: pkgconfig
Done.

Spec file and rpm have been updated.


Comment 10 Mamoru TASAKA 2007-09-19 03:47:51 UTC
Parag, I will leave this review to how you judge.

Comment 11 Parag AN(पराग) 2007-09-19 04:11:45 UTC
Also,
SHOULD:
Change 
http://www.citi.umich.edu/projects/nfsv4/linux/libgssglue/libgssglue-0.1.tar.gz
to
http://www.citi.umich.edu/projects/nfsv4/linux/%{name}/%{name}-%{version}.tar.gz

Then,
  I tried to install built rpm on F7 machine and I got
rpm -Uvh libgssglue-0.1-2.fc8.i386.rpm 
Preparing...                ########################################### [100%]
        file /etc/gssapi_mech.conf from install of libgssglue-0.1-2.fc8
conflicts with file from package libgssapi-0.11-1.fc7


Comment 12 Parag AN(पराग) 2007-09-19 04:13:04 UTC
(In reply to comment #10)
> Parag, I will leave this review to how you judge.
  Did I miss something in this review? or you want to officially review this?
  

Comment 13 Mamoru TASAKA 2007-09-19 04:19:56 UTC
(In reply to comment #12)
> (In reply to comment #10)
> > Parag, I will leave this review to how you judge.
>   Did I miss something in this review? or you want to officially review this?

I just wanted to say that when you can think this review can
be approved I will agree with your judgment.

Comment 14 Parag Nemade 2007-09-19 04:38:59 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #10)
> > > Parag, I will leave this review to how you judge.
> >   Did I miss something in this review? or you want to officially review this?
> 
> I just wanted to say that when you can think this review can
> be approved I will agree with your judgment.

cool. thanks. Sorry I didn't get what you said in your last comment and thought
I took over your review.

Comment 15 Steve Dickson 2007-09-19 13:45:41 UTC
> file /etc/gssapi_mech.conf from install of libgssglue-0.1-2.fc8
> conflicts with file from package libgssapi-0.11-1.fc7
I have addressed this issue with upstream, and they
recommeded what we stick with the gssapi_mech.conf file
name. 

Is this review done?



Comment 16 Parag AN(पराग) 2007-09-19 16:01:52 UTC
(In reply to comment #15)
> > file /etc/gssapi_mech.conf from install of libgssglue-0.1-2.fc8
> > conflicts with file from package libgssapi-0.11-1.fc7
> I have addressed this issue with upstream, and they
> recommeded what we stick with the gssapi_mech.conf file
> name. 
> 
 Can you explain a bit more here. If this package got approved then how this
package can be used on fedora? conflicts are coming from its installation.
people cannot install this package.
  you should look at
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-3cfc1ea19d28975faad9d56f70a6ae55661d3c3d


> Is this review done?
 Not yet.
  Source URL Change needed.

Comment 17 Steve Dickson 2007-09-25 14:38:32 UTC
> Can you explain a bit more here.
This is a rename of package so there is bound to
be some conflictions, which I don't believe are
show stoppers. Now I could remove those conflictions
but that would be breaking away from upstream which
is something I do not want to do.

> Also,
> SHOULD:
> Change 
>http://www.citi.umich.edu/projects/nfsv4/linux/libgssglue/libgssglue-0.1.tar.gz
> to
>http://www.citi.umich.edu/projects/nfsv4/linux/%{name}/%{name}-%{version}.tar.gz
Key word "SHOULD"... URL do not HAVE to be in this format. I personally
find this format incredibly hard to read and basically useless.
But to get this 3week review process over with, I will do it... 

The spec file and srpm have been updated.

Comment 18 Parag AN(पराग) 2007-09-26 03:23:53 UTC
Remember this is new package review and not Merge review.
I would like to see package links and not to see wordings "The spec file and
srpm have been updated."
Provide me package links as you did in initial comment.
If you check a normal procedure of review is that for each change in SPEC during
review submitter has to update release tag and give direct download links to
SRPM in each update he provides.

Comment 19 Parag AN(पराग) 2007-09-26 07:18:27 UTC
thanks for your updates to SPEC.
But sorry I can't approve this package as I can still see package         
"file /etc/gssapi_mech.conf from install of libgssglue-0.1-2.fc8 conflicts with
file from package libgssapi-0.11-1.fc7"

What workaround I can suggest here is to add 
Provides: libgssapi = 0.11-1.fc7
Obsoletes: libgssapi <= 0.11-1.fc7
below following line in SPEC file
Requires: krb5-libs >= 1.5

When built with above changes and tried to install on F7 machine as it gave me
 rpm -Uvh libgssglue-0.1-2.fc7.i386.rpm libgssglue-devel-0.1-2.fc7.i386.rpm
error: Failed dependencies:
        libgssapi.so.2 is needed by (installed) libtirpc-0.1.7-7.fc7.i386
        libgssapi.so.2 is needed by (installed) nfs-utils-1.1.0-3.fc7.i386
        libgssapi.so.2(libgssapi_CITI_2) is needed by (installed)
libtirpc-0.1.7-7.fc7.i386
        libgssapi.so.2(libgssapi_CITI_2) is needed by (installed)
nfs-utils-1.1.0-3.fc7.i386

That mean you need to ask all packages that depends on libgssapi.so.2 to build
against libgssglue.



Comment 20 Steve Dickson 2007-09-26 13:39:52 UTC
> Provide me package links as you did in initial comment.
I just assumed since the spec file and srpm were in the
exact same place you could continue to used the original
links. Sorry for such a bad assumption. I'll guess I go head 
and cut and past them from the original posting.

> What workaround I can suggest here is to add 
> Provides: libgssapi = 0.11-1.fc7
> Obsoletes: libgssapi <= 0.11-1.fc7
> below following line in SPEC file
> Requires: krb5-libs >= 1.5
updated spec and srpm.
Spec URL: http://people.redhat.com/steved/libgssglue/libgssglue.spec
SRPM URL: http://people.redhat.com/steved/libgssglue/libgssglue-0.1-2.fc8.src.rpm

> rpm -Uvh libgssglue-0.1-2.fc7.i386.rpm libgssglue-devel-0.1-2.fc7.i386.rpm
> error: Failed dependencies:
Of course there will be dependency problem because I
have not been able to check this package in!
As soon as I can check this package in, those 
dependency problem will go away because I will 
be able to update the packages at are depending
it. Please note *all* the packages that are dependent
on this I maintain. 


Comment 21 Parag AN(पराग) 2007-09-26 14:06:06 UTC
* thanks very much that finally you got my point that links must be provided
each time to reviewer to avoid confusion between old and new SPEC.

* But that doesn't mean you should not increment release tag. And for above
change, do you think its so small to previous SPEC you provided?
No. You must update release tag and provide new links for final review.

*Remember to run rpmlint on all SRPM,RPM files.


Comment 22 Steve Dickson 2007-09-26 15:58:36 UTC
> * But that doesn't mean you should not increment release tag. And for above
> change, do you think its so small to previous SPEC you provided?
I did, I went from -1 to -2 and group all the comments under
the "RPM review" changelog which I think *very* appropriate. 
Why artificially increase version numbers for basically the 
same bug (or process in this case). Also, Isn't  up the 
the discretion of the maintainer to decide when and
why to increase a version number? I would surely hope so.

> No. You must update release tag and provide new links for final review.
No. I feel the way I'm managing this tag for my package is just
fine. Now if so choose not to let this package into fedora due to
silly issue like tag number (after working on this for 3 weeks)
so be it... 

> *Remember to run rpmlint on all SRPM,RPM files.
I did and I see no errors. 



Comment 23 Parag AN(पराग) 2007-09-27 03:40:21 UTC
Fine.
Though I am not happy with "not updating SPEC changelog and providing new SRPM
each time SPEC got some noticeable changes", as per your wish I am approving
this package here.
You should have added at least review bug number in changelog if you preferred
not to update release.

Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
ce1b4c758e6de01b712d154c5c97e540  libgssglue-0.1.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ libgssglue.pc file present.
+ -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ ldconfig scriptlets are used.
+ Package libgssglue-0.1-2.fc8 ->
  Provides: config(libgssglue) = 0.1-2.fc8 libgssglue.so.1
libgssglue.so.1(HIDDEN) libgssglue.so.1(libgssapi_CITI_2)
  Requires: config(libgssglue) = 0.1-2.fc8 krb5-libs >= 1.5 libc.so.6
libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.1.3)
libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libdl.so.2
libdl.so.2(GLIBC_2.0) libdl.so.2(GLIBC_2.1) libgssglue.so.1 rtld(GNU_HASH)
  Package libgssglue-devel-0.1-2.fc8 ->
  Requires: libgssglue = 0.1-2.fc8 libgssglue.so.1 pkgconfig
+ Not a GUI app.
APPROVED.



Comment 24 Mamoru TASAKA 2007-10-07 06:02:41 UTC
Steve, ping?

Comment 25 Steve Dickson 2007-10-09 18:35:41 UTC
Not sure what I need to do since I can not import the package

./common/cvs-import.sh  libgssglue-0.1-2.fc8.src.rpm
Checking out module: 'libgssglue'
cvs server: cannot find module `libgssglue' - ignored
cvs [checkout aborted]: cannot expand modules
ERROR: "libgssglue" module does not exist in cvs.


Comment 26 Mamoru TASAKA 2007-10-10 01:53:39 UTC
Please check http://fedoraproject.org/wiki/PackageMaintainers/NewPackageProcess .
Next you have to do "CVSAdminProcedure".

Comment 27 Steve Dickson 2007-10-12 19:46:57 UTC
New Package CVS Request
=======================
Package Name: libgssglue
Short Description: Renaming libgssapi to libgssglue
Owners: steved
Branches: devel
InitialCC: steved



Comment 28 Kevin Fenzi 2007-10-12 21:43:48 UTC
Please check your Provides/Obsoletes. See: 
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-3cfc1ea19d28975faad9d56f70a6ae55661d3c3d

in particular, I don't think .fc7 should be in there. 

cvs done. 
(note also that Short Description: should be the package short description, and
that Owners: should be your fedora account name). 

Comment 29 Steve Dickson 2007-10-15 14:53:14 UTC
When I check the package out and do a 'make i386'
It hangs after the following error messages

   rpmq: no arguments given for query
   rpmq: no arguments given for query

Any idea as to what he problem could be?

Comment 30 Kevin Fenzi 2007-10-15 15:06:44 UTC
You don't appear to have anything checked in yet? 
The Makefile looks for a *.spec file to see what it's making, and I don't see
one in cvs. 

You should be able to use the 'cvs-import.sh' script to just import a src.rpm, 
or manually 'cvs add' and 'cvs commit' a .spec, sources and .cvsignore files. 


Comment 31 Steve Dickson 2007-10-15 16:12:18 UTC
ah... I thought "cvs done." meant that you already did the import... 

Comment 32 Mamoru TASAKA 2007-10-20 11:57:29 UTC
Closing as this package is already in rawhide tree.