Bug 165963 - Review Request: libnet. Packet construction and injection library
Review Request: libnet. Packet construction and injection library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Paul Howarth
David Lawrence
http://www.packetfactory.net/libnet/
:
Depends On:
Blocks: FE-ACCEPT 165964
  Show dependency treegraph
 
Reported: 2005-08-15 07:25 EDT by Patrice Dumas
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2005-09-12 17:26:26 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora‑cvs+


Attachments (Terms of Use)
Update + Split into -devel, -samples and the main package (2.57 KB, patch)
2005-08-22 17:42 EDT, Oliver Falk
no flags Details | Diff
Patch implementing review suggestions (2.48 KB, patch)
2005-08-30 10:43 EDT, Paul Howarth
no flags Details | Diff

  None (edit)
Description Patrice Dumas 2005-08-15 07:25:21 EDT
SRPM Name or Url: http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.1.2-1.src.rpm
Description:

Libnet is an API to help with the construction and handling of network packets.
It provides a portable framework for low-level network packet writing and
handling (use libnet in conjunction with libpcap and you can write some really
cool stuff).  Libnet includes packet creation at the IP layer and at the link
layer as well as a host of supplementary and complementary functionality.
Libnet is very handy with which to write network tools and network test code.
See the manpage and sample test code for more detailed information.
Comment 1 Patrice Dumas 2005-08-15 07:27:24 EDT
Sorry, a typo in the srpm url the right is:

http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.2.1-1.src.rpm
Comment 2 Patrice Dumas 2005-08-15 07:33:36 EDT
I borrowed the spec from http://manta.univ.gda.pl/~mgarski/RPMS/libnet/

libnet is allready in the extras cvs. There are 2 libnet, one is libnet10 which
coresponds with the previous libnet version and has a sligtly different API. It
is a published package.

The other is called libnet it corresponds with the same version than libnet10
but has only entries for RHL-8 and RHL-9.
Comment 3 Patrice Dumas 2005-08-15 08:33:15 EDT
It is a prerequisite for intuitively
Comment 4 Oliver Falk 2005-08-22 07:59:47 EDT
Good:
 - rpmlint is quiet
 - License is OK

