[Bug 349621] Review Request: compiz-manager - A wrapper script to start compiz with proper options

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: compiz-manager - A wrapper script to start compiz with proper options


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





------- Additional Comments From fedora at deadbabylon.de  2007-10-24 11:30 EST -------
(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

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.