Bug 575170 (kmymoney)

Summary: Review Request: kmymoney - Personal finance
Product: [Fedora] Fedora Reporter: Rex Dieter <rdieter>
Component: Package ReviewAssignee: Toshio Ernie Kuratomi <a.badger>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: a.badger, fedora-package-review, notting
Target Milestone: ---Flags: a.badger: fedora‑review+
kevin: fedora‑cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-04-21 11:48:18 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 504493    

Description Rex Dieter 2010-03-19 12:32:28 EDT
Spec URL: http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney-3.96.1-1.fc14.src.rpm
Description:
KMyMoney strives to be the best personal finance manager.
The ultimate objectives of KMyMoney are...
* Accuracy.  Using time tested double entry accounting principles
  helps ensure that your finances are kept in correct order.
* Ease of use.  Strives to be the easiest open source personal
  finance manager to use, especially for the non-technical user.
* Familiar Features.  Intends to provide all important features
  found in the commercially-available, personal finance managers.


This is a kde4 port of kmymoney2 in fedora now.  That big change, as well as the pkg rename kmymoney2->kmymoney warrants a re-review.

This will target F-14+
Comment 1 Rex Dieter 2010-03-19 12:46:06 EDT
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2063262
Comment 2 Rex Dieter 2010-03-19 12:51:14 EDT
rpmlint *.rpm x86_64/*.rpm
kmymoney.x86_64: W: hidden-file-or-dir /usr/share/kde4/apps/kmymoney/pics/l10n/de/.directory
kmymoney.x86_64: W: hidden-file-or-dir /usr/share/kde4/apps/kmymoney/pics/l10n/ro/.directory
kmymoney.x86_64: E: script-without-shebang /usr/share/kde4/apps/kmymoney/misc/financequote.pl
kmymoney-devel.x86_64: W: obsolete-not-provided kmymoney2-devel
kmymoney-devel.x86_64: W: no-documentation
kmymoney-libs.x86_64: W: spelling-error Summary(en_US) runtime -> run time, run-time, untimely
kmymoney-libs.x86_64: W: summary-not-capitalized C kmymoney runtime libraries
kmymoney-libs.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, untimely
kmymoney-libs.x86_64: W: obsolete-not-provided kmymoney2-libs
kmymoney-libs.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 1 errors, 9 warnings.
Comment 3 Rex Dieter 2010-03-19 16:21:11 EDT
toshio found a bundled copy of libkdchart (from koffice), adding blocker bug #504493
Comment 4 Toshio Ernie Kuratomi 2010-03-19 23:18:24 EDT
Full review:

GOOD:

* Upstream tarball is kmymoney, name of package and srpm follows naming guidelines
* License is GPLv2 or GPLv3 which is acceptable
* License file included
* spec file is legible
* Source matches upstream
* Locales handled with %find_lang
* Shared libraries in -libs subpackage.  ldconfig is properly run there.
* Package owns all directories and files it creates and nothing more
* Proper %clean
* Macros used consistently
* Package contains code, not content
* Headers in the -devel subpacakge
* No static libs
* No pkgconfig files
* .so files are in the devel package
* devel package properly requires the exact vesion/release of the libs package.
* No libtool .la files
* Properly contains a .desktop file
* No file dependencies
* kmymoney has a man page
* Package builds in koji
* kmymoney starts, runs, imports files from kmymoney2

NEEDSWORK:
* License field says GPLv2+ but it's really GPLv2 or GPLv3 due to the license of libkdchart.
  - Note: not sure if kmymoney upstream is aware of this subtle difference -- it doesn't affect us at the moment
* Bundled libkdchart: talked with rdieter.  Big can of worms here.  Upstream is officially www.kdab.com.  However, it looks  like their version of the product isn't open source or perhaps just isn't available for download.  Koffice has a bundled copy.
  - Unclear at what point in time the koffice version forked off from upstream.
  - Unclear if the koffice version is being developed with API changes.
  - Unclear if they're syncing with the KDAB version.
  - Unclear if the kmymoney version is a koffice or a kdab fork.
  - Unclear whether the version in kmymoney is modified from upstream
  - Unclear what point in time the kmymoney version forked from upstream
  - Note: one mitigating factor is that the current, kmymoney2 package (which this replaces entirely) contains a bundled libkdchart as well.  So this is something of a case of the package continuing to contain a bundled copy since it wasn't caught on initial review of kmymoney2.  Those packages are not grandfathered (under the current Guidelines) but we haven't yet removed a package from the distro, preferring to give the maintainer time to figure out a way to fix the problem.
  - Note:  rdieter is working on changes to the current Guidelines that might allow this and other packages in if they fit certain criteria.

QUESTIONING:

These may need to be fixed here or maybe should result in Guideline changes.

* Large amount of docs in the main package but they're intended for online help (Help::KMyMoney Handbook)  This seems okay.  OTOH, these files are marked as %doc but online help won't work if they're excluded which could fall under the "files marked as %doc should not affect runtime".  I note that gnome apps seem to not mark these as %doc.  This probably needs to be discussed and either one method decided on or documented in the Guidelines as to why they differ.

* (nonblocker) desktop-file-validate is run in %check rather than %install.  Should we amend the Guidelines to say to run this in %install or %check? http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

* (nonblocker) You're doing two differences from what's on the ScriptletSnippets page -- both look like they're improvements over what we have there:
  1. updates in %postun are done only on package removal.  
  2. Use of %posttrans instead of %post for update-mime-database and update-desktop-database.
  Do we want to propose these as updates to the Guidelines?

RPMLINT:

kmymoney.x86_64: W: hidden-file-or-dir /usr/share/kde4/apps/kmymoney/pics/l10n/de/.directory
kmymoney.x86_64: W: hidden-file-or-dir /usr/share/kde4/apps/kmymoney/pics/l10n/ro/.directory

- Are these needed?

kmymoney.x86_64: E: script-without-shebang /usr/share/kde4/apps/kmymoney/misc/financequote.pl

- This should either be set to nonexecutable or have a /usr/bin/perl shebang line added

[... Lots of errors like the following for files in debuginfo ..]
kmymoney-debuginfo.x86_64: E: world-writable /usr/src/debug/kmymoney-3.96.1/x86_64-redhat-linux-gnu/kmymoney/widgets/ui_kmymoneyreportconfigtab3decl.h 0666
[...]

- Probably need to run find . -type f -name '*.h' -exec chmod 0644 kmymoney

kmymoney-devel.x86_64: W: obsolete-not-provided kmymoney2-devel
kmymoney-libs.x86_64: W: obsolete-not-provided kmymoney2-libs

- I'm assuming the API has changed enough that the old package name shouldn't be provided for the library related subpackags.  Correct this if that's untrue.

kmymoney-devel.x86_64: W: no-documentation

- I notice that there's some developer documentation in deverloper-docs but I don't know if it's of any use for developing against kmymoney-devel.  If it is, it can be included.

kmymoney-libs.x86_64: W: spelling-error Summary(en_US) runtime -> run time, run-time, untimely
kmymoney-libs.x86_64: W: spelling-error %description -l en_US runtime -> run time, run-time, untimely

- runtime is a common spelling so no problem here.

kmymoney-libs.x86_64: W: summary-not-capitalized C kmymoney runtime libraries

- Up to you -- Kmymoney would be okay when taken as a project name, kmymoney if you just think of it as a program.

kmymoney-libs.x86_64: W: no-documentation

- I don't think there's any documentation needed here.
Comment 5 Rex Dieter 2010-03-22 10:57:20 EDT
After discussing the kdchart situation with koffice,kmymoney developers as well as kdab representative, we came up with a tentative plan:

short term:  treat the koffice copy as the canonical one, and patch kmymoney to use that (system) copy

long term: kdab is working through some administrative hurdles to possibly releaese kdchart itself sometime.  when/if that happens, then treat that one as the canonical version, and then patch koffice to use that one too.  (as it stands, kdab only releases the GPL code to it's paying customers)

See also:
http://lists.kde.org/?l=koffice-devel&m=126902833513539&w=2
http://sourceforge.net/mailarchive/forum.php?thread_name=64e15f8f1003191630yb3d0657y769329823d3b0742%40mail.gmail.com&forum_name=kmymoney2-developer
Comment 6 Toshio Ernie Kuratomi 2010-03-22 13:19:25 EDT
Sounds like a good plan to me.  Without a way to get direct access to the kdab provided library we can't treat it as the canonical upstream in this case so going with the koffice version is a very reasonable choice.
Comment 7 Rex Dieter 2010-04-02 16:14:03 EDT
Spec URL: http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney.spec
SRPM URL:
http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney-3.97.0-1.fc12.src.rpm

%changelog
* Fri Apr 02 2010 Rex Dieter <rdieter@fedoraproject.org> - 3.97.0-1
- kmymoney-3.97.0
- use external/shared kdchart


koffice build with koffice-kdchart-devel subpkg going now,
http://koji.fedoraproject.org/koji/taskinfo?taskID=2092160
Comment 8 Toshio Ernie Kuratomi 2010-04-02 20:44:32 EDT
kdchart bundling taken care of.  The other items from comment #4 are still outstanding:

NEEDSWORK:
* License field says GPLv2+ but it's really GPLv2 or GPLv3 due to the license
of libkdchart.
  - I took a brief look at the source in koffice -- looks like anything that links to libkdgantt or libkdchart (both kdab products?) need to be GPLv2 OR GPLv3 instead of GPLv2+.

* The question of online docs that are marked %doc
* Whether to propose the desktop-file-validate and ScriptletSnippets changes to the packaging committee (nonblocker but we should figure out if this is something that we should update the best practices for).
* The .directory files
* /usr/share/kde4/apps/kmymoney/misc/financequote.pl => script without shebang
* The world writable source files ending up in debug packages (I think the files might be generated during build so - do this during %install:: find . -type f -name '*.h' -exec chmod 0644 kmymoney)
Comment 9 Rex Dieter 2010-04-07 11:42:14 EDT
working on 3.97.2 update now, looks like shebang and world writable files are fixed in the new tarball.

Marking handbooks as %doc has historically been done (and is included in rpm %find_)ang macro).

Scriptlet snippet changes probably should be considered by the committee.  Doing as much as possible in %posttrans instead of %post is largely a no-brainer, imo. (I'll work on writing up a proposal when I get a chance).
Comment 10 Rex Dieter 2010-04-07 12:00:56 EDT
I take it back about the world-writable files... those are generated at runtime.  eww.  

Spec URL: http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney.spec
SRPM URL:
http://rdieter.fedorapeople.org/rpms/kmymoney/kmymoney-3.97.2-1.fc14.src.rpm


%changelog
* Wed Apr 07 2010 Rex Dieter <rdieter@fedoraproject.org> - 3.97.2-1
- kmymoney-3.97.2
- License: GPLv2 or GPLv3
- omit .directory files from packaging
- -debuginfo: fix world-writable perms in generated headers
Comment 11 Toshio Ernie Kuratomi 2010-04-12 22:04:15 EDT
All problems taken care of.

APPROVED
Comment 12 Rex Dieter 2010-04-13 08:55:44 EDT
New Package CVS Request
=======================
Package Name: kmymoney
Short Description: Personal finance
Owners: rdieter
Branches: F-13
InitialCC: tuxbrewr
Comment 13 Kevin Fenzi 2010-04-14 00:44:04 EDT
CVS done (by process-cvs-requests.py).
Comment 14 Rex Dieter 2010-04-21 11:48:18 EDT
imported, built for rawhide.