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 190911 - Review Request: ctapi-common - Common infrastructure for CT-API modules
Summary: Review Request: ctapi-common - Common infrastructure for CT-API modules
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 188369
TreeView+ depends on / blocked
 
Reported: 2006-05-06 11:21 UTC by Ville Skyttä
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-05-07 13:53:22 UTC
Type: ---
Embargoed:
ville.skytta: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)
ctapi-common.spec, first cut (1.42 KB, text/plain)
2006-05-06 11:25 UTC, Ville Skyttä
no flags Details
ctapi-common.spec, fixed (1.39 KB, text/plain)
2006-05-06 11:27 UTC, Ville Skyttä
no flags Details
ctapi-common.spec, fixed even better (1.45 KB, text/plain)
2006-05-06 11:33 UTC, Ville Skyttä
no flags Details

Description Ville Skyttä 2006-05-06 11:21:24 UTC
Common files and packaging infrastructure for CT-API modules.

For reference, see discussion in bug 188369.

No URLs; the package is built from a selfcontained specfile which will be attached shortly.

Comment 1 Ville Skyttä 2006-05-06 11:25:23 UTC
Created attachment 128690 [details]
ctapi-common.spec, first cut

Here's the specfile.  Contents of README intentionally kept generic for all
architectures.	I'm not aware of a way to specify arch-qualified dependencies
in packages, so this one adds the %{name}(%{_target_cpu}) Provides for that
purpose.

Others are welcome to take ownership of this package, I'm not particularly
interested in maintaining it (although there's not much to maintain here) as I
don't have use for any CT-API modules myself at the moment.

Comment 2 Ville Skyttä 2006-05-06 11:27:33 UTC
Created attachment 128691 [details]
ctapi-common.spec, fixed

And of course I posted a wrong version of the specfile.  Here's the fixed one.

Comment 3 Ville Skyttä 2006-05-06 11:33:15 UTC
Created attachment 128692 [details]
ctapi-common.spec, fixed even better

*blush*

Comment 4 Hans de Goede 2006-05-06 18:30:31 UTC
Oops hit save too soon (before typing the comment I wanted to type)

I was away this afternoon (outside, fresh air and sun you know) and while
cycling to the city I was wondering about the arch problem. And I came up with a
nice and clean solution which imho is much better then in the current spec file:

Why not make ctapi lib packages require %{_libdir}/ctapi instead of ctapi-cmmon,
then on dual arch systems like x86_64 they will automaticly drag in the right
one. Since both an i386 and an x86_64 version of ctapi-common could get
installed we do still need the arch in the filename under /etc/ld.so.conf.d

Also in this case I think it would be better to put the README in a seperate
file and "install -p" it so that both arch packages have the same timestamp for it.

What do you think if you agree please attach a new version and I'll review it.
If you don't agree any better suggestions? I find your current solution not so
elegant.


Comment 5 Ville Skyttä 2006-05-06 19:00:43 UTC
The dir based dependency is doable, but it'll break as soon as some other
package owns %{_libdir}/ctapi.  We can make sure it doesn't happen within FE,
but I can imagine a 3rd party package doing that eg. for cross distro
compatibility issues.  I find the provided thingy less likely to break, but I
don't have too strong opinion on this.

Regarding README, I have both i386 and x86_64 ctapi-common with differing README
timestamp installed here, and rpm doesn't raise a conflict and even rpm -V is
silent for both.  Simply having it as SourceX isn't enough to guarantee the same
timestamp in packages due to eg. cvs checkouts, it would have to be put into a
tarball or touch'd with a timestamp to ensure that.  I'm not sure if it's worth
the trouble, especially considering that rpm doesn't seem to have any issues
with the current implementation.

Comment 6 Hans de Goede 2006-05-06 19:17:35 UTC
Ok,

I kinda strongly prefer the %{_libdir} approach for the dependency handling I
concider the other approach somewhat ugly. If a third part package causes things
to break we will handle that the. If rpm doesn't mind different timestamps on
the README I'm happy with things as they are too. Summarazing, could you attach
a new spec with the %{_libdir} approach, then I'll review it,

Thanks.


Comment 7 Ville Skyttä 2006-05-06 20:34:10 UTC
Ok, let's try that out.  I thought it'd be a good idea to include a license file
so I split README out anyway while at it.  Hopefully I can assign the copyright
to the Fedora Project...?

http://cachalot.mine.nu/5/SRPMS/ctapi-common-1.0-2.src.rpm

* Sat May  6 2006 Ville Skyttä <ville.skytta at iki.fi> - 1.0-2
- Encourage dir based dependency on %{_libdir}/ctapi in packages (#190911).
- Split contents of README into a separate file.
- Change license to MIT, include license text.
- Add URL.

Comment 8 Hans de Goede 2006-05-07 09:56:04 UTC
MUST:
=====
* rpmlint output is:
E: ctapi-common no-binary
W: ctapi-common non-conffile-in-etc /etc/ld.so.conf.d/ctapi-x86_64.conf
  The error can be ignored, I don't know about the warning wine also drops a 
  file under /etc/ld.so.conf.d and doesn't mark it config either, but that could
  actually be a wine packaging bug. I'll leave this one up to your discretion.
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License (MIT) ok, license file included
* spec file is legible and in Am. English.
* Source matches upstream NA, no upstream
* Compiles and builds on devel-x86_64
* BR: ok
* No locales
* No shared libraries
* Not relocatable
* Package owns all dirs it should own
* 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 gui -> no .desktop file required


Approved.


Comment 9 Ville Skyttä 2006-05-07 13:53:22 UTC
mysql and qt have their ld.so.conf.d snippets non-%config which is why I made it
so in this package too.  This can be changed later if needed.  Imported and
built for devel, FC-4 and FC-5 builds will follow when the branches are ready.
http://buildsys.fedoraproject.org/build-status/job.psp?uid=8974

Comment 10 Ville Skyttä 2007-07-08 08:38:27 UTC
Package Change Request
======================
Package Name: ctapi-common
Updated Fedora Owners: ville.skytta,frank-buettner

Comment 11 Ville Skyttä 2007-07-14 10:26:03 UTC
Package Change Request
======================
Package Name: ctapi-common
New Branches: EL-4 EL-5
Updated EPEL Owners: frank-buettner,ville.skytta


Comment 12 Kevin Fenzi 2007-07-15 04:23:42 UTC
cvs done.


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