Bug 190911 - Review Request: ctapi-common - Common infrastructure for CT-API modules
Review Request: ctapi-common - Common infrastructure for CT-API modules
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 188369
  Show dependency treegraph
 
Reported: 2006-05-06 07:21 EDT by Ville Skyttä
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-05-07 09:53:22 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
ville.skytta: fedora‑review+
kevin: fedora‑cvs+


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

  None (edit)
Description Ville Skyttä 2006-05-06 07:21:24 EDT
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 07:25:23 EDT
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 07:27:33 EDT
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 07:33:15 EDT
Created attachment 128692 [details]
ctapi-common.spec, fixed even better

*blush*
Comment 4 Hans de Goede 2006-05-06 14:30:31 EDT
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 15:00:43 EDT
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 15:17:35 EDT
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 16:34:10 EDT
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 05:56:04 EDT
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 09:53:22 EDT
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 04:38:27 EDT
Package Change Request
======================
Package Name: ctapi-common
Updated Fedora Owners: ville.skytta@iki.fi,frank-buettner@gmx.net
Comment 11 Ville Skyttä 2007-07-14 06:26:03 EDT
Package Change Request
======================
Package Name: ctapi-common
New Branches: EL-4 EL-5
Updated EPEL Owners: frank-buettner@gmx.net,ville.skytta@iki.fi
Comment 12 Kevin Fenzi 2007-07-15 00:23:42 EDT
cvs done.

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