[Bug 280541] Review Request: setools - SELinux policy analysis tools

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

Summary: Review Request: setools - SELinux policy analysis tools


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





------- Additional Comments From mtasaka at ioa.s.u-tokyo.ac.jp  2007-09-09 12:59 EST -------
Created an attachment (id=191091)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=191091&action=view)
rpmlint complaint for 3.3.1-1

Well, for 3.3.1-1 there are not a few issues to be fixed.
Please check the following URLs for general packaging issue.

http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines

A. %description stage

* AutoReqProv
  - Please explain you want to disable AutoReqProv.

    The fact that you check the libraries' dependencies is _not_
    a good reason because even in the case it is still preferable
    that the dependency for libraries are _also_ automatically
    checked by __find_requires. Usually you must only write
    "version specific Requires" for libraries and leave the other 
    libraries' dependency to __find_requires unless you have special 
    reason you don't want to.

   - Also, please consider to remove redundant version
     specific dependency. For example, even FC6 has GTK 2.10.13
     and I don't find any reason you want to write
     "%define gtk_ver 2.8".

* EVR specific dependency
  - Dependency between subpackages must be EVR (epoch:version:*release*) 
    specific.

* Vertual provides
  - Please explain why you want to add vertual provides such as
    "Provides: libqpol = %{libqpol_ver}". This may cause some
    complicated problems for upgrade paths by yum.

* Typo
  - "sqlite >= ${sqlite_ver}" and so on are apparent typo.

B. %prep/%build/%install stage
* Timestamp
  - Please keep timestamp. When using "install" or "cp", use
    "-p" option.
  - Also,
--------------------------------------------------------
make DESTDIR=${RPM_BUILD_ROOT} INSTALL="install -p" install
--------------------------------------------------------
    is more preferable to keep timestamps on more files.

* Macros
  - Please use macros. For example, /usr/share must be %_datadir.
    http://fedoraproject.org/wiki/Packaging/RPMMacros

* Desktop file
  - desktop file must be installed by using "desktop-file-install"
    (BuildRequires: desktop-file-utils is needed).

C. %files
* defattr
  - Now we recommend %defattr(-,root,root,-)
  - Note: %defattr(0755,root,root) is usually wrong (see
    the debuginfo issue below).

* Directory ownership issue
  - Please make it ensure that all the directories created by
    setools related rpms are surely owned by setools related rpms.
    Actually many directories are not owned.
----------------------------------------------------
/usr/include/apol
/usr/include/poldiff
/usr/include/qpol
/usr/lib/python2.5/site-packages/setools/
.... (and many others)
----------------------------------------------------

  ! Note: When you write
----------------------------------------------------
%files
foo/
----------------------------------------------------
    (where foo is a directory), this means the directory foo/
    itself and all files/directories/etc.. under foo/.
    This way of writing %files entry cleans up and shorten
    %files entry and makes directory ownership issue more
    visible than writing verbose file lists.

* static archive
  - Static archive must be seperated from -devel subpackage
    and must be packages in another subpackage.

D. rpmlint
The result for rpmlint is attached (please check how rpmlint
complains before submitting)
You can get the explanation of each rpmlint by using
"rpmlint -I". For example:
--------------------------------------------------------
[tasaka1 at localhost ~]$ rpmlint -I mixed-use-of-spaces-and-tabs
mixed-use-of-spaces-and-tabs :
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.
--------------------------------------------------------

Summary:
* mixed-use-of-spaces-and-tabs
  - See above.

* non-executable-script
  - /usr/share/setools-3.3/seaudit-report-service must have
    executable permission

* script-without-shebang
  - Scripts without shebang must not have executable permission

* unstripped-binary-or-object
  - Setting executable permission by %attr like
--------------------------------------------------------------
%attr(755,root,root) %{pkgpyexecdir}/_qpol.so
--------------------------------------------------------------
    is actually not right.
    Unless binaries have executable permission at the time %install
    ends, the binaries are not stripped by find-debuginfo.sh or
    so.
    i.e. you have to change those binaries by the time %install
    ends "manually", not by trying to set executable permission
    by %attr.

* use-old-pam-stack
  - Using "pam_stack.so" is deprecated, and moreover, pam_stack.so
    module is *removed* from pam. So %_sysconfdir/pam.d/seaudit
    must be updated not to use pam_stack.so.

* invalid-desktopfile
  - See above (desktop-file-install)

* devel-file-in-non-devel-package
  - Symlinks %_libdir/*.so should be moved to -devel subpackage

* symlink-should-be-relative
  - Please change symlinks to relative, not absolute.

-- 
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, or are watching someone who is.