[Bug 782456] Review Request: kredentials - kde kerberos ticket system tray monitor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=782456

James Laska <jlaska@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |jlaska@xxxxxxxxxx
         AssignedTo|nobody@xxxxxxxxxxxxxxxxx    |jlaska@xxxxxxxxxx
               Flag|                            |fedora-review?

--- Comment #1 from James Laska <jlaska@xxxxxxxxxx> 2012-01-23 14:13:23 EST ---
> [ 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

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]