[Bug 461849] Review Request: garmintools - Communication tools for Garmin devices

[bugzilla at redhat.com]

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=461849


Peter Lemenkov <lemenkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lemenkov at gmail.com




--- Comment #7 from Peter Lemenkov <lemenkov at gmail.com>  2009-08-19 09:48:11 EDT ---
Brian, I'm sorry for shameless breaking of your review process, but it seems,
that you forgot about this ticket :)

REVIEW:

- rpmlint is not silent (I added custom message numbering for the sake of
simplicity)

[petro at Sulaco SPECS]$ rpmlint ~/Desktop/garmintools-
1. garmintools.ppc: W: shared-lib-calls-exit /usr/lib/libgarmintools.so.4.2.0
exit at GLIBC_2.0
2. garmintools.ppc: E: postin-without-ldconfig /usr/lib/libgarmintools.so.4.2.0
3. garmintools.ppc: E: library-without-ldconfig-postun
/usr/lib/libgarmintools.so.4.2.0
4. garmintools.ppc: W: devel-file-in-non-devel-package
/usr/lib/libgarmintools.so
3 packages and 0 specfiles checked; 2 errors, 2 warnings.
[petro at Sulaco SPECS]$

The 1st message is indicated a possible design flaw in garmintools. You should
consider asking upstream about it.

The 2nd and the 3rd messaged should be fixed. Please remove "devel" from the
%post and %postun sections (looks like the leftover), and be careful - don't
forget the %post section which contains "rmmod garmin_gps &>/dev/null || true"

The 4th message should be fixed - you accidentally listed file
%{_libdir}/libgarmintools.so twice - in main package and in devel-subpackage. I
advise you to change in %files section this line 

%{_libdir}/lib*.so*

to that one:

%{_libdir}/lib*.so.*

+ The package is named according to the Package Naming Guidelines .
+ The spec file name matches the base package %{name}, in the format
%{name}.spec .

- The package must meet the Packaging Guidelines, but there is an issue, in the
%files section, with owned directories which are already owned by other
applications. See my note below.

+ The package is licensed with a Fedora approved license and meets the
Licensing Guidelines .
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included
in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package matches the upstream source, as
provided in the spec URL.

[petro at Sulaco SOURCES]$ sha256sum garmintools-0.10.tar.gz*
ffd50b7f963fa9b8ded3223c4786b07906c887ed900de64581a24ff201444cee 
garmintools-0.10.tar.gz
ffd50b7f963fa9b8ded3223c4786b07906c887ed900de64581a24ff201444cee 
garmintools-0.10.tar.gz.1
[petro at Sulaco SOURCES]$

+ The package successfully compiles and builds into binary rpms on at least one
primary architecture.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1614626

+ All build dependencies are listed in BuildRequires.

- 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 %post and %postun. See my notes, regarding rpmlint messages above.

+ The package owns all directories that it creates (none, actually).

- A Fedora package must not list a file more than once in the spec file's
%files listings. See my notes, regarding rpmlint messages above.

+ Permissions on files were set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
+ Everything, the package includes as %doc, does not affect the runtime of the
application.
+ Header files are in a -devel package.
+ No static libraries.

- 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. See
my notes, regarding rpmlint messages above.

+ The devel subpackage requires the base package using a fully versioned
dependency: Requires: %{name} = %{version}-%{release}
+ The package does NOT contain any .la libtool archives.

- The package must not own files or directories already owned by other
packages. Unfortunately, main package owns /etc/udev/rules.d and
/etc/modprobe.d
Please use the correct form (note the asterisk mark at the end of the
strings)):

%config(noreplace) %{_sysconfdir}/udev/rules.d/*
%config(noreplace) %{_sysconfdir}/modprobe.d/*

+ At the beginning of %install, the package runs rm -rf %{buildroot} (or
$RPM_BUILD_ROOT). 
+ All filenames in rpm package are valid UTF-8. 

Please, fix issues noted above.

-- 
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.