Bug 461106 - Review Request: libnotifymm - C++ interface for libnotify
Summary: Review Request: libnotifymm - C++ interface for libnotify
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-09-04 08:51 UTC by Denis Leroy
Modified: 2009-01-14 10:21 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-01-14 10:21:00 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Denis Leroy 2008-09-04 08:51:28 UTC
Spec URL: http://www.poolshark.org/src/libnotifymm.spec
SRPM URL: http://www.poolshark.org/src/libnotifymm-0.6.1-1.fc10.src.rpm

Description:
libnotifymm provides a C++ interface to the libnotify library. Highlights include typesafe callbacks, widgets extensible via inheritance and a comprehensive set of widget classes that can be freely combined to quickly create complex user interfaces. libnotifymm is part of the gtkmm project.

Comment 1 Rakesh Pandit 2008-09-04 09:20:11 UTC
Failed to build on rawhide test box x86_64. Snippet from build log:


g++ -DHAVE_CONFIG_H -DG_LOG_DOMAIN=\"libnotifymm\" -I../../libnotify -I../../libnotify -I/usr/include/glibmm-2.4 -I/usr/lib64/glibmm-2.4/include -I/usr/include/sigc++-2.0 -I/usr/lib64/sigc++-2.0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/gtkmm-2.4 -I/usr/lib64/gtkmm-2.4/include -I/usr/include/giomm-2.4 -I/usr/lib64/giomm-2.4/include -I/usr/include/gdkmm-2.4 -I/usr/lib64/gdkmm-2.4/include -I/usr/include/pangomm-1.4 -I/usr/include/atkmm-1.6 -I/usr/include/gtk-2.0 -I/usr/lib64/gtk-2.0/include -I/usr/include/cairomm-1.0 -I/usr/include/pango-1.0 -I/usr/include/cairo -I/usr/include/atk-1.0 -I/usr/include/freetype2 -I/usr/include/dbus-1.0 -I/usr/lib64/dbus-1.0/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -c notify.cc  -fPIC -DPIC -o .libs/notify.o
notification.cc: In static member function 'static void Notify::Notification_Class::class_init_function(void*, void*)':
notification.cc:168: error: invalid conversion from 'void (*)(NotifyNotification*)' to 'void (*)(NotifyNotification*, gint)'
notification.cc: In static member function 'static void Notify::Notification_Class::closed_callback(NotifyNotification*)':
notification.cc:214: error: too few arguments to function
notification.cc: In member function 'virtual void Notify::Notification::on_closed()':
notification.cc:477: error: too few arguments to function
make[4]: Leaving directory `/builddir/build/BUILD/libnotifymm-0.6.1/libnotify/libnotifymm'
make[3]: Leaving directory `/builddir/build/BUILD/libnotifymm-0.6.1/libnotify/libnotifymm'
make[4]: *** [notification.lo] Error 1
make[3]: *** [all-recursive] Error 1
make[2]: Leaving directory `/builddir/build/BUILD/libnotifymm-0.6.1/libnotify'
make[2]: *** [all-recursive] Error 1
make[1]: *** [all] Error 2
make[1]: Leaving directory `/builddir/build/BUILD/libnotifymm-0.6.1/libnotify'
make: *** [all-recursive] Error 1
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.8NC4b5 (%build)
    Bad exit status from /var/tmp/rpm-tmp.8NC4b5 (%build)
Child returncode was: 1
EXCEPTION: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target x86_64 --nodeps builddir/build/SPECS/libnotifymm.spec']
Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/mock/trace_decorator.py", line 70, in trace
    result = func(*args, **kw)
  File "/usr/lib/python2.5/site-packages/mock/util.py", line 316, in do
    raise mock.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), child.returncode)
Error: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target x86_64 --nodeps builddir/build/SPECS/libnotifymm.spec']
LEAVE do --> EXCEPTION RAISED

Comment 2 Denis Leroy 2008-09-05 09:42:02 UTC
*!@#$, rawhide package includes an API-breaking patch.

Here's the fix, but this will only build on rawhide. For F-9, comment out the first patch from the spec file.

