Bug 321711 - Review Request: shorewall-perl - Perl-based compiler for Shoreline Firewall
Summary: Review Request: shorewall-perl - Perl-based compiler for Shoreline Firewall
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 321731
TreeView+ depends on / blocked
 
Reported: 2007-10-07 00:42 UTC by Jonathan Underwood
Modified: 2007-11-30 22:12 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-10-07 23:38:03 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jonathan Underwood 2007-10-07 00:42:51 UTC
Spec URL: http://jgu.fedorapeople.org/shorewall-perl.spec
SRPM URL: http://jgu.fedorapeople.org/shorewall-perl-4.0.4-1.fc7.src.rpm
Description: 

The Shoreline Firewall, more commonly known as "Shorewall", is a
Netfilter (iptables) based firewall that can be used on a dedicated
firewall system, a multi-function gateway/ router/server or on a
standalone GNU/Linux system. 

The version 3 release series of Shorewall is already available in Fedora. With the release of version 4, upstream has added a new perl based rule compiler and completely changed the way the package is distributed. The shell-based and perl-based compilers are each distributed as individual tarballs, and files required to run shorewall with either compiler are packaged as a third tarball, shorewall-common. 

The shorewall-perl compilers is suggested for new
installed systems and shorewall-shell is provided for backwards
compatibility and smooth legacy system upgrades because shorewall perl
is not fully compatible with all legacy configurations.

This package contains the files for the perl-based rule compiler.

Comment 1 Jonathan Underwood 2007-10-07 00:50:16 UTC
$ rpmlint -i ../RPMS/noarch/shorewall-perl-4.0.4-1.fc7.noarch.rpm 
shorewall-perl.noarch: E: useless-explicit-provides perl(Shorewall::Ports)
This package provides 2 times the same capacity. It should only provide it
once.

--> This is bogus, and is caused by a problem with the way rpm generates
automatic Provides. The package contains a perl script (buildports.pl) which
parses /etc/services and /etc/protocols to generate a module Ports.pm. This is
done at package build time. Because buildports.pl contains the text "package
Shorewall::Ports;" which it echo's out to Ports.pm during generation, RPM
believes that both Ports.pm and buildports.pl provide Shorewall::Ports. This
could be solved by not including buildports.pl in the package, but this file has
utility for people who make local mods to /etc/services or /etc/protocols.

shorewall-perl.noarch: W: empty-%pre
shorewall-perl.noarch: W: empty-%post
shorewall-perl.noarch: W: empty-%preun

--> These 3 can be ignored.

Comment 2 Jonathan Underwood 2007-10-07 00:56:51 UTC
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.

