GNOME Bugzilla – Bug 615168
drop slib dependency, scheme cleanups, support guile 2.0 (patches)
Last modified: 2018-06-29 22:37:42 UTC
Hello, Andy Wingo (guile developer) sent the following message to the gnucash-devel list (see http://article.gmane.org/gmane.comp.gnome.apps.gnucash.devel/28080): "The attached patches drop the slib dependency, clean up some Scheme code, with the end result that Guile 2.0 seems to work with Gnucash. This process found a couple of bugs in Guile, so if you try with 2.0, try from git." so it would be nice if those could be applied and be ready for 2.4 release. I'm opening the ticket not seeing those patches are applied and let them not be forgotten. Sincerely, Gour
Created attachment 158193 [details] [review] 0001-remove-spurious-require-hash-table-instances.patch Here is the 1st patch.
Created attachment 158194 [details] [review] 0002-pull-in-printf-from-slib-change-all-code-to-use-it.patch 2ns patch.
Created attachment 158195 [details] [review] 0003-replace-calls-to-simple-format-with-calls-to-format.patch 3rd patch
Created attachment 158196 [details] [review] 0004-remove-bundled-srfi-8.patch 4th patch
Created attachment 158197 [details] [review] 0005-remove-instances-of-use-modules-ice-9-slib.patch 5th patch
Created attachment 158198 [details] [review] 0006-remove-configure.in-check-for-slib.patch 6th patch
Created attachment 158199 [details] [review] 0007-fixup-one-last-simple-format-bit.patch 7th patch
Created attachment 158200 [details] [review] 0008-scm_c_string_length-is-the-proper-spelling-now.patch 8th patch
Created attachment 158201 [details] [review] 0009-fix-bogus-gnc-safe-strcmp-definition.patch 9th patch
Created attachment 158202 [details] [review] 0010-we-always-have-hash-fold.patch 10th patch
Created attachment 158203 [details] [review] 0011-fix-a-number-of-scheme-syntax-errors.patch 11th patch
Created attachment 158204 [details] [review] 0012-make-sure-that-printf-is-available-whereever-it-is-u.patch 12th patch
Created attachment 158205 [details] [review] 0013-N_-in-the-root-module.patch 13th (last) patch Sincerely, Gour
Thanks for putting these on the bugzilla, Gour! Now someone just needs to apply them :)
Comment on attachment 158193 [details] [review] 0001-remove-spurious-require-hash-table-instances.patch The patches should indeed be committed. However, handling 15 attachments here is somewhat cumbersome, but I received all of them already by the above noted email message.
Comment on attachment 158196 [details] [review] 0004-remove-bundled-srfi-8.patch This one doesn't apply anymore - I think this srfi has already been deleted.
Dear Andy, I have the following comments: - In 0001, you wrote "Typo in c-interface.scm"; however, the hunk in question is the (_ ... ) function in the "lookup" function. The _ function should return the values in translated form, which is consistent to the documentation of the gnc:make-string-database function above. This does not seem like a typo to me but is rather intentional. - The 0013 patch (N_ in the root module) doesn't work with guile-1.6.8 here (guile-1.6.8 as shipped with ubuntu 9.10) and gnucash crashes on startup: In unknown file: ?: 8* [resolve-module (gnucash app-utils)] ?: 9 (let ((full-name #)) (let (#) (if already # #))) ... ?: 10 (begin (if # #) (make-modules-in # full-name)) ?: 11* (if (or # #) (try-load-module name)) ?: 12 [try-load-module (gnucash app-utils)] ?: 13 (or (begin (try-module-linked name)) (try-module-autoload name) ...) ?: 14* [try-module-autoload (gnucash app-utils)] ?: 15 (let* (# # # #) (resolve-module dir-hint-module-name #f) (and # #)) ... ?: 16 (letrec ((load-file #)) (dynamic-wind (lambda () #) (lambda () #) ...) ...) ?: 17* [dynamic-wind #<procedure #f ()> #<procedure #f ()> #<procedure #f ()>] ?: 18* [#<procedure #f ()>] ?: 19* (let ((file #)) (cond (# => #) (# => #))) ?: 20 [#<procedure #f #> "/home/chs/usr/share/gnucash/guile-modules/gnucash/app-$ ?: 21 [load-file #<primitive-procedure primitive-load> ...] ?: 22* [save-module-excursion #<procedure #f ()>] ?: 23 (let (# #) (dynamic-wind # thunk #)) ?: 24 [dynamic-wind #<procedure #f ()> #<procedure #f ()> #<procedure #f ()>] ?: 25* [#<procedure #f ()>] ?: 26* [primitive-load "/home/chs/usr/share/gnucash/guile-modules/gnucash/app-uti$ In /home/chs/usr/share/gnucash/guile-modules/gnucash/app-utils.scm: 267: 27* (define-syntax N_ (syntax-rules () ((_ x) x))) /home/chs/usr/share/gnucash/guile-modules/gnucash/app-utils.scm:267:1: In expression (define-syntax N_ (syntax-rules () #)): /home/chs/usr/share/gnucash/guile-modules/gnucash/app-utils.scm:267:1: Unbound variable: define-syntax
Comment on attachment 158193 [details] [review] 0001-remove-spurious-require-hash-table-instances.patch This crashes with guile-1.6.8, Backtrace: In unknown file: ?: 0* [gnc:send-options 0 #<procedure dispatch (key)>] In /home/chs/usr/share/gnucash/scm/options.scm: 1521: 1* [gnc:options-for-each #<procedure #f (option)> #<procedure dispatch (key)>] 1468: 2 [options-for-each #<procedure #f (option)>] ... 1308: 3 (hash-for-each (lambda # #) option-hash) /home/chs/usr/share/gnucash/scm/options.scm:1308:5: In expression (hash-for-each (lambda # #) option-hash): /home/chs/usr/share/gnucash/scm/options.scm:1308:5: Unbound variable: hash-for-each
Comment on attachment 158200 [details] [review] 0008-scm_c_string_length-is-the-proper-spelling-now.patch This one doesn't apply with current trunk anymore.
Created attachment 159459 [details] [review] Remaining 5 patches against current svn trunk r19069, but crash with guile-1.6.8 I've merged the remaining patches on current trunk, but they need fixing for guile-1.6.8.
Comment on attachment 159459 [details] [review] Remaining 5 patches against current svn trunk r19069, but crash with guile-1.6.8 (Sorry, the file is a .tar.gz with 5 git-am patches included to make downloading easier...)
Created attachment 159543 [details] [review] 6 patches Thanks for the review, Christian. Here are the remaining patches in a tarball. commit 44d7d426204a519e4d600e85bedaeeb8fff50a75 Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 20:08:35 2010 +0200 remove spurious (require 'hash-table) instances There is nothing that the slib hash-table module provides that was used in any of these files; they all used Guile's stock hash tables. src/app-utils/c-interface.scm | 2 -- src/app-utils/prefs.scm | 2 -- src/business/business-reports/aging.scm | 2 -- src/business/business-reports/easy-invoice.scm | 2 -- src/business/business-reports/fancy-invoice.scm | 2 -- src/business/business-reports/invoice.scm | 2 -- src/report/report-system/report-system.scm | 2 -- src/report/standard-reports/standard-reports.scm | 2 -- 8 files changed, 0 insertions(+), 16 deletions(-) commit d8c0cea158eac73856fb5cc96194bc4128b74427 Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 20:59:32 2010 +0200 remove instances of (use-modules (ice-9 slib)) There are no more instances of (require ...), so Guile's slib support is no longer needed. src/app-utils/c-interface.scm | 1 - src/business/business-reports/aging.scm | 1 - src/business/business-reports/easy-invoice.scm | 1 - src/business/business-reports/fancy-invoice.scm | 1 - src/business/business-reports/invoice.scm | 1 - src/business/business-reports/job-report.scm | 1 - src/business/business-reports/owner-report.scm | 1 - src/business/business-reports/payables.scm | 1 - src/business/business-reports/receivables.scm | 1 - src/import-export/qif-import/qif-import.scm | 1 - src/report/locale-specific/us/taxtxf-de_DE.scm | 1 - src/report/locale-specific/us/taxtxf.scm | 1 - src/report/report-system/eguile-utilities.scm | 2 +- src/report/report-system/report-system.scm | 1 - src/report/standard-reports/account-piecharts.scm | 1 - src/report/standard-reports/advanced-portfolio.scm | 1 - src/report/standard-reports/average-balance.scm | 2 -- src/report/standard-reports/balance-sheet.scm | 1 - src/report/standard-reports/balsheet-eg.scm | 2 +- .../standard-reports/budget-balance-sheet.scm | 1 - src/report/standard-reports/budget-barchart.scm | 1 - src/report/standard-reports/budget-flow.scm | 1 - .../standard-reports/budget-income-statement.scm | 1 - src/report/standard-reports/budget.scm | 1 - src/report/standard-reports/cash-flow.scm | 1 - src/report/standard-reports/category-barchart.scm | 1 - src/report/standard-reports/daily-reports.scm | 1 - src/report/standard-reports/equity-statement.scm | 1 - src/report/standard-reports/general-journal.scm | 1 - src/report/standard-reports/general-ledger.scm | 1 - src/report/standard-reports/income-statement.scm | 1 - src/report/standard-reports/net-barchart.scm | 1 - src/report/standard-reports/portfolio.scm | 1 - src/report/standard-reports/price-scatter.scm | 1 - src/report/standard-reports/register.scm | 1 - src/report/standard-reports/standard-reports.scm | 1 - src/report/standard-reports/transaction.scm | 1 - src/report/standard-reports/trial-balance.scm | 1 - src/report/utility-reports/view-column.scm | 1 - 39 files changed, 2 insertions(+), 40 deletions(-) commit baa07cad70d13ad168f1a77330a5e266e4299fd6 Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 21:00:18 2010 +0200 remove configure.in check for slib Gnucash no longer requires slib. configure.in | 14 -------------- 1 files changed, 0 insertions(+), 14 deletions(-) commit 947eb1a93774e421739becd965cf20470a34a71d Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 21:20:59 2010 +0200 scm_c_string_length is the proper spelling now Fix up a couple uses of scm_i_string_length, and add a back-compat shim for earlier versions. src/engine/engine-helpers.c | 4 ++-- src/gnome-utils/dialog-options.c | 2 +- src/guile-mappings.h | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) commit 4887ab4529efb0632ed2669b9cfe89d03d5d9a7a Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 21:53:17 2010 +0200 don't define hash-fold for guile 2.x * src/scm/main.scm (hash-fold): Only do the conditional hash-fold dance if we don't have guile 2.0. (Definitions in expression context are actually invalid Scheme, but older Guiles are more permissive.) src/scm/main.scm | 21 ++++++++++++--------- 1 files changed, 12 insertions(+), 9 deletions(-) commit 23e92285f409389f34bd490db44581c5201fd1db Author: Andy Wingo <wingo@pobox.com> Date: Sun Apr 25 21:57:12 2010 +0200 N_ in the root module * src/app-utils/app-utils.scm: * src/app-utils/c-interface.scm: Make N_ available to all of Gnucash. A hack, but a correct hack that permits the Scheme code to be compiled. src/app-utils/app-utils.scm | 17 +++++++++++++++++ src/app-utils/c-interface.scm | 11 ----------- 2 files changed, 17 insertions(+), 11 deletions(-)
Same situation as in comment 18: Already the 0001 patch "remove spurious (require 'hash-table) instances" doesn't work with guile-1.6.8 here. It crashes on startup (if I have some reports open) with the following error: Backtrace: In current input: 1: 0* [gnc:report-menu-setup] ?: 1 (letrec (# # # ...) (gnc-add-scm-extension #) ...) In /home/chs/usr/share/gnucash/guile-modules/gnucash/report/report-gnome.scm: 129: 2* [gnc:add-report-template-menu-items] In unknown file: ?: 3 (letrec (# # # ...) (gnc:report-templates-for-each add-template) ...) In /home/chs/usr/share/gnucash/guile-modules/gnucash/report/report-gnome.scm: 92: 4* [gnc:report-templates-for-each #<procedure add-template #>] In /home/chs/usr/share/gnucash/scm/report.scm: 679: 5 (hash-for-each (lambda # #) *gnc:_report-templates_*) /home/chs/usr/share/gnucash/scm/report.scm:679:3: In expression (hash-for-each (lambda # #) *gnc:_report-templates_*): /home/chs/usr/share/gnucash/scm/report.scm:679:3: Unbound variable: hash-for-each
But I've applied your patches 0004 and 0005 already as they didn't concern any of the above crashes.
I think the issue (w.r.t. comment #23) is that saved reports use the hash-for-each -- so we will need to continue to support the symbol.
Created attachment 160665 [details] [review] Remaining 4 patches against SVN trunk r19148 I've applied most hunks of the 0001 and 0002 patches so that one last instance of (require 'hash-table) and (use-modules (ice-9 slib)) remain in report-system/report-system.scm. Different to the behaviour in comment #23 gnucash now start without crashing, but if those two lines were removed, it crashes again on startup with guile-1.6.8. Re comment #25: The file ~/.gnucash/saved-reports-2.0 or any of the files in ~/.gnucash/books/ exist and contain scheme code, but they do not contains any hash-for-each function (and also no line including the string "hash"), so I don't know why those might cause a crash on startup.
Thanks for looking at these patches. I don't really have the bandwidth to help until the end of the week, but if this bug is still open then I'll have a ping. The intent of my patch was not to remove hash-for-each, but remove a definition in expression context, assuming that it was available; but realizing that 1.6 didn't have it, my revised patch tried to make it available only within a cond-expand, but it seems to not have worked on 1.6.
As reported in bug#618331 and on our mailing list, the two patches 0001 and 0002 shouldn't have been committed in the first place as they are causing crashes due to "Unbound variable: hash-for-each" in even more places than I thought. In SVN, we will revert them for now. @Andy: Sure, take your time. Feel free to submit updated patches and I'll check whether they work in guile-1.6.
Created attachment 161023 [details] [review] Remaining 4 patches against SVN trunk r19155 As noted in comment #28, the applied parts of the first two patches still caused crashes in guile-1.6, so I reverted more parts of it. Attached is the part of your contributions which (after my revert) are not (yet) applied to SVN.
Created attachment 161025 [details] Remaining 4 patches against SVN trunk r19156 (trunk r19155 was still crashing on guile-1.6)
src/report/report-system/eguile-utilities.scm also needs slib's sub-vicinity and related routines in the find-file routine that returns a file's full path name. With slib removed from eguile-utilities, the Tax Invoice report fails because sub-vicinity is not defined. There may well be better ways to do what find-file does that don't require slib, but that's beyond my limited knowledge of Guile (and GnuCash).
@Andy: Any updates here?
No updates, no; been busy with Guile itself...
Committed as revisions 20435 to 20438 as part of bug 621238. Thank you very much for these patches!
Hey, thanks for committing them, and apologies for not doing more on my side to follow through. Happy hacking :)
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=615168. Please update any external references or bookmarks.