Bug 202439 - Review Request: frozen-bubble - Frozen Bubble arcade gam
Summary: Review Request: frozen-bubble - Frozen Bubble arcade gam
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Wart
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 202437
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-08-14 14:15 UTC by Hans de Goede
Modified: 2007-11-30 22:11 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-26 06:01:15 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Move per libs to app directory (1.66 KB, patch)
2006-08-20 05:12 UTC, Wart
no flags Details | Diff

Description Hans de Goede 2006-08-14 14:15:28 UTC
Spec URL: http://people.atrpms.net/~hdegoede/frozen-bubble.spec
SRPM URL: http://people.atrpms.net/~hdegoede/frozen-bubble-1.0.0-6.src.rpm
Description:
Full-featured, colorful animated penguin eyecandy, 100 levels of 1p game, hours
and hours of 2p game, 3 professional quality 20-channels musics, 15 stereo
sound effects, 7 unique graphical transition effects and a level editor.  
You need this game.

Comment 1 Warren Togami 2006-08-14 18:25:55 UTC
I think this game was removed from Fedora in the past for some legal reason.  I
don't have any details of this though.


Comment 2 Christopher Stone 2006-08-14 18:37:50 UTC
There is no reason this game can't be in extras.  See bug #202437 for more
information.

Comment 3 Hans de Goede 2006-08-14 19:22:01 UTC
(In reply to comment #2)
> There is no reason this game can't be in extras.  See bug #202437 for more
> information.

Exactly, this game wasn't in FE because perl-SDL wasn't in FE. perl-SDL wasn't
in FE because it can use smpeg, however it turns out that perl-SDL is just fine
without smpeg as (sofar) no perl-SDL using packages use the smpeg part.


Comment 4 Hans de Goede 2006-08-15 10:32:33 UTC
New version to work with the new perl-SDL-2.1.3:
Spec URL: http://people.atrpms.net/~hdegoede/frozen-bubble.spec
SRPM URL: http://people.atrpms.net/~hdegoede/frozen-bubble-1.0.0-7.src.rpm


Comment 5 Wart 2006-08-16 03:20:27 UTC
GOOD
====
* rpmlint output clean
* Package and spec named appropriately
* Source matches upstream:
  2be5ead2aee72adc3fb643630a774b59  frozen-bubble-1.0.0.tar.bz2
* GPL license ok, license file included
* RPM_BUILD_ROOT cleaned where it ought to be
* .desktop file and icons installed correctly
* No duplicate %files
* Spec file legible and in Am. English
* Builds in mock on FC5-i386, FC5-x86_64, FC6-i386, FC6-x86_64
* No need for -doc subpackage
* No need for -devel subpackage
* No locales
* Owns all directories that it creates
* Does not own any directories that it should not
* Contains code and allowable game content
* File permissions look ok
* Not relocatable

MUSTFIX
=======
* BR: perl is not necessary.  It is already picked up by perl-SDL.

SHOULD
======
* There is some inconsistency in the use of $RPM_BUILD_ROOT vs.
  ${RPM_BUILD_ROOT}.  Both work.  Pick one and stick with it.

* The perl autoprovider in rpm adds some automatic provides since this package
  installs files into %{perl_vendorarch}.  If these files are only going
  to be used by frozen-bubble, wouldn't it be better to put them somewhere
  in %{_datadir}/frozen-bubble instead?

Comment 6 Hans de Goede 2006-08-16 04:47:13 UTC
(In reply to comment #5)
> 
> MUSTFIX
> =======
> * BR: perl is not necessary.  It is already picked up by perl-SDL.
> 
Ok, will fix as soon as we've got agreement on the things below

> SHOULD
> ======
> * There is some inconsistency in the use of $RPM_BUILD_ROOT vs.
>   ${RPM_BUILD_ROOT}.  Both work.  Pick one and stick with it.
> 
Agree, thats because I didn't do the original specfile I'll make it all
$RPM_BUILD_ROOT

> * The perl autoprovider in rpm adds some automatic provides since this package
>   installs files into %{perl_vendorarch}.  If these files are only going
>   to be used by frozen-bubble, wouldn't it be better to put them somewhere
>   in %{_datadir}/frozen-bubble instead?

Erm, there are .so files installed to %{perl_vendorarch}, hence the package is
not noarch. I could try dropping them in %{_libdir}/frozen-bubble instead, but
I've got no clue howto make perl find them there (first perl package ever for me).

Or we could just leave them there :) (in which case I could add a provides
filter to make it stop providing this if you want).


Comment 7 Wart 2006-08-16 06:14:33 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > * The perl autoprovider in rpm adds some automatic provides since this package
> >   installs files into %{perl_vendorarch}.  If these files are only going
> >   to be used by frozen-bubble, wouldn't it be better to put them somewhere
> >   in %{_datadir}/frozen-bubble instead?
> 
> Erm, there are .so files installed to %{perl_vendorarch}, hence the package is
> not noarch.

Yes, my bad.  The main point was that if these aren't perl modules that can be
used by other applications then they should live in an application-specific
directory instead of the system-wide perl tree.

> I could try dropping them in %{_libdir}/frozen-bubble instead, but
> I've got no clue howto make perl find them there (first perl package ever for me).
> 
> Or we could just leave them there :) (in which case I could add a provides
> filter to make it stop providing this if you want).

