Bug 349621 - Review Request: compiz-manager - A wrapper script to start compiz with proper options
Summary: Review Request: compiz-manager - A wrapper script to start compiz with proper...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-10-23 21:33 UTC by Sebastian Vahl
Modified: 2011-04-05 15:18 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-10-30 10:24:46 UTC
Type: ---
mtasaka: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Sebastian Vahl 2007-10-23 21:33:55 UTC
Spec URL: http://www.deadbabylon.de/fedora/review/compiz-manager.spec
SRPM URL: http://www.deadbabylon.de/fedora/review/compiz-manager-0.6.0-1.fc8.src.rpm
Description: This script will detect what options we need to pass to compiz to get it 
started, and start a default plugin and possibly window decorator.


Some notes:
Group: I'm not really sure if this group fits. 
Options for starting compiz: I've not modified the options that would be started with compiz. So if there are better solutions please let me know.

Comment 1 Mamoru TASAKA 2007-10-24 14:29:14 UTC
For 0.6.0-1:

* %_libdir
  - This script is broken on 64 bits architecture.
  ! Note: to make this package "noarch", some effort
    is needed to deal with both 32 bits and 64 bits arch.

* LIBGL_NVIDIA and so on..
  - What happens if sover of LIBGL_NVIDIA changes?
    IMO, autodetection like:
-----------------------------------------------------
for driver in $libdir/nvidia/libGL.so.*.xlibmesa ; do
	LIBGL_NVIDIA=$driver
done
-----------------------------------------------------
    is better.

* USE_EMERALD
  - Please explain why you want to disable emerald by
    default.
    - If you want, the comment on the above line is not correct.
    - And something like:
-----------------------------------------------------
USE_EMERALD=${USE_EMERALD:-no}
-----------------------------------------------------
      is better IMO.

* Dependency
  - It seems that this scripts uses some commands in
    xorg-x11-utils (this is not installed by default).
  - Also, adding requirement for pciutils is better (for /sbin/lspci)
  - glx-utils also seems needed.
  - Something else may be also needed. Would you check the dependency
    again?

* Timestamp
  - Please use "-p" option when using "cp" or "install" command
    to keep timestamps.

* ExcludeArch
  - compiz is not available on ppc64, so please write in the spec file
    "ExcludeArch: ppc64" even if this is noarch
    ref:
    https://www.redhat.com/archives/fedora-devel-list/2007-October/msg00262.html

? Desktop file
  - Is it preferable that this package provides desktop file entry?

