Bug 231746

Assigning to myself for review. Expect results early next week.

At the first glance: according to
http://fedoraproject.org/wiki/PackagingDrafts/SourceUrl?highlight=%28downloads.sourceforge.net%29#head-27442167fe28eb345470e8db56716d62b508978c
the recommended Source0 is
http://downloads.sourceforge.net/ettercap/ettercap-NG-%{version}.tar.gz

My, that was fast, the ink on the spec wasn't even dry yet. :)

Fixed.
Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-2.src.rpm

I would prefer to have at least two ettercap packages, one without the graphical
frontend and one with. Also you should add openssl to the BR according to the
homepage:

| If you want SSH1 and/or HTTPS support, ettercap requires OpenSSL libraries

And maybe also these two:
| To enable plugins: libltdl (part of libtool)
| To have perl regexp in the filters: libpcre

I've updated the BRs.  To create the two packages (ettercap and ettercap-gui?),
will I have to do two builds, as the text/curses/gtk interfaces are all called
by command line flags?  

(In reply to comment #5)
> I've updated the BRs.  To create the two packages (ettercap and ettercap-gui?),
> will I have to do two builds, as the text/curses/gtk interfaces are all called
> by command line flags?  

It seems to be so, debian does it this way. Configure --enable-gtk, make, cp
ettercap ettercap-gtk, make clean, configure --disable-gtk, make and creates
three packages:
ettercap
ettercap-gtk
ettercap-common

I guess ettercap and ettercap-gtk should both require ettercap-common and
conflict with each other, because both should install ettercap as /usr/sbin/ettercap

So should only /usr/sbin/ettercap be in ettercap and ettercap-gtk and the rest
in ettercap-common?

I've got the above finished, but no matter how I compile, the resulting binary
still requires a choice of interface set at the command line, -T, -C, or -G.  Is
this a candidate for Alternates, and if so how would that work?

(In reply to comment #7)
> So should only /usr/sbin/ettercap be in ettercap and ettercap-gtk and the rest
> in ettercap-common?

This sounds to me to be the best way.

(In reply to comment #8)
> I've got the above finished, but no matter how I compile, the resulting binary
> still requires a choice of interface set at the command line, -T, -C, or -G.  Is
> this a candidate for Alternates, and if so how would that work?

Alternatives would not help here. This helps only, when you may install several
packages, that contain e.g. a binary with the same name but different code. The
choice of interface is something that has to be changed in the source code of
ettercap, if you want to change it. A simple patch to the ec_parser.c file would
do it and a simple patch for ettercap.1.in to show that the -G option only works
with the -gtk package. I will attach a patch that selects the UI depending on
whether ettercap was invoked as ettercap-{text,gtk,curses} and demanding a UI
option when invoked as ettercap. The patch also makes ettercap display a warning
when someone wants to use "-G" and gtk support is not supported (same for
curses) and the display text only displays the supported options. Also in the
right places I changed GUI to UI in the help text. Maybe upstream agrees with
this patch, too.

The different packages help only to install ettercap without having to install
the gtk dependencies. 

In the gtk subpackage, there should also be a .desktop file and the regarding
scriptlets, I just realized.

Created attachment 149800 [details]
Makes ettercap only display supported UI options, fixes some typos and supports different names for the invocation

Wouldn't it be enough to install the binaries with different names (say
tettercap + ettercap -- as ethereal used to be packaged, or ettercap +
ettercap-gui) so that to avoid the need to make the text-only and the gtk
version conflict with each other ? I guess the help file could be left as it was
and included in the -common part... Of course the ettercap-gtk should pull in
the needed deps.

(In reply to comment #11)
> Wouldn't it be enough to install the binaries with different names (say
> tettercap + ettercap -- as ethereal used to be packaged, or ettercap +
> ettercap-gui) so that to avoid the need to make the text-only and the gtk
> version conflict with each other ? 

What is the advantage of this? The ettercap-gtk package includes all the
features, the ettercap package includes. So there is no need to install both
binaries.

> I guess the help file could be left as it was
> and included in the -common part... Of course the ettercap-gtk should pull in
> the needed deps.

Imho the help should be as accurate as possible. Do you mean only not to change
the manpage or also the commandline help?

(In reply to comment #12)

> What is the advantage of this? The ettercap-gtk package includes all the
> features, the ettercap package includes. So there is no need to install both
> binaries.

If there is a text-only subpackage that doesn't need any X library
it may be installed in a very minimal setup.

Advantage? Avoid conflicts. I very much prefer yum install to "yum install X;
oh, wait... conflict ? why, oh why ?"
I really have in mind the ethereal package, where the GUI and text-only versions
did not conflict.

As for help, I was thinking of the man pages only.


WRT to comment #7 and #9:
As far as I understand the setup (correct me if I am wrong) says "text only
binary in ettercap and gtk version + the rest in ettercap-common". This would
leave ettercap-common as the only package needed for graphic only installs. I
find the name a bit strange and not very explanatory. I would rather suggest
"ettercap" for the GUI version and "ettercap-tui" or "tettercap" for the text
only one. However, I think that splitting in three is a better choice
- text only rpm, including %{_bindir}/ettercap (+ Requires + ncurses) + man
pages if they are different to the GUI
- GUI, including %{_bindir}/ettercap-gtk + .desktop + scriptlets + Requires for
GUI + man pages if different to the text only
- common: the rest

(In reply to comment #14)
> Advantage? Avoid conflicts. I very much prefer yum install to "yum install X;
> oh, wait... conflict ? why, oh why ?"
> I really have in mind the ethereal package, where the GUI and text-only versions
> did not conflict.

Maybe then use "Provides: ettercap" in ettercap-gtk, so that when one runs
"yum install ettercap ettercap-gtk ettercap-common" only the ettercap-gtk and
the ettercap-comman package are installed? Or when the ettercap package is
already installed and one runs "yum install ettercap-gtk" the ettercap is
deinstalled and ettercap-gtk is installed. Maybe Conflicts is not the correct
rpm Tag. But the described beheaviour would be my favourite. If this is not
possible with RPM-Tags, than using alternatives would be the best.

> WRT to comment #7 and #9:

> only one. However, I think that splitting in three is a better choice

Three packages is what I want to suggest, too.

ettercap package:
Requires: ettercap-common
%{_bindir}/ettercap (without support for gtk)
%{_bindir}/ettercap-text (symlink to ettercap)
%{_bindir}/ettercap-curses (symlink to ettercap)

ettercap-gtk package:
Requires: ettercap-common
%{_bindir}/ettercap (with support for gtk)
%{_bindir}/ettercap-text (symlink to ettercap)
%{_bindir}/ettercap-curses (symlink to ettercap)
%{_bindir}/ettercap-gtk (symlink to ettercap)

ettercap-common package:
Everything else.

(In reply to comment #13)

> If there is a text-only subpackage that doesn't need any X library
> it may be installed in a very minimal setup.

This is the advantage of building different binaries and package them
differently. But not a advantage of avoiding conflicts.

(In reply to comment #15)

> ettercap-gtk package:
> Requires: ettercap-common
> %{_bindir}/ettercap (with support for gtk)
> %{_bindir}/ettercap-text (symlink to ettercap)
> %{_bindir}/ettercap-curses (symlink to ettercap)
> %{_bindir}/ettercap-gtk (symlink to ettercap)
Also the .desktop + scriptlets + Requires for
GUI + man pages if different to the text only should go into the -gtk package.

I'd like to avoid Conflicts as well, and honestly I think it'd be better to keep
the text/curses binary called ettercap, and rename the gtk version, since 9/10
users will be calling it from the .desktop anyway.  If the GIMP maintainers
renamed the main binary to fluffybunnyfeet, very few would notice.

I'm also somewhat loathe to depart from upstream unless really necessary.  While
I like the Busybox-esqe patch to handing the inferface flag issue, would this
not confuse existing ettercap users?  The patch appears only to add the new
functionality, and not break the existing arg handling.  IF this is correct,
then I see no issue.

Are we looking at a consensus around a 3-package plan, with symlinks, the above
patch, and Provides instead of Obsoletes?  If so, I'll get on it.

3 package plan: OK
No conflicts: OK
rename the gtk binary: OK
patch: if it works, fine by me
The only problem I see comes from the fact that both the GUI and non-GUI
packages would provide the symlinks (hence a conflict) and the "alternatives"
plan is meant to be used for the situation when multiple installed packages
provide the same capabilities. Since the gtk version also provides text
capabilities, there is not much sense in keeping the text version installed.

Why would both packages have to provide both symlinks?  If we only include the
GTK symlink in the GTK package, include the whole text version in the GTK
version, and use Provides to remove the text-only package, the problem goes
away, does it not?

>Why would both packages have to provide both symlinks? 
This was the suggestion in comment #15

As long as you allow for transitions from text only to -gtk AND back (imagine I
have installed the gtk version but I decide to settle for just ncurses), fine by me.

Will putting Provides: ettercap in both rpms do this?

As far as I know (by all means, please do correct me if I am wrong), there are
two options. I'll presume we use ettercap-tui as name of the text-only rpm,
ettercap-gui for the full GUI version and "ettercap" for the capability
provided; also the names of installed binaries are just for the sake of the
theory (on purpose using different names than the package in order to avoid
confusion)
a)
- ettercap-tui installes the binary as tettercap, ettercap-gtk installs it's
binary as gettercap
- both packages provide "ettercap"
- symlinks are treated by the alternatives system, so that which ever of the
packages is installed last will win (symlink-wise)
yum install ettercap will only install the -gtk version (shorter name)
Advantages:
- if someone wants to explicitely have any or both versions, there is no problem;
- both packages can be updated at will, as needed
Disadvantages:
- the presence of the text-only version is redundant if the full version exists. 
- require on alternatives
b)
- ettercap-tui installes the binary as tettercap, ettercap-gtk installs it's
binary as gettercap
- both provide "ettercap"
- ettercap-gtk obsoletes ettercap-tui
Advantage
- only one of the packages will be installed at a given time
Disadvantage
- unless special care is taken (yum --exclude ettercap-tui), a yum
update/upgrade will attempt to replace the text-only version with the GUI


In both cases "ettercap" could be the name of one of the packages.

(In reply to comment #22)

> b)

> - ettercap-gtk obsoletes ettercap-tui

> Disadvantage
> - unless special care is taken (yum --exclude ettercap-tui), a yum
> update/upgrade will attempt to replace the text-only version with the GUI

I don't think this is acceptable. There should not be a package and
another that obsoletes it in the same repo.

Second that 100%.
Which is why I think a) as a way better solution, unless I have missed something.

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-3.src.rpm

Split, with symlinks, .desktop and icon.  rpmlint still complains about
documentation.  Should I ignore this since it's in -common?

The more I think about it, the more I believe that a variant of comment #11 is
the simplest way to go:
- create 3 packages; ettercap-common, ettercap-gui and ettercap; last ones
should _both_ provide "ettercap"
- include in ettercap-common a README.FEDORA explaining that the -gui provides
all capabilities of the -tui + gtk interface; include here the command switches
needed  in order to use the -gui as a text only/ ncurses interface (we all know
that people do not read manuals, so including same info in two places won't
hurt; we might just get a supplemental chance to actually pass the info to the
end user) and the instructions on how to remove the text-only package if one
decides to use the gtk version
- ettercap would install in addition to the ettercap binary the -curses and
-text symlinks
- assuming that ettercap-gui would be installed by people who actually want a
GUI, a .desktop file will be installed which runs just that: the GUI. If the
user wants to use the -gtk enabled binary with a text only interface, the
instructions for achieving this are available both in the man page and in the
README.FEDORA doc.
- the description of ettercap (the text only package) should mention that, for a
GUI variant, "yum install ettercap-gui" should be used

Advantages
- no conflicts
- no need for alternatives
- the name being shorter, yum install ettercap would not pull the GUI + deps
- since the gtk version provides ettercap, the text-only package will not be
pulled once the gtk is installed

Opinions ?

WRT to comment #25: we must be twins, given the way we sync :)

I'll review that in a couple of hours

Crud. I knew I forgot something.  Here's a version with the proper Provides.  I
think.

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-4.src.rpm

I'll work on the README.fedora. Would that not be better placed in -gtk and
ettercap, so if fulfills the doc requirement, and we could have two, slightly
tailored?  Vanilla for ettercap, one explaining how to run text or curses with
the GTK version for -gtk.

wrt to comment #28: If you want it so, I will not object. But being in the
-common would be simpler and could/would explain the split. Plus it would not
create conflicts between the tui-fedora-readme and gui-fedora-readme.
As of doc requirements, we DO provide the common docs in the package which is
... surprise surprise... common. Given the split and the presence of docs in the
-common package, I see no reason for the binary packages to contain anything but
the specific man page(s).

(In reply to comment #28)
> Crud. I knew I forgot something.  Here's a version with the proper Provides.  I
> think.
> 
> Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
> SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-4.src.rpm

- %post and % postun should be %post gtk and %postun gtk, otherwise the will be
in the ettercap package.
- In oposite to my previous messages, the -curses and -text symlinks only need
to be in the -common package
- you add ettercap-gtk twice

In addition to the things that Till has noticed:

- %{_bindir}/ettercap is listed both under -common and in the main package
- -common should not provide "ettercap", we do not want it to be installed/left
without one of the other two
- in -common: no need to require glibc; few things would work without it :)
- in main package: no need to require glibc-devel; just like glibc it's on the
exception list ( http://fedoraproject.org/wiki/Extras/FullExceptionList )

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-5.src.rpm

30 and 31 are addressed.  I'll look at the README tomorrow.

I've just built the package in mock and run the output via rpmlint. I will not
post now a full review, because there are some changes to be done.

1. Please settle with either %{rpmbuildroot} or $RPM_BUILD_ROOT.
2. zlib-devel is a duplicate BR, openssl-devel already pulls it
3. desktop-file-utils is a needed BR in order to install the .desktop file

The explicit provides you have included are not needed. rpm will figure out
itself. And as far as I see from rpm -q --provides / rpm -q --requires, it looks
like a versioned require on ettercap-common is needed.

rpmlint gives a warning on the symlinks created in -common:
[wolfy@wolfy64 result]$ rpmlint ettercap-common-0.7.3-5.fc7.x86_64.rpm
W: ettercap-common dangling-relative-symlink /usr/bin/ettercap-text ./ettercap
W: ettercap-common dangling-relative-symlink /usr/bin/ettercap-curses ./ettercap


I have attached a revised version of ettercap-spec, please take a look at it.
Using it, the two main binary packages will give:
[wolfy@wolfy64 result]$ rpm -qp --provides ettercap-0.7.3-5.fc7.x86_64.rpm
ettercap = 0.7.3-5.fc7
[wolfy@wolfy64 result]$ rpm -qp --provides ettercap-gtk-0.7.3-5.fc7.x86_64.rpm
ettercap
ettercap-gtk = 0.7.3-5.fc7
I am almost sure that the unversioned provides in ettercap-gtk should be
modified to ettercap-name-version

Created attachment 150043 [details]
spec diff

Fixes unconsistent usage of RPM_BUILD_DIR/%{rpmbuilddir}, fixes BR and Requires

Forgot to mention, you should also look into using parallel make (
$RPM_OPT_FLAGS/%{optflags} )

I've applied your changes.  The reason the symlinks give that warning is because
it warned me to use relative paths, not absolute paths.  Should I go back to
absolute?

WRT comment #35: s/$RPM_OPT_FLAGS/%{optflags} / %{?_smp_mflags}/
(just add it to the make lines, works OK, just tested)

WRT comment #36: I think it's OK like that. The paths are resolved correctly at
install time. I hope I am not wrong...

Added smp flags.  WRT 36, You mean OK relative, or OK absolute?

OK relative. But I would like to hear a second opinion.

(In reply to comment #39)
> OK relative. But I would like to hear a second opinion.

Use relatives. Absolutes are bad, because they do not work whith anaconda / the
rescue system.

Ok.  Then here's the current iteration:
Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-6.src.rpm

Please keep in mind that you have to edit the .desktop file. It's content is
included in the final rpm as provided and because of the %datadir you have used
the path for the icon is not correct.


Full review in a while...

(In reply to comment #39)
> OK relative. But I would like to hear a second opinion.

Relative are best, and the rpmlint warning about dangling
symlinks is ignorable if it points to a file or dir which is
part of a package it depends on.

Fixed desktop file.

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-7.src.rpm

OK, here we go.
Between square brackets I have included references to items listed under
"Problems" below. Please observe the "MUSTFIX"es included over there.

GOOD:
- rpmlint output:
[wolfy@wolfy64 ettercap]$ rpmlint ~/ettercap-0.7.3-6.src.rpm
W: ettercap unversioned-explicit-provides ettercap
->I assume it's triggered by the line in "%package gtk" and can be safely ignored
[wolfy@wolfy64 ettercap]$ rpmlint ettercap-common-0.7.3-6.fc7.x86_64.rpm
W: ettercap-common dangling-relative-symlink /usr/bin/ettercap-text ./ettercap
W: ettercap-common dangling-relative-symlink /usr/bin/ettercap-curses ./ettercap
-> can be ignored, the target will be there when either ettercap or ettercap-gtk
is installed
[wolfy@wolfy64 ettercap]$ rpmlint ettercap-0.7.3-6.fc7.x86_64.rpm
[wolfy@wolfy64 ettercap]$ rpmlint ettercap-gtk-0.7.3-6.fc7.x86_64.rpm
W: ettercap-gtk no-documentation
-> can be ignored, docs are in -common

- package meets naming guidelines
- package meets packaging guidelines
- license (GPL) OK, text is included in %doc, matches source 
- spec file legible, in am. english
- source matches upstream, sha1sum 
7a2c3f848ca4f39c07fddeb0d6308641265bc4ff  ettercap-NG-0.7.3.tar.gz [1]
- package compiles on devel (x86)
- no missing BR [2]
- no unnecessary BR
- no locales
- not relocatable
- owns all files/directories that it creates, does not take ownership of foreign
files/folders
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- most docs are included in the -common package, except for the man page
specific to the ncurses interface [2]
- nothing in %doc affects runtime
- a .desktop file is conveniently installed for the gtk interface [3]
- no .la/static/pkconfig files
- scriptlets are sane


SHOULD:

OK: runs as advertised
OK: builds happily in mock for devel and fc6 (x86_64)
OK: require of subpackage is fully versioned


Problems

MUSTFIX: [1] - Timestamp of original file is not preserved in the src.rpm;
please use wget -N ( or similar tool ) to fix that;the timestamp of the man
pages is not preserved either
[2] - adding libtool-ltdl-devel as BR (instead of libtool) allows bulding of the
plugins. I have verified and there are no problems, they just need to be
included in a package. I suggest a 4th package (ettercap-plugins) for that (this
is a nice to have, technically is not a blocker, just a possible, easy to add
but missing feature)
MUSTFIX: [3] - the desktop file has an error: wrong path (includes an
unexpandable rpm macro) for the icon; just drop the full path, rpm will take
care at install time (the error is still present in the release 7 of the spec)
MUSTFIX: [4] - There is a whole folder with docs (/doc, huh), missing; I suggest
removing INSTALL, README.BINARIES, README.CVS and even README.PLATFORMS, they
provide no useful info for a final user who only wants to run the application.
MUSTFIX: the -common packages creates symlinks which are not valid unless the
text-only rpm is installed. This is not nice. I think that a bit of magic is
needed here. One fix could be to create the symlinks in %post in the text and
gtk rpms, and only if they do not exist already.
- you can dump "Provides ettercap" in the -gtk package. I have just tested after
a build without it and everything seems fine:
[root@wolfy64 x86_64]# yum provides ettercap
Loading "skip-broken" plugin
Loading "installonlyn" plugin
Setting up repositories
Reading repository metadata in from local files
updates   : ################################################## 1613/1613
core      : ################################################## 2931/2931
extras    : ################################################## 6071/6071
LOCALx86_6: ################################################## 5/5
ettercap-gtk.x86_64                      0.7.3-6.fc6            LOCALx86_64
Matched from:
ettercap
ettercap.x86_64                          0.7.3-6.fc6            LOCALx86_64
Matched from:
ettercap

(In reply to comment #33)
> [wolfy@wolfy64 result]$ rpm -qp --provides ettercap-gtk-0.7.3-5.fc7.x86_64.rpm
> ettercap
> ettercap-gtk = 0.7.3-5.fc7
> I am almost sure that the unversioned provides in ettercap-gtk should be
> modified to ettercap-name-version

I guess you mean it should be (I hope this is the correct syntax):
Provides: ettercap = %{version}-%{release}

Also the -common package should have:
Requires: ettercap-common = %{version}-%{release}

And from the description of the ettercap package:
| This package contains the NCURSES version.
Better change it to "This package supports the daemon mode and contains the text
and ncurses user interface."

(In reply to comment #45)

> [2] - adding libtool-ltdl-devel as BR (instead of libtool) allows bulding of the
> plugins. I have verified and there are no problems, they just need to be
> included in a package. I suggest a 4th package (ettercap-plugins) for that (this
> is a nice to have, technically is not a blocker, just a possible, easy to add
> but missing feature)

Please do this :-)

> MUSTFIX: the -common packages creates symlinks which are not valid unless the
> text-only rpm is installed. This is not nice. I think that a bit of magic is
> needed here. One fix could be to create the symlinks in %post in the text and
> gtk rpms, and only if they do not exist already.

Using alternatives seems not to be that complicated, too. Install ettercap in
the ettercap package to %{_bindir}/ettercap.ettercap and use this:

%post
%{_sbindir}/alternatives --install  %{_bindir}/ettercap ettercap
{_bindir}/ettercap.ettercap 30

%post gtk
%{_sbindir}/alternatives --install  %{_bindir}/ettercap ettercap
{_bindir}/ettercap-gtk 30

%preun
if [ "$1" = 0 ]; then
    {_sbindir}/alternatives --remove ettercap %{_bindir}/ettercap.ettercap
fi
%preun gtk
if [ "$1" = 0 ]; then
    {_sbindir}/alternatives --remove ettercap %{_bindir}/ettercap-gtk
fi

The ettercap and -gtk package also need:
Requires(post) %{_sbindir}/alternatives
Requires(preun) %{_sbindir}/alternatives

see man alternatives for help.

> - you can dump "Provides ettercap" in the -gtk package. I have just tested after

It still should be there, because the -common package should require
ettercap-version-release.

Fixed 2 (leaving a plugin package for later, perhaps), 3, 4.  Not sure how to do
1.  Is 5(symlinks) really an issue? Why would someone install ettercap-common
without ettercap or ettercap-gtk?
Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-8.src.rpm

(In reply to comment #47)

>  Is 5(symlinks) really an issue? Why would someone install ettercap-common
> without ettercap or ettercap-gtk?

You get the situation when you install ettercap and then deinstall it, because
ettercap-common is left when you do not add a Requires: ettercap-version-release

(In reply to comment #45)
> MUSTFIX: [1] - Timestamp of original file is not preserved in the src.rpm;
> please use wget -N ( or similar tool ) to fix that;the timestamp of the man
> pages is not preserved either

Afaik are the manpages gziped by rpm, this will always change the man pages
except to gzip them first. Is preserving the timestap for man pages really
neccessary?

WRT comment#47
Minor: The spec from release 8 has an unneeded BR (libtool, pulled by
libtool-ltdl-devel, as specified in comment $45 /2 ). 
Mustfix: If you do not include in a package the additional files ( %configure
sees the additional available libs and build the plugins by default) which are
generated by enabling ltdl, build is terminated with an error (unpackaged
files). Pay attention to the .la files which are generated during the build of
plugins, they must be removed.

WRT comment #48: I will not hold approval because of this, but preserving
timestamps is heavily recommended. I have to recheck why is the date modified.
AFAIK rpm by itself does not do that, simply because it calls gzip which
preserves timestamps. But not today, I really had a rough day today.

(In reply to comment #49)

> WRT comment #48: I will not hold approval because of this, but preserving
> timestamps is heavily recommended. I have to recheck why is the date modified.

Just a guess: They are rebuilt after the "make clean" which removes them.

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-9.src.rpm

- Removed libtool BR.
- Removed .la files.
- Moved plugins to subpackage.
- Re-added Provides to GTK package.

Seems all that remains is the symlink/alternatives issue.  Would alternatives
outright replace the symlinks?  I'm somewhat muddled, though the pre/post
scripts for it in 46 make sense.

WRT 50: I suppose that makes a solution to the timestamp issue moot.

You have forgotten to include the /doc folder in %doc (or at least explain why
it is not to be included; to me the files over there seem useful for people who
want to expand the existing capabilities of the program). While at the docs, I
think that including README is a good idea, as it talks about the license and
also gives some explanations on the internals of the program.

I've found the explanation of the modified timestamps for man pages: the
original tar.gz includes the man pages both as a final version and as automake
generated files; %configure overwrites the existing pages because it generates
them again:
Writing output files...

configure: creating ./config.status
config.status: creating Makefile
config.status: creating Makefile.mingw
config.status: creating man/Makefile
config.status: creating man/ettercap.8
config.status: creating man/ettercap_curses.8
config.status: creating man/ettercap_plugins.8
config.status: creating man/etterlog.8
config.status: creating man/etterfilter.8
config.status: creating man/etter.conf.5


The solution for solving the symlink/alternative issue is up to you, use
whatever looks cleaner. You can go straight ahead and use Till's suggestion
(don't forget to require alternatives in this case), or emulate alternatives
using %post ("if link exists, exit; otherwise create it) and %postun (if link
exists and points to self, remove). I guess alternatives is less error prone...

I fixed the doc and README.

I implemented Till's alternatives solution.  I'll post the resulting SRPM/SPEC,
but first, may I assume that the alternatives solution effectively deprecates
Till's patch https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=149800 ?

No, it does not deprecate the patch. The patch changes some strings that are
displayed (from GUI to UI) and gives nice error messages if ncurses / gtk are
not available when the user selects one of the -C/-G command line options.
Therefore please keep the patch.

Makes sense, I forgot about those aspects. Kept.

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-10.src.rpm

One small error at the first glance: you've split each one of the "alternatives
install" instructions on two lines, but forgot to end the first line with a
continuation marker.
A more important error are the typos in the same lines, each one of the
{_bindir} should have been %{_bindir}; same error is present in the
"alternatives remove" lines
Also, there is no need to use both "Requires: alternatives" and "Requires(post):
%{_sbindir}/alternatives / Requires(preun): %{_sbindir}/alternatives" (second
Requires line in the main package)


Since you'll have to edit the spec anyway, I have a few additional suggestions
(feel free to ignore them, they are just that, suggestions)
- in the %doc line, replace doc with doc/ in order to make it clear it's a directory
- rename the cryptic ettercap.ettercap to ettercap-tui
- dump the custom defined ettercapdir macro, it's used only once, in the %files
section, so there is no saving after all

Addressed all of the above.  I'll see if I can have 3/15 declared International
Typo Day. :)

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-11.src.rpm

All's well ... And the International Typo Day should be PI day (3.14).
Alternatives system working OK, cross dependencies seem OK, both binaries do
what they are supposed to do. Please think for a convenient Readme.Fedora and
have it prepared for inclusion when doing the CVS upload.


As far I can see, except for the timestamp of man pages which needs fiddling
with the configure script (I've spent half an hour with it but to no avail),
everything seems OK. Till, do you happen to spot anything else which requires
fixing ? 

(In reply to comment #58)

> everything seems OK. Till, do you happen to spot anything else which requires
> fixing ? 

From the RevieGuidelines:
- SHOULD: Usually, subpackages other than devel should require the base package
using a fully versioned dependency.

ettercap-common still does not require ettercap. Afaik this causes
ettercap-common beeing left on the system, when someone deinstalls ettercap and
ettercap-gtk.

I attach a patch that fixes this and also adds a fully versioned provides to the
-gtk package.

Created attachment 150165 [details]
modifies the requrires/provides

Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-12.src.rpm

Applied Till's patch and added a README.fedora.

OK, one more thing and I guess we are done. When deciding to go the
"alternatives" way we completely forgot that Till's patch treats the particular
cases when the program is lauched with the name of "ettercap-curses",
"ettercap-text" and "ettercap-gtk". Could you please expand the scriptlets for
alternatives to add (as slaves) the symlinks for -text and -curses?

The package is APPROVED, I trust that you'll correct the scriptlets before
uploading to CVS. Please also add a line in the readme.fedora file, explaining
that "alternatives --config ettercap" allows switching between default tui and
default gui if both packages are installed

Scriptlets corrected, README updated.  Thanks for the assistance.  This is
undoubtedly my most complex RPM to date.

New Package CVS Request
=======================
Package Name: ettercap
Short Description: Network traffic sniffer/analyser
Owners: limb
Branches: FC-5 FC-6
InitialCC: 

Built on FC-5, 6, devel.  FC-5 required modified BR.
Closing.

Not tested, but I think that
%if "0%{?dist}" == "0fc5"
BuildRequires: libpcap
%elif "0%{?dist}" == "0el4"
BuildRequires: libpcap
%else
BuildRequires: libpcap-devel
BuildRequires: libtool-ltdl-devel
%endif


would allow the same spec on all fedora releases and epel 4 (yes, I know
conditions for fc5 and epel4 could be summed; they are separate on purpose, in
case something else is still different)
Actually epel4 has almost the same requirements with fc5, plus a conditional for
the non-existent libtool-ltdl-devel. I have built and tested with:
%if "0%{?dist}" != "0el4"
BuildRequires: libtool-ltdl-devel
BuildRequires: libpcap
%endif
I have no fc5 around so I did not build/test for that.

I haven't FC5 either.  I'll update this for the next release, unless you think
this'll get pulled into epel before that and merits a rebuild.

I for one find ettercap to be a valuable tool. As I live in a Centos 3/4 world,
adapting ettercap for it was both fun and useful. It's up to you to decide if
you want to include & maintain it in EPEL (if not, I volunteer to do that, if
you agree). I would not wait for next release for the simple reason that the
program is quite stable and releases are rare.
I have attached my patch (cleaned a bit) which allows build in Centos 4. Beware,
it probably needs further adjusting in order to accomodate fc5 and epel5.

Created attachment 150460 [details]
adjusts spec file for epel4

As I'm not in a position to test on RHEL or CentOS (though I wish I was), I'll
let you take this to EPEL.  I'll apply your patch, and get it to work with fc5
and epel4.  If you get it to work with epel5, feel free to submit a patch and
I'll add that to the Fedora version, so we can keep this as consistent as
possible from version to version.

I'll be glad to maintain it in EPEL. However, please wait a while because
- the current version of the patch completely removes the plugins in EPEL4
- I have no EL 5 currently
- I am a bit busy with creating a stripped down Centos3 for an embedded router
(no HDD)
I'll ping you once the patch is in better shape.

Ok. I'll hold off then.

Package Change Request
======================
Package Name: ettercap
New Branches: EL-4 EL-5

el branches made