Bug 575170 (kmymoney) - Review Request: kmymoney - Personal finance
Summary: Review Request: kmymoney - Personal finance
Keywords:
Status: CLOSED RAWHIDE
Alias: kmymoney
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Toshio Ernie Kuratomi
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: DuplicSysLibsTracker
TreeView+ depends on / blocked
 
Reported: 2010-03-19 16:32 UTC by Rex Dieter
Modified: 2010-04-21 15:48 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-04-21 15:48:18 UTC
Type: ---
Embargoed:
a.badger: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Rex Dieter 2010-03-19 16:32:28 UTC
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 16:46:06 UTC
Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2063262

Comment 2 Rex Dieter 2010-03-19 16:51:14 UTC
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 20:21:11 UTC
toshio found a bundled copy of libkdchart (from koffice), adding blocker bug #504493

Comment 4 Toshio Ernie Kuratomi 2010-03-20 03:18:24 UTC
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 14:57:20 UTC
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 17:19:25 UTC
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 20:14:03 UTC
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> - 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-03 00:44:32 UTC
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 15:42:14 UTC
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 16:00:56 UTC
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> - 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-13 02:04:15 UTC
All problems taken care of.

APPROVED

Comment 12 Rex Dieter 2010-04-13 12:55:44 UTC
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 04:44:04 UTC
CVS done (by process-cvs-requests.py).

Comment 14 Rex Dieter 2010-04-21 15:48:18 UTC
imported, built for rawhide.


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