[Bug 1178911] Review Request: cairo-dock - Light eye-candy fully themable animated dock

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

 



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



--- Comment #4 from Jan Pokorný <jpokorny@xxxxxxxxxx> ---
Briefly:
- minor BSD-licensed file discovered
- some dates in changelog would deserve fixing

Below I commented also on some items that are OK:


> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> [ ] = Manual review needed
> 
> 
> Issues:
> =======
> - 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 is included in %doc.
>   Note: Cannot find LICENSE in rpm(s)
>   See:
>   http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
> - Package do not use a name that already exist
>   Note: A package already exist with this name, please check
>   https://admin.fedoraproject.org/pkgdb/acls/name/cairo-dock
>   See:
>   https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Conflicting_Package_Names

I am not sure here, I suppose the existing orhpaned package will get un-retired
(https://admin.fedoraproject.org/pkgdb/package/cairo-dock/)

> 
> ===== MUST items =====
> 
> C/C++:
> [x]: Package does not contain kernel modules.
> [x]: Package contains no static executables.
> [x]: Development (unversioned) .so files in -devel subpackage, if present.
>      Note: Unversioned so-files in private %_libdir subdirectory (see
>      attachment). Verify they are not in ld path.

cairo-dock-core: /usr/lib64/cairo-dock/libcd-Help.so

... but this is not a development .so, and is correctly in private subdir

Furthermore, CMakeList.txt makes it clear it is a special plugin for cairo-dock
that cannot be packaged in extra plug-ins package.

> [x]: Header files in -devel subpackage, if present.
> [x]: ldconfig called in %post and %postun if required.
> [x]: Package does not contain any libtool archives (.la)
> [x]: Rpath absent or only used for internal libs.
> 
> Generic:
> [x]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.

You mentioned there were some concerns in the past but now it was ACK'd
by spot.  It would be best if it there was a public record of that, couldn't
find anything on fedora-legal/fedora-devel MLs.  Only that it is better to
have a transparency in this regard.

IIUIC, the patent stuff is being removed prior to creating the resulting
source tarball thanks cairo-dock-create-fedora-tarball.sh.

> [!]: License field in the package spec file matches the actual license.
>      Note: Checking patched sources after %prep for licenses. Licenses found:
>      "GPL (v2 or later)", "LGPL (v2 or later)", "GPL (unversioned/unknown
>      version) GPL (v3)", "GPL (v3 or later)", "Unknown or generated". 13 files
>      have unknown license. Detailed output of licensecheck in /home/mock
>      /cairo-dock/1178911-cairo-dock/cairo-dock/licensecheck.txt

Everything seems OK except for BSD license of
    cairo-dock-3.4.0/cmake_modules/GNUInstallDirs.cmake
which refers to BSD license of CMake (and, as per instructions in the
header, it should be included in full in that very file!), which turns
to be BSD 3-Clause "New" or "Revised" license.

Hence I think the license should be: GPLv3+ and BSD

Alternatively, a fact that native CMake version exists can be utilized:
$ rpm -qf /usr/share/cmake/Modules/GNUInstallDirs.cmake
cmake-3.0.2-2.fc21.x86_64

> [ ]: License file installed when any subpackage combination is installed.