Comment 2 Sebastian Vahl 2007-10-24 15:30:11 UTC
(In reply to comment #1)
> For 0.6.0-1:
> 
> * %_libdir
>   - This script is broken on 64 bits architecture.
>   ! Note: to make this package "noarch", some effort
>     is needed to deal with both 32 bits and 64 bits arch.

Thx. I've added a if-else section to the script and replaced all libdirs.
Is it right that ppc also uses /usr/lib and only x86_64 /usr/lib64? (I'm only 
using x86)

> * LIBGL_NVIDIA and so on..
>   - What happens if sover of LIBGL_NVIDIA changes?
>     IMO, autodetection like:
> -----------------------------------------------------
> for driver in $libdir/nvidia/libGL.so.*.xlibmesa ; do
>         LIBGL_NVIDIA=$driver
> done
> -----------------------------------------------------
>     is better.

Thanks. added (and also for LIBGL_FGLRX)

> * USE_EMERALD
>   - Please explain why you want to disable emerald by
>     default.
>     - If you want, the comment on the above line is not correct.
>     - And something like:
> -----------------------------------------------------
> USE_EMERALD=${USE_EMERALD:-no}
> -----------------------------------------------------
>       is better IMO.

I've disabled emerald to not require more dependencies than needed. I've 
replaced the comment in the script with this.

> * Dependency
>   - It seems that this scripts uses some commands in
>     xorg-x11-utils (this is not installed by default).
>   - Also, adding requirement for pciutils is better (for /sbin/lspci)
>   - glx-utils also seems needed.
>   - Something else may be also needed. Would you check the dependency
>     again?

Thanks. This three seems to be the only not mentioned requirements (except 
grep and sed).
Added.

> * Timestamp
>   - Please use "-p" option when using "cp" or "install" command
>     to keep timestamps.

Added.

> 
> * ExcludeArch
>   - compiz is not available on ppc64, so please write in the spec file
>     "ExcludeArch: ppc64" even if this is noarch
>     ref:
>     
https://www.redhat.com/archives/fedora-devel-list/2007-October/msg00262.html

Added.

> ? Desktop file
>   - Is it preferable that this package provides desktop file entry?

Sorry. That was my fault. I've uploaded a wrong srpm and spec first but 
replaced them a few minutes later. I haven't thought that anybody has 
downloaded them so I've not increased the release.

New ones:
SPEC Url: http://www.deadbabylon.de/fedora/review/compiz-manager.spec
SRPM Url: 
http://www.deadbabylon.de/fedora/review/compiz-manager-0.6.0-2.fc8.src.rpm

ChangeLog:
- update patch: differ between different archs
- update patch: don't hardcode the libGl versions
- update patch: add description why not ot use emerald by default
- add requires: xorg-x11-utils
- add requires: pci-utils
- add requires: glx-utils
- add ExcludeArch for ppc64 because compiz isn't available there
- keep timestamp of compiz-manager

Comment 3 Mamoru TASAKA 2007-10-24 17:53:01 UTC
(In reply to comment #2)
> Is it right that ppc also uses /usr/lib and only x86_64 /usr/lib64? (I'm only 
> using x86)
Yes (as ppc64 is disabled).


> > * Timestamp
> >   - Please use "-p" option when using "cp" or "install" command
> >     to keep timestamps.
> 
> Added.
Please add "-p" option also for "cp"ing COPYING file.

 > ? Desktop file
> >   - Is it preferable that this package provides desktop file entry?
Well, what I wanted to ask you is whether it is preferable to
install some desktop file (e.g. named "feodra-compiz-manager.desktop)
under %_datadir/applications so that this script can be used
from GNOME panel, for example (not from terminal).

Almost okay.

Comment 4 Sebastian Vahl 2007-10-25 08:48:04 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Is it right that ppc also uses /usr/lib and only x86_64 /usr/lib64? (I'm 
only 
> > using x86)
> Yes (as ppc64 is disabled).

Thanks

> > > * Timestamp
> > >   - Please use "-p" option when using "cp" or "install" command
> > >     to keep timestamps.
> > 
> > Added.
> Please add "-p" option also for "cp"ing COPYING file.

Sry, overseen. Fixed.

>  > ? Desktop file
> > >   - Is it preferable that this package provides desktop file entry?
> Well, what I wanted to ask you is whether it is preferable to
> install some desktop file (e.g. named "feodra-compiz-manager.desktop)
> under %_datadir/applications so that this script can be used
> from GNOME panel, for example (not from terminal).

Gnome has the "Desktop Effects" application to switch from metacity to compiz 
and back again. compiz-manager only starts compiz and has no ability to switch 
back to the former used wm. And with the script only you don't have any 
feedback (except starting compiz). So I don't think a additional desktop-file 
would be a good idea.
For KDE there are some plans to include a dialog which uses compiz-manager to 
check and start compiz. But this script should go into compiz-kde. My 
intention with compiz-manager was to only add this "helper script" so that an 
additional GUI could easily be provided by another package.

New ones:
SPEC Url: http://www.deadbabylon.de/fedora/review/compiz-manager.spec
SRPM Url: 
http://www.deadbabylon.de/fedora/review/compiz-manager-0.6.0-3.fc8.src.rpm

Changelog:
- keep timestamp also of COPYING
- Update patch: always use kwin as fallback in KDE_FULL_SESSION

Comment 5 Mamoru TASAKA 2007-10-25 09:04:26 UTC
Okay.

------------------------------------------------------------------
   This package (compiz-manager) is APPROVED by me
------------------------------------------------------------------

Comment 6 Sebastian Vahl 2007-10-25 09:21:18 UTC
Thx!

New Package CVS Request
=======================
Package Name: compiz-manager
Short Description: A wrapper script to start compiz with proper options
Owners: svahl
Branches: F-8
InitialCC: drago01
Cvsextras Commits: no

Comment 7 Sebastian Vahl 2007-10-30 10:24:46 UTC
Imported and built for f8 and devel.

Comment 8 leigh scott 2011-04-01 15:52:19 UTC
Hi Sebastian,

Is it ok for me to request a epel6 branch + commit rights?

https://bugzilla.redhat.com/show_bug.cgi?id=677365

Thanks

Leigh

Comment 9 leigh scott 2011-04-04 23:04:50 UTC
Package Change Request
======================
Package Name: compiz-manager
New Branches: el6
Owners: leigh123linux

Comment 10 Jason Tibbitts 2011-04-05 15:18:03 UTC
Git done (by process-git-requests).


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