Is there a reason why the gconf2 --makefile-uninstall-rule is missing?
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

Obviously, because once you install it, you will never ever want to uninstall
it, evar.

Or, I screwed up. Fixed & uploaded.

ok, I would be happy to review this package... 

Look for a full review in a bit (after I can figure out why doing a mockbuild 
of this on my test box causes it to lockup). 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name. 
OK - Spec has consistant macro usage. 
OK - Meets Packaging Guidelines. 
OK - License (GPL/GFDL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
5755b05a3eaebab392fe9ad49073beb2  gnucash-2.0.4.tar.bz2
5755b05a3eaebab392fe9ad49073beb2  gnucash-2.0.4.tar.bz2.1
ffc058efd0283a4b43ca31980c40db49  gnucash-docs-2.0.1.tar.bz2
ffc058efd0283a4b43ca31980c40db49  gnucash-docs-2.0.1.tar.bz2.1
afa10712d00b6a90aef0dc7fbb116ff30ded91cb  gnucash-2.0.4.tar.bz2
afa10712d00b6a90aef0dc7fbb116ff30ded91cb  gnucash-2.0.4.tar.bz2.1
ce04f51e8eeb8324b7abca6bf84ddb18562cf6b4  gnucash-docs-2.0.1.tar.bz2
ce04f51e8eeb8324b7abca6bf84ddb18562cf6b4  gnucash-docs-2.0.1.tar.bz2.1

See below - BuildRequires correct
OK - Spec handles locales/find_lang
See below - Package has %defattr and permissions on files is good. 
OK - Package has a correct %clean section. 
OK - Package has correct buildroot
OK - Package is code or permissible content. 
OK - Packages %doc files don't affect runtime. 
OK - Package is a GUI app and has a .desktop file
OK - Package compiles and builds on at least one arch. 
OK - Package has no duplicate files in %files. 
OK - Package doesn't own any directories other packages own. 
OK - Package owns all the directories it creates. 
See below - No rpmlint output. 
OK - final provides and requires are sane

SHOULD Items:

See below - Should build in mock. 
OK - Should build on all supported archs
OK - Should have dist tag
OK - Should package latest version

Issues: 

1. Is this package built often from svn snapshots? 
ie, are the %if's for svn building needed anymore?

2. Is the 
%defattr(-,root,root,755)
needed? Or will
%defattr(-,root,root,-) 
work?

3. Does rpm fail at finding the perl requires?
Is the '%define __perl_requires  %{nil}' still needed?

4. Doesn't seem to build here in mock/devel. 
The build.log has at the end: 

checking for libgsf-1 >= 1.12.2 libgsf-gnome-1 >= 1.12.2... Package libgsf-
gnome-1 was not found in the pkg-config search path. Perhaps you should add the 
directory containing `libgsf-gnome-1.pc' to the PKG_CONFIG_PATH environment 
variable No package 'libgsf-gnome-1' found
configure: error: Library requirements (libgsf-1 >= 1.12.2 libgsf-gnome-1 >= 
1.12.2) not met; consider adjusting the PKG_CONFIG_PATH environment variable if 
your libraries are in a nonstandard prefix so pkg-config can find them.
error: Bad exit status from /var/tmp/rpm-tmp.55382 (%build)

Looks like missing 'Buildrequires: libgsf-gnome-devel'
Adding that gets it building on devel here. 

5. The "--disable-sql" seems to have been added back in fc4. 
Is it worth re-enabling now?

6. Should the --with-cairo be commented in or out? 

7. Our friend rpmlint says: 

E: gnucash obsolete-not-provided gnucash-backend-postgres

I don't know how long ago the gnucash-backend-postgres was removed, 
but it might be good to provide gnucash-backend-postgres as long 
as the obsolete is still there.

E: gnucash invalid-soname /usr/lib/libgncqof-backend-qsf.so libgncqof-backend-
qsf.so
E: gnucash invalid-soname /usr/lib/libgnc-backend-file.so libgnc-backend-file.so

Can be ignored. I think rpmlint can't handle things with - in the filename
when it's not a major version number.  

W: gnucash non-conffile-in-etc /etc/gconf/schemas/apps_gnucash_warnings.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/apps_gnucash_history.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_prices.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_window_pages_register.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_reconcile.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_hbci.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_common.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/apps_gnucash_general.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_business_common.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_print_checks.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_import_generic_matcher.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_totd.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_window_pages_account_tree.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_scheduled_transctions.schemas
W: gnucash non-conffile-in-etc /etc/gconf/schemas/
apps_gnucash_dialog_commodities.schemas

I think as you say that all these can be ignored. 

E: gnucash non-executable-script /usr/share/xml/gnucash/xsl/vcard-
gnccustomer.pl 0644

Might nuke the #!/usr/bin/perl from this script or make it executable? 

E: gnucash shell-syntax-error-in-%post

There seems to be a unattached
done
in the %post. 

W: gnucash unversioned-explicit-obsoletes gnucash-backend-postgres

typically it's good to add a version thats being obsoleted. 

E: gnucash hardcoded-library-path in /usr/lib/debug

Is the '%exclude /usr/lib/debug' needed? 

E: gnucash-debuginfo script-without-shebang /usr/src/debug/gnucash-2.0.4/src/
import-export/import-commodity-matcher.c

Should be mode 644?

8. .la files should be nuked unless there is a good reason to keep them. 

9. It looks like the docs aren't released for each main gnucash release. 
Perhaps it would make sense to split them into a gnucash-docs package? 
That would save people 10MB of update when just gnucash was updated. 

10. Does %{?_smp_mflags} not work with this package?
Unless it breaks something, that should be added to both the main make 
and the docs make.

FYI, I just did a test build here... the smp_flags is a no go. It blows up... 
scratch item 10 off. 

On item 2: The %defattr(-,root,root,-) seems to generate ok permissions from a 
quick look. 

(In reply to comment #4)
> Issues: 
> 
> 1. Is this package built often from svn snapshots? 
> ie, are the %if's for svn building needed anymore?

It was in the run-up to 2.0. I haven't really built it much from SVN since then.
It could be removed as it does dirty up the spec file a lot.

> 2. Is the 
> %defattr(-,root,root,755)
> needed? Or will
> %defattr(-,root,root,-) 
> work?

755 is the default, so just (-,root,root) should work. Fixed.

> 3. Does rpm fail at finding the perl requires?
> Is the '%define __perl_requires  %{nil}' still needed?

Ooh, good catch. This was done to avoid generating perl requires on something
that we didn't ship that was satisfied by installing an Extras package. I'll try
without it to see if it can be removed.

> 4. Doesn't seem to build here in mock/devel. 
> The build.log has at the end: 
> 
> checking for libgsf-1 >= 1.12.2 libgsf-gnome-1 >= 1.12.2... Package libgsf-
> gnome-1 was not found in the pkg-config search path. Perhaps you should add the 
> directory containing `libgsf-gnome-1.pc' to the PKG_CONFIG_PATH environment 
> variable No package 'libgsf-gnome-1' found
> configure: error: Library requirements (libgsf-1 >= 1.12.2 libgsf-gnome-1 >= 
> 1.12.2) not met; consider adjusting the PKG_CONFIG_PATH environment variable if 
> your libraries are in a nonstandard prefix so pkg-config can find them.
> error: Bad exit status from /var/tmp/rpm-tmp.55382 (%build)
> 
> Looks like missing 'Buildrequires: libgsf-gnome-devel'
> Adding that gets it building on devel here. 

Hm, wonder if that was split. Will check.

> 5. The "--disable-sql" seems to have been added back in fc4. 
> Is it worth re-enabling now?

The SQL support is deprecated, lacking a maintainer,  and not supported upstream.

> 6. Should the --with-cairo be commented in or out? 

It's an option that the builtin libgoffice/libgsf snapshots took, that ended up
not working. (In Core, we built this with the builtin goffice support.) As we're
using separate goffice/gsf, the comment can be removed.

> 7. Our friend rpmlint says: 
> 
> E: gnucash obsolete-not-provided gnucash-backend-postgres
> 
> I don't know how long ago the gnucash-backend-postgres was removed, 
> but it might be good to provide gnucash-backend-postgres as long 
> as the obsolete is still there.

Well, we don't provide the functionality, and it was always built from
the gnucash SRPM. I'm not seeing how you could get into a situation where
you'd need the provide.

> E: gnucash non-executable-script /usr/share/xml/gnucash/xsl/vcard-
> gnccustomer.pl 0644
> 
> Might nuke the #!/usr/bin/perl from this script or make it executable? 

It's an example script, so having it non-executable makes sense to me. (Not sure
why they install it in datadir instead of doc) 

