Spec URL: http://people.redhat.com/nhorman/kredentials.spec SRPM URL: http://people.redhat.com/nhorman/kredentials-2.0-1.fc16.src.rpm Description: kredentials is a kde system tray application that is used to obtain/destroy and monitor the status of kerberos tokens. This applet obviates the need to open a terminal for the sole purpose of running kinit/klog/kdestroy/etc.
> [ PASS ] MUST: rpmlint must be run on the source rpm and all binary rpms the > build produces. The output should be posted in the review.(refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint) # rpmlint ~/Downloads/kredentials-2.0-1.fc16.src.rpm kredentials.src: W: spelling-error Summary(en_US) kerberos -> kerosene kredentials.src: W: spelling-error %description -l en_US kerberos -> kerosene 1 packages and 0 specfiles checked; 0 errors, 2 warnings. > [ FAIL ] MUST: The package must be named according to the > http://fedoraproject.org/wiki/Packaging/NamingGuidelines The name is correct, but I think the rpm %{version} should match the upstream version. Specifically, it seems that this is 2.0-pre1. Perhaps consult the pre-release / snapshot version suggestions at: https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages Based on that, I might recomment %{version}-%{release} = 2.0-0.1.pre1 > [ PASS ] MUST: The spec file name must match the base package > <code>%{name}</code>, in the format <code>%{name}.spec</code> unless your > package has an exemption. (refer to > http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Spec_file_name). > [ PASS ] MUST: The package must meet the > http://fedoraproject.org/wiki/Packaging/Guidelines. > [ PASS ] MUST: The package must be licensed with a Fedora approved license and > meet the http://fedoraproject.org/wiki/Packaging/LicensingGuidelines. Looks good in the matrix at https://fedoraproject.org/wiki/Licensing > [ PASS ] MUST: The License field in the package spec file must match the actual > license. (refer to > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#ValidLicenseShortNames) > [ FAIL ] MUST: If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package must be included in <code>%doc</code>.(refer to > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License Text) Your %files is missing a %doc for the following files: * kredentials/AUTHORS * kredentials/AdditionalInfo.txt * kredentials/CMakeLists.txt * kredentials/COPYING * kredentials/ChangeLog * kredentials/INSTALL * kredentials/NEWS * kredentials/README * kredentials/TODO > [ PASS ] MUST: The spec file must be written in American English. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#summary) > [ WARN ] MUST: The spec file for the package '''MUST''' be legible. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#Spec_Legibility) Not required, but I'd recommend replacing mkdir, cp make w/ their macro equivalents. This is nit-picky, I know ... so feel free to ignore if you don't agree. > [ PASS ] MUST: The sources used to build the package must match the upstream > source, as provided in the spec URL. Reviewers should use md5sum for this task. > If no upstream URL can be specified for this package, please see the > http://fedoraproject.org/wiki/Packaging/SourceURL for how to deal with > this. c999b200377706785ec8e1ae5ef61216 kredentials_2.0-pre1.tar.bz2 c999b200377706785ec8e1ae5ef61216 /tmp/jlaska-rpm/SOURCES/kredentials_2.0-pre1.tar.bz2 > [ FAIL ] MUST: The package '''MUST''' successfully compile and build into > binary rpms on at least one primary architecture. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Support) See failed scratch build at http://koji.fedoraproject.org/koji/taskinfo?taskID=3726309 > [ NA ] MUST: If the package does not successfully compile, build or work on > an architecture, then those architectures should be listed in the spec in > <code>ExcludeArch</code>. Each architecture listed in <code>ExcludeArch</code> > '''MUST''' have a bug filed in bugzilla, describing the reason that the package > does not compile/build/work on that architecture. The bug number '''MUST''' be > placed in a comment, next to the corresponding <code>ExcludeArch</code> line. > (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#Architecture_Build_Failures) > [ WARN ] MUST: All build dependencies must be listed in > <code>BuildRequires</code>, except for any that are listed in the > http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 section of the > Packaging Guidelines ; inclusion of those as <code>BuildRequires</code> is > optional. Apply common sense. This may be why the scratch build fails, unclear > [ NA ] MUST: The spec file MUST handle locales properly. This is done by > using the <code>%find_lang</code> macro. Using > <code>%{_datadir}/locale/*</code> is strictly forbidden.(refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files) > [ NA ] MUST: Every binary RPM package (or subpackage) which stores shared > library files (not just symlinks) in any of the dynamic linker's default paths, > must call ldconfig in <code>%post</code> and <code>%postun</code>. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#Shared_Libraries) > [ NA ] MUST: Packages must NOT bundle copies of system libraries.(refer to > http://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries) > [ NA ] MUST: If the package is designed to be relocatable, the packager must > state this fact in the request for review, along with the rationalization for > relocation of that specific package. Without this, use of Prefix: /usr is > considered a blocker. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#RelocatablePackages) > [ PASS ] MUST: A package must own all directories that it creates. If it does > not create a directory that it uses, then it should require a package which > does create that directory. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership) > [ PASS ] MUST: A Fedora package must not list a file more than once in the spec > file's %files listings. (Notable exception: license texts in specific > situations)(refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles) > [ PASS ] MUST: Permissions on files must be set properly. Executables should be > set with executable permissions, for example. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions) > [ FAIL ] MUST: Each package must consistently use macros. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#macros) Please use either %{buildroot} or $RPM_BUILD_ROOT ... not both > [ PASS ] MUST: The package must contain code, or permissable content. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#CodeVsContent) > [ NA ] MUST: Large documentation files must go in a -doc subpackage. (The > definition of large is left up to the packager's best judgement, but is not > restricted to size. Large can refer to either size or quantity). (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation) > [ PASS ] MUST: If a package includes something as %doc, it must not affect the > runtime of the application. To summarize: If it is in %doc, the program must > run properly if it is not present. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#PackageDocumentation) > [ NA ] MUST: Header files must be in a -devel package. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages) > [ NA ] MUST: Static libraries must be in a -static package. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries) > [ NA ] MUST: If a package contains library files with a suffix (e.g. > libfoo.so.1.1), then library files that end in .so (without suffix) must go in > a -devel package. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#DevelPackages) > [ NA ] MUST: In the vast majority of cases, devel packages must require the > base package using a fully versioned dependency: <code>Requires: > %{name}%{?_isa} = %{version}-%{release} </code> (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#RequiringBasePackage) > [ NA ] MUST: Packages must NOT contain any .la libtool archives, these must > be removed in the spec if they are built.(refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#StaticLibraries) > [ PASS ] MUST: Packages containing GUI applications must include a > %{name}.desktop file, and that file must be properly installed with > desktop-file-install in the %install section. If you feel that your packaged > GUI application does not need a .desktop file, you must put a comment in the > spec file with your explanation. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#desktop) > [ PASS ] MUST: Packages must not own files or directories already owned by > other packages. The rule of thumb here is that the first package to be > installed should own the files or directories that other packages may rely > upon. This means, for example, that no package in Fedora should ever share > ownership with any of the files or directories owned by the > <code>filesystem</code> or <code>man</code> package. If you feel that you have > a good reason to own a file or directory that another package owns, then please > present that at package review time. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership) > [ PASS ] MUST: All filenames in rpm packages must be valid UTF-8. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#FilenameEncoding) > [ FAIL ] MUST: The upstream project URL should be http://freecode.com/projects/kredentials Also, note that you can use more macros in the Source0 URL Perhaps you'd want ... Source0: http://people.csail.mit.edu/noahm/%{name}/%{version}/%{name}_%{version}-pre1.tar.bz2 Other nit ... You might consider calling out files (instead of using globs) in your %files. Moreso for the _bindir stuff. Just a thought
copy that, thanks james. I'll fix this up today.
James, new spec and src rpm for you: SPEC: http://people.redhat.com/nhorman/kredentials.spec SRPM: http://people.redhat.com/nhorman/kredentials-2.0-0.1.pre1.fc16.src.rpm I've made changes according to all your notes above (since you asked, the build failure was a missing BuildRequires on kdelibs-devel, corrected now). The only thing I took liberties with was the %doc set. Specifically I didn't include CMakeLists.txt and AdditionalChanges.txt as those are artifact files that are part of the kde build process and hold no relevant package information. Thanks!
> [ PASS ] MUST: rpmlint must be run on the source rpm and all binary rpms the > build produces. The output should be posted in the review.(refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint) > # rpmlint /var/lib/mock/fedora-16-x86_64/result/kredentials-*rpm > kredentials.src: W: spelling-error Summary(en_US) kerberos -> kerosene > kredentials.src: W: spelling-error %description -l en_US kerberos -> kerosene > kredentials.x86_64: W: spelling-error Summary(en_US) kerberos -> kerosene > kredentials.x86_64: W: spelling-error %description -l en_US kerberos -> kerosene Safe to ignore > kredentials.x86_64: W: incoherent-version-in-changelog 2.0-1 ['2.0-0.1.pre1.fc16', '2.0-0.1.pre1'] Ah, this would be nice to fix. Looks like the first %changelog entry uses the old version/release format > kredentials.x86_64: W: no-manual-page-for-binary kredentials > kredentials.x86_64: W: install-file-in-docs /usr/share/doc/kredentials-2.0/INSTALL > 3 packages and 0 specfiles checked; 0 errors, 7 warnings. Safe to ignore > [ PASS ] MUST: The package must be named according to the > http://fedoraproject.org/wiki/Packaging/NamingGuidelines Looks good > [ PASS ] MUST: The upstream project URL should be http://freecode.com/projects/kredentials Fixed > [ PASS ] MUST: If (and only if) the source package includes the text of the > license(s) in its own file, then that file, containing the text of the > license(s) for the package must be included in <code>%doc</code>.(refer to > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License Text) The %doc files look good. > [ WARN ] MUST: All build dependencies must be listed in > <code>BuildRequires</code>, except for any that are listed in the > http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 section of the > Packaging Guidelines ; inclusion of those as <code>BuildRequires</code> is > optional. Apply common sense. Apologies, I missed this previously ... looks like you'll need to add "BuildRequires: desktop-file-utils" (https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage) > [ WARN ] MUST: Each package must consistently use macros. (refer to > http://fedoraproject.org/wiki/Packaging/Guidelines#macros) Nice, looks like consistent use around $RPM_BUILD_ROOT I've added a small patch to consistently use some of the other %build macros. Feel free to take it or toss it ... entirely optional. > [ PASS ] MUST: The upstream project URL should be http://freecode.com/projects/kredentials Looks good. > Other nit ... You might consider calling out files (instead of using globs) in > your %files. Moreso for the _bindir stuff. Just a thought Looks good, thanks. So just a few minor adjustments, and I think you'll be in good shape.
Created attachment 557297 [details] kredentials.patch
SPEC: http://people.redhat.com/nhorman/kredentials.spec SRPM: http://people.redhat.com/nhorman/kredentials-2.0-0.1.pre1.fc16.src.rpm There you go, fixed up as per your comments. Thanks for the through review!
Looks great, thanks for your patience. By the power vested in me, I approve this review request :)
New Package SCM Request ======================= Package Name: kredentials Short Description: kde system tray application to monitor kerberos tickets Owners: nhorman Branches: f16 InitialCC:
Git done (by process-git-requests).
kredentials-2.0-0.1.pre1.fc16 has been submitted as an update for Fedora 16. https://admin.fedoraproject.org/updates/kredentials-2.0-0.1.pre1.fc16
kredentials-2.0-0.1.pre1.fc16 has been pushed to the Fedora 16 testing repository.
kredentials-2.0-0.1.pre1.fc16 has been pushed to the Fedora 16 stable repository.