Bug 190911
Summary: | Review Request: ctapi-common - Common infrastructure for CT-API modules | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Ville Skyttä <scop> | ||||||||
Component: | Package Review | Assignee: | Hans de Goede <hdegoede> | ||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||
Severity: | medium | Docs Contact: | |||||||||
Priority: | medium | ||||||||||
Version: | rawhide | Flags: | ville.skytta:
fedora-review+
kevin: fedora-cvs+ |
||||||||
Target Milestone: | --- | ||||||||||
Target Release: | --- | ||||||||||
Hardware: | All | ||||||||||
OS: | Linux | ||||||||||
Whiteboard: | |||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||
Doc Text: | Story Points: | --- | |||||||||
Clone Of: | Environment: | ||||||||||
Last Closed: | 2006-05-07 13:53:22 UTC | Type: | --- | ||||||||
Regression: | --- | Mount Type: | --- | ||||||||
Documentation: | --- | CRM: | |||||||||
Verified Versions: | Category: | --- | |||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||
Embargoed: | |||||||||||
Bug Depends On: | |||||||||||
Bug Blocks: | 163779, 188369 | ||||||||||
Attachments: |
|
Description
Ville Skyttä
2006-05-06 11:21:24 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.
Created attachment 128691 [details]
ctapi-common.spec, fixed
And of course I posted a wrong version of the specfile. Here's the fixed one.
Created attachment 128692 [details]
ctapi-common.spec, fixed even better
*blush*
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. 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. 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. 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. 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. 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 Package Change Request ====================== Package Name: ctapi-common Updated Fedora Owners: ville.skytta,frank-buettner Package Change Request ====================== Package Name: ctapi-common New Branches: EL-4 EL-5 Updated EPEL Owners: frank-buettner,ville.skytta cvs done. |