[Bug 225769] Merge Review: freeradius

Mon Jul 27 12:11:30 UTC 2009

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

Peter Lemenkov <lemenkov at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |487059

--- Comment #4 from Peter Lemenkov <lemenkov at gmail.com>  2009-07-27 08:11:27 EDT ---
REVIEW (sorry for the delay):

+/- rpmlint outputs a LOT of messages:

http://peter.fedorapeople.org/freeradius_rpmlint.txt

However, as John Dennis already mentioned in bz#487059, most of these issues
may be omitted safely:

* non-standard GID:
* non-readable files
* non-config files under /etc
* no-documentation

The rest of rpmlint messages:

freeradius.i586: W: obsolete-not-provided freeradius-dialupadmin
freeradius.i586: W: obsolete-not-provided freeradius-dialupadmin-ldap
freeradius.i586: W: obsolete-not-provided freeradius-dialupadmin-mysql
freeradius.i586: W: obsolete-not-provided freeradius-dialupadmin-postgresql

I think, this may be omitted too.

freeradius.i586: E: zero-length /var/log/radius/radius.log

Likewise.

freeradius.i586: E: incoherent-logrotate-file /etc/logrotate.d/radiusd

Due to historical reasons, the main application name differs from the package's
name. We cannot change anything here. So this issue should be omitted.

freeradius.i586: W: file-not-utf8
/usr/share/doc/freeradius-2.1.6/rfc/pppext-eap-sim-12.txt

This should be fixed at the %prep stage.

freeradius.i586: E: zero-length /var/log/radius/radutmp

May be omitted.

freeradius.i586: W: file-not-utf8 /usr/share/doc/freeradius-2.1.6/rlm_dbm

This should be fixed at the %prep stage.

freeradius.i586: E: executable-marked-as-config-file /etc/rc.d/init.d/radiusd

Does it really should be marked as a config? I'm sure it doesn't. All
installation-specific changes should be placed in /etc/sysconfig/%{name} or
something similar.

freeradius.i586: W: file-not-utf8
/usr/share/doc/freeradius-2.1.6/rfc/draft-sterman-aaa-sip-00.txt

This should be fixed at the %prep stage.

freeradius.i586: E: non-standard-executable-perm /etc/raddb/certs/bootstrap
0750

I don't like the idea to place executables (scripts) into /etc/ (exept known
exeptions like /etc/init.d ). However looks like this should be placed here due
to historical reasons. So this issue may be omitted too.

freeradius.i586: W: incoherent-init-script-name radiusd ('freeradius',
'freeradiusd')

Due to historical reasons, the main application name differs from the package's
name. We cannot change anything here. So this issue should be omitted.

freeradius-debuginfo.i586: W: hidden-file-or-dir
/usr/src/debug/freeradius-server-2.1.6/src/main/.libs
freeradius-debuginfo.i586: W: hidden-file-or-dir
/usr/src/debug/freeradius-server-2.1.6/src/main/.libs

Looks like a leftover from wrongly picked up binaries byt script to generate
debuginfo. I think this may be ignored.

freeradius-debuginfo.i586: W: spurious-executable-perm
/usr/src/debug/freeradius-server-2.1.6/src/modules/rlm_ippool/rlm_ippool_tool.c
freeradius-debuginfo.i586: W: spurious-executable-perm
/usr/src/debug/freeradius-server-2.1.6/src/modules/rlm_linelog/rlm_linelog.c

This should be fixed at the %prep stage.

freeradius-postgresql.i586: W: summary-not-capitalized postgresql support for
freeradius
freeradius-unixODBC.i586: W: summary-not-capitalized unixODBC support for
freeradius

This may be omitted

+ 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 meets the Packaging Guidelines. At least, I cannot find any
issues. Yet, some small issues still remains:

* I'm sure, that %description for -devel subpackage must be changed, since it
has nothing to do with static libraries.
* See my note regarding ldconfig below.
* I advice you to consider adding support for Firebird and iodbc, since both of
them are included in Fedora. However this is not a blocker - just my
suggestion.

+ 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 (as legible, as this quite complex
package can be).
+ The sources used to build the package match the upstream source, as provided
in the spec URL. 
+ The package successfully compiles and builds into binary rpms on at least one
primary architecture. 

dist-5E-epel
http://koji.fedoraproject.org/koji/taskinfo?taskID=1536573

dist-f11
http://koji.fedoraproject.org/koji/taskinfo?taskID=1536583

+ All build dependencies are listed in BuildRequires.

- The main package calls ldconfig in %post and %postun, however I didn't see
any library objects stored in the default ldconfig paths. For me it seems like
the leftofver. Please, correct me if I wrong.

+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files
listings.
+ Permissions on files are set properly. Note, that freeradius is a
security-sensitive application, so many files marked as non-readable for
non-privileged users.
+ 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.
+ No extremely large documentation files.
+ Everything, the package includes as %doc, does not affect the runtime of the
application.
+ The C header files are in a -devel subpackage.
+ No static libraries.
+ No pkgconfig(.pc) files.

+/- In the vast majority of cases, devel packages must require the base package
using a fully versioned dependency: Requires: %{name} = %{version}-%{release},
but freeradius-devel requires freeradius-libs instead. However, I suspect, that
there is some rationale behind this. Actually, I suspect also, that something
is broken here, since these two libraries located not in any of the standart
ldconfig paths.

+ The package does NOT contain any .la libtool archives.
+ The package does not own files or directories already owned by other
packages.
+ At the beginning of %install, the package runs rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Please, comment the issues, noted above, and I'll continue.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.