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
Summary: | Review Request: libnet. Packet construction and injection library | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Patrice Dumas <pertusus> | ||||||
Component: | Package Review | Assignee: | Paul Howarth <paul> | ||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> | ||||||
Severity: | medium | Docs Contact: | |||||||
Priority: | medium | ||||||||
Version: | rawhide | CC: | fedora-package-review, oliver | ||||||
Target Milestone: | --- | Flags: | petersen:
fedora-cvs+
|
||||||
Target Release: | --- | ||||||||
Hardware: | All | ||||||||
OS: | Linux | ||||||||
URL: | http://www.packetfactory.net/libnet/ | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2005-09-12 21:26:26 UTC | Type: | --- | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 163779, 165964 | ||||||||
Attachments: |
|
Description
Patrice Dumas
2005-08-15 11:25:21 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 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. It is a prerequisite for intuitively 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. 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 (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} 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 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.
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) (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 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 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. 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.
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 Approved. 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. 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? 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). 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. In fact there are many more but I think they should be taken up to date as all seem a bit unmaintained. As a buildreq, both package versions can still be used, so libnet10 would still be available (if needed). 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 updated libnet10 packages been done for: devel, FC-4, FC-3 Can I import and build libnet now? Is that package approved? Comment 15 said so, and the issue with libnet10 has been resolved. Please add Branches: EL-4 EL-5 and add to owners: jwilson 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. Package Change Request ====================== libnet: New Branches: EPEL-4, EPEL-5 Updated EPEL Owners: pertusus [AT] free dot fr, jwilson [AT] redhat dot com thanks, done 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 |