[Bug 460887] Review Request: libpcapnav - a libpcap trace file navigation library
bugzilla at redhat.com
bugzilla at redhat.com
Sat Oct 11 10:52:43 UTC 2008
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=460887
Michael Schwendt <bugs.michael at gmx.net> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|nobody at fedoraproject.org |bugs.michael at gmx.net
Flag| |fedora-review?,
| |needinfo?(christian at whoop.o
| |rg)
--- Comment #2 from Michael Schwendt <bugs.michael at gmx.net> 2008-10-11 06:52:41 EDT ---
> %define prefix /usr
Please give the rationale for making this package relocatable:
https://fedoraproject.org/wiki/Packaging/ReviewGuidelines
Note that in your case it is a mistake:
pcapnav-config isn't relocatable.
> $ rpmlint libpcapnav-0.8-1.src.rpm
> libpcapnav.src:18: W: hardcoded-packager-tag Christian
Don't set "Packager". Your name appears in the %changelog.
Setting the Packager tag should only be done in your local
build-system for binary rpms *you* have built. Publishing
src.rpms with a default Packager tag bears the risk that
other people build and publish bad binary rpms with your
name in the package header.
> libpcapnav.src:20: W: hardcoded-prefix-tag %{prefix}
See top of review.
> libpcapnav.src:47: W: configure-without-libdir-spec
Prefer the %configure macro where it works.
Run "rpm --eval %configure" to see what it does.
It also defines libdir as necessary.
> libpcapnav.src:82: W: macro-in-%changelog prefix
Avoid macros in %changelog. Some cause damage when they expand.
You can escape them with a double %% as in %%prefix.
> libpcapnav.src: W: no-version-in-last-changelog
It's preferred if you hardcode the package version-release
at the right of every changelog entry. Like:
* Tue Sep 02 2008 Christian Kreibich <christian at whoop.org> - 0.8-1
- Fix /usr/lib/ file list.
> $ rpmlint libpcapnav-devel-0.8-1.i386.rpm
> libpcapnav-devel.i386: W: no-documentation
This can be ignored.
> libpcapnav-devel.i386: E: library-without-ldconfig-postin
> /usr/lib/libpcapnav.so.0.0.0
> libpcapnav-devel.i386: E: library-without-ldconfig-postun
> /usr/lib/libpcapnav.so.0.0.0
These errors are only because your %files section is wrong.
The *.so.* files belong into the main package, not the -devel pkg.
> libpcapnav-devel.i386: W: no-version-dependency-on libpcapnav/libpcapnav-libs/liblibpcapnav 0.8
Most -devel packages must
Requires: %{name} = %{version}-%{release}
so main pkg and -devel pkg are kept in sync for any changes.
> libpcapnav-devel.i386: W: summary-ended-with-dot Development
> and documentation files for libpcapnav.
Most times "Summary" is not a full sentence. It's preferred to
not end it with a dot.
> $ rpmlint libpcapnav-0.8-1.i386.rpm
> libpcapnav.i386: E: zero-length /usr/share/doc/libpcapnav-0.8/NEWS
> libpcapnav.i386: E: explicit-lib-dependency libpcap
See comment on -devel pkg. This must be versioned.
> %define version 0.8
Just do
Version: 0.8
instead of redefining %version earlier.
> Release: 1
Using the %{dist} macro is not mandatory, but may be helpful if
using exactly the same src.rpm for several Fedora releases.
-> Release: 1%{?dist}
> # Using FTP to get around SourceForge's habit of adding something after .tar.gz
> Source: ftp://ftp.sf.net/pub/sourceforge/netdude/%{name}-%{version}.tar.gz
There are recommendations on working http links somewhere in the
Fedora Packaging Wiki.
> Requires: libpcap
Delete this. There is an automatic dependency on the
libpcap SONAME added by rpmbuild. We rely on these dependencies.
Query the libpcapnav package to see.
> %description devel
> Static libraries, header files and documentation
> for libpcapnav
We don't build and include static libs unless there is very good
reason to do that. Libtool archives (*.la) should be deleted, too.
> %build
Build log contains many format string type warnings, e.g.
using %u for long int.
> if [ "$SMP" != "" ]; then
There is %{?_smp_mflags} that can simply be appended to "make".
> make prefix=$RPM_BUILD_ROOT%{prefix} install
Use: make DESTDIR=$RPM_BUILD_ROOT install
Can the tests be run in the %check section of the spec?
Should they pass?
$ ./run-tests.sh
Running test 1 ... FAILED.
> %files
> %defattr(-,root,root,-)
> %doc AUTHORS COPYING ChangeLog NEWS README
> %{prefix}/lib/lib*.so.*
This would break on 64-bit multiarch platforms where there
is /usr/lib64 (and /usr/lib for 32-bit libs). Hence use:
%{_libdir}/*.so.*
> %files devel
> %defattr(-,root,root,-)
> %doc %{prefix}/share/gtk-doc/html/pcapnav
This is empty and only adds empty directories to the pkg.
> %{prefix}/lib/libpcapnav*
%_libdir
and include only the *.so symlink here:
%{_libdir}/*.so
> %{prefix}/include/pcapnav.h
%_includedir
> %{prefix}/bin/pcapnav-config
%_bindir
> drwxr-xr-x /usr/share/gtk-doc/html/pcapnav
> drwxr-xr-x /usr/share/gtk-doc/html/pcapnav/images
> $ pcapnav-config --cflags --libs
> -I/usr/include
> -L/usr/lib -lpcapnav -lpcap
The -I and -L here are bad as they alter the search order.
Can you make it not override default search paths with
default search paths?
Hope this catches all packaging issues. But only an updated src.rpm
will tell.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
More information about the package-review
mailing list