[Bug 487392] New: Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

Summary: Review Request: crash-catcher - apps crash detecting daemon

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

Summary: Review Request: crash-catcher - apps crash detecting
                    daemon
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail], [e-mail]
   Estimated Hours: 0.0
    Classification: Fedora
    Target Release: ---

Spec URL: http://people.fedoraproject.org/~jmoskovc/crash-catcher.spec
SRPM URL:
http://people.fedoraproject.org/~jmoskovc/crash-catcher-0.0.1-2.fc10.src.rpm
Description: CrashCatcher is a tool to help users to detect defects in
applications and to create a bug report with all informations needed by
maintainer to fix it. It uses plugin system to extend its functionality.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

Jiri Moskovcak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail],
                   |                            |[e-mail]

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #1 from Dan Horák   2009-02-26 03:18:21 EDT ---
formal review is here, see the notes below:

BAD source files match upstream:
OK package meets naming and versioning guidelines.
BAD specfile is properly named, is cleanly written and uses macros
consistently.
OK dist tag is present.
OK license field matches the actual license.
OK license is open source-compatible (GPLv2+). License text included in
package.
OK latest version is being packaged.
OK* BuildRequires are proper.
OK compiler flags are appropriate.
OK %clean is present.
OK package builds in mock (Rawhide/x86_64).
OK debuginfo package looks complete.
BAD rpmlint is silent.
OK final provides and requires look sane.
N/A %check is present and all tests pass.
OK shared libraries are added to the regular linker search paths, correct
scriptlets exists
BAD owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
Ok file permissions are appropriate.
BAD scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK* no headers.
OK no pkgconfig files.
OK no libtool .la droppings.
BAD not a GUI app

- full URL to source archive is missing
- no need to specify License in all sub-packages, it is inherited from the main
package
- parallel make is not used (use make %{?_smp_mflags})
- when you decide to create a -devel subpackage, then you must move the
libraries from the main package to a -libs sub-package to be multilib compliant
- use %{_initddir} instead of /etc/rc.d/init.d
- no need to specify BR: glib2-devel, it is resolved automatically via
gtkmm24-devel

- rpmlint complains a bit
crash-catcher.x86_64: W: no-documentation
 => mark COPYING and README as %doc

crash-catcher.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libUtils.so
crash-catcher.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/libMiddleWare.so
 => you probably want a devel subpackage

crash-catcher.x86_64: E: executable-marked-as-config-file
/etc/rc.d/init.d/crash-catcher
crash-catcher.x86_64: W: conffile-without-noreplace-flag
/etc/rc.d/init.d/crash-catcher
 => remove %config from the initsctript

crash-catcher.x86_64: E: binary-or-shlib-defines-rpath /usr/sbin/crash-catcher
['/usr/lib64']
crash-catcher.x86_64: E: binary-or-shlib-defines-rpath
/usr/lib64/libMiddleWare.so.0.0.1 ['/usr/lib64']
crash-catcher-addon-ccpp.x86_64: E: binary-or-shlib-defines-rpath
/usr/libexec/hookCCpp ['/usr/lib64']
 => http://fedoraproject.org/wiki/Packaging:Guidelines#Removing_Rpath

crash-catcher-gui.x86_64: E: non-executable-script
/usr/share/crash-catcher/CCMainWindow.py 0644
crash-catcher-gui.x86_64: E: non-executable-script
/usr/share/crash-catcher/CCDBusBackend.py 0644
 => drop the shebang lines from the scrips, they are not intended to be run on
their own

crash-catcher-gui.x86_64: E: script-without-shebang /usr/bin/cc-gui
 => add it

crash-catcher-addon-ccpp.x86_64: W: no-documentation
crash-catcher-applet.x86_64: W: no-documentation
crash-catcher-gui.x86_64: W: no-documentation
crash-catcher-plugin-logger.x86_64: W: no-documentation
crash-catcher-plugin-mailx.x86_64: W: no-documentation
crash-catcher-plugin-sqlite3.x86_64: W: no-documentation
 => can be ignored now

crash-catcher-plugin-mailx.x86_64: E: description-line-too-long
 => fix

crash-catcher-addon-ccpp.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/crash-catcher/libCCpp.so
crash-catcher-plugin-logger.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/crash-catcher/libLogger.so
crash-catcher-plugin-mailx.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/crash-catcher/libMailx.so
crash-catcher-plugin-sqlite3.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/crash-catcher/libSQLite3.so
 => link plugins with "-avoid-version" in LDFLAGS, then it is ok to have *.so
in a non-devel package

- unowned directories:
    %{_sysconfdir}/%{name}
    %{_sysconfdir}/%{name}/plugins
    %{_libdir}/%{name}
    %{_datadir}/%{name}
  first 3 should be owned by the main package, the last one by the -gui