To be changed:
 - Make should be: make %{?_smp_mflags}
   (Works at least with 1.1.3 RC

Ideas:
 - Subpackage a -devel package
 - Subpackage a -sample package or put the samples in the -devel package

Everything else seems to be fine.
Comment 5 Patrice Dumas 2005-08-22 12:33:09 EDT
I put everything in a -devel package since there are no binaries nor dynamic
libraries.

rpmlint gave a lot of complaints on the rpm file. I cleaned everything except
the warning about the end of files.

http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.2.1-2.src.rpm
Comment 6 Paul Howarth 2005-08-22 13:20:33 EDT
(In reply to comment #5)
> rpmlint gave a lot of complaints on the rpm file. I cleaned everything except
> the warning about the end of files.

You can fix that by adding to %setup:

# Fix DOS line-ends
sed -i -e 's/\r$//' doc/{CHANGELOG,CONTRIB}


Comment 7 Patrice Dumas 2005-08-22 14:30:56 EDT
Thanks, I used it. I also added a Provides of libnet in libnet-devel.
It is in: 

http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.2.1-3.src.rpm
Comment 8 Oliver Falk 2005-08-22 17:42:09 EDT
Created attachment 117984 [details]
Update + Split into -devel, -samples and the main package

This patch is a suggestion. I merged your spec with mine, but please keep in
mind, that I allready have a package for 1.1.3_RC_01.
Comment 9 Patrice Dumas 2005-08-26 06:36:41 EDT
I have some comments about your patch

* you shouldn't use perl but sed
* is the doxygen.conf file really needed?

I used pushd/popd from your patch.

Otherwise I guess the dynamic libraries and installation cleanups are only in
1.1.3. 

(and I think unified diffs are more handy, I couldn't apply yours)

 
Comment 10 Oliver Falk 2005-08-26 06:50:18 EDT
(In reply to comment #9)
> I have some comments about your patch
> 
> * you shouldn't use perl but sed

I'm a perl-fan :-) But of course sed would be better...

> * is the doxygen.conf file really needed?

No it is not - you're correct.

> I used pushd/popd from your patch.

Fine.

> Otherwise I guess the dynamic libraries and installation cleanups are only in
> 1.1.3. 

Yes, they are only in 1.1.3, but I hope/guess 1.1.3 will come out soon. You can
also think about building the RC 1.1.3 for -devel only...

> (and I think unified diffs are more handy, I couldn't apply yours)

Next time... :-)

-of
Comment 11 Patrice Dumas 2005-08-26 09:10:45 EDT
I think we should wait for 1.1.3 release to package it. 
I still think the samples are best in -devel, in my opinion they are very
usefull as documentation and tiny.

Here is the srpm with pushd/popd:
http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.2.1-4.src.rpm
Comment 12 Oliver Falk 2005-08-29 03:49:36 EDT
This seems fine to me now. Maybe Paul has something more to say? If not, it's
approved from my side.

Paul, could you review it once more and approve it - as I'm quite busy currently
and don't have time to fully review it - Merci.
Comment 13 Paul Howarth 2005-08-30 10:43:14 EDT
Created attachment 118251 [details]
Patch implementing review suggestions

Review:

- rpmlint clean
- package naming OK
- specfile name OK
- package meets guidelines (static library included because dynamic not yet
  available)
- license is BSD, matches spec, text included
- spec file written in English and is legible
- sources match upstream
- package builds OK in FC4 and in mock for rawhide (i386)
- no explicit BRs
- no locales to worry about
- no shared libraries
- not relocatable
- no directory ownership or permissions issues
- no duplicate files
- %clean section present and correct
- code, not content
- docs not large enough to warrant a -docs subpackage
- docs don't affect runtime
- everything in -devel subpackage
- no libtool archives
- no scriptlets

Needswork:

- macro usage is inconsistent - use $RPM_BUILD_ROOT or %{buildroot} throughout
  rather than using both in different places

Nitpicks:

- try to be consistent in the usage of tabs and spaces for lining up the tags
  at the top of the specfile - I suggest all spaces because not everyone uses
  the same tab settings
- try to avoid using the %{name} macro except where it's clearly a generic
  usage, e.g. in the BuildRoot definition and the Provides: entry;
  substituting the actual package name results in a clearer spec file
- the "pushd sample; make clean; popd" construct can be simplified to
  "make -C sample clean"
- removal of CVS directories and other shouldn't-have-been-in-tarball junk is
  best done in %prep

Note for anyone else watching:

- this package contains a static library and a bunch of include files and some
  docs, all of which belong in a -devel package. The "main" package therefore
  contains no files and only a -devel binary subpackage is built. The -devel
  subpackage "provides" the main package for the time being, for forward
  compatibility reasons.


Attached patch addresses issues raised in this comment.
Comment 14 Patrice Dumas 2005-08-30 15:11:37 EDT
Much thanks for your nice review and patch. I applied it, the result is there:

http://www.environnement.ens.fr/docs/fc-srpms/libnet-1.1.2.1-5.src.rpm
Comment 15 Paul Howarth 2005-08-31 05:17:21 EDT
Approved.
Comment 16 Michael Schwendt 2005-08-31 06:53:20 EDT
Veto. Does it still conflict with libnet10? Then either libnet10 needs the
fix/work-around I've prepared in CVS (devel), or libnet-devel must obsolete
libnet10 which is not used by any other package anymore. Also notice my
cvs commit comment for that.
Comment 17 Patrice Dumas 2005-08-31 17:48:18 EDT
I agree that this issue must be solved somehow. libnet10 is deprecated, so one
may think that it is a good idea to let it be obsoleted. However there may still
be softwares that use the old interface, for example the previous release of
intuitively used that interface, until sept 2004. I don't know of security issue
with libnet10. Forcing upstream to use the new interface shouldn't be that bad,
though. 

Maybe the best thing would have been to have parallel installable libs, but as
it is not the case I am wondering whether it wouldn't be better to have
conflicts: tags such that users are forced to remove libnet10 by hand, let some
time flow and change conflicts to obsoletes?
Comment 18 Michael Schwendt 2005-09-01 09:48:57 EDT
Well, the change I've applied to libnet10 in devel CVS is to add
versioned "Provides" for libnet and libnet-devel. With them, your
new libnet-devel package is seen as an update/upgrade. If you
agree with that, somebody would just need to push updates for
libnet10 for devel (and possibly also update FC-4 and FC-3 like
that).
Comment 19 Patrice Dumas 2005-09-02 03:42:10 EDT
Yes I agree. After searching a bit on the web it seems that there is a debian
patch for dsniff and for snort the issue has been raised. I don't know other
packages using libnet.
Comment 20 Patrice Dumas 2005-09-02 03:51:52 EDT
In fact there are many more but I think they should be taken up to date as all
seem a bit unmaintained.
Comment 21 Michael Schwendt 2005-09-02 06:30:49 EDT
As a buildreq, both package versions can still be used, so libnet10 would still
be available (if needed).
Comment 22 Patrice Dumas 2005-09-02 17:52:41 EDT
Yes, just go on and build a new version of libnet10, afterwards I'll commit this
version. Maybe I could add something like that in the %changelog:

- this libnet package updates libnet10 
Comment 23 Michael Schwendt 2005-09-03 09:30:00 EDT
updated libnet10 packages been done for: devel, FC-4, FC-3
Comment 24 Patrice Dumas 2005-09-03 11:30:17 EDT
Can I import and build libnet now?
Comment 25 Patrice Dumas 2005-09-12 05:52:19 EDT
Is that package approved?
Comment 26 Michael Schwendt 2005-09-12 07:46:48 EDT
Comment 15 said so, and the issue with libnet10 has been resolved.
Comment 27 Patrice Dumas 2007-03-20 18:19:22 EDT
Please add
Branches: EL-4 EL-5
and add to owners:
jwilson
Comment 28 Jens Petersen 2007-03-20 20:44:23 EDT
I just updated PackageMaintainers/CVSAdminProcedure to clarify the procedure.
Sorry, but could you please use the new Package Change Request template to specify
the changes you want, since it is not completely clear if the owners change is for
both Fedora and EPEL; and then set fedora-cvs to '?' again.  Please use the
bugzilla email address of jwilson too.

Thanks.
Comment 29 Patrice Dumas 2007-03-21 15:34:00 EDT
Package Change Request
======================
libnet:
New Branches: EPEL-4, EPEL-5
Updated EPEL Owners: pertusus [AT] free dot fr, jwilson [AT] redhat dot com
Comment 30 Jens Petersen 2007-03-21 21:04:27 EDT
thanks, done
Comment 31 Jens Petersen 2007-03-21 21:12:35 EDT
Just for the future if you can write it like this it would be even better:

Package Change Request
======================
Package Name: libnet
New Branches: EL-4 EL-5
Updated EPEL Owners: pertusus [AT] free dot fr, jwilson [AT] redhat dot com

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