Spec URL: http://adiemus.org/~kdreyer/rpms/pam-afs-session.spec SRPM URL: http://adiemus.org/~kdreyer/rpms/pam-afs-session-2.1-1.src.rpm Description: pam-afs-session is a PAM module intended for use with a Kerberos v5 PAM module to obtain an AFS PAG (Process Authentication Group) and AFS tokens on login. It puts every new session in a PAG regardless of whether it was authenticated with Kerberos and runs a configurable external program to obtain tokens. I would like to get this package into Fedora and EPEL 5. I'm a long-time Fedora user, and I'm looking to be a developer. I maintain a few RPMs at work, but this is my first package in Fedora, so I'm seeking a sponsor. $ rpmlint SPECS/pam-afs-session.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. $ rpmlint SRPMS/pam-afs-session-2.1-1.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint RPMS/i686/pam-afs-session-2.1-1.i686.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. $ rpmlint RPMS/i686/pam-afs-session-debuginfo-2.1-1.i686.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Koji scratch builds: EPEL5: https://koji.fedoraproject.org/koji/taskinfo?taskID=2850664 F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2850668 F14: http://koji.fedoraproject.org/koji/taskinfo?taskID=2850671 F15: http://koji.fedoraproject.org/koji/taskinfo?taskID=2850674
Hi Ken, Some initial comments: 1) Name: pam-afs-session There are some guidelines on the naming of pam modules: http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Addon_Packages_.28httpd.2C_pam.2C_and_SDL.29 though in this case I appreciate it supersedes the normal "use name of tar ball" which you followed. 2) A very recent addition to the "rules" is to add %{?_isa} to the end of your -devel Buildrequires. e.g. BuildRequires: pam-devel%{?_isa} Other than that at first site it's looking pretty good. Generally have a look at the "Getting Sponsored" pages. Provide some informal reviews of other pending packages from here: http://fedoraproject.org/PackageReviewStatus/NEW.html I'd avoid the "Need sponsor" ones. Report back here with some links to some reviews you have made. Steve.
Steve, Thank you for your suggestions. (In reply to comment #1) > though in this case I appreciate it supersedes the normal "use > name of tar ball" which you followed. I'm fine with changing it to pam_afs_session. You are right, I was trying to follow the tarball name convention. Do I need to change the name to use underscores? > 2) > A very recent addition to the "rules" is to add %{?_isa} to the > end of your -devel Buildrequires. e.g. I incorporated your %{?_isa} suggestion in 2.1-2 Updated Spec (2.1-2): http://adiemus.org/~kdreyer/rpms/pam-afs-session.spec F14 build (2.1-2): http://koji.fedoraproject.org/koji/taskinfo?taskID=2859436 > Report back here with some links to some reviews you have made. A few days ago I reviewed #678199 . I can do more reviews next week if necessary.
Some more reviews would be good. Preferably one you can find problems with, submitting another package is also builds a stronger case. Either pam_afs_session or pam_afs-session , probably the latter. Please provide direct links to the .src.rpm and .spec at each revision.
I've submitted a second package for review at https://bugzilla.redhat.com/681393 .
I've also informally reviewed https://bugzilla.redhat.com/664390 .
Okay I'll look over all of this , just to warn you for me this is a first time so excuse any clunkiness in the process.
Steve, thank you. I hope I'm not interrupting you mid-review, but I've taken your naming suggestion and renamed it to pam_afs_session. This matches the module and the manpage. http://adiemus.org/~kdreyer/rpms/pam_afs_session.spec http://adiemus.org/~kdreyer/rpms/pam_afs_session-2.1-3.src.rpm Koji scratch build: EL5: http://koji.fedoraproject.org/koji/taskinfo?taskID=2879001 F14: http://koji.fedoraproject.org/koji/taskinfo?taskID=2879014 F15: http://koji.fedoraproject.org/koji/taskinfo?taskID=2879011
removing FE-NEEDSPONSOR from this review, per https://fedorahosted.org/fesco/ticket/569
spectool pam_afs_session.spec Source0: http://archives.eyrie.org/software/afs/pam-afs-session-2.1.tar.gz this URL looks to have vanished, you need to update to 2.2 I guess. It's a shame if upstream deletes old versions? Also you include examples/debian and examples/solaris in your docs and these are irrelavent in this context so should be left out. Steve
It looks like upstream just moves older versions to an ARCHIVE directory, eg. http://archives.eyrie.org/software/ARCHIVE/pam-afs-session/pam-afs-session-2.1.tar.gz I've updated to 2.2, and removed the examples for other platforms. 2.2-1: http://adiemus.org/~kdreyer/rpms/pam_afs_session.spec http://adiemus.org/~kdreyer/rpms/pam_afs_session-2.2-1.src.rpm Koji scratch builds: F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2887900 F14: http://koji.fedoraproject.org/koji/taskinfo?taskID=2887933 F15: http://koji.fedoraproject.org/koji/taskinfo?taskID=2887936 EL5: http://koji.fedoraproject.org/koji/taskinfo?taskID=2887922 EL6: http://koji.fedoraproject.org/koji/taskinfo?taskID=2887907
This is really pedantic - sorry. If you are going to modify the source with find %{_builddir}/%{buildsubdir}/examples -mindepth 1 -maxdepth 1 -not -name "redhat" -exec rm -rf {} ';' then that should be in %prep since this is where you prepare the source. Otherwise just change %doc LICENSE NEWS README TODO examples to %doc LICENSE NEWS README TODO examples/redhat
(In reply to comment #11) > This is really pedantic - sorry. I appreciate your help and the thoroughness of your review. In the interest of staying closer to upstream's conventions, I want to keep the "examples/redhat/" directory structure. I've moved the find command to the %prep section per your suggestion. Here is 2.2-2: http://adiemus.org/~kdreyer/rpms/pam_afs_session.spec http://adiemus.org/~kdreyer/rpms/pam_afs_session-2.2-2.src.rpm Koji scratch builds: F13: http://koji.fedoraproject.org/koji/taskinfo?taskID=2889613 EL5: http://koji.fedoraproject.org/koji/taskinfo?taskID=2889616
- Package meets naming and packaging guidelines It does, pam module ones in particular. - Spec file matches base package name. Yes - Spec has consistant macro usage. Yes. - Meets Packaging Guidelines. Yes - License MIT - License field in spec matches The LICENSE file is very verbose in specifying all the various copy rights but yes it seems to MIT. - License file included in package Yes - Spec in American English Yes - Spec is legible. Yes - Sources match upstream md5sum: $ md5sum pam-afs-session-2.2.tar.gz ../SOURCES/pam-afs-session-2.2.tar.g 5621adc56319582b71b61546face4409 pam-afs-session-2.2.tar.gz 5621adc56319582b71b61546face4409 ../SOURCES/pam-afs-session-2.2.tar.gz - Package needs ExcludeArch It does not - BuildRequires correct Yes. - Spec handles locales/find_lang Not present. - Package is relocatable and has a reason to be. Not relocatable. - Package has %defattr and permissions on files is good. They are. - Package has a correct %clean section. Yes - Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) - Package is code or permissible content. Yes. - Doc subpackage needed/used. Not needed. - Packages %doc files don't affect runtime. They don't - Headers/static libs in -devel subpackage. Not needed. - Spec has needed ldconfig in post and postun Not needed. - .pc files in -devel subpackage/requires pkgconfig Not needed - .so files in -devel subpackage. Not needed. - -devel package Requires: %{name} = %{version}-%{release} - .la files are removed. They are. - Package is a GUI app and has a .desktop file No gui - Package compiles and builds on at least one arch. See koji. - Package has no duplicate files in %files. No - Package doesn't own any directories other packages own. /lib/security is pulled in via pam which is required - good. - Package owns all the directories it creates. - No rpmlint output. pam_afs_session.x86_64: W: spelling-error %description -l en_US afs -> ads, as, oafs pam_afs_session-debuginfo.x86_64: W: spelling-error Summary(en_US) afs -> ads, as, oafs pam_afs_session-debuginfo.x86_64: W: spelling-error %description -l en_US afs -> ads, as, oafs pam_afs_session.src: W: spelling-error %description -l en_US afs -> ads, as, oafs - final provides and requires are sane: $ rpm -qp --provides pam_afs_session-2.2-2.x86_64.rpm pam_afs_session.so()(64bit) pam_afs_session = 2.2-2 pam_afs_session(x86-64) = 2.2-2 $ rpm -qp --requires pam_afs_session-2.2-2.x86_64.rpm | grep -v rpm | grep -v libc libk5crypto.so.3()(64bit) libkrb5.so.3()(64bit) libkrb5.so.3(krb5_3_MIT)(64bit) libpam.so.0()(64bit) libpam.so.0(LIBPAM_1.0)(64bit) libpam.so.0(LIBPAM_EXTENSION_1.0)(64bit) libpam.so.0(LIBPAM_MODUTIL_1.0)(64bit) pam_krb5 SHOULD Items: - Should build in mock. See koji. - Should build on all supported archs See koji - Should function as described. Not checked. - Should have sane scriptlets. None - Should have subpackages require base package with fully versioned depend. Not relavent. - Should have dist tag Yes - Should package latest version Yes - check for outstanding bugs on package. Issues: All seems good. APPROVED. Steve.
New Package SCM Request ======================= Package Name: pam_afs_session Short Description: AFS PAG and AFS tokens on login Owners: ktdreyer Branches: f13 f14 f15 el5 el6
Git done (by process-git-requests).
Hi Ken, You can close this now with "closed nextrelease" Steve.
Thanks Steve, done.