Spec URL: http://www.poolshark.org/src/libnotifymm.spec
SRPM URL: http://www.poolshark.org/src/libnotifymm-0.6.1-2.fc10.src.rpm

Comment 3 Jason Tibbitts 2008-12-20 22:19:49 UTC
Sorry nobody's looked at this in a while, but I just tried to build on current rawhide (x86_64) and got:

notification.cc: In static member function 'static void  
  Notify::Notification_Class::class_init_function(void*, void*)':
notification.cc:192: error: invalid conversion from 'void (*)(NotifyNotification*, 
  gint)' to 'void (*)(NotifyNotification*)'
notification.cc: In static member function 'static void 
  Notify::Notification_Class::closed_callback(NotifyNotification*, gint)':
notification.cc:238: error: too many arguments to function
notification.cc: In member function 'virtual void
  Notify::Notification::on_closed(gint)':
notification.cc:501: error: too many arguments to function

It does build fine on F-10, though, and there's no requirement for a package to build on rawhide in order to be reviewed, although of course you'll want to get it building relatively soon.

There are a pile of rpmlint unused-direct-shlib-dependency complaints, against things like libgtk-x11 and libatk.  I don't think these are especially problematic as those libraries will be in memory anyway, but they're trivially fixed with the usual one-liner:
  sed -i -e 's! -shared ! -Wl,--as-needed\0!g' libtool
after the %configure call.  With that, rpmlint is silent.

Note that the examples are GPL, not LGPL, but they aren't installed so there's no issue.

Really I don't see anything which should block approval of this package.

* source files match upstream.  sha256sum:
  fea7f6f7e436d343a14ceed749b5e9ad78e34c0a62c7790520710de5236c402f  
   libnotifymm-0.6.1.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (F-10, x86_64).
* package installs properly.
* debuginfo package looks complete.
O rpmlint has some fixable complaints which aren't blockers.
* final provides and requires are sane:
  libnotifymm-0.6.1-2.fc10.x86_64.rpm
   libnotifymm-1.0.so.7()(64bit)
   libnotifymm = 0.6.1-2.fc10
   libnotifymm(x86-64) = 0.6.1-2.fc10
  =
   /sbin/ldconfig
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libglib-2.0.so.0()(64bit)
   libglibmm-2.4.so.1()(64bit)
   libgobject-2.0.so.0()(64bit)
   libnotify.so.1()(64bit)
   libnotifymm-1.0.so.7()(64bit)
   libsigc-2.0.so.0()(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)

  libnotifymm-devel-0.6.1-2.fc10.x86_64.rpm
   pkgconfig(libnotifymm-1.0) = 0.6.1
   libnotifymm-devel = 0.6.1-2.fc10
   libnotifymm-devel(x86-64) = 0.6.1-2.fc10
  =
   gtkmm24-devel
   libnotify-devel
   libnotifymm = 0.6.1-2.fc10
   libnotifymm-1.0.so.7()(64bit)
   pkgconfig

* shared libraries installed:
   ldconfig called properly.
   unversioned .so link is in the -devel package.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files.
* scriptlets are OK (ldconfig).
* code, not content.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel package.
* pkgconfig are in the -devel package; pkgconfig dependency is present.
* no static libraries.
* no libtool .la files.

APPROVED

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 4 Denis Leroy 2008-12-23 06:04:18 UTC
Tibbs, thanks for the review. I'll fix the rpmlint warning and devel build issue.


New Package CVS Request
=======================
Package Name: libnotifymm
Short Description: C++ interface for libnotify
Owners: denis
Branches: F-9 F-10
InitialCC:

Comment 5 Kevin Fenzi 2008-12-25 00:37:28 UTC
cvs done.

Comment 6 Jason Tibbitts 2009-01-13 23:25:27 UTC
Did you want to get this built and pushed out?  I see an F-10 build, but no rawhide or F-9 builds and no update notice.

Comment 7 Denis Leroy 2009-01-14 10:21:00 UTC
Yes, had to figure out why it didn't build on rawhide. Fixed now, thanks.


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