Comment 3 Ville Skyttä 2007-10-07 07:14:39 UTC
(In reply to comment #1)
> Because buildports.pl contains the text "package
> Shorewall::Ports;" which it echo's out to Ports.pm during generation, RPM
> believes that both Ports.pm and buildports.pl provide Shorewall::Ports.

rpmbuild doesn't pick up automatic perl Provides from anything else than *.pm,
see /usr/lib/rpm/{,redhat/}find-provides.  In this case the dupe comes from
Shorewall/FallbackPorts.pm, not buildports.pl.


Comment 4 Jonathan Underwood 2007-10-07 11:58:18 UTC
Thanks Ville, that is really helpful. Clearly there's no point packaging
FallbackPorts.pm, I'll remove that from the package.

As an aside, I wonder if it would be better to generate Ports.pm on installation
in the %post script, so that it takes into account any local modifications to
/etc/{services,protocols}. OTOH, do people ever edit those files locally?

Comment 5 Ville Skyttä 2007-10-07 13:39:54 UTC
Packages can add stuff to those files in their scriptlets, IIRC at least
something LTSP related did.  If you decide to generate the file in %post, be
sure to move the file somewhere in /etc or /var so it won't blow in in read-only
/usr setups (%_netsharedpath), and probably mark as %ghost %config as well as
decide what should happen to an existing Ports.pm on upgrades.

Comment 6 Jonathan Underwood 2007-10-07 23:37:30 UTC
Hi Ville - I can see a way of possibly doing that by having a symlink to
/var/lib/shorewall-perl/Ports.pm in
/usr/share/shorewall-perl/Shorewall/Ports.pm, and creating
/var/lib/shorewall-perl/Ports.pm in %post on package install or upgrade. Would
that be ok?

However, I am sure there is some aspect of the issue I am missing, because doing
an installation/upgrade of an rpm with a read only /usr wouldn't work anyway,
and so I don't actually understand what is wrong with creating Ports.pm
somewhere on /usr? [Please note, I'm not challenging what you say, as I am sure
you're right, but there is something I am missing]

However, this problem has seemingly larger scope. Take your LTSP example - if
the user installs shorewall, and then LTSP which modifies
/etc/{services,protocols}, the Ports.pm would then be incorrect until shorewall
is upgraded again. The only way around that would be with triggers, which is...
horrific...

So all in all, I can't help thinking the simple solution of creating Ports.pm at
build time from the build root /etc/{services,protocols} is probably as good as
we can get. Shouldn't the LTSP packager be getting his changes to these files
incororated into the setup package, rather than changing those files on package
installation?



Comment 7 Jonathan Underwood 2007-10-07 23:38:03 UTC
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 multitarball package will continue in
BZ #321731

Comment 8 Ville Skyttä 2007-10-08 06:48:38 UTC
(In reply to comment #6)
> 
> However, I am sure there is some aspect of the issue I am missing, because doing
> an installation/upgrade of an rpm with a read only /usr wouldn't work anyway,
> and so I don't actually understand what is wrong with creating Ports.pm
> somewhere on /usr? [Please note, I'm not challenging what you say, as I am sure
> you're right, but there is something I am missing]

You're missing what %_netsharedpath does.  For example %_netsharedpath /usr
makes rpm not drop any files under /usr but just assume the files it'd drop
there are already there (useful for eg. a central NFS-shared /usr - the server
installs rpms with files there, clients install the same RPMs locally but don't
touch anything in /usr).  However, %_netsharedpath only affects files in package
payloads, scriptlets need to be taken care of by packagers.

> Shouldn't the LTSP packager be getting his changes to these files
> incororated into the setup package, rather than changing those files on package
> installation?

Possibly (but that might be harder than it seems).  But that's not my point, the
point is that more that those files are marked as %config and thus supposedly...
configurable, modifiable.

Comment 9 Jonathan Underwood 2007-10-08 23:46:11 UTC
(In reply to comment #8)
> (In reply to comment #6)
> > 
> > However, I am sure there is some aspect of the issue I am missing, because doing
> > an installation/upgrade of an rpm with a read only /usr wouldn't work anyway,
> > and so I don't actually understand what is wrong with creating Ports.pm
> > somewhere on /usr? [Please note, I'm not challenging what you say, as I am sure
> > you're right, but there is something I am missing]
> 
> You're missing what %_netsharedpath does.  For example %_netsharedpath /usr
> makes rpm not drop any files under /usr but just assume the files it'd drop
> there are already there (useful for eg. a central NFS-shared /usr - the server
> installs rpms with files there, clients install the same RPMs locally but don't
> touch anything in /usr).  However, %_netsharedpath only affects files in package
> payloads, scriptlets need to be taken care of by packagers.
> 

Aha, crystal clear, thanks for taking the time to explain.

> > Shouldn't the LTSP packager be getting his changes to these files
> > incororated into the setup package, rather than changing those files on package
> > installation?
> 
> Possibly (but that might be harder than it seems).  But that's not my point, the
> point is that more that those files are marked as %config and thus supposedly...
> configurable, modifiable.

OK, I understand. I think in this case Ports.pm doesn't need %config though - it
should certainly be in /var/lib/shorewall-perl but it isn't editable - it merely
reflects local state.


Note You need to log in before you can comment on or make changes to this bug.