> E: gnucash shell-syntax-error-in-%post
> 
> There seems to be a unattached
> done
> in the %post. 

Good catch, fixed.

> W: gnucash unversioned-explicit-obsoletes gnucash-backend-postgres
> 
> typically it's good to add a version thats being obsoleted. 

See above - not sure how you'd get to where it's needed.

> E: gnucash hardcoded-library-path in /usr/lib/debug
> 
> Is the '%exclude /usr/lib/debug' needed? 

Otherwise %{_libdir}/* will pick it up.

> E: gnucash-debuginfo script-without-shebang /usr/src/debug/gnucash-2.0.4/src/
> import-export/import-commodity-matcher.c
> 
> Should be mode 644?

Possibly. Not sure it matters in debuginfo, but fixed.

> 8. .la files should be nuked unless there is a good reason to keep them. 

gnucash uses g-wrap, which uses libltdl to load shared objects. Unfortunate, but
required.

> 9. It looks like the docs aren't released for each main gnucash release. 
> Perhaps it would make sense to split them into a gnucash-docs package? 
> That would save people 10MB of update when just gnucash was updated. 

Not sure how it helps when you have the main package always requiring gnucash-docs.

New stuff uploaded to http://people.redhat.com/notting/review/ with said fixes.

(In reply to comment #6)

1. ok.
2. ok.
3. ok.
4. ok. Yeah, libgsf-gnome-devel was split off recently in devel.
5. ok.
6. ok.
7. ok. All makes sense.
8. ok.
9.
>> 9. It looks like the docs aren't released for each main gnucash release.
>> Perhaps it would make sense to split them into a gnucash-docs package?
>> That would save people 10MB of update when just gnucash was updated.
>
>Not sure how it helps when you have the main package always requiring gnucash-
docs.

Well, for instance if you have Requires: gnucash-docs = 2.0.1 now, you
can upgrade the main package without needing docs updates until the next
time they update the docs. I don't know how often upstream updates the
main package without the docs also being updated however, so this might
not be worth the trouble.

A few additional items:

- There is a 'Requires: qbanking'. Nothing currently provides that...
Should that be 'aqbanking'? Any particular version?

- Oddly the changes you made in the rpath disabling seem to have fixed
whatever I was seeing with the smp_mflags not working. This cuts buildtime
here from about 70min to about 23min. Can you confirm adding smp_mflags
works now?

(In reply to comment #8)
> >> 9. It looks like the docs aren't released for each main gnucash release.
> >> Perhaps it would make sense to split them into a gnucash-docs package?
> >> That would save people 10MB of update when just gnucash was updated.
> >
> >Not sure how it helps when you have the main package always requiring gnucash-
> docs.
> 
> Well, for instance if you have Requires: gnucash-docs = 2.0.1 now, you
> can upgrade the main package without needing docs updates until the next
> time they update the docs. I don't know how often upstream updates the
> main package without the docs also being updated however, so this might
> not be worth the trouble.

Hm, possibly. Let me see how much pain it is to split, shouldn't be
too much.

> A few additional items:
> 
> - There is a 'Requires: qbanking'. Nothing currently provides that...
> Should that be 'aqbanking'? Any particular version?

See currently-under-review aqbanking package - it's split off.

> - Oddly the changes you made in the rpath disabling seem to have fixed
> whatever I was seeing with the smp_mflags not working. This cuts buildtime
> here from about 70min to about 23min. Can you confirm adding smp_mflags
> works now?

I'll need to test - I've still seen errors where the guile bindings/C libraries
get out of sync and fail.

gnucash-docs split off, bug 227210. New gnucash spec & srpm uploaded to reflect
this.

Sounds good. I can review that at some point here if no one beats me to it. ;) 

On this package, the only thing I see left is to check if smp_mflags works all
the time, or if it still causes problems. If you could do that before the next
build that would be fine... 

This package is APPROVED. Don't forget to close it NEXTRELEASE once you have
tested the smp_mflags and such. 

make with -j33 yields...

 gcc -DHAVE_CONFIG_H -I. -I. -I../.. -I../.. -I../.. -I../.. -I../../lib/libc
-I../../src -I../../src/core-utils -pthread -I/usr/include/glib-2.0
-I/usr/lib/glib-2.0/include -std=gnu99 -pthread -I/usr/lib/g-wrap/include
-pthread -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -Wall -Wunused -Wmissing-prototypes
-Wmissing-declarations -Wdeclaration-after-statement -Wno-pointer-sign
-D_FORTIFY_SOURCE=2 -MT gw-gnc-module.lo -MD -MP -MF .deps/gw-gnc-module.Tpo -c
gw-gnc-module.c  -fPIC -DPIC -o .libs/gw-gnc-module.o
gw-gnc-module.c:167: error: expected identifier or '(' before 'if'
gw-gnc-module.c:183: warning: data definition has no type or storage class
gw-gnc-module.c:183: warning: type defaults to 'int' in declaration of 'typespec'
gw-gnc-module.c:183: error: conflicting types for 'typespec'

So... no.

Ah well, it would have been nice to have. 
I guess my test was on a dual core box, so smp_mflags was only 2... perhaps 
the higher number of jobs triggers something. ;( 

Is built now.

Package CVS Request
===================
Package Name: gnucash
New Branches: EL-4 EL-5

CVS done.