[Bug 210187] Review Request: libassa - C++ Object-Oriented network library

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: libassa - C++ Object-Oriented network library


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=210187





------- Additional Comments From bugs.michael at gmx.net  2006-10-10 15:05 EST -------
> %define debug_package %{nil}

This must be removed. Disabling debuginfo packages is the wrong
thing to do.

> %define rel           2
> %define disttag       fc5
> %define release       %{rel}.%{disttag}

Overused macroism. %rel is used only once in the entire spec file.
%disttag serves no purpose since %{?dist} ought to be used, and
"Release" tag defines %release. Use just

Release: 	2%{?dist}

and expand %rel in the Source tag.

> Packager:	Vladislav Grinchenko (vld at users.sourceforge.net)
> Vendor:     3rdShift, Inc.

Set these always via ~/.rpmmacros instead. When set in a
spec file, anybody who would built non-working binary rpms would
pretend that they are from you. Further, the build system shall set
these.

> Source: 	%{name}-%{version}-%{rel}.tar.gz

Download URL is missing.

> Prefix: 	/usr

Doubtful. If this package shall be made reloctable, at least
use %{prefix} here instead of /usr.

> BuildRoot: 	/tmp/%{name}-%{version}-root

Not the recommended buildroot from the packaging guidelines.

> %package	devel
> Summary: 	Headers for developing programs with libassa library
> Group: 		Development/Libraries

Missing "Requires: %{name} = %{version}-%{release}"

>	CFLAGS="$RPM_OPT_FLAGS"           \
>		./configure  $ARCH_FLAGS      \
			--prefix=%{prefix}        \

Use the %configure macro instead of "./configure". It sets many other
parameters beyond --prefix, e.g. --libdir and --datadir.

> %install
> if [ -d $RPM_BUILD_ROOT ]; then rm -rf $RPM_BUILD_ROOT; fi

This is neither necessary nor safe. Just use "rm -rf $RPM_BUILD_ROOT".

> # new redhat versions don't use .la
> rm -f %{buildroot}%{_libdir}/*.la

Don't mix %{buildroot} and $RPM_BUILD_ROOT

> %post
> %preun
> %postun

With these scriptlets, the package is missing:

Requires(post): /sbin/install-info /sbin/ldconfig
Requires(preun): /sbin/install-info
Requires(postun): /sbin/ldconfig

> %postun
> /sbin/ldconfig
>
> #===============================================================================
> # clenup section
> #===============================================================================

Don't place any "#-----" comments directly after scriptlet sections.
They are included in the binary rpms.

Query your binary rpms with "rpm --query --scripts libassa" to see!

> %files
> %defattr(-, root, root)
>
> %doc AUTHORS COPYING ChangeLog INSTALL NEWS README

Verify whether the INSTALL file is relevant to RPM package users.
If it's the standard FSF file, it's irrevelant.

> %{prefix}/lib/*.so.*

This will be wrong on platforms where the library must be installed in
%{_libdir} instead, so use %{_libdir} instead of %{prefix}/lib

Same for -devel package.

> %files  devel
> %defattr(-, root, root, 755)

Any particular reason why %defattr(-,root,root,-) is not enough?

> %{prefix}/bin/*

Use %{_bindir}

> %{prefix}/include/assa-3.4

Use %{_includedir}

> %{prefix}/lib/pkgconfig/*.pc
> %{prefix}/lib/*.so

Use %{_libdir} and "Requires: pkgconfig"

> #%doc  AUTHORS COPYING ChangeLog INSTALL NEWS README
> %{prefix}/share/doc/*
> %{prefix}/share/doc/%{name}-%{version}/*

Files marked as %doc are included automatically in an internal _docdir
path, so it's weird to see files included here again. 

Further, %{prefix}/share is %{_datadir}, so prefer it.

> %files doc

Missing %defattr(-,root,root,-)

> %doc doc/html

> %changelog
> * Wed Jul 19 2006 Vladislav Grinchenko <vlg at users.sourceforge.net>
> - disabled tests and examples in configure step

As you add more changelog entries, don't forget to add the package
version and release to every entry.

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.