Bug 231746 - Review Request: ettercap - Network traffic sniffer/analyser
Review Request: ettercap - Network traffic sniffer/analyser
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: manuel wolfshant
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-10 20:51 EST by Jon Ciesla
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-19 13:06:13 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wolfy: fedora‑review+
petersen: fedora‑cvs+


Attachments (Terms of Use)
Makes ettercap only display supported UI options, fixes some typos and supports different names for the invocation (2.65 KB, patch)
2007-03-11 18:10 EDT, Till Maas
no flags Details | Diff
spec diff (2.00 KB, application/octet-stream)
2007-03-14 11:15 EDT, manuel wolfshant
no flags Details
modifies the requrires/provides (698 bytes, patch)
2007-03-15 16:05 EDT, Till Maas
no flags Details | Diff
adjusts spec file for epel4 (2.30 KB, patch)
2007-03-20 00:52 EDT, manuel wolfshant
no flags Details | Diff

  None (edit)
Description Jon Ciesla 2007-03-10 20:51:41 EST
Spec URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap.spec
SRPM URL: http://zanoni.jcomserv.net/extras/ettercap/ettercap-0.7.3-1.src.rpm
Description: Ettercap is a suite for man in the middle attacks on LAN. It features
sniffing of live connections, content filtering on the fly and many other
interesting tricks. It supports active and passive dissection of many
protocols (even ciphered ones) and includes many feature for network and host
analysis.
Comment 1 manuel wolfshant 2007-03-10 21:02:29 EST
Assigning to myself for review. Expect results early next week.
Comment 3 Jon Ciesla 2007-03-10 21:26:43 EST
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
Comment 4 Till Maas 2007-03-11 07:23:05 EDT
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
Comment 5 Jon Ciesla 2007-03-11 14:28:57 EDT
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?  
Comment 6 Till Maas 2007-03-11 14:57:53 EDT
(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
Comment 7 Jon Ciesla 2007-03-11 15:01:36 EDT
So should only /usr/sbin/ettercap be in ettercap and ettercap-gtk and the rest
in ettercap-common?
Comment 8 Jon Ciesla 2007-03-11 16:40:39 EDT
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?
Comment 9 Till Maas 2007-03-11 18:07:45 EDT
(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.
Comment 10 Till Maas 2007-03-11 18:10:05 EDT
Created attachment 149800 [details]
Makes ettercap only display supported UI options, fixes some typos and supports different names for the invocation
Comment 11 manuel wolfshant 2007-03-12 05:54:27 EDT
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.
Comment 12 Till Maas 2007-03-12 06:02:52 EDT
(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?
Comment 13 Patrice Dumas 2007-03-12 06:21:37 EDT
(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.
Comment 14 manuel wolfshant 2007-03-12 06:42:45 EDT
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
Comment 15 Till Maas 2007-03-12 07:53:07 EDT
(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.
Comment 16 Till Maas 2007-03-12 07:55:18 EDT
(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.

Comment 17 Jon Ciesla 2007-03-12 08:09:06 EDT
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.
Comment 18 manuel wolfshant 2007-03-12 08:38:40 EDT
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.
Comment 19 Jon Ciesla 2007-03-12 09:19:47 EDT
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?
Comment 20 manuel wolfshant 2007-03-12 09:25:53 EDT
>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.
Comment 21 Jon Ciesla 2007-03-12 09:33:18 EDT
Will putting Provides: ettercap in both rpms do this?
Comment 22 manuel wolfshant 2007-03-12 10:20:53 EDT
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.
Comment 23 Patrice Dumas 2007-03-12 11:39:35 EDT
(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.
Comment 24 manuel wolfshant 2007-03-12 11:46:44 EDT
Second that 100%.
Which is why I think a) as a way better solution, unless I have missed something.
Comment 25 Jon Ciesla 2007-03-13 14:48:24 EDT
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?
Comment 26 manuel wolfshant 2007-03-13 14:57:12 EDT
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 ?
Comment 27 manuel wolfshant 2007-03-13 14:58:59 EDT
WRT to comment #25: we must be twins, given the way we sync :)

I'll review that in a couple of hours
Comment 28 Jon Ciesla 2007-03-13 15:21:31 EDT
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.
Comment 29 manuel wolfshant 2007-03-13 15:30:44 EDT
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).
Comment 30 Till Maas 2007-03-13 17:03:44 EDT
(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
Comment 31 manuel wolfshant 2007-03-13 18:11:44 EDT
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 )
Comment 32 Jon Ciesla 2007-03-13 23:43:04 EDT
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.
Comment 33 manuel wolfshant 2007-03-14 11:11:58 EDT
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
Comment 34 manuel wolfshant 2007-03-14 11:15:28 EDT
Created attachment 150043 [details]
spec diff

Fixes unconsistent usage of RPM_BUILD_DIR/%{rpmbuilddir}, fixes BR and Requires
Comment 35 manuel wolfshant 2007-03-14 11:26:53 EDT
Forgot to mention, you should also look into using parallel make (
$RPM_OPT_FLAGS/%{optflags} )
Comment 36 Jon Ciesla 2007-03-14 11:32:31 EDT
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?
Comment 37 manuel wolfshant 2007-03-14 11:50:20 EDT
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...
Comment 38 Jon Ciesla 2007-03-14 11:55:38 EDT
Added smp flags.  WRT 36, You mean OK relative, or OK absolute?
Comment 39 manuel wolfshant 2007-03-14 12:01:28 EDT
OK relative. But I would like to hear a second opinion.
Comment 40 Till Maas 2007-03-14 12:41:31 EDT
(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.
Comment 41 Jon Ciesla 2007-03-14 12:45:48 EDT
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
Comment 42 manuel wolfshant 2007-03-14 12:50:01 EDT
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...
Comment 43 Patrice Dumas 2007-03-14 13:23:18 EDT
(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.
Comment 45 manuel wolfshant 2007-03-14 14:53:33 EDT
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
Comment 46 Till Maas 2007-03-14 15:21:45 EDT
(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.

Comment 47 Jon Ciesla 2007-03-14 15:23:17 EDT
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
Comment 48 Till Maas 2007-03-14 15:43:36 EDT
(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?
Comment 49 manuel wolfshant 2007-03-14 16:19:35 EDT
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.
Comment 50 Till Maas 2007-03-14 16:28:24 EDT
(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.
Comment 51 Jon Ciesla 2007-03-15 08:11:55 EDT
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.
Comment 52 manuel wolfshant 2007-03-15 10:21:56 EDT
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...
Comment 53 Jon Ciesla 2007-03-15 11:33:26 EDT
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 ?
Comment 54 manuel wolfshant 2007-03-15 11:40:45 EDT
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.
Comment 55 Jon Ciesla 2007-03-15 11:43:22 EDT
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
Comment 56 manuel wolfshant 2007-03-15 13:00:47 EDT
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
Comment 57 Jon Ciesla 2007-03-15 13:59:28 EDT
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
Comment 58 manuel wolfshant 2007-03-15 15:16:20 EDT
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 ? 

Comment 59 Till Maas 2007-03-15 15:59:28 EDT
(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.
Comment 60 Till Maas 2007-03-15 16:05:05 EDT
Created attachment 150165 [details]
modifies the requrires/provides
Comment 61 Jon Ciesla 2007-03-16 08:28:28 EDT
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.
Comment 62 manuel wolfshant 2007-03-16 09:32:30 EDT
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?
Comment 63 manuel wolfshant 2007-03-16 09:36:25 EDT
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
Comment 64 Jon Ciesla 2007-03-16 09:48:07 EDT
Scriptlets corrected, README updated.  Thanks for the assistance.  This is
undoubtedly my most complex RPM to date.
Comment 65 Jon Ciesla 2007-03-16 09:49:58 EDT
New Package CVS Request
=======================
Package Name: ettercap
Short Description: Network traffic sniffer/analyser
Owners: limb@jcomserv.net
Branches: FC-5 FC-6
InitialCC: 
Comment 66 Jon Ciesla 2007-03-19 13:06:13 EDT
Built on FC-5, 6, devel.  FC-5 required modified BR.
Closing.
Comment 67 manuel wolfshant 2007-03-19 13:34:17 EDT
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.
Comment 68 Jon Ciesla 2007-03-19 13:37:20 EDT
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.
Comment 69 manuel wolfshant 2007-03-20 00:51:13 EDT
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.
Comment 70 manuel wolfshant 2007-03-20 00:52:20 EDT
Created attachment 150460 [details]
adjusts spec file for epel4
Comment 71 Jon Ciesla 2007-03-20 07:56:40 EDT
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.
Comment 72 manuel wolfshant 2007-03-20 08:07:34 EDT
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.
Comment 73 Jon Ciesla 2007-03-20 08:12:56 EDT
Ok. I'll hold off then.
Comment 74 Jon Ciesla 2007-05-21 13:59:18 EDT
Package Change Request
======================
Package Name: ettercap
New Branches: EL-4 EL-5
Comment 75 Jens Petersen 2007-05-26 23:34:43 EDT
el branches made

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