Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 215568 - Review Request: beryl-dbus - Beryl OpenGL window and compositing manager dbus plug-in
Summary: Review Request: beryl-dbus - Beryl OpenGL window and compositing manager dbus...
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 Package Reviews List
URL:
Whiteboard:
Depends On: 209259
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-11-14 17:27 UTC by Jarod Wilson
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-11-21 21:09:09 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Jarod Wilson 2006-11-14 17:27:27 UTC
Spec URL: http://wilsonet.com/packages/beryl/beryl-dbus.spec
SRPM URL: http://wilsonet.com/packages/beryl/beryl-dbus-0.1.2-1.fc6.src.rpm
Description:
Beryl is a combined window manager and compositing
manager that runs on top of Xgl or AIGLX using OpenGL
to provide effects accelerated by a 3D graphics card
on the desktop. Beryl is a community-driven fork of
Compiz.

Beryl has a flexible plug-in system, which the
contents of this package take advantage of.


Depends on beryl-core, submitted for FE-review under bug 209259.

Comment 1 Martin Sourada 2006-11-14 17:47:15 UTC
You should probably fix this rpmlint output:
E: beryl-dbus zero-length /usr/share/doc/beryl-dbus-0.1.2/AUTHORS

Comment 2 Jarod Wilson 2006-11-14 18:15:48 UTC
(In reply to comment #1)
> You should probably fix this rpmlint output:
> E: beryl-dbus zero-length /usr/share/doc/beryl-dbus-0.1.2/AUTHORS

Done, along with switching over to using upstream tarball.

http://wilsonet.com/packages/beryl/beryl-dbus-0.1.2-2.fc6.src.rpm


Comment 3 Mamoru TASAKA 2006-11-16 16:54:48 UTC
Well,
1. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines :

* BuildRequires:
  - Well, too much redundant BuildRequires (I don't like this...)
    All needed are:
-------------------------------------------------
beryl-core-devel >= %{version}
dbus-devel
++++++++
libXcomposite-devel
libXdamage-devel
libSM-devel
libpng-devel
libXext-devel
libXinerama-devel
startup-notification-devel
libXrandr-devel
libXrender-devel
++++++++
-------------------------------------------------
    Actually these dependency finally 98 minimal build enviroment + 
    42 other package (tol: 140), while your original package tries
    to install 276 package.

NOTE: The packages between +(plus) symbols, i.e.
-------------------------------------------------
libXcomposite-devel
libXdamage-devel
libSM-devel
libpng-devel
libXext-devel
libXinerama-devel
startup-notification-devel
libXrandr-devel
libXrender-devel
-------------------------------------------------
   should be required by beryl-core-devel (check /usr/lib/pkgconfig/beryl.pc)
   so beryl-core-devel package should be fixed. After that this package
   should only require:
--------------------------------------------------
beryl-core-devel >= %{version}
dbus-devel
--------------------------------------------------
   for BuildRequires (I want to check this again so that would you
   fix beryl-core-devel first?)

* Requires:
  - dbus
    This is not necessary as libraries' dependency automatically pulls this.
    

2. Other things I have noticed :
* %doc
  - Well /usr/share/doc/beryl-dbus-0.1.2/ChangeLog says:
-------------------------------------------------
see debian/changelog
-------------------------------------------------
    ... however, where is debian/changelog?

99. For other packages:
99-A For beryl-core package:
* Well, rpmlint is not silent.
--------------------------------------------------
W: beryl-core undefined-non-weak-symbol /usr/lib/libberylsettings.so.0.0.0 g_free
W: beryl-core undefined-non-weak-symbol /usr/lib/libberylsettings.so.0.0.0 g_free
W: beryl-core undefined-non-weak-symbol /usr/lib/libberylsettings.so.0.0.0
g_slist_remove
W: beryl-core undefined-non-weak-symbol /usr/lib/libberylsettings.so.0.0.0
g_mkdir_with_parents
W: beryl-core undefined-non-weak-symbol /usr/lib/libberylsettings.so.0.0.0
g_key_file_has_key
....(too much)
---------------------------------------------------
  Perhaps linking against glib or something else is not correct.


Comment 4 Mamoru TASAKA 2006-11-16 18:57:32 UTC
Well, it seems that you write many unneeded BuildRequires on
beryl-related packages, which should be fixed.

Comment 5 Jarod Wilson 2006-11-16 19:00:40 UTC
Thanks much for the leg-work! I'll work on cleaning that all up. I think I
copied over the beryl-core spec to the first sub-package, and largely left the
BR in place, should have trimmed them then. I'll fix up beryl-core-devel's
dependencies first up, then work on the rest...

Comment 6 Jarod Wilson 2006-11-16 20:39:06 UTC
Okay, I've got a new build of beryl-dbus that has only BR: beryl-core-devel and
dbus-devel, and it builds perfectly in mock, with an updated beryl-core that
adds all the other bits as requirements of beryl-core-devel. I've also deleted
the ChangeLog file from %doc, I don't see debian/changelog anywhere in the
tarball (this project is, um, a little lax on docs...). I haven't yet pushed a
build of that updated beryl-core version through Extras though, as I want to
trim the BR on all the othe beryl bits first.

http://wilsonet.com/packages/beryl/beryl-dbus-0.1.2-3.fc6.src.rpm

Comment 7 Mamoru TASAKA 2006-11-17 14:52:14 UTC
Okay.
-----------------------------------------------
   This package (beryl-dbus) is APPROVED by me.
-----------------------------------------------

Two comments.
1. Requires: dbus
   This is not necessary because this package (beryl-dbus) requires
   libdbus-1.so.3 and this adds dbus dependency automatically.
2. For beryl-core package:
   /usr/lib/libberylsettings.so.0.0.0 contains lots of undefined non-weak
   symbols (as said above).
[tasaka1@localhost ~]$ ldd -r /usr/lib/libberylsettings.so.0.0.0
undefined symbol: g_free        (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_free        (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_slist_remove        (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_mkdir_with_parents  (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_key_file_has_key    (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_key_file_get_string_list    (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: g_dir_close   (/usr/lib/libberylsettings.so.0.0.0)
undefined symbol: XStringToKeysym       (/usr/lib/libberylsettings.so.0.0.0)
.......
.......
          Please fix the linkage against this library.


Comment 8 Jarod Wilson 2006-11-17 18:13:14 UTC
(In reply to comment #7)
> Okay.
> -----------------------------------------------
>    This package (beryl-dbus) is APPROVED by me.
> -----------------------------------------------
> 
> Two comments.
> 1. Requires: dbus
>    This is not necessary because this package (beryl-dbus) requires
>    libdbus-1.so.3 and this adds dbus dependency automatically.

Done for the version I'll import once cvs is resurrected. Thank you!

> 2. For beryl-core package:
>    /usr/lib/libberylsettings.so.0.0.0 contains lots of undefined non-weak
>    symbols (as said above).
[...]
> .......
>           Please fix the linkage against this library.

Looking into it now...




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