Bug 204579 - Review Request: libatomic_ops - Atomic memory update operations
Review Request: libatomic_ops - Atomic memory update operations
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-08-30 02:44 EDT by Pierre Ossman
Modified: 2008-04-14 20:22 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-09 14:10:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Pierre Ossman 2006-08-30 02:44:14 EDT
Spec URL: http://homes.drzeus.cx/~drzeus/atomic/libatomic_ops.spec
SRPM URL: http://homes.drzeus.cx/~drzeus/atomic/libatomic_ops-devel-1.1-1.src.rpm
Description:
Provides implementations for atomic memory update operations on a
number of architectures. This allows direct use of these in reasonably
portable code. Unlike earlier similar packages, this one explicitly
considers memory barrier semantics, and allows the construction of code
that involves minimum overhead across a variety of architectures.


As most of this library is done in its headers, the ABI isn't considered stable. The package therefore only builds to a devel package, containing headers and static libs. This is also how Debian packages it.
Comment 1 Hans de Goede 2006-09-09 04:43:57 EDT
I see that this indeed is a special package and that making a .so for it is of
of little use.

MUST:
=====
O rpmlint output is:
E: libatomic_ops-devel invalid-spec-name libatomic_ops.spec
Tihs must be fixed, see below
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok (but license file not included)
* spec file is legible and in Am. English.
* Source matches upstream (sortof)
* "Compiles" and builds on FC-5 i386
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* no -devel package needed, no libs / .la files.
* no .desktop file required


Must Fix
========
* "Name:" should be "libatomic_ops" not "libatomic_ops-devel", then
  add a subpackage -devel, with the same Summary and Description and
  only add a "%files devel" and not a normal "%files", then rpmbuild will only
  generate a -devel package
* Remove "%define debug_package %{nil}" code gets generated from both the .c
  and the .h files, people may need this for debugging
* Remove the execute rights from src/atomic_ops.h in "%prep", one we get a
  debuginfo rpmlint will start complaining about the unneeded execute rights, as
  these stay preserved in the debuginfo package
* Since you have no use for these remove them instead of commenting them: "
#BuildRequires:
#Requires:     "
* Once you've fixed the "Name:" drop the "-n xxx" argument to "%setup"
* Once you've fixed the "Name:" do a search and replace for atomic_ops with
  %{name}, everywhere except in the "URL:".


Should fix:
===========
* You can write all this:
%dir %{_includedir}/atomic_ops    
%{_includedir}/atomic_ops/*.h     
%dir %{_includedir}/atomic_ops/sysdeps
%{_includedir}/atomic_ops/sysdeps/*
  As just:
%{_includedir}/%{name}
* And this:
%dir %{_datadir}/libatomic_ops
%{_datadir}/libatomic_ops/*   
  as:
%{_datadir}/lib%{name}
* Also concider replacing:
%{_libdir}/libatomic_ops.a
%{_libdir}/libatomic_ops_gpl.a
  With:
%{_libdir}/lib%{name}*.a
* Etc.
Comment 2 Hans de Goede 2006-09-09 04:58:44 EDT
Erm,

The above MUST list is a copy paste from another review and I forgot to update a
few items, please ignore it and use this one. The Must Fix and Should Fix still
apply!

MUST:
=====
O rpmlint output is:
E: libatomic_ops-devel invalid-spec-name libatomic_ops.spec
This must be fixed, see Must Fix
O Package and spec file NOT named appropriately, see Must Fix
* Packaged according to packaging guidelines
* License GPL/MIT ok and included
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean OK
O macro usage not OK due to wrong "Name:", see Must Fix
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* no .so / .la files.
* no .desktop file required


Comment 3 Pierre Ossman 2006-09-09 07:07:14 EDT
> * "Name:" should be "libatomic_ops" not "libatomic_ops-devel", then
>   add a subpackage -devel, with the same Summary and Description and
>   only add a "%files devel" and not a normal "%files", then rpmbuild will only
>   generate a -devel package

I didn't quite know how to produce a package that wasn't called %{name}, which
was my initial intention anyway. :)

> * Remove "%define debug_package %{nil}" code gets generated from both the .c
>  and the .h files, people may need this for debugging

I added that because rpmbuild complained that the debug package was empty. So on
at least my version, this doesn't work.


I'll have a new version up in a bit.
Comment 4 Hans de Goede 2006-09-09 07:14:31 EDT
(In reply to comment #3)
> > * Remove "%define debug_package %{nil}" code gets generated from both the .c
> >  and the .h files, people may need this for debugging
> 
> I added that because rpmbuild complained that the debug package was empty. So on
> at least my version, this doesn't work.
> 
Erm,

Yes ofcourse it won't generate any debbuginfo for a .a as that info gets
generated for the package linking against the .a, unfortunatly at that time the
.c files and system installed .h files won't be added to the -debuginfo package
of the apps being linked against it. I really don't know how to make a debuginfo
package with just the sources in there, so I guess that people who need the
sources to debug must just install the src.rpm . To make a long story short, you
are right and the "%define debug_package %{nil}" can stay.

Comment 6 Hans de Goede 2006-09-09 07:44:41 EDT
Looks good, approved!

One last remark, you could use %{name}*.h for the header files and plain %{name}
for /usr/include/atomic_ops
Comment 7 Lubomir Kundrak 2008-04-14 13:17:59 EDT
I'd be very thiankful if you could request and maintain a EPEL-5 branch for this
package. In case you don't want or can't do that, let me know and I'll do that.

Thanks!
Comment 8 Pierre Ossman 2008-04-14 16:28:42 EDT
Please do. I don't really have enough time for the Fedora packages. So feel free
to give those some love as well. ;)
Comment 9 Lubomir Kundrak 2008-04-14 16:59:15 EDT
Package Change Request
======================
Package Name: libatomic_ops
New Branches: EL-5
New branch owner: lkundrak
New branch cvsextras commits: yes
Comment 10 Kevin Fenzi 2008-04-14 20:22:05 EDT
cvs done.

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