sub-package
- the preun scriptlet controls rarpd
- plugins are usually linked with "-avoid-version" in LDFLAGS
- desktop file is missing for the gui

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

Adam Williamson  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #2 from Adam Williamson   2009-02-26 13:55:04 EDT ---
Here's an updated .src.rpm with fixes for all issues identified by the review.
The code fixes are implemented as a patch, which obviously the crash-catcher
guys should apply to the code in trac rather than keeping as a patch.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #3 from Adam Williamson   2009-02-26 13:55:43 EDT ---
Created an attachment (id=333368)
 --> (https://bugzilla.redhat.com/attachment.cgi?id=333368)
Fixed crash-catcher package

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #4 from Adam Williamson   2009-02-26 13:56:48 EDT ---
Oh, except obviously I couldn't fix the tarball location as I'm not a
crash-catcher developer :) You guys need to put a download section on the site
and put the tarball there.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #5 from Adam Williamson   2009-02-26 14:06:42 EDT ---
sorry, one more thing - remove 'autoreconf' from the spec once the patch is
applied upstream, it's only needed while the patch is touching a Makefile.am.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #6 from Dan Horák   2009-03-02 10:03:29 EDT ---
few issues are still remaining
- %{_libdir}/%{name} is unowned - it should belong to the main package
- desktop file for the gui doesn't exist

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #7 from Jiri Moskovcak   2009-03-02 16:01:51 EDT ---
Ok, latest update, fixed desktop file, %{_libdir}/%{name} is owned by main
package.
http://jmoskovc.fedorapeople.org/crash-catcher-0.0.1-10.fc10.src.rpm
http://jmoskovc.fedorapeople.org/crash-catcher.spec

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #8 from Adam Williamson   2009-03-02 18:31:34 EDT ---
sorry, somehow I missed those two.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #9 from Dan Horák   2009-03-03 03:09:02 EDT ---
another round
- desktop-file-utils is missing as BR
- the main package must use "%dir %{_libdir}/%{name}" in the %files section
instead of %{_libdir}/%{name} only
- you should replace all occurrences of the string "crash-catcher" with
%{name}, it will make it easier to change the name of the package
- the package must use a released source archive or follow the guideline for
pre-release packages

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #10 from Jiri Moskovcak   2009-03-03 06:21:04 EDT ---
Fixed spec and srpm:
http://jmoskovc.fedorapeople.org/crash-catcher-0.0.1-12.fc10.src.rpm
http://jmoskovc.fedorapeople.org/crash-catcher.spec

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #11 from Dan Horák   2009-03-03 09:10:04 EDT ---
resolved issue
- source archive available to download and is the same as included in the srpm
    d026802acb81aa2ec26154fd09361b1c30eb70c9  crash-catcher-0.0.1.tar.gz

remaining issues
- the crash-catcher service is on by default
- don't use "--vendor fedora" when installing the desktop file and use only
%{_datadir}/applications/%{name}.desktop in %files section for the gui
subpackage
- remove "%{_libdir}/%{name}" from %files section of the libs subpackage,
because then the directory and all plugins are incorrectly owned by the libs
subpackage

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #12 from Jiri Moskovcak   2009-03-03 12:41:29 EDT ---
Updated files:
http://jmoskovc.fedorapeople.org/crash-catcher-0.0.1-12.fc10.src.rpm
http://jmoskovc.fedorapeople.org/crash-catcher.spec

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

Dan Horák  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #13 from Dan Horák   2009-03-03 13:31:41 EDT ---
All issues are fixed, this package is APPROVED.

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

--- Comment #14 from Jiri Moskovcak   2009-03-03 13:45:38 EDT ---
New Package CVS Request
=======================
Package Name: abrt
Short Description: Automatic bug detection and reporting tool
Owners: [e-mail] [e-mail]
Branches: F-10 F-11

#abrt is a new name for crash-catcher, since it's already a trademark..

[Bug 487392] Review Request: crash-catcher - apps crash detecting daemon

[Red Hat Bugzilla]

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

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

Jiri Moskovcak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs?

[Bug 487392] Review Request: abrt - Automatic bug detection and reporting tool

[Red Hat Bugzilla]

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

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

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Review Request:             |Review Request: abrt -
                   |crash-catcher - apps crash  |Automatic bug detection and
                   |detecting daemon            |reporting tool
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #15 from Kevin Fenzi   2009-03-03 15:32:19 EDT ---
Please use FAS names in Owners field.

[Bug 487392] Review Request: abrt - Automatic bug detection and reporting tool

[Red Hat Bugzilla]

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

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

Jiri Moskovcak  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE