[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

Ondrej Vasik <ovasik at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #17 from Ondrej Vasik <ovasik at redhat.com> 2012-02-13 12:46:18 EST ---
Sorry for delay...
Second round of 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.
OK license field matches the actual license.
  GPLv2+
OK license is open source-compatible (GPLv2+). License text included in
package.
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.
OK* rpmlint is silent.
OK final provides and requires look sane.
N/A %check is present and all tests pass.
OK shared libraries are added to the regular linker search paths with correct
scriptlets
OK owns the directories it creates.
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.

OK* rpmlint output:

[Reset at dhcp-24-196 rpmbuild]$ rpm -q rpmlint
rpmlint-1.4-6.fc18.noarch
[Reset at dhcp-24-196 rpmbuild]$ rpmlint SPECS/strongswan.spec
RPMS/i686/strongswan-4.6.1-4.fc18.i686.rpm
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan_ipsec.secrets.5.gz 135: warning: `TQ' not
defined
strongswan.i686: E: non-standard-dir-perm /etc/strongswan 0700L
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan_ipsec.conf.5.gz 1370: warning: `EX' not defined
strongswan.i686: W: manual-page-warning
/usr/share/man/man5/strongswan_ipsec.conf.5.gz 1372: warning: `EE' not defined
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
1 packages and 1 specfiles checked; 1 errors, 8 warnings.

This one error for 700 configuration directory is ok ... for me it seems to be
better option to protect the whole directory. But if the upstream has different
opinion and they want to have special permissions just on the config file, I'm
fine with that.

8 warnings coming from manpages marked TODO in spec file - worksforme as
well... I think these are not displayed to you because you are using different
version of rpmlint...and they are in the upstream versions of manpages.

You could reproduce these warnings for e.g. strongswan.conf by : zcat
strongswan.conf.5.gz | gtbl | groff -mtty-char -Tutf8 -P-c -mandoc -wmac
>/dev/null

So the only one comment left - is the format of the source tarball - please
consider switch to bzip2 format in next release - it will make srpm smaller :).

Anyway - other than that package looks sane now, APPROVED.

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