[Bug 753354] Review Request: strongswan - IKEv1 and IKEv2 daemon

[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=753354

--- Comment #8 from Ondrej Vasik <ovasik at redhat.com> 2012-01-20 05:58:10 EST ---
formal review is here, see the notes explaining OK* and BAD (BAD*) statuses
below:

OK source files match upstream:
   sha256sum strongswan-4.6.1.tar.gz :
   d750ec16bc32c3d7f41fdbc7ac376defb1acde9f4d95d32052cdb15488ca3c34 
strongswan-4.6.1.tar.gz

   One comment here - is there any reason for shipping .tar.gz sources? I see
upstream provides .tar.bz2 as well, which is about 30% smaller.

  sha256sum strongswan-4.6.1.tar.bz2 :
  3d6dcdb3ce46dab51783b98c9bb54ebc931ff80941a0507d3cf3e3ac813eb439 
strongswan-4.6.1.tar.bz2

OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK dist tag is present.
BAD license field matches the actual license.
  Still GPL - You should use GPLv2+
BAD license is open source-compatible (GPLv2+). License text included in
package.
   at least
   %doc COPYING README
   is missing (maybe even NEWS could be helpful)
OK latest version is being packaged.
   Only developer version for 4.6.2 available atm.
OK BuildRequires are proper.
OK compiler flags are appropriate.
OK package builds in mock (Rawhide/i386).
OK debuginfo package looks complete.
BAD* rpmlint is silent.
OK final provides and requires look sane.
N/A %check is present and all tests pass.
BAD shared libraries are added to the regular linker search paths with correct
scriptlets

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig

is missing ... package adds some shared libraries ...

Idea - would it make sense to move them into -libs subpackage?

BAD owns the directories it creates.
  /usr/libexec/ipsec/ , /usr/lib/ipsec/ , /usr/lib/ipsec/plugins directories
  are added but not owned.

OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
OK correct scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK headers in devel subpackage
OK pkgconfig files in devel subpackage.
OK no libtool .la droppings.
OK not a GUI app.

rpmlint output:
strongswan.i686: W: invalid-license GPL

Use GPLv2+

strongswan.i686: E: non-readable /etc/strongswan.conf 0640L

Really strange permissions for the configure file... Is that intentional?
I would recommend changing to 644...

strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan.conf.5.gz 27: warning: `EX' not defined
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan.conf.5.gz 31: warning: `EE' not defined
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan.conf.5.gz 141: warning: `TQ' not defined
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan.conf.5.gz 1274: warning: `UR' not defined
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan.conf.5.gz 1276: warning: `UE' not defined
strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.conf.5.gz
1370: warning: `EX' not defined
strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.conf.5.gz
1372: warning: `EE' not defined
strongswan.i686: W: manual-page-warning /usr/share/man/man5/ipsec.secrets.5.gz
135: warning: `TQ' not defined

Please check the manpages, it seems they are not completely gramatically
correct.

strongswan.i686: W: no-manual-page-for-binary strongswan

I guess this is caused by incomplete renaming binary from ipsec to
strongswan... please adjust the fix...

1 packages and 0 specfiles checked; 1 errors, 13 warnings.

Small comments:
There are trailing spaces in the spec file in %post section

When building on RHEL it would emit also warning about empty sections of
post/postun ... but this is something what could be solved later (you want this
package in epel6, so it would probably be better if you do that soon)

Please fix the issues.

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