Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 165963 - Review Request: libnet. Packet construction and injection library
Summary: Review Request: libnet. Packet construction and injection library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Paul Howarth
QA Contact: David Lawrence
URL: http://www.packetfactory.net/libnet/
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 165964
TreeView+ depends on / blocked
 
Reported: 2005-08-15 11:25 UTC by Patrice Dumas
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-09-12 21:26:26 UTC
Type: ---
Embargoed:
petersen: fedora-cvs+


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

Description Patrice Dumas 2005-08-15 11:25:21 UTC
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 11:27:24 UTC
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 11:33:36 UTC
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 12:33:15 UTC
It is a prerequisite for intuitively

Comment 4 Oliver Falk 2005-08-22 11:59:47 UTC
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 16:33:09 UTC
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 17:20:33 UTC
(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 18:30:56 UTC
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 21:42:09 UTC
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 10:36:41 UTC
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 10:50:18 UTC
(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 13:10:45 UTC
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 07:49:36 UTC
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 14:43:14 UTC
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 19:11:37 UTC
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 09:17:21 UTC
Approved.

Comment 16 Michael Schwendt 2005-08-31 10:53:20 UTC
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 21:48:18 UTC
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 13:48:57 UTC
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 07:42:10 UTC
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 07:51:52 UTC
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 10:30:49 UTC
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 21:52:41 UTC
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 13:30:00 UTC
updated libnet10 packages been done for: devel, FC-4, FC-3


Comment 24 Patrice Dumas 2005-09-03 15:30:17 UTC
Can I import and build libnet now?

Comment 25 Patrice Dumas 2005-09-12 09:52:19 UTC
Is that package approved?

Comment 26 Michael Schwendt 2005-09-12 11:46:48 UTC
Comment 15 said so, and the issue with libnet10 has been resolved.


Comment 27 Patrice Dumas 2007-03-20 22:19:22 UTC
Please add
Branches: EL-4 EL-5
and add to owners:
jwilson

Comment 28 Jens Petersen 2007-03-21 00:44:23 UTC
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 19:34:00 UTC
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-22 01:04:27 UTC
thanks, done

Comment 31 Jens Petersen 2007-03-22 01:12:35 UTC
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.