This doesn't seem to be true, e.g., one may install cairo-dock-libs and
miss the licenses/* files.

> [x]: %build honors applicable compiler flags or justifies otherwise.

Perhaps %cmake macro is robust enough so thar -O3 -> -O2 in CMakeLists.txt
is not necessary?

> [x]: Package contains no bundled libraries without FPC exception.
> [ ]: Changelog in prescribed format.

OK except rpmlint reports:

cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Thu Dec 21
2011 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxx> - 2.4.0.2-2
cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Sat Nov  6
2009 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> - 2.1.1.2-1
cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Wed May 27
2008 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> - 1.5.6-1.date20080528

> [x]: Sources contain only permissible code or content.
> [x]: Each %files section contains %defattr if rpm < 4.4
>      Note: %defattr present but not needed
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> [x]: Development files must be in a -devel package
> [x]: Package uses nothing in %doc for runtime.
> [x]: The spec file handles locales properly.
> [x]: Package consistently uses macros (instead of hard-coded directory names).

(/sbin/ldconfig based on
https://fedoraproject.org/wiki/Packaging:Guidelines#Shared_Libraries
contradicts a bit, and there is no macro for that command)

> [x]: Package is named according to the Package Naming Guidelines.
> [x]: Package does not generate any conflict.
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [-]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [x]: Requires correct, justified where necessary.

note wrt. cairo-dock, one can still install just cairo-dock-{core,libs} and
then particular plugins, so this is OK

> [x]: Spec file is legible and written in American English.
> [-]: Package contains systemd file(s) if in need.
> [x]: Useful -debuginfo package or justification otherwise.
> [x]: Package is not known to require an ExcludeArch tag.
> [x]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 30720 bytes in 3 files.
> [x]: Package complies to the Packaging Guidelines
> [x]: Package successfully compiles and builds into binary rpms on at least one
>      supported primary architecture.
> [x]: Rpmlint is run on all rpms the build produces.
>      Note: There are rpmlint messages (see attachment).
> [x]: Package requires other packages for directories it uses.
> [x]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: All build dependencies are listed in BuildRequires, except for any that
>      are listed in the exceptions section of Packaging Guidelines.
> [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
> [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
>      beginning of %install.
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Package contains desktop file if it is a GUI application.
> [x]: Package installs a %{name}.desktop using desktop-file-install or desktop-
>      file-validate if there is such a file.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
>      work.
> [x]: Package is named using only allowed ASCII characters.
> [x]: Package is not relocatable.
> [x]: Sources used to build the package match the upstream source, as provided
>      in the spec URL.
> [x]: Spec file name must match the spec package %{name}, in the format
>      %{name}.spec.
> [x]: File names are valid UTF-8.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> 
> ===== SHOULD items =====
> 
> Generic:
> [ ]: If the source package does not include license text(s) as a separate file
>      from upstream, the packager SHOULD query upstream to include it.

Related to the possible BSD issue (License field in the package spec...)

> [x]: Final provides and requires are sane (see attachments).
> [x]: Fully versioned dependency in subpackages if applicable.
>      Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in cairo-dock-
>      libs , cairo-dock-core , cairo-dock-devel
> [x]: Package functions as described.
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [x]: Scriptlets must be sane, if used.
> [x]: SourceX tarball generation or download is documented.
>      Note: Package contains tarball without URL, check comments
> [ ]: Description and summary sections in the package spec file contains
>      translations for supported Non-English languages, if available.

This sound merely like nice-to-have :)

> [ ]: Package should compile and build into binary rpms on all supported
>      architectures.

Haven't verified this.

> [ ]: %check is present and all tests pass.

Tests are present in tests subdir, but perhaps impractical to run.

> [x]: Packages should try to preserve timestamps of original installed files.
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [x]: Reviewer should test that the package builds in mock.
> [x]: Buildroot is not present
> [x]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
> [x]: Dist tag is present (not strictly required in GL).
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: Uses parallel make %{?_smp_mflags} macro.
> [x]: The placement of pkgconfig(.pc) files are correct.
> [x]: SourceX is a working URL.
> [x]: Spec use %global instead of %define unless justified.


> Rpmlint
> -------
> Checking: cairo-dock-3.4.0-9.fc21.x86_64.rpm
>           cairo-dock-libs-3.4.0-9.fc21.x86_64.rpm
>           cairo-dock-core-3.4.0-9.fc21.x86_64.rpm
>           cairo-dock-devel-3.4.0-9.fc21.x86_64.rpm
>           cairo-dock-3.4.0-9.fc21.src.rpm

> cairo-dock.x86_64: W: spelling-error Summary(en_US) themable -> them able, them-able, fathomable
> cairo-dock.x86_64: W: spelling-error %description -l en_US metapackage -> meta package, meta-package, prepackage
> cairo-dock-core.x86_64: W: spelling-error %description -l en_US themable -> them able, them-able, fathomable

> cairo-dock.x86_64: E: no-binary

This cannot be noarch as then no subpackage cannot be reverted to "arch"
(was experimenting with something similar for clufter).

> cairo-dock.x86_64: W: no-documentation
> cairo-dock-libs.x86_64: W: no-documentation
> cairo-dock-devel.x86_64: W: no-documentation

> cairo-dock-core.x86_64: W: obsolete-not-provided cairo-dock-plug-ins-gecko
> cairo-dock-core.x86_64: W: obsolete-not-provided cairo-dock-themes

justified

> cairo-dock-core.x86_64: E: incorrect-fsf-address /usr/share/licenses/cairo-dock-core/copyright

OK for now, to be discussed with upstream as per [comment 1].

> cairo-dock-devel.x86_64: W: only-non-binary-in-usr-lib

non-versioned lib symlink to versioned one

> cairo-dock.src: W: spelling-error Summary(en_US) themable -> them able, them-able, fathomable
> cairo-dock.src: W: spelling-error %description -l en_US metapackage -> meta package, meta-package, prepackage

not sure if there is any strong preference in other packages; themable OK
(~parsable, movable).

> cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Thu Dec 21 2011 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxx> - 2.4.0.2-2
> cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Sat Nov  6 2009 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> - 2.1.1.2-1
> cairo-dock.src: E: specfile-error warning: bogus date in %changelog: Wed May 27 2008 Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> - 1.5.6-1.date20080528

might be worth fixing otherwise it will be reported over and over in any
(local) rebuild, etc.

> 5 packages and 0 specfiles checked; 5 errors, 11 warnings.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
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]