I've seen python programs work with python libraries that don't live in
%{python_sitearch} and figured that perl could work the same way.  But if that's
a problem, then a Provides: filter should be fine.

Comment 8 Hans de Goede 2006-08-16 10:30:29 UTC
(In reply to comment #7)
> I've seen python programs work with python libraries that don't live in
> %{python_sitearch} and figured that perl could work the same way.
I know I'vr actually done that with python programs. I think perl can do this
too, but don't ask how.

> But if that's
> a problem, then a Provides: filter should be fine.

Its a problem as I lack the nescesarry knowledge to make this happen, so I've
added a provides filter. Notice that this filter does not filters out the 
fb_c_stuff.so()(64bit) provides, even though that is private too, as moving the
perl stuff to an other dir wouldn't have removed that provides.

This has made me thinking many packages have .so's which are mean't as plugins
not as libraries and which thus are kinda private, yet these files are currently
listed in the autogenerated provide lists, which doesn't seem entirely correct?

REDO FROM START

Erm, I just tried installing the version with the filtered provides and that
gives me the following error:
[hans@shalem SPECS]$ sudo rpm -Uvh
/usr/src/redhat/RPMS/x86_64/frozen-bubble-1.0.0-8.x86_64.rpm
error: Failed dependencies:
        perl(FBLE) is needed by frozen-bubble-1.0.0-8.x86_64
        perl(fb_c_stuff) is needed by frozen-bubble-1.0.0-8.x86_64
        perl(fb_stuff) is needed by frozen-bubble-1.0.0-8.x86_64
        perl(fbsyms) is needed by frozen-bubble-1.0.0-8.x86_64

So I think that filtering the provides is a bad idea, I think/hope that these
provides will even be generated with the perl stuff in another dir, because I'm
pretty sure the deps will still be generated since those come from the main
script. So I think that filtering them is a bad idea, and we will just have to
leave things as they are.


Comment 9 Jason Tibbitts 2006-08-16 17:34:48 UTC
Adding a directory to Perl's library search path is as simple as sticking:

use lib '/blah';

near the top of the script.



Comment 10 Hans de Goede 2006-08-16 17:39:17 UTC
(In reply to comment #9)
> Adding a directory to Perl's library search path is as simple as sticking:
> 
> use lib '/blah';
> 
> near the top of the script.
> 
> 

Thanks, does that work for binary stuff (.so) which is hidden under an auto dir
too? Also should regular perl apps but their stuff in a seperate dir. The
current behaviour is upstreams default.


Comment 11 Hans de Goede 2006-08-17 07:07:15 UTC
Any chance we can get some progress on this, this is blocking the building of
perl-SDL as I want to build them in quick succession so that it doesn't break
peoples systems from updating.


Comment 12 Wart 2006-08-18 15:02:22 UTC
Sorry for the delay.  I'm playing with a few things to see if I can move the
perl bits from %{perl_vendorarch} to %{_libdir}.  I'll have an update later today.

Comment 13 Wart 2006-08-19 06:46:33 UTC
Quick update:  I moved the files from %{perl_vendorarch} to %{_libdir}/%{name},
without changing anything else, and the game still seemed to run with no
problems.  I'll try it on a clean system tomorrow just to be sure, though.

Comment 14 Wart 2006-08-20 05:12:25 UTC
Created attachment 134522 [details]
Move per libs to app directory

This is a patch to the spec file to move the application-specific perl
libraries out of the global perl library tree and into %{_libdir}/%{name}

Comment 15 Hans de Goede 2006-08-20 05:43:09 UTC
Thanks!

Applied and I've also fixed the inconsistent RPM_BUILD_ROOT usage, new version here:
Spec URL: http://people.atrpms.net/~hdegoede/frozen-bubble.spec
SRPM URL: http://people.atrpms.net/~hdegoede/frozen-bubble-1.0.0-8.src.rpm


Comment 16 Wart 2006-08-20 06:34:26 UTC
All necessary items addressed.  Game still functions as expected.

APPROVED

Comment 17 Ville Skyttä 2006-08-20 07:28:44 UTC
I think moving the files but leaving the perl(...) Provides in is worse than
leaving the files in the usual locations (and leaving the Provides intact). 
Moving should be coupled with Provides (and probably Requires due to comment 8)
filtering but that's of questionable gain anyway, I'd revert moving the files
and doing things as usual.

Comment 18 Hans de Goede 2006-08-20 11:19:27 UTC
Wart, I'm waiting for a reply to comment 17 from you before continuing, the
raised concern seems valid. (Although I'm getting the strange feeling with this
that for some reason people are more stricy about perl(xxx) Provides/Requires
then about .so Provides/Requires, because we have similar issues with .so
Provides in a gazillion packages).


Comment 19 Ville Skyttä 2006-08-20 15:02:17 UTC
That's probably because things are in better shape with perl(...)
autoprovides/requires than generic .so elsewhere, problems elsewhere are not
really a good reason to inflict them everywhere, the cases where this arises in
perl related packages are pretty rare, and can be more cleanly filtered in these
cases than others -- eg. no need to disable rpmbuild's internal dep generator.

Note however that comment 17 is just an opinion, not a veto.  But thanks for
considering it anyway.

Comment 20 Wart 2006-08-20 19:46:34 UTC
(In reply to comment #17)
> I think moving the files but leaving the perl(...) Provides in is worse than
> leaving the files in the usual locations (and leaving the Provides intact). 
> Moving should be coupled with Provides (and probably Requires due to comment 8)
> filtering but that's of questionable gain anyway, I'd revert moving the files
> and doing things as usual.

My main concerns with leaving the files in %{perl_vendorarch} is that these are
application-specific perl modules.  They have no use outside of this package.  I
strongly prefer moving things like this into application specific directories
such as %{_libdir}/%{name} instead of polluting the language library tree.

I know languages like python and Tcl can handle this with no problem, which is
why I advocated doing it here as well.  It seems to me that we have two choices
here:

1) Move the files to %{_libdir}/%{name} and turn off the
autoprovides/autorequires for the perl modules

2) Leave the files where they are and live with obscurities like perl(fbsyms)
and perl(fb_c_stuff) in the system-wide perl library tree.

If the current standard practice for app-specific perl modules is #2 then I
won't argue, even though I think it's a bad policy.

Comment 21 Paul Howarth 2006-08-21 11:28:21 UTC
(In reply to comment #20)
> (In reply to comment #17)
> > I think moving the files but leaving the perl(...) Provides in is worse than
> > leaving the files in the usual locations (and leaving the Provides intact). 
> > Moving should be coupled with Provides (and probably Requires due to comment 8)
> > filtering but that's of questionable gain anyway, I'd revert moving the files
> > and doing things as usual.
> 
> My main concerns with leaving the files in %{perl_vendorarch} is that these are
> application-specific perl modules.  They have no use outside of this package.  I
> strongly prefer moving things like this into application specific directories
> such as %{_libdir}/%{name} instead of polluting the language library tree.

And I'm strongly in agreement with Wart. Whilst it's a little hassle in the spec
file, it's cleaner on the system the package gets installed into and is less
likely to result in future namespace collisions.


Comment 22 Hans de Goede 2006-08-21 12:43:35 UTC
Okay,

I'll add filters for the bogus provides then, do the same to the requires since
otherwise things won't work and then post a new version.

This may take a few days though since my vacation is over, so my spare time has
been drasticly reduced.

Comment 23 Hans de Goede 2006-08-23 13:40:52 UTC
* Wed Aug 23 2006 Hans de Goede <j.w.r.degoede> 1.0.0-9
- Filter out the autogenerated Provides for our private perl modules and also
  filter out the matching AutoRequires to still get an installable package

Go get it here:
Spec URL: http://people.atrpms.net/~hdegoede/frozen-bubble.spec
SRPM URL: http://people.atrpms.net/~hdegoede/frozen-bubble-1.0.0-9.src.rpm



Comment 24 Hans de Goede 2006-08-25 19:25:52 UTC
Erm,

Oops, this was already approved and here I'm waiting for approval. I guess that
the approval still stands? Then I'll omport 1.0.0-9


Comment 25 Wart 2006-08-25 21:03:21 UTC
I just did a quick double check of the -9 package and it both looks and plays ok.

APPROVED (still)

Comment 26 Hans de Goede 2006-08-26 06:01:15 UTC
Thanks,

Importad and build, closing



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