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 210187 (libassa) - Review Request: libassa - C++ Object-Oriented network library
Summary: Review Request: libassa - C++ Object-Oriented network library
Keywords:
Status: CLOSED RAWHIDE
Alias: libassa
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michael Schwendt
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT granule
TreeView+ depends on / blocked
 
Reported: 2006-10-10 17:45 UTC by Vladislav Grinchenko
Modified: 2007-11-30 22:11 UTC (History)
0 users

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-11-19 11:41:23 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
improved libassa.spec (4.37 KB, text/plain)
2006-10-15 23:44 UTC, Michael Schwendt
no flags Details

Comment 1 Vladislav Grinchenko 2006-10-10 17:48:24 UTC
The library's home page is: http://libassa.sourceforge.net/

It has been distributed as RPM packages for RedHat and Fedora for a number of
years. I want to ease end-user installation with `yum' via Fedora extras.

Comment 2 Vladislav Grinchenko 2006-10-10 18:02:04 UTC
This is my first package and I seek a sponsor.

Comment 3 Michael Schwendt 2006-10-10 19:05:37 UTC
> %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.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.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.

Comment 4 Vladislav Grinchenko 2006-10-12 21:15:20 UTC
Michael,

thanks for your comments. I have made all of the recommended modifications,
rebuilt the package on my local system and verified that it at least builds and
installs with no errors.

Please, find the latest version of the spec file, assa-3.4.2-2.spec, at the
aforementioned Spec URL.

thanks for your detailed comments,
-Vlad

Comment 5 Vladislav Grinchenko 2006-10-14 02:38:28 UTC
I have updated (Fri Oct 13 22:26 EDT 2006) both the .spec and .src.rpm again. It
should be sufficient enough to get going with the acceptance process.

Can someone sponsor this project?

thanks,
-Vlad

Comment 6 Jason Tibbitts 2006-10-15 04:34:04 UTC
Vladislav, it is an unfortunate fact that our current procedures are geared more
towards someone who will maintain several packages and generally participate in
the process of reviewing other packages and general project maintenance.  This
leaves out upstream maintainers who would simply like their software to be
easily available to Fedora users.

One proposed solution to this involves finding an existing Fedora maintainer who
would like to co-maintain your package.  Let me see if I can help with that.  I
will post a message to fedora-extras-list and see if anyone is interested.

Comment 7 Michael Schwendt 2006-10-15 23:44:03 UTC
Created attachment 138539 [details]
improved libassa.spec

* Mon Oct 16 2006 Michael Schwendt <mschwendt[AT]users.sf.net> - 3.4.2-3
- disable rpaths
- disable static libs
- build tests and add %%check section for them
- BR doxygen and fix inclusion of HTML files
- execute /sbin/ldconfig in scriptlets directly
- don't use %%makeinstall


* libassa-doc package built empty, missing BuildRequires doxygen

* -devel package contained the same %doc files as main package due to
unclean inclusion of %_docdir paths

* "make check" fails in one self-test -- see %check section of the spec!

* minor spec fixes - visible with diff ;)

Comment 8 Vladislav Grinchenko 2006-10-16 21:10:04 UTC
Michael, 

thanks for further editing of the spec file. I applied your changes and uploaded
new versions of both the spec file and SRPM to the URL above.

I don't want to deal with 'make check' for now 'cause its success highly depends
on various services available to the build host.

thanks,
-Vlad

Comment 9 Michael Schwendt 2006-10-16 23:45:06 UTC
Okay, 3.4.2-3 is good then and APPROVED. Just notice that if you
keep re-adding things like "summary-ended-with-dot" or re-naming
the spec file (ought to be %{name}.spec => libassa.spec), you won't
manage to make rpmlint shut up about things like that:  ;-)

  $ rpmlint libassa-3.4.2-3.src.rpm 
  E: libassa invalid-spec-name assa.spec
  W: libassa mixed-use-of-spaces-and-tabs (spaces: line 11, tab: line 38)

But this is not crucial and could be fixed in CVS any time.

[...]

Re-running the sighands_test suite consecutively makes Test 5
sometimes PASS and sometimes FAIL, which indicates that there's
a problem somewhere.

=== Test 5  failed: USR1 rcvd signalscount != 3 ===
=== Test 5  failed: USR1 rcvd signalscount != 3 ===
=== Test 5  failed: "C" rcvd signalscount != 3 ===



Comment 10 Vladislav Grinchenko 2006-10-17 21:44:25 UTC
Michael,

thanks for the APPROVAL.

I have renamed assa.spec to libassa.spec in CVS to take care of the error and
ran 'sed -i -e 's/\t/ /g' libassa.spec' to fix the warnings.

Both the spec and the SRPM have been updated accordingly.

Thanks for your help!
-Vlad


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