Bug 321731

Added current shorewall package owner to cc. 

Robert - I'm not trying to usurp your package here, but I thought that because
upstream has changed so much, and because Id done the packaging work for other
reasons, it would be useful to put them into BZ for review. I am more than happy
if you want to continue owning this package. Am also happy to co-maintain
shorewall with you, if you like.

Following discussion with Robert, and also on #fedora-devel, consensus seems to
be that it is better to have a single package with all tarballs. Therefore, I'm
closing this review, and discussion of the multi-tarball package will continue in
this bugzilla entry.

Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.4-1.fc7.src.rpm

This is the combined tarball package. Plus it now contains the shorewall-lite
functionality. 

The main package (shorewall) simply Requires the sub-packages shorewall-common,
shorewall-perl and shorewall-shell. This is the pragmatic approach, aimed at not
breaking existing installations that require the legacy shell compiler. The
clued up user could install only shorewall-common and shorewall-perl or
shorewall-shell if she only wanted one compiler. There is also a sub-package for
shorewall-lite. 

Running rpmlint on the subpackages:
$ rpmlint ../RPMS/noarch/shorewall-common-4.0.4-1.fc7.noarch.rpm 
shorewall-common.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall
--> Bogus, as the Defaul-Start entry is empty

shorewall-common.noarch: E: subsys-not-used /etc/rc.d/init.d/shorewall
--> This is ok, as although the init file doesn't create a lockfile, shorewall
itself does.

shorewall-common.noarch: W: incoherent-init-script-name shorewall
--> Irrelevant

$ rpmlint ../RPMS/noarch/shorewall-perl-4.0.4-1.fc7.noarch.rpm
No warnings or errors

$ rpmlint ../RPMS/noarch/shorewall-shell-4.0.4-1.fc7.noarch.rpm 
shorewall-shell.noarch: E: non-executable-script
/usr/share/shorewall-shell/prog.header 0644
--> prog.header has a hash-bang at the top, but isn't executable as this file is
simply a template for shorewall to use when generating a shell script, and so it
shouldn't be executeable.

$ rpmlint ../RPMS/noarch/shorewall-lite-4.0.4-1.fc7.noarch.rpm 
shorewall-lite.noarch: W: non-conffile-in-etc /etc/shorewall-lite/Makefile
--> A weird one. that Makefile is used by shorewall-lite and is intentionally
put in /etc by upstream... not sure waht to do about this really.

shorewall-lite.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall-lite
--> bogus again - Default-Start is empty, Default-Stop has all runlevels.

shorewall-lite.noarch: E: subsys-not-used /etc/rc.d/init.d/shorewall-lite
--> Again, shorewall-lite handles lock file creation itself, there's no need for
the init script to create one.

A remaining question is what to do about Ports.pm. Please see BZ #321711 for a
discussion about this. 

Synopsis: shorewall-perl requires the file:
/usr/share/shorewall-perl/Shorewall/Ports.pm

Ports.pm is built by buildports.pl which looks at entries in
/etc/{services,protocols} and builds Ports.pm accordingly. 

/etc/{services,protocols} are part of the setup package.

The current strategy in the spec file is that Ports.pm is created at package
build time by Build:requiring the setup package. However, the following issues
then arise:
A) What if the user has modified locally /etc/{services,protocols}
B) What if another package modifies /etc/{services,protocols} on package
installation (Ville recalled that the LTSP stuff might do this)

The only way around that would be to create Ports.pm on package installation.
This could be done in %post, and indeed the upstream sample spec does just that.

But, Ville also points out that creating files under /usr on package
installation fails if /usr is read-only (but then again, so will package
installation). So, then we'd have to do the building of Ports.pm under
/var/lib/shorewall-perl, and package a symlink there from
/usr/share/shorewall-perl. 

This is all do-able, but I can't help preferring the KISS approach of generating
Ports.pm at package build time using /etc/{services,protocols} from the setup
package. For case B above I would argue that the LTSP packager should have his
changes to /etc/{services,protocols} incoorporated into the setup package files.

... Thoughts?

Oops - in Comment #2, please ignore the part that reads "I'm closing this review"!

For background, the reason that Ports.pm is generated rather than direclty
parsing /etc/{services,protocols} is given by upstream:
3)  Previously, Shorewall-perl read /etc/protocols and /etc/services
    during compiler startup to build internal protocol and service
    tables. This had a fixed cost of up to one half second or more,
    depending on the speed of the system and the distribution
    (The /etc/services released with OpenSuSE 10.2 is over 14,000
    lines!!) These tables are now initialized by the Perl compiler
    which speeds up compilation considerably.

    During installation, Shorewall generates the Perl module
    /usr/share/shorewall-perl/Shorewall/Ports.pm, using your
    /etc/protocols and /etc/services as input.

    To re-generate the module from those two files:
        1. Backup your current /usr/share/shorewall-perl/Shorewall/Ports.pm
           file.
        2. /usr/share/shorewall-perl/buildports.pl > \
           /usr/share/shorewall-perl/Shorewall/Ports.pm

    Note: If the buildports.pl program fails to run to a successful
    completion during installation, a fallback version of
    module will be installed. That fallback module was generated from
    the /etc/protocols and /etc/services shipped with Ubuntu Feisty
    Fawn.

    Even if the buildports.pl program runs successfully, the fallback
    module is also installed as
    /usr/share/shorewall-perl/Shorewall/FallbackPorts.pm. So if you
    encounter problems with the generated module, simply copy the
    fallback module to /usr/share/shorewall-perl/Shorewall/Ports.pm.

(In reply to comment #3)
> $ rpmlint ../RPMS/noarch/shorewall-common-4.0.4-1.fc7.noarch.rpm 
> shorewall-common.noarch: W: service-default-enabled /etc/rc.d/init.d/shorewall
> --> Bogus, as the Defaul-Start entry is empty

Indeed, rpmlint bug (ditto the -lite one), now fixed in rpmlint upstream svn.

Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.4-2.fc7.src.rpm

* Mon Oct  8 2007 Jonathan G. Underwood <jonathan.underwood> - 4.0.4-2
- Add ghost files for /var/lib/shorewall/.modules and /var/lib/shorewall/.modulesdir
- Fix ownership of /var/lib/shorewall-lite

I have discussed the whole buildports/Ports.pm issue with upstream maintainers,
and a better way of dealing with this has been established which removes the
need for Ports.pm and buildports - this will happen in the next release 4.0.5 in
the very near future. So i see no point tackling the issue in this package
currently. I think therefore there's nothing more to be done with this package,
it's ready to go, subject to review. Technically, since shorewall exists already
as a Fedora package it doesn't need a review, but in this case, with so many
changes, it is probably adviseable to review.

Spec URL: http://jgu.fedorapeople.org/shorewall.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-4.0.5-1.fc7.src.rpm

This is an updated package for version 4.0.5. With this new version, there is no
longer any need to cache /etc/services,protocols and so the buildports.pl issue
has gone away. 

With this version, there are no outstanding issues from a packaging perspective
that I am aware of, so hopefully we can get this into the repos soon!

Since the packaging of shorewall hasn't changed substantially with this version
(unlike my initial package split into shorewall-common, -perl, -lite, -shell), I
don't think this necessarily needs review, it's merely an update to an existing
package (as opposed to creating 4 new packages).

Robert, are you ok with this? If so, please can you grant me the rights on the
packagedatabase that I asked for to allow me to co-maintain this package with you?

done

Thanks Robert  - will start updating packages this evening.

Updated packages built for F-7, F-8 and devel. Closing this bug.

shorewall-4.0.5-1.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update shorewall'

shorewall-4.0.5-1.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.