Spec URL: http://chkr.fedorapeople.org/review/perl-Authen-Krb5-Admin.spec SRPM URL: http://chkr.fedorapeople.org/review/perl-Authen-Krb5-Admin-0.11-1.fc10.src.rpm Description: Authen::Krb5::Admin is an object-oriented interface to the Kerberos 5 admin server. Currently only MIT KDCs are supported, but the author envisions seamless integration with other KDCs. rpmlint: RPMS/i386/perl-Authen-Krb5-Admin-* SRPMS/perl-Authen-Krb5-Admin-0.11-1.fc10.src.rpm SPECS/perl-Authen-Krb5-Admin.spec 3 packages and 1 specfiles checked; 0 errors, 0 warnings. koji: F11: https://koji.fedoraproject.org/koji/taskinfo?taskID=1235646 F10: https://koji.fedoraproject.org/koji/taskinfo?taskID=1235667 F9: https://koji.fedoraproject.org/koji/taskinfo?taskID=1235674
I don't see your name in the Fedora account system. Is this your first package?
Erm, duh, please ignore me. Your information is private and as such prevents searches.
Please note that the License: tag should indicate the license of the final, built package. You have: # admin.h - MIT # ppport.h - GPL+ or Artisic (same as any version of Perl) # everything else: BSD (two clause) License: MIT and BSD and ( GPL+ or Artistic ) yet admin.h and pport.h are not in the final package; they are merely compiled with the rest of the code. Honestly I don't know "GPL+ or Artistic" combines with both MIT and BSD code when all of it is compiled together. My guess is that GPL+ dominates, but I don't know what happens to Artistic. Blocking FE-Legal for an opinion.
Other than the license issue brought up by Jason, I will bring up some issues, questions and suggestions. (About the license issue, I think we should go with BSD, since everything that goes into the final binary RPM is BSD.) * In the build.log I see: checking for libk5crypto ... not found (using libcrypto) This is not found because we have this line in Makefile.PL my $KRB5_LIBDIR = "$PREFIX/lib" ; I think this line need to be patched or sed'ed to use the correct %{_lib} ! BR: krb5-devel is unnecessary. openssl-devel will pull that up. ? This package owns the same directories with the perl-Authen-Krb5 package. Is this intentional? Or should this package require perl-Authen-Krb5? I know that there is an exception rule for perl packages. I was wondering if this package makes use of that exception rule. ! Please make the description span the 80 columns. * Each package must consistently use macros. You should either use the "perl, make, chmod, ..." notation or "%{__perl}, %{__make}, %{__chmod}, ..." notation. A mixture is not desired.
Well, we're not using Artistic, only GPL. We only list Artistic for historical reasons, at this point. There aren't any compatibility issues. Either: License: GPL+ or License: MIT and BSD and (GPL+ or Artistic) would be correct here. Up to the packager. Lifting FE-Legal.
Well, the latter is what's already there, so I'll just go ahead. * source files match upstream. sha256sum: 273a9866a5b927315e98518ac2aeb3083effb58816815122923d3fac4ae5c29e Authen-Krb5-Admin-0.11.tar.gz * package meets naming and versioning guidelines. * specfile is properly named, is cleanly written and uses macros consistently. * summary is OK. * description is OK. * dist tag is present. * build root is OK. * license field matches the actual license. * license is open source-compatible. * license text included in package. * latest version is being packaged. * BuildRequires are proper. * compiler flags are appropriate. * %clean is present. * package builds in mock (rawhide, x86_64). * package installs properly. * debuginfo package looks complete. * rpmlint is silent. * final provides and requires are sane: Admin.so()(64bit) perl(Authen::Krb5::Admin) = 0.11 perl-Authen-Krb5-Admin = 0.11-1.fc11 perl-Authen-Krb5-Admin(x86-64) = 0.11-1.fc11 = libcom_err.so.2()(64bit) libcrypto.so.8()(64bit) libkadm5clnt.so.5()(64bit) libkadm5clnt.so.5(kadm5clnt_5_MIT)(64bit) libkrb5.so.3()(64bit) perl >= 0:5.004 perl(:MODULE_COMPAT_5.10.0) perl(AutoLoader) perl(Carp) perl(DynaLoader) perl(Exporter) perl(strict) perl(vars) * %check is necessarily disabled. * no shared libraries are added to the regular linker search paths. * owns the directories it creates. * doesn't own any directories it shouldn't. * no duplicates in %files. * file permissions are appropriate. * no generically named files * code, not content. * documentation is small, so no -doc subpackage is necessary. * %docs are not necessary for the proper functioning of the package. * no headers. * no pkgconfig files. * no static libraries. * no libtool .la files. APPROVED The package review process needs reviewers! If you haven't done any package reviews recently, please consider doing one.
Jason, Is it really okay to ignore the build.log warning (the multilib error) and the macro inconsistency that I indicated above (along with other things)?
(In reply to comment #7) > Is it really okay to ignore the build.log warning (the multilib error) and the > macro inconsistency that I indicated above (along with other things)? I'll take care of these items before committing the package to CVS. I'll update SRPM/Spec file later today and I would be glad if you could verify the fixes.
I don't see any macro inconsistency in the spec I'm looking at. I didn't see a build log error when I built this but perhaps I'm just missing it. There was no directory ownership issue that I saw; the rules for Perl module directory ownership are complex enough to get their own section in the guidelines. Was there something else you believe that I've missed?
New Package CVS Request ======================= Package Name: perl-Authen-Krb5-Admin Short Description: Perl extension for MIT Kerberos 5 admin interface Owners: chkr Branches: F-9 F-10
Thanks to all for the reviews and clarifications! (In reply to comment #4) > * In the build.log I see: > checking for libk5crypto ... not found (using libcrypto) > This is not found because we have this line in Makefile.PL > my $KRB5_LIBDIR = "$PREFIX/lib" ; > I think this line need to be patched or sed'ed to use the correct %{_lib} Fixed. Package builds now without the missing lib message on both 64bit archs: https://koji.fedoraproject.org/koji/taskinfo?taskID=1240524 > ! BR: krb5-devel is unnecessary. openssl-devel will pull that up. Removed. > ? This package owns the same directories with the perl-Authen-Krb5 package. Is > this intentional? Or should this package require perl-Authen-Krb5? I know that > there is an exception rule for perl packages. I was wondering if this package > makes use of that exception rule. Yes, I think that the mentioned exception from http://fedoraproject.org/wiki/Packaging/Perl#Directory_Ownership applies. > ! Please make the description span the 80 columns. Fixed. > * Each package must consistently use macros. You should either use the "perl, > make, chmod, ..." notation or "%{__perl}, %{__make}, %{__chmod}, ..." notation. > A mixture is not desired. Fixed. Regarding the license I've chosen the 2nd option offered by spot. The new packages are uploaded: Spec URL: http://chkr.fedorapeople.org/review/perl-Authen-Krb5-Admin.spec SRPM URL: http://chkr.fedorapeople.org/review/perl-Authen-Krb5-Admin-0.11-2.fc10.src.rpm
cvs done.
perl-Authen-Krb5-Admin-0.11-2.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/perl-Authen-Krb5-Admin-0.11-2.fc10
perl-Authen-Krb5-Admin-0.11-2.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/perl-Authen-Krb5-Admin-0.11-2.fc9
perl-Authen-Krb5-Admin-0.11-2.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
perl-Authen-Krb5-Admin-0.11-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.