Bug 232873

Summary: Review Request: compat-guichan05 - compatibility libraries for guichan
Product: [Fedora] Fedora Reporter: Wart <wart>
Component: Package ReviewAssignee: Hans de Goede <hdegoede>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideFlags: hdegoede: fedora-review+
wtogami: 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: 2007-04-10 21:18:14 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: 225044    

Description Wart 2007-03-19 06:13:23 UTC
Spec URL: http://www.kobold.org/~wart/fedora/compat-guichan05.spec
SRPM URL: http://www.kobold.org/~wart/fedora/compat-guichan05-0.5.0-1.fc7.src.rpm
Description: 
This package contains compatibility libraries for guichan 0.5.0.  This will allow the main guichan package to be updated to the latest version while providing support for applications that have not yet been updated to use the latest version.

Once this package is approved, I plan to update the main guichan package and push builds for both of these at the same time, as well as rebuilding the one packages that needs this (sear).

Note that I will be away until the end of March, and will likely not be able to respond to review comments until I return.

Comment 1 Hans de Goede 2007-03-19 11:16:19 UTC
Hi,

1 problem and one question:

Problem: it doesn't build on rawhide i386, and I cannot see the errors as it
adds: "> /dev/null 2>&1" to all compile commands, please remove this so that I
can see the error (and warnings).

Question / remark:
I notice that you've added -release %{version} to the LDFLAGS for this. This
hads pros and cons:
-pro, this way non Fedora packages / binaries (which may expect libguichan.so.0)
 will not find this, which is good as there is on way to know which version of'
 guichan they are actually looking for
-con, this way it will not be ABI compatible (because of the different soname)
 with the current guichan package, and thus guichan may only be updated in 
 devel.

What I actually had in mind was adding -release %{version} to the new
guichan-0.6.1 . Notice that we must do this independend of what we do with
compat-guichan, to avoid having the same problem in the future.


Comment 2 Wart 2007-03-20 04:13:07 UTC
I'll try to see where it's hiding the output.  I think it's buried in one of the
libtool invocations.  Odd that I don't see the build failures in rawhide i386 in
mock, though.

As for the con, I'm not concerned about it not being ABI compatible, as upstream
tends to make each release ABI incompatible with the previous one anyway.  Since
there are very few packages that depend on guichan (2 that I'm aware of), I'd
rather coordinate upgrades of the main guichan with the addition of
compat-guichan packages as needed.

I do plan on adding the -release %{version} to the new guichan 0.6.1 as well,
but not until compat-guichan05 is available and sear has been updated to use the
compat-guichan05 package.

Comment 3 Hans de Goede 2007-03-20 08:32:56 UTC
(In reply to comment #2)
> As for the con, I'm not concerned about it not being ABI compatible, as upstream
> tends to make each release ABI incompatible with the previous one anyway.  Since
> there are very few packages that depend on guichan (2 that I'm aware of), I'd
> rather coordinate upgrades of the main guichan with the addition of
> compat-guichan packages as needed.
> 

I understand, but although there are still no guidelines for this (GRRR) I only
make ABI changes in -devel, iow if you use -release in the compat package, and
follow my rules for ABI stability, then the compat package may only go into
-devel. Even though only the soname changes, this is still an ABI breakage and
IMHO the ABI should be kept stable for releases. AFAIK this is sort of an
unwritten rule. People may have build local binaries against guichan, and
breaking those in the middle of a release is a bit rude IMHO.

The easy fix for this is to not use -release for compat-guichan and if we ever
need a compat-guichan6 then ofcourse leave the -release in there.





Comment 4 Wart 2007-04-08 00:10:42 UTC
This makes sense as well, and I guess it would minimize the chance of breaking
other packages.  I'll have to upgrade the non-compat guichan package with the
-release fix before I can submit a build for this one in order to avoid filename
conflicts.

Updated package without the -release %{version} fix:

http://www.kobold.org/~wart/fedora/compat-guichan05.spec
http://www.kobold.org/~wart/fedora/compat-guichan05-0.5.0-3.src.rpm

rpmlint now complains:
compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan_sdl.so ../libguichan_sdl.so.0.0.0

...but these files are provided in the base package on which -devel requires, so
I don't think it's a real problem.

Comment 5 Hans de Goede 2007-04-08 06:00:41 UTC
(In reply to comment #4)
> This makes sense as well, and I guess it would minimize the chance of breaking
> other packages.  I'll have to upgrade the non-compat guichan package with the
> -release fix before I can submit a build for this one in order to avoid filename
> conflicts.
> 
Correct.


MUST:
=====
* rpmlint output is:
E: compat-guichan05 obsolete-not-provided guichan
E: compat-guichan05-devel only-non-binary-in-usr-lib
W: compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan_sdl.so ../libguichan_sdl.so.0.0.0
W: compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan.so ../libguichan.so.0.0.0
W: compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan_allegro.so ../libguichan_allegro.so.0.0.0
W: compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan_glut.so ../libguichan_glut.so.0.0.0
W: compat-guichan05-devel dangling-relative-symlink
/usr/lib64/guichan-0.5/libguichan_opengl.so ../libguichan_opengl.so.0.0.0
These can all be ignored
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel x86_64
* BR: ok
* No locales
* ldconfig correctly run for shared libraries
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* no .desktop file required

Could fix
=========
* Why instead off:
%dir %{_libdir}/guichan-0.5
%{_libdir}/guichan-0.5/libguichan.so
%{_libdir}/guichan-0.5/libguichan_allegro.so
%{_libdir}/guichan-0.5/libguichan_glut.so
%{_libdir}/guichan-0.5/libguichan_opengl.so
%{_libdir}/guichan-0.5/libguichan_sdl.so
  not just write:
%{_libdir}/guichan-0.5
  ? this would also be consistent with how you handle the .h files

Approved!


Comment 6 Wart 2007-04-08 20:05:29 UTC
(In reply to comment #5)
> Could fix
> =========
> * Why instead off:
> %dir %{_libdir}/guichan-0.5
> %{_libdir}/guichan-0.5/libguichan.so
> %{_libdir}/guichan-0.5/libguichan_allegro.so
> %{_libdir}/guichan-0.5/libguichan_glut.so
> %{_libdir}/guichan-0.5/libguichan_opengl.so
> %{_libdir}/guichan-0.5/libguichan_sdl.so
>   not just write:
> %{_libdir}/guichan-0.5
>   ? this would also be consistent with how you handle the .h files

The set of libraries that guichan provides may (and has) vary from release to
release depending on the whims of upstream.  Listing each library explicitly
makes it easier to detect when the list of libraries changes, so that I can
check any dependencies for breakage.

Comment 7 Wart 2007-04-08 20:05:57 UTC
New Package CVS Request
=======================
Package Name: compat-guichan05
Short Description: compatibility libraries for older guichan versions
Owners: wart
Branches: FC-6
InitialCC:

Comment 8 Wart 2007-04-10 21:18:14 UTC
Imported and built on -devel.  Thanks!