Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Oops, this push inadvertently added unfinished work on GSettings. I have 
reverted it for now.

Regards,

Geert

Op zaterdag 16 oktober 2021 10:49:14 CEST schreef Geert Janssens:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/d4e4062c 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/24fa2899 (commit)
>via  https://github.com/Gnucash/gnucash/commit/26b2d7ca (commit)
>via  https://github.com/Gnucash/gnucash/commit/c6103a5c (commit)
>   from  https://github.com/Gnucash/gnucash/commit/16184daf (commit)
> 
> 
> 
> commit d4e4062c9310f50729c94b73e9f2bd80382fabfb
> Author: Geert Janssens 
> Date:   Sat Oct 16 10:48:55 2021 +0200
> 
> Remove redundant function declaration
> 
> gnc_load_scm_configuration is declared and defined in gnucash-core-app.
> 
> commit 24fa289952257cb8a2f2b9d59024be59d05888e3
> Author: Geert Janssens 
> Date:   Wed Oct 6 18:53:36 2021 +0200
> 
> GSettings - rework internal helper functions to use more C++
> 
> commit 26b2d7ca7878776a7c6777cd30e1b19185b72983
> Author: Geert Janssens 
> Date:   Wed Oct 6 17:11:00 2021 +0200
> 
> GSettings - make most of the api private
> 
> All preference calls should happen via the gnc_prefs_... apis.
> The gnc_gsettings_... apis are an internal implementation of this.
> 
> commit c6103a5c171f459b6ca070c61bcde939885ae613
> Author: Geert Janssens 
> Date:   Wed Oct 6 16:50:57 2021 +0200
> 
> Add structure to map migrated preferences to old ones
> 
> This can be used to keep both in sync in the period between
> initial migration and eventual obsolence.
> Note only non-obsoleted, migrated preferences are tracked.
> We don't want to resync preferences that have been
> obsoleted (reset). That would nullify the whole idea
> of making them obsolete for future removal.
> 
> This commit only adds the mapping, synching will follow in a future
> commit.
> 
> 
> 
> Summary of changes:
>  gnucash/gnucash-commands.hpp   |   4 -
>  libgnucash/app-utils/gnc-gsettings.cpp | 277 +++--
>  libgnucash/app-utils/gnc-gsettings.h   | 550
> - libgnucash/app-utils/gnc-prefs-utils.c | 
>  1 +
>  4 files changed, 189 insertions(+), 643 deletions(-)
> 
> ___
> gnucash-patches mailing list
> gnucash-patc...@gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-patches




___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Nov 13, 2020, at 2:13 AM, Geert Janssens  
> wrote:
> 
> Op vrijdag 13 november 2020 00:52:44 CET schreef John Ralls:
>> Updated   via  https://github.com/Gnucash/gnucash/commit/d751913c 
>> (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/4e91a33b (commit)
>>  from  https://github.com/Gnucash/gnucash/commit/a519b913 (commit)
>> 
>> 
>> 
>> commit d751913cb9d422be91595f42f89674248ffe90aa
>> Author: John Ralls 
>> Date:   Mon Nov 9 14:26:15 2020 -0800
>> 
>>Fix typo in .gitattributes, text not test.
>> 
>> commit 4e91a33be358ac952ea31c2c62c816070c15b453
>> Author: John Ralls 
>> Date:   Mon Nov 9 13:50:51 2020 -0800
>> 
>>Use LTDL_LIBRARY_PATH instead of (DY)LD_LIBRARY_PATH
>> 
>>With GnuCash's main libraries now linked at startup instead of being
>>dlopened at runtime except by Guile we can restrict the library path
>>renaming to Gnu Libtool's environment variable.
> 
> Gnucash is no longer using gnc-module itself but the gnc-module interface is 
> still present though. So in theory we still support user-built custom gnc-
> modules. Would they still work after your change on MacOS ?
> 
> If not, we should either just drop gnc-module completely (which Derek opposed 
> IIRC) or clearly document the extra  steps the user should take to make it 
> work, which IIUC on MacOS means setting (DY)LD_LIBRARY_PATH such that the 
> custom gnc-module is found.

GnuCash does still use gnc-module for aqbanking and import-binary, and yes, 
they still work because the actual paths are set by gnc_find_module and the 
values of $GNC_MODULE_PATH. A user trying to load their own gnc-module from 
somewhere else would need to add that somewhere else to $GNC_MODULE_PATH, but 
they'd also need to modify and recompile GnuCash to actually load their module.


Regards,
John Ralls

[1] https://github.com/Gnucash/gnucash/blob/maint/gnucash/gnucash.cpp#L143

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op vrijdag 13 november 2020 00:52:44 CET schreef John Ralls:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/d751913c 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/4e91a33b (commit)
>   from  https://github.com/Gnucash/gnucash/commit/a519b913 (commit)
> 
> 
> 
> commit d751913cb9d422be91595f42f89674248ffe90aa
> Author: John Ralls 
> Date:   Mon Nov 9 14:26:15 2020 -0800
> 
> Fix typo in .gitattributes, text not test.
> 
> commit 4e91a33be358ac952ea31c2c62c816070c15b453
> Author: John Ralls 
> Date:   Mon Nov 9 13:50:51 2020 -0800
> 
> Use LTDL_LIBRARY_PATH instead of (DY)LD_LIBRARY_PATH
> 
> With GnuCash's main libraries now linked at startup instead of being
> dlopened at runtime except by Guile we can restrict the library path
> renaming to Gnu Libtool's environment variable.

Gnucash is no longer using gnc-module itself but the gnc-module interface is 
still present though. So in theory we still support user-built custom gnc-
modules. Would they still work after your change on MacOS ?

If not, we should either just drop gnc-module completely (which Derek opposed 
IIRC) or clearly document the extra  steps the user should take to make it 
work, which IIUC on MacOS means setting (DY)LD_LIBRARY_PATH such that the 
custom gnc-module is found.

Regards,

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Christopher Lam]

Unfortunately cleaning up advanced-portfolio.scm is much more challenging
than I expected, so, I've now reverted it to its original (as of 4.0)
state, with suitable changes to support guile-3.0 only.

On Sat, 1 Aug 2020 at 05:55, Christopher Lam  wrote:

> Updated  via  https://github.com/Gnucash/gnucash/commit/12ab85fa (commit)
>  via  https://github.com/Gnucash/gnucash/commit/6f196031 (commit)
> from  https://github.com/Gnucash/gnucash/commit/4df6493b (commit)
>
>
>
> commit 12ab85fa6c147df2714a14c778412f930f89ed40
> Author: Christopher Lam 
> Date:   Sat Aug 1 10:12:38 2020 +0800
>
> [advanced-portfolio] use G_ for guile-3.0
>
> diff --git a/gnucash/report/reports/standard/advanced-portfolio.scm
> b/gnucash/report/reports/standard/advanced-portfolio.scm
> index 279fcb91f..192e97e63 100644
> --- a/gnucash/report/reports/standard/advanced-portfolio.scm
> +++ b/gnucash/report/reports/standard/advanced-portfolio.scm
> @@ -1048,8 +1048,8 @@ by preventing negative stock balances.")
> (lambda (foreign domestic date)
>  (find-price
> (gnc-pricedb-lookup-nearest-in-time-any-currency-t64
>  pricedb foreign (time64CanonicalDayTime date))
> domestic)
> -  (headercols (list (_ "Account")))
> -  (totalscols (list (gnc:make-html-table-cell/markup
> "total-label-cell" (_ "Total"
> +  (headercols (list (G_ "Account")))
> +  (totalscols (list (gnc:make-html-table-cell/markup
> "total-label-cell" (G_ "Total"
>(sum-total-moneyin (gnc-numeric-zero))
>(sum-total-income (gnc-numeric-zero))
>(sum-total-both-gains (gnc-numeric-zero))
> @@ -1060,37 +1060,37 @@ by preventing negative stock balances.")
>
>   ;;begin building lists for which columns to display
>(if show-symbol
> - (begin (append! headercols (list (_ "Symbol")))
> + (begin (append! headercols (list (G_ "Symbol")))
>  (append! totalscols (list " "
>
>   (if show-listing
> - (begin (append! headercols (list (_ "Listing")))
> + (begin (append! headercols (list (G_ "Listing")))
>  (append! totalscols (list " "
>
>   (if show-shares
> - (begin (append! headercols (list (_ "Shares")))
> + (begin (append! headercols (list (G_ "Shares")))
>  (append! totalscols (list " "
>
>   (if show-price
> - (begin (append! headercols (list (_ "Price")))
> + (begin (append! headercols (list (G_ "Price")))
>  (append! totalscols (list " "
>
>   (append! headercols (list " "
> -   (_ "Basis")
> -   (_ "Value")
> -   (_ "Money In")
> -   (_ "Money Out")
> -   (_ "Realized Gain")
> -   (_ "Unrealized Gain")
> -   (_ "Total Gain")
> -   (_ "Rate of Gain")
> -   (_ "Income")))
> +   (G_ "Basis")
> +   (G_ "Value")
> +   (G_ "Money In")
> +   (G_ "Money Out")
> +   (G_ "Realized Gain")
> +   (G_ "Unrealized Gain")
> +   (G_ "Total Gain")
> +   (G_ "Rate of Gain")
> +   (G_ "Income")))
>
>   (if (not (eq? handle-brokerage-fees 'ignore-brokerage))
> - (append! headercols (list (_ "Brokerage Fees"
> + (append! headercols (list (G_ "Brokerage Fees"
>
> - (append! headercols (list (_ "Total Return")
> -   (_ "Rate of Return")))
> + (append! headercols (list (G_ "Total Return")
> +   (G_ "Rate of Return")))
>
>(append! totalscols (list " "))
>
> @@ -1187,14 +1187,14 @@ by preventing negative stock balances.")
>(gnc:html-document-add-object! document table)
>(if warn-price-dirty
>(gnc:html-document-append-objects! document
> - (list
> (gnc:make-html-text (_ "* this commodity data was built using transaction
> pricing instead of the price list."))
> + (list
> (gnc:make-html-text (G_ "* this commodity data was built using transaction
> pricing instead of the price list."))
>(gnc:make-html-text
> (gnc:html-markup-br))
> -  (gnc:make-html-text
> 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

Testing the Swig bindings would be a good thing, the more thorough the better. 
For the most part the Swig binding tests just make sure that they can load a 
module, which isn't much of a test.

Regards,
John Ralls


> On Jul 19, 2019, at 8:44 AM, Christopher Lam  
> wrote:
> 
> Hi, not sure if .scm tests are welcome in engine/tests;
> test-business-core.scm in maint is running well however in master it fails
> to access the engine. ACCT-TYPE-LIABILITY is undefined, ditto
> gnc-commodity-table-lookup etc. The following is not adequate to allow
> test-business-core.scm to access core engine. How to fix?
> 
> modified   libgnucash/engine/test/CMakeLists.txt
> @@ -247,7 +247,14 @@ if (HAVE_SRFI64)
> test-business-core.scm
> )
> 
> -  gnc_add_scheme_tests("${scm_tests_with_srfi64_SOURCES}")
> +  gnc_add_scheme_test_targets (scm-test-with-srfi64
> +"${scm_tests_with_srfi64_SOURCES}"
> +"tests"
> +
> "${GUILE_DEPENDS};scm-test-engine-extras;scm-srfi64-extras;gncmod-engine"
> +FALSE
> +)
> +
> +gnc_add_scheme_tests ("${scm_tests_with_srfi64_SOURCES}")
> endif (HAVE_SRFI64)
> 
> gnc_add_scheme_test_targets(scm-test-engine
> 
> -- Forwarded message -
> From: Christopher Lam 
> Date: Fri, 19 Jul 2019 at 14:25
> Subject: gnucash maint: Multiple changes pushed
> To: 
> 
> 
> Updated  via  https://github.com/Gnucash/gnucash/commit/0c433e02 (commit)
> via  https://github.com/Gnucash/gnucash/commit/e654bd34 (commit)
> via  https://github.com/Gnucash/gnucash/commit/57f291af (commit)
> via  https://github.com/Gnucash/gnucash/commit/75d5d810 (commit)
>from  https://github.com/Gnucash/gnucash/commit/8adcacbd (commit)
> 
> 
> 
> commit 0c433e02f7d0003c10e1244f572d0a9badd46e33
> Author: Christopher Lam 
> Date:   Fri Jul 19 02:15:13 2019 +0800
> 
>[business-core] deprecate gnc:entry-type-percent-p
> 
>This function is only used once. deprecate it.
> 
> diff --git a/gnucash/report/business-reports/invoice.scm
> b/gnucash/report/business-reports/invoice.scm
> index 7b99e1d08..b5c9e3f32 100644
> --- a/gnucash/report/business-reports/invoice.scm
> +++ b/gnucash/report/business-reports/invoice.scm
> @@ -105,7 +105,7 @@
>   (_ "Total"
> 
> (define (monetary-or-percent numeric currency entry-type)
> -  (if (gnc:entry-type-percent-p entry-type)
> +  (if (eqv? entry-type GNC-AMT-TYPE-PERCENT)
>   (string-append (gnc:default-html-gnc-numeric-renderer numeric #f) "
> " (_ "%"))
>   (gnc:make-gnc-monetary currency numeric)))
> 
> diff --git a/libgnucash/engine/business-core.scm
> b/libgnucash/engine/business-core.scm
> index 80e9737f9..f20d39044 100644
> --- a/libgnucash/engine/business-core.scm
> +++ b/libgnucash/engine/business-core.scm
> @@ -94,6 +94,8 @@
>   (else ""
> 
> (define (gnc:entry-type-percent-p type-val)
> +  (issue-deprecation-warning
> +   "gnc:entry-type-percent-p is deprecated.")
>   (let ((type type-val))
> (equal? type GNC-AMT-TYPE-PERCENT)))
> 
> 
> commit e654bd34af64235b5f9ac0dd8a05f6db9d8c912b
> Author: Christopher Lam 
> Date:   Fri Jul 19 02:24:45 2019 +0800
> 
>[business-core] simplify functions
> 
> diff --git a/libgnucash/engine/business-core.scm
> b/libgnucash/engine/business-core.scm
> index a05204528..80e9737f9 100644
> --- a/libgnucash/engine/business-core.scm
> +++ b/libgnucash/engine/business-core.scm
> @@ -50,35 +50,25 @@
> ;
> 
> (define (gnc:owner-get-name-dep owner)
> -  (define (just-name name)
> -(if name name ""))
> -
> -  (let ((type (gncOwnerGetType owner)))
> -(cond
> -  ((eqv? type GNC-OWNER-JOB)
> -   (gnc:owner-get-name-dep (gncJobGetOwner
> -   (gncOwnerGetJob owner
> -  (else (just-name (gncOwnerGetName owner))
> +  (cond
> +   ((eqv? (gncOwnerGetType owner) GNC-OWNER-JOB)
> +(gnc:owner-get-name-dep (gncJobGetOwner (gncOwnerGetJob owner
> +   (else (or (gncOwnerGetName owner) ""
> 
> (define (gnc:owner-get-address-dep owner)
> -  (define (add-if-exists lst new)
> -(if (and new (> (string-length new) 0))
> -   (cons new lst)
> -   lst))
> -  (define (build-string lst)
> -(cond
> - ((null? lst) "")
> - ((null? (cdr lst)) (car lst))
> - (else (string-append (build-string (cdr lst)) "\n" (car lst)
> -  (let ((lst '())
> -   (addr (gnc:owner-get-address owner)))
> -; Added gncAddressGetName  
> -(set! lst (add-if-exists lst (gncAddressGetName  addr)))
> -(set! lst (add-if-exists lst (gncAddressGetAddr1 addr)))
> -(set! lst (add-if-exists lst (gncAddressGetAddr2 addr)))
> -(set! lst (add-if-exists lst (gncAddressGetAddr3 addr)))
> -(set! lst (add-if-exists lst (gncAddressGetAddr4 addr)))
> -(build-string lst)))
> +  (define (addif elt)
> +(if (and elt (> (string-length elt) 0))
> +(list elt)
> +'()))
> +  (let ((addr (gnc:owner-get-address owner)))
> +(string-join
> + (append
> +  (addif (gncAddressGetName  addr))
> +  

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Wm via gnucash-devel]

On 26/02/2019 11:56, Christopher Lam wrote:

Hi Geert,
Sharp eye indeed.
I haven't used the new gnc:gui-error function because report.scm is being
simulateously attacked:
(1) refactoring in maint-scheme-progress
(2) slowly creating nearly 100% coverage for tests
(3) fixing an invalid code path introduced about 8 months ago, which was
returning #f to signify failure for a report-definition-without-guid, when
in reality the report-definition-without-guid was correctly handled by
autogenerating guid.
I considered gnc:gui-error to be a proper refactoring job belonging to (1)
in my unpushed maint-scheme-progress branch but needed to create tests
during (2) first to ensure the refactoring was safe, and the tests needed
to handle non-gui code properly.
I felt that completing (2) was more important than (1).
But you're right, we could easily have used gnc:gui-error here.


I love your confidence, Cristopher Lam.

--
Wm


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op dinsdag 26 februari 2019 12:56:47 CET schreef Christopher Lam:
> Hi Geert,
> Sharp eye indeed.
> I haven't used the new gnc:gui-error function because report.scm is being
> simulateously attacked:
> (1) refactoring in maint-scheme-progress
> (2) slowly creating nearly 100% coverage for tests
> (3) fixing an invalid code path introduced about 8 months ago, which was
> returning #f to signify failure for a report-definition-without-guid, when
> in reality the report-definition-without-guid was correctly handled by
> autogenerating guid.
> I considered gnc:gui-error to be a proper refactoring job belonging to (1)
> in my unpushed maint-scheme-progress branch but needed to create tests
> during (2) first to ensure the refactoring was safe, and the tests needed
> to handle non-gui code properly.
> I felt that completing (2) was more important than (1).
> But you're right, we could easily have used gnc:gui-error here.
> 
Sure. That's fine. If it makes sense in your opinion you can also do a 
followup commit to use gnc:gui-error instead. I can't tell, I'm not following 
all your work in detail.

Regards,

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Christopher Lam]

Hi Geert,
Sharp eye indeed.
I haven't used the new gnc:gui-error function because report.scm is being
simulateously attacked:
(1) refactoring in maint-scheme-progress
(2) slowly creating nearly 100% coverage for tests
(3) fixing an invalid code path introduced about 8 months ago, which was
returning #f to signify failure for a report-definition-without-guid, when
in reality the report-definition-without-guid was correctly handled by
autogenerating guid.
I considered gnc:gui-error to be a proper refactoring job belonging to (1)
in my unpushed maint-scheme-progress branch but needed to create tests
during (2) first to ensure the refactoring was safe, and the tests needed
to handle non-gui code properly.
I felt that completing (2) was more important than (1).
But you're right, we could easily have used gnc:gui-error here.

On Tue, 26 Feb 2019 at 19:38, Geert Janssens 
wrote:

> Op dinsdag 26 februari 2019 11:24:26 CET schreef Christopher Lam:
> > Updatedvia  https://github.com/Gnucash/gnucash/commit/d980bb50
> (commit)
> >via  https://github.com/Gnucash/gnucash/commit/0ce6999f (commit)
> >via  https://github.com/Gnucash/gnucash/commit/5cdc5158 (commit)
> >   from  https://github.com/Gnucash/gnucash/commit/316a22a2 (commit)
> >
> >
> >
> > commit d980bb50f7353a16cc4d144be89b140e55eab1d6
> > Author: Christopher Lam 
> > Date:   Tue Feb 26 14:00:15 2019 +0800
> >
> > [report] display gnc-error-dialog only when UI is running
> >
> > This is preparation work for creating report.scm tests.
> >
> > diff --git a/gnucash/report/report-system/report.scm
> > b/gnucash/report/report-system/report.scm index 017dcd347..bace494d6
> 100644
> > --- a/gnucash/report/report-system/report.scm
> > +++ b/gnucash/report/report-system/report.scm
> > @@ -398,8 +398,10 @@
> >  )
> >  report-id
> >)
> > -  (begin
> > - (gnc-error-dialog '() (string-append "Report Failed! One of your
> > previously opened reports has failed to open. The template on which it
> was
> > based: " template-name ", was not found.")) +  (let ((errmsg
> > (string-append "Report Failed! One of your previously opened reports has
> > failed to open. The template on which it was based: " template-name ",
> was
> > not found."))) +  (if (gnucash-ui-is-running)
> > +(gnc-error-dialog '() errmsg)
> > +(gnc:warn errmsg))
> >   #f))
> >)
>
> Any reason why you didn't replace gnc-error-dialog with gnc:gui-error
> instead
> of adding these conditionals ?
>
> Regards,
> Geert
>
>
> ___
> gnucash-devel mailing list
> gnucash-devel@gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
>
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op dinsdag 26 februari 2019 11:24:26 CET schreef Christopher Lam:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/d980bb50 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/0ce6999f (commit)
>via  https://github.com/Gnucash/gnucash/commit/5cdc5158 (commit)
>   from  https://github.com/Gnucash/gnucash/commit/316a22a2 (commit)
> 
> 
> 
> commit d980bb50f7353a16cc4d144be89b140e55eab1d6
> Author: Christopher Lam 
> Date:   Tue Feb 26 14:00:15 2019 +0800
> 
> [report] display gnc-error-dialog only when UI is running
> 
> This is preparation work for creating report.scm tests.
> 
> diff --git a/gnucash/report/report-system/report.scm
> b/gnucash/report/report-system/report.scm index 017dcd347..bace494d6 100644
> --- a/gnucash/report/report-system/report.scm
> +++ b/gnucash/report/report-system/report.scm
> @@ -398,8 +398,10 @@
>  )
>  report-id
>)
> -  (begin
> - (gnc-error-dialog '() (string-append "Report Failed! One of your
> previously opened reports has failed to open. The template on which it was
> based: " template-name ", was not found.")) +  (let ((errmsg
> (string-append "Report Failed! One of your previously opened reports has
> failed to open. The template on which it was based: " template-name ", was
> not found."))) +  (if (gnucash-ui-is-running)
> +(gnc-error-dialog '() errmsg)
> +(gnc:warn errmsg))
>   #f))
>)

Any reason why you didn't replace gnc-error-dialog with gnc:gui-error instead 
of adding these conditionals ?

Regards,
Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op vrijdag 18 januari 2019 23:54:07 CET schreef Christian Stimming:
> commit 6f34995901dcfc999c675e5a4bc095eaf52a2d6f
> Author: Christian Stimming 
> Date:   Fri Jan 18 23:32:31 2019 +0100
> 
> Usability improvements for Bayes editor window
> 
> Remove the "Are you sure" question as it is simply annoying but does
> not help. 

Christian,

In what way does it not help or is it annoying ?

I don't use bayes data so I don't know the specifics of how this works. 
However in general when gnucash is about to delete data we ask the user 
whether that's what s/he really wants to do. In many cases we also provide the 
option to remember the answer.

Is there an undo feature (other than restoring from backup) if the user 
deleted these bayes entries by accident ?

Regards,

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op vrijdag 11 januari 2019 01:42:41 CET schreef John Ralls:
> > On Jan 10, 2019, at 12:44 PM, Geert Janssens 
> > wrote:> 
> > Op donderdag 10 januari 2019 21:11:26 CET schreef John Ralls:
> >>> On Jan 10, 2019, at 8:32 AM, Geert Janssens 
> >>> wrote:>
> >>> 
> >>> Op donderdag 10 januari 2019 16:35:57 CET schreef John Ralls:
> > On Jan 10, 2019, at 2:12 AM, Geert Janssens
> > 
> > wrote:>
> > 
> > Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> >>> On Jan 9, 2019, at 9:06 AM, Geert Janssens
> >>> 
> >>> wrote:>
> >>> 
> >>> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> > On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> > 
> > HI,
> > 
> > On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
> >> I like the idea of caching the system locale for future use. Too
> >> bad
> >> the
> >> std::locale is working so poorly on Windows :(
> >> 
> >> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> >>> +const std::locale&
> >>> +gnc_get_locale()
> >>> +{
> >>> +  static std::locale cached;
> >>> +  static bool tried_already = false;
> >> 
> >> If I understand it correctly using static variables makes the
> >> function
> >> unsuitable for multi-threading, right ?
> > 
> > Not necessarily.  There is a race condition on first-use, but
> > beyond
> > that
> > I don't see a MT issue here.  The data is read-only, right?
> > Multiple
> > threads could read from the same read-only data simultaneously, so
> > that
> > should be fine.
> >>> 
> >>> It was this first-use race condition I was referring to.
> >>> 
> >>> Particularly, for thread safety I think setting tried_already = true
> >>> should
> >>> happen at the very end of the function, after we're sure cached has
> >>> a
> >>> proper value. Otherwise a competing thread could try to get the
> >>> uninitialized cached value if thread execution of the first thread
> >>> is
> >>> interrupted right after tried_already is set true.
> >>> 
> > Static data is ont MT-unsafe if it's being changed on a per-call
> > basis
> > (e.g. a time_t -> string API returning a static string buffer).
> > 
> >> Any idea how difficult would it be to fix that ?
> > 
> > You could add a mutex around the initialization.  That's all I
> > think
> > you
> > would need here.
> > 
> >> I know GnuCash is not thread-safe by a long shot and gtk itself
> >> is
> >> single
> >> threaded so it doesn't matter that much.
> >> 
> >> However I silently set a personal goal of changing that sometime.
> >> The
> >> C
> >> code
> >> is a lost cause IMO, but it might be worth to keep
> >> multi-threading
> >> in
> >> mind
> >> as
> >> we gradually convert to c++. In my basic understanding of
> >> threading
> >> this
> >> particular function should not be too hard to make tread safe.
>  
>  Right, and I decided against making this the first introduction of
>  mutex
>  into GnuCash. I think std::atomic
>  (https://en.cppreference.com/w/cpp/atomic/atomic
>  ) is the correct
>  modern
>  C++ way for a simple case like this.
> >>> 
> >>> I was hoping indeed modern C++ has some answer to this. This is my
> >>> first
> >>> foray into multi-threading by the way so I'm pretty green in this
> >>> area
> >>> and wishing to learn more about it.
> >>> 
> >>> In this particular case would declaring the cached and tried_already
> >>> as
> >>> atomic be sufficient to make this thread safe ?
> >>> 
> >>> It seems to me this would still allow multiple threads to go in and
> >>> run
> >>> the
> >>> initialization code for setting cached. Given they all should end up
> >>> setting cached to the same locale it's probably not dangerous to the
> >>> application, only potentially running the same code multiple times
> >>> where
> >>> only once would be sufficient.
> >> 
> >> My knowledge of concurrency is pretty dated. I had an operating
> >> system
> >> course 30 years ago that included a discussion of concurrency, and
> >> some
> >> parts of Glib are thread-safe, so I’ve bounced up against it doing
> >> maintenance a few times. Just mutex and semaphore stuff.
> >> 
> >> I haven’t even yet read the concurrency chapter in Josuttis, but my
> >> understanding of std::atomic is that a std::atomic variable magically
> >> 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 10, 2019, at 5:25 PM, Derek Atkins  wrote:
> 
> 
> On Thu, January 10, 2019 7:42 pm, John Ralls wrote:
>> 
> [snip]
>> 
>> Gtk has gtk_init. You call it, set up your GUI, then call gtk_main to
>> start the event loop. libguile has several initialization functions to
>> choose from:
>> https://www.gnu.org/software/guile/manual/html_node/Initialization.html#Initialization.
>> No "initialize on first use" there.
>> 
>> Consider that "initialize on first use" means that *every* exported
>> function has to have a clause or belong to a class whose constructor
>> checks for and initializes the global state. If we just need to do it for
>> localizing the C++ UI locale, that's what get_cpp_locale() does and we're
>> done, except that we'd have to make that first call thread safe. If we
>> need to do much more, a library init function is cleaner and simpler...
>> and might be cleaner and simpler than making get_cpp_locale thread safe.
>> 
>> This is all for the rather distant future, though, so let's defer figuring
>> it out til then. We'll forget by then anything we decide now. ;-)
> 
> Add a comment in the code to remind us?

There's nothing special about it. GnuCash is written in a way that pretty much 
everything except function local variables is global thanks to QofContainer. 
Transactions are lightly protected in the obvious case where one might try to 
edit the same one in two registers, but that's about it. 
qof_begin_edit/qof_commit_edit afford no protection at all... quite the 
opposite, in fact.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Derek Atkins]

On Thu, January 10, 2019 7:42 pm, John Ralls wrote:
>
[snip]
>
> Gtk has gtk_init. You call it, set up your GUI, then call gtk_main to
> start the event loop. libguile has several initialization functions to
> choose from:
> https://www.gnu.org/software/guile/manual/html_node/Initialization.html#Initialization.
> No "initialize on first use" there.
>
> Consider that "initialize on first use" means that *every* exported
> function has to have a clause or belong to a class whose constructor
> checks for and initializes the global state. If we just need to do it for
> localizing the C++ UI locale, that's what get_cpp_locale() does and we're
> done, except that we'd have to make that first call thread safe. If we
> need to do much more, a library init function is cleaner and simpler...
> and might be cleaner and simpler than making get_cpp_locale thread safe.
>
> This is all for the rather distant future, though, so let's defer figuring
> it out til then. We'll forget by then anything we decide now. ;-)

Add a comment in the code to remind us?

> Regards,
> John Ralls

-derek

-- 
   Derek Atkins 617-623-3745
   de...@ihtfp.com www.ihtfp.com
   Computer and Internet Security Consultant

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 10, 2019, at 12:44 PM, Geert Janssens  
> wrote:
> 
> Op donderdag 10 januari 2019 21:11:26 CET schreef John Ralls:
>>> On Jan 10, 2019, at 8:32 AM, Geert Janssens 
>>> wrote:> 
>>> Op donderdag 10 januari 2019 16:35:57 CET schreef John Ralls:
> On Jan 10, 2019, at 2:12 AM, Geert Janssens 
> wrote:>
> 
> Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
>>> On Jan 9, 2019, at 9:06 AM, Geert Janssens
>>> 
>>> wrote:>
>>> 
>>> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> 
> HI,
> 
> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
>> I like the idea of caching the system locale for future use. Too
>> bad
>> the
>> std::locale is working so poorly on Windows :(
>> 
>> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
>>> +const std::locale&
>>> +gnc_get_locale()
>>> +{
>>> +  static std::locale cached;
>>> +  static bool tried_already = false;
>> 
>> If I understand it correctly using static variables makes the
>> function
>> unsuitable for multi-threading, right ?
> 
> Not necessarily.  There is a race condition on first-use, but beyond
> that
> I don't see a MT issue here.  The data is read-only, right? 
> Multiple
> threads could read from the same read-only data simultaneously, so
> that
> should be fine.
>>> 
>>> It was this first-use race condition I was referring to.
>>> 
>>> Particularly, for thread safety I think setting tried_already = true
>>> should
>>> happen at the very end of the function, after we're sure cached has a
>>> proper value. Otherwise a competing thread could try to get the
>>> uninitialized cached value if thread execution of the first thread is
>>> interrupted right after tried_already is set true.
>>> 
> Static data is ont MT-unsafe if it's being changed on a per-call
> basis
> (e.g. a time_t -> string API returning a static string buffer).
> 
>> Any idea how difficult would it be to fix that ?
> 
> You could add a mutex around the initialization.  That's all I think
> you
> would need here.
> 
>> I know GnuCash is not thread-safe by a long shot and gtk itself is
>> single
>> threaded so it doesn't matter that much.
>> 
>> However I silently set a personal goal of changing that sometime.
>> The
>> C
>> code
>> is a lost cause IMO, but it might be worth to keep multi-threading
>> in
>> mind
>> as
>> we gradually convert to c++. In my basic understanding of threading
>> this
>> particular function should not be too hard to make tread safe.
 
 Right, and I decided against making this the first introduction of
 mutex
 into GnuCash. I think std::atomic
 (https://en.cppreference.com/w/cpp/atomic/atomic
 ) is the correct
 modern
 C++ way for a simple case like this.
>>> 
>>> I was hoping indeed modern C++ has some answer to this. This is my
>>> first
>>> foray into multi-threading by the way so I'm pretty green in this area
>>> and wishing to learn more about it.
>>> 
>>> In this particular case would declaring the cached and tried_already
>>> as
>>> atomic be sufficient to make this thread safe ?
>>> 
>>> It seems to me this would still allow multiple threads to go in and
>>> run
>>> the
>>> initialization code for setting cached. Given they all should end up
>>> setting cached to the same locale it's probably not dangerous to the
>>> application, only potentially running the same code multiple times
>>> where
>>> only once would be sufficient.
>> 
>> My knowledge of concurrency is pretty dated. I had an operating system
>> course 30 years ago that included a discussion of concurrency, and some
>> parts of Glib are thread-safe, so I’ve bounced up against it doing
>> maintenance a few times. Just mutex and semaphore stuff.
>> 
>> I haven’t even yet read the concurrency chapter in Josuttis, but my
>> understanding of std::atomic is that a std::atomic variable magically
>> eliminates races and has certain operations (construction and operator=
>> among them) that are guaranteed to be, well, atomic: The compiler will
>> always create a contiguous uninterruptible sequence of machine
>> instructions
>> for atomic operations. There are also ways to order execution of some
>> functions with std::memory_model that can make locks (i.e. mutexes and
>> 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op donderdag 10 januari 2019 21:11:26 CET schreef John Ralls:
> > On Jan 10, 2019, at 8:32 AM, Geert Janssens 
> > wrote:> 
> > Op donderdag 10 januari 2019 16:35:57 CET schreef John Ralls:
> >>> On Jan 10, 2019, at 2:12 AM, Geert Janssens 
> >>> wrote:>
> >>> 
> >>> Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> > On Jan 9, 2019, at 9:06 AM, Geert Janssens
> > 
> > wrote:>
> > 
> > Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> >>> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> >>> 
> >>> HI,
> >>> 
> >>> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
>  I like the idea of caching the system locale for future use. Too
>  bad
>  the
>  std::locale is working so poorly on Windows :(
>  
>  Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> > +const std::locale&
> > +gnc_get_locale()
> > +{
> > +  static std::locale cached;
> > +  static bool tried_already = false;
>  
>  If I understand it correctly using static variables makes the
>  function
>  unsuitable for multi-threading, right ?
> >>> 
> >>> Not necessarily.  There is a race condition on first-use, but beyond
> >>> that
> >>> I don't see a MT issue here.  The data is read-only, right? 
> >>> Multiple
> >>> threads could read from the same read-only data simultaneously, so
> >>> that
> >>> should be fine.
> > 
> > It was this first-use race condition I was referring to.
> > 
> > Particularly, for thread safety I think setting tried_already = true
> > should
> > happen at the very end of the function, after we're sure cached has a
> > proper value. Otherwise a competing thread could try to get the
> > uninitialized cached value if thread execution of the first thread is
> > interrupted right after tried_already is set true.
> > 
> >>> Static data is ont MT-unsafe if it's being changed on a per-call
> >>> basis
> >>> (e.g. a time_t -> string API returning a static string buffer).
> >>> 
>  Any idea how difficult would it be to fix that ?
> >>> 
> >>> You could add a mutex around the initialization.  That's all I think
> >>> you
> >>> would need here.
> >>> 
>  I know GnuCash is not thread-safe by a long shot and gtk itself is
>  single
>  threaded so it doesn't matter that much.
>  
>  However I silently set a personal goal of changing that sometime.
>  The
>  C
>  code
>  is a lost cause IMO, but it might be worth to keep multi-threading
>  in
>  mind
>  as
>  we gradually convert to c++. In my basic understanding of threading
>  this
>  particular function should not be too hard to make tread safe.
> >> 
> >> Right, and I decided against making this the first introduction of
> >> mutex
> >> into GnuCash. I think std::atomic
> >> (https://en.cppreference.com/w/cpp/atomic/atomic
> >> ) is the correct
> >> modern
> >> C++ way for a simple case like this.
> > 
> > I was hoping indeed modern C++ has some answer to this. This is my
> > first
> > foray into multi-threading by the way so I'm pretty green in this area
> > and wishing to learn more about it.
> > 
> > In this particular case would declaring the cached and tried_already
> > as
> > atomic be sufficient to make this thread safe ?
> > 
> > It seems to me this would still allow multiple threads to go in and
> > run
> > the
> > initialization code for setting cached. Given they all should end up
> > setting cached to the same locale it's probably not dangerous to the
> > application, only potentially running the same code multiple times
> > where
> > only once would be sufficient.
>  
>  My knowledge of concurrency is pretty dated. I had an operating system
>  course 30 years ago that included a discussion of concurrency, and some
>  parts of Glib are thread-safe, so I’ve bounced up against it doing
>  maintenance a few times. Just mutex and semaphore stuff.
>  
>  I haven’t even yet read the concurrency chapter in Josuttis, but my
>  understanding of std::atomic is that a std::atomic variable magically
>  eliminates races and has certain operations (construction and operator=
>  among them) that are guaranteed to be, well, atomic: The compiler will
>  always create a contiguous uninterruptible sequence of machine
>  instructions
>  for atomic operations. There are also ways to order execution of some
>  functions with std::memory_model that can make locks (i.e. mutexes and
>  semaphores) unnecessary, and the compiler is able to figure out when
>  

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 10, 2019, at 8:32 AM, Geert Janssens  
> wrote:
> 
> Op donderdag 10 januari 2019 16:35:57 CET schreef John Ralls:
>>> On Jan 10, 2019, at 2:12 AM, Geert Janssens 
>>> wrote:> 
>>> Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> On Jan 9, 2019, at 9:06 AM, Geert Janssens 
> wrote:>
> 
> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
>>> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
>>> 
>>> HI,
>>> 
>>> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
 I like the idea of caching the system locale for future use. Too bad
 the
 std::locale is working so poorly on Windows :(
 
 Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> +const std::locale&
> +gnc_get_locale()
> +{
> +  static std::locale cached;
> +  static bool tried_already = false;
 
 If I understand it correctly using static variables makes the
 function
 unsuitable for multi-threading, right ?
>>> 
>>> Not necessarily.  There is a race condition on first-use, but beyond
>>> that
>>> I don't see a MT issue here.  The data is read-only, right?  Multiple
>>> threads could read from the same read-only data simultaneously, so
>>> that
>>> should be fine.
> 
> It was this first-use race condition I was referring to.
> 
> Particularly, for thread safety I think setting tried_already = true
> should
> happen at the very end of the function, after we're sure cached has a
> proper value. Otherwise a competing thread could try to get the
> uninitialized cached value if thread execution of the first thread is
> interrupted right after tried_already is set true.
> 
>>> Static data is ont MT-unsafe if it's being changed on a per-call basis
>>> (e.g. a time_t -> string API returning a static string buffer).
>>> 
 Any idea how difficult would it be to fix that ?
>>> 
>>> You could add a mutex around the initialization.  That's all I think
>>> you
>>> would need here.
>>> 
 I know GnuCash is not thread-safe by a long shot and gtk itself is
 single
 threaded so it doesn't matter that much.
 
 However I silently set a personal goal of changing that sometime. The
 C
 code
 is a lost cause IMO, but it might be worth to keep multi-threading in
 mind
 as
 we gradually convert to c++. In my basic understanding of threading
 this
 particular function should not be too hard to make tread safe.
>> 
>> Right, and I decided against making this the first introduction of
>> mutex
>> into GnuCash. I think std::atomic
>> (https://en.cppreference.com/w/cpp/atomic/atomic
>> ) is the correct
>> modern
>> C++ way for a simple case like this.
> 
> I was hoping indeed modern C++ has some answer to this. This is my first
> foray into multi-threading by the way so I'm pretty green in this area
> and wishing to learn more about it.
> 
> In this particular case would declaring the cached and tried_already as
> atomic be sufficient to make this thread safe ?
> 
> It seems to me this would still allow multiple threads to go in and run
> the
> initialization code for setting cached. Given they all should end up
> setting cached to the same locale it's probably not dangerous to the
> application, only potentially running the same code multiple times where
> only once would be sufficient.
 
 My knowledge of concurrency is pretty dated. I had an operating system
 course 30 years ago that included a discussion of concurrency, and some
 parts of Glib are thread-safe, so I’ve bounced up against it doing
 maintenance a few times. Just mutex and semaphore stuff.
 
 I haven’t even yet read the concurrency chapter in Josuttis, but my
 understanding of std::atomic is that a std::atomic variable magically
 eliminates races and has certain operations (construction and operator=
 among them) that are guaranteed to be, well, atomic: The compiler will
 always create a contiguous uninterruptible sequence of machine
 instructions
 for atomic operations. There are also ways to order execution of some
 functions with std::memory_model that can make locks (i.e. mutexes and
 semaphores) unnecessary, and the compiler is able to figure out when
 that’s
 true and when it isn’t and to take care of the locking without the
 programmer needing to explicitly code it.
 
 It’s not something I think worth worrying about now. GnuCash is very far
 away from being multi-threaded. Besides, the natural home for the
 instantiation of the cached C++ local is in main(), after all of the
 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op donderdag 10 januari 2019 16:35:57 CET schreef John Ralls:
> > On Jan 10, 2019, at 2:12 AM, Geert Janssens 
> > wrote:> 
> > Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> >>> On Jan 9, 2019, at 9:06 AM, Geert Janssens 
> >>> wrote:>
> >>> 
> >>> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> > On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> > 
> > HI,
> > 
> > On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
> >> I like the idea of caching the system locale for future use. Too bad
> >> the
> >> std::locale is working so poorly on Windows :(
> >> 
> >> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> >>> +const std::locale&
> >>> +gnc_get_locale()
> >>> +{
> >>> +  static std::locale cached;
> >>> +  static bool tried_already = false;
> >> 
> >> If I understand it correctly using static variables makes the
> >> function
> >> unsuitable for multi-threading, right ?
> > 
> > Not necessarily.  There is a race condition on first-use, but beyond
> > that
> > I don't see a MT issue here.  The data is read-only, right?  Multiple
> > threads could read from the same read-only data simultaneously, so
> > that
> > should be fine.
> >>> 
> >>> It was this first-use race condition I was referring to.
> >>> 
> >>> Particularly, for thread safety I think setting tried_already = true
> >>> should
> >>> happen at the very end of the function, after we're sure cached has a
> >>> proper value. Otherwise a competing thread could try to get the
> >>> uninitialized cached value if thread execution of the first thread is
> >>> interrupted right after tried_already is set true.
> >>> 
> > Static data is ont MT-unsafe if it's being changed on a per-call basis
> > (e.g. a time_t -> string API returning a static string buffer).
> > 
> >> Any idea how difficult would it be to fix that ?
> > 
> > You could add a mutex around the initialization.  That's all I think
> > you
> > would need here.
> > 
> >> I know GnuCash is not thread-safe by a long shot and gtk itself is
> >> single
> >> threaded so it doesn't matter that much.
> >> 
> >> However I silently set a personal goal of changing that sometime. The
> >> C
> >> code
> >> is a lost cause IMO, but it might be worth to keep multi-threading in
> >> mind
> >> as
> >> we gradually convert to c++. In my basic understanding of threading
> >> this
> >> particular function should not be too hard to make tread safe.
>  
>  Right, and I decided against making this the first introduction of
>  mutex
>  into GnuCash. I think std::atomic
>  (https://en.cppreference.com/w/cpp/atomic/atomic
>  ) is the correct
>  modern
>  C++ way for a simple case like this.
> >>> 
> >>> I was hoping indeed modern C++ has some answer to this. This is my first
> >>> foray into multi-threading by the way so I'm pretty green in this area
> >>> and wishing to learn more about it.
> >>> 
> >>> In this particular case would declaring the cached and tried_already as
> >>> atomic be sufficient to make this thread safe ?
> >>> 
> >>> It seems to me this would still allow multiple threads to go in and run
> >>> the
> >>> initialization code for setting cached. Given they all should end up
> >>> setting cached to the same locale it's probably not dangerous to the
> >>> application, only potentially running the same code multiple times where
> >>> only once would be sufficient.
> >> 
> >> My knowledge of concurrency is pretty dated. I had an operating system
> >> course 30 years ago that included a discussion of concurrency, and some
> >> parts of Glib are thread-safe, so I’ve bounced up against it doing
> >> maintenance a few times. Just mutex and semaphore stuff.
> >> 
> >> I haven’t even yet read the concurrency chapter in Josuttis, but my
> >> understanding of std::atomic is that a std::atomic variable magically
> >> eliminates races and has certain operations (construction and operator=
> >> among them) that are guaranteed to be, well, atomic: The compiler will
> >> always create a contiguous uninterruptible sequence of machine
> >> instructions
> >> for atomic operations. There are also ways to order execution of some
> >> functions with std::memory_model that can make locks (i.e. mutexes and
> >> semaphores) unnecessary, and the compiler is able to figure out when
> >> that’s
> >> true and when it isn’t and to take care of the locking without the
> >> programmer needing to explicitly code it.
> >> 
> >> It’s not something I think worth worrying about now. GnuCash is very far
> >> away from being multi-threaded. Besides, the natural home for the
> >> instantiation of the cached C++ local is in main(), after all of the
> >> environment options are parsed but before the GUI is loaded or the first

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 10, 2019, at 2:12 AM, Geert Janssens  
> wrote:
> 
> Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
>>> On Jan 9, 2019, at 9:06 AM, Geert Janssens 
>>> wrote:> 
>>> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> 
> HI,
> 
> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
>> I like the idea of caching the system locale for future use. Too bad
>> the
>> std::locale is working so poorly on Windows :(
>> 
>> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
>>> +const std::locale&
>>> +gnc_get_locale()
>>> +{
>>> +  static std::locale cached;
>>> +  static bool tried_already = false;
>> 
>> If I understand it correctly using static variables makes the function
>> unsuitable for multi-threading, right ?
> 
> Not necessarily.  There is a race condition on first-use, but beyond
> that
> I don't see a MT issue here.  The data is read-only, right?  Multiple
> threads could read from the same read-only data simultaneously, so that
> should be fine.
>>> 
>>> It was this first-use race condition I was referring to.
>>> 
>>> Particularly, for thread safety I think setting tried_already = true
>>> should
>>> happen at the very end of the function, after we're sure cached has a
>>> proper value. Otherwise a competing thread could try to get the
>>> uninitialized cached value if thread execution of the first thread is
>>> interrupted right after tried_already is set true.
>>> 
> Static data is ont MT-unsafe if it's being changed on a per-call basis
> (e.g. a time_t -> string API returning a static string buffer).
> 
>> Any idea how difficult would it be to fix that ?
> 
> You could add a mutex around the initialization.  That's all I think you
> would need here.
> 
>> I know GnuCash is not thread-safe by a long shot and gtk itself is
>> single
>> threaded so it doesn't matter that much.
>> 
>> However I silently set a personal goal of changing that sometime. The C
>> code
>> is a lost cause IMO, but it might be worth to keep multi-threading in
>> mind
>> as
>> we gradually convert to c++. In my basic understanding of threading
>> this
>> particular function should not be too hard to make tread safe.
 
 Right, and I decided against making this the first introduction of mutex
 into GnuCash. I think std::atomic
 (https://en.cppreference.com/w/cpp/atomic/atomic
 ) is the correct modern
 C++ way for a simple case like this.
>>> 
>>> I was hoping indeed modern C++ has some answer to this. This is my first
>>> foray into multi-threading by the way so I'm pretty green in this area
>>> and wishing to learn more about it.
>>> 
>>> In this particular case would declaring the cached and tried_already as
>>> atomic be sufficient to make this thread safe ?
>>> 
>>> It seems to me this would still allow multiple threads to go in and run
>>> the
>>> initialization code for setting cached. Given they all should end up
>>> setting cached to the same locale it's probably not dangerous to the
>>> application, only potentially running the same code multiple times where
>>> only once would be sufficient.
>> 
>> My knowledge of concurrency is pretty dated. I had an operating system
>> course 30 years ago that included a discussion of concurrency, and some
>> parts of Glib are thread-safe, so I’ve bounced up against it doing
>> maintenance a few times. Just mutex and semaphore stuff.
>> 
>> I haven’t even yet read the concurrency chapter in Josuttis, but my
>> understanding of std::atomic is that a std::atomic variable magically
>> eliminates races and has certain operations (construction and operator=
>> among them) that are guaranteed to be, well, atomic: The compiler will
>> always create a contiguous uninterruptible sequence of machine instructions
>> for atomic operations. There are also ways to order execution of some
>> functions with std::memory_model that can make locks (i.e. mutexes and
>> semaphores) unnecessary, and the compiler is able to figure out when that’s
>> true and when it isn’t and to take care of the locking without the
>> programmer needing to explicitly code it.
>> 
>> It’s not something I think worth worrying about now. GnuCash is very far
>> away from being multi-threaded. Besides, the natural home for the
>> instantiation of the cached C++ local is in main(), after all of the
>> environment options are parsed but before the GUI is loaded or the first
>> session started. This isn’t the function to worry about concurrent access.
>> 
> Fair enough. Though I'd think it should happen in libgnucash initialization 
> at 
> some point rather than in the gui code. But that's just a matter of sorting 
> out all startup configuration we do at some point and estimate which part 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op donderdag 10 januari 2019 06:45:15 CET schreef John Ralls:
> > On Jan 9, 2019, at 9:06 AM, Geert Janssens 
> > wrote:> 
> > Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> >>> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> >>> 
> >>> HI,
> >>> 
> >>> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
>  I like the idea of caching the system locale for future use. Too bad
>  the
>  std::locale is working so poorly on Windows :(
>  
>  Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> > +const std::locale&
> > +gnc_get_locale()
> > +{
> > +  static std::locale cached;
> > +  static bool tried_already = false;
>  
>  If I understand it correctly using static variables makes the function
>  unsuitable for multi-threading, right ?
> >>> 
> >>> Not necessarily.  There is a race condition on first-use, but beyond
> >>> that
> >>> I don't see a MT issue here.  The data is read-only, right?  Multiple
> >>> threads could read from the same read-only data simultaneously, so that
> >>> should be fine.
> > 
> > It was this first-use race condition I was referring to.
> > 
> > Particularly, for thread safety I think setting tried_already = true
> > should
> > happen at the very end of the function, after we're sure cached has a
> > proper value. Otherwise a competing thread could try to get the
> > uninitialized cached value if thread execution of the first thread is
> > interrupted right after tried_already is set true.
> > 
> >>> Static data is ont MT-unsafe if it's being changed on a per-call basis
> >>> (e.g. a time_t -> string API returning a static string buffer).
> >>> 
>  Any idea how difficult would it be to fix that ?
> >>> 
> >>> You could add a mutex around the initialization.  That's all I think you
> >>> would need here.
> >>> 
>  I know GnuCash is not thread-safe by a long shot and gtk itself is
>  single
>  threaded so it doesn't matter that much.
>  
>  However I silently set a personal goal of changing that sometime. The C
>  code
>  is a lost cause IMO, but it might be worth to keep multi-threading in
>  mind
>  as
>  we gradually convert to c++. In my basic understanding of threading
>  this
>  particular function should not be too hard to make tread safe.
> >> 
> >> Right, and I decided against making this the first introduction of mutex
> >> into GnuCash. I think std::atomic
> >> (https://en.cppreference.com/w/cpp/atomic/atomic
> >> ) is the correct modern
> >> C++ way for a simple case like this.
> > 
> > I was hoping indeed modern C++ has some answer to this. This is my first
> > foray into multi-threading by the way so I'm pretty green in this area
> > and wishing to learn more about it.
> > 
> > In this particular case would declaring the cached and tried_already as
> > atomic be sufficient to make this thread safe ?
> > 
> > It seems to me this would still allow multiple threads to go in and run
> > the
> > initialization code for setting cached. Given they all should end up
> > setting cached to the same locale it's probably not dangerous to the
> > application, only potentially running the same code multiple times where
> > only once would be sufficient.
> 
> My knowledge of concurrency is pretty dated. I had an operating system
> course 30 years ago that included a discussion of concurrency, and some
> parts of Glib are thread-safe, so I’ve bounced up against it doing
> maintenance a few times. Just mutex and semaphore stuff.
> 
> I haven’t even yet read the concurrency chapter in Josuttis, but my
> understanding of std::atomic is that a std::atomic variable magically
> eliminates races and has certain operations (construction and operator=
> among them) that are guaranteed to be, well, atomic: The compiler will
> always create a contiguous uninterruptible sequence of machine instructions
> for atomic operations. There are also ways to order execution of some
> functions with std::memory_model that can make locks (i.e. mutexes and
> semaphores) unnecessary, and the compiler is able to figure out when that’s
> true and when it isn’t and to take care of the locking without the
> programmer needing to explicitly code it.
> 
> It’s not something I think worth worrying about now. GnuCash is very far
> away from being multi-threaded. Besides, the natural home for the
> instantiation of the cached C++ local is in main(), after all of the
> environment options are parsed but before the GUI is loaded or the first
> session started. This isn’t the function to worry about concurrent access.
> 
Fair enough. Though I'd think it should happen in libgnucash initialization at 
some point rather than in the gui code. But that's just a matter of sorting 
out all startup configuration we do at some point and estimate which part is 
needed for libgnucash and which part is only for the gui code.

Geert

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op donderdag 10 januari 2019 02:08:29 CET schreef John Ralls:
> > On Jan 9, 2019, at 11:52 AM, Geert Janssens 
> > wrote:> 
> > Op woensdag 9 januari 2019 18:06:49 CET schreef Geert Janssens:
> >> Op woensdag 9 januari 2019 15:57:07 CET schreef John Ralls:
>  On Jan 9, 2019, at 4:24 AM, Geert Janssens 
>  wrote:>
>  
>  Op dinsdag 1 januari 2019 22:14:16 CET schreef John Ralls:
> > Updated  via  https://github.com/Gnucash/gnucash/commit/3a105f07
> >> 
> >> (commit)
> >> 
> >  via  https://github.com/Gnucash/gnucash/commit/95bee405 
> > (commit)
> >  via  https://github.com/Gnucash/gnucash/commit/cc3bb4ef 
> > (commit)
> > 
> > from  https://github.com/Gnucash/gnucash/commit/0f53b6c8 
> > (commit)
> > 
> > commit 3a105f0728984df7f063110acc8390c93722d581
> > Author: John Ralls 
> > Date:   Tue Jan 1 13:12:39 2019 -0800
> > 
> >   Catch boost::locale character-conversion exceptions.
> >   
> >   Partial cause of the crash reported in Bug 797002.
>  
>  I suppose you meant 796996 ?
>  
>  Also it looks like you're really only catching the errors. The source
>  of
>  the conversion issue itself is not really determined yet ?
> >>> 
> >>> Yes, wrong bug.
> >>> 
> >>> Yes, in this one I’m only catching the exceptions because uncaught
> >>> exceptions cause crashes. The root cause was libc’s not-quite-right
> >>> creation of e.g. “Spanish_Spain.1252” locale strings, the .1252 part
> >>> choking gen(“”) (and even Spanish_Spain chokes std::locale(“”). That’s
> >>> addressed with the more thorough error handling and use of gen(“”) in
> >>> b4fedff90e.
> >>> 
> >>> Regards,
> >>> John Ralls
> >> 
> >> Oh right. I saw that commit later on, but didn't make the connection.
> >> Thanks.
> > 
> > Returning to this: it occurred to me bug 796996 is about a crash on MacOS,
> > though the example you give looks like a Windows locale name.
> > 
> > Does libc MacOS also create improper locale names and does b4fedff90e fix
> > this as well ?
> 
> Yes, and so do some Linux distros. What chokes gen(“”) is the appended
> encoding.  That’s why b4fedff90e strips everything after ‘.’ in the locale
> string and tries again.
> 
> The wider Windows case came up on IRC:
> https://wiki.gnucash.org/logs/2019/01/04.html#T15:58:41
> . That fail was
> from std::locale and is what led me to use gen(“”) for all C++ locale
> retrievals.
> 
Ok, thanks for the clarifications.

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 9, 2019, at 9:06 AM, Geert Janssens  wrote:
> 
> Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
>>> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
>>> 
>>> HI,
>>> 
>>> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
 I like the idea of caching the system locale for future use. Too bad the
 std::locale is working so poorly on Windows :(
 
 Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> +const std::locale&
> +gnc_get_locale()
> +{
> +  static std::locale cached;
> +  static bool tried_already = false;
 
 If I understand it correctly using static variables makes the function
 unsuitable for multi-threading, right ?
>>> 
>>> Not necessarily.  There is a race condition on first-use, but beyond that
>>> I don't see a MT issue here.  The data is read-only, right?  Multiple
>>> threads could read from the same read-only data simultaneously, so that
>>> should be fine.
>>> 
> 
> It was this first-use race condition I was referring to.
> 
> Particularly, for thread safety I think setting tried_already = true should 
> happen at the very end of the function, after we're sure cached has a proper 
> value. Otherwise a competing thread could try to get the uninitialized cached 
> value if thread execution of the first thread is interrupted right after 
> tried_already is set true.
> 
>>> Static data is ont MT-unsafe if it's being changed on a per-call basis
>>> (e.g. a time_t -> string API returning a static string buffer).
>>> 
 Any idea how difficult would it be to fix that ?
>>> 
>>> You could add a mutex around the initialization.  That's all I think you
>>> would need here.
>>> 
 I know GnuCash is not thread-safe by a long shot and gtk itself is single
 threaded so it doesn't matter that much.
 
 However I silently set a personal goal of changing that sometime. The C
 code
 is a lost cause IMO, but it might be worth to keep multi-threading in
 mind
 as
 we gradually convert to c++. In my basic understanding of threading this
 particular function should not be too hard to make tread safe.
>> 
>> Right, and I decided against making this the first introduction of mutex
>> into GnuCash. I think std::atomic
>> (https://en.cppreference.com/w/cpp/atomic/atomic
>> ) is the correct modern
>> C++ way for a simple case like this.
> 
> I was hoping indeed modern C++ has some answer to this. This is my first 
> foray 
> into multi-threading by the way so I'm pretty green in this area and wishing 
> to learn more about it.
> 
> In this particular case would declaring the cached and tried_already as 
> atomic 
> be sufficient to make this thread safe ?
> 
> It seems to me this would still allow multiple threads to go in and run the 
> initialization code for setting cached. Given they all should end up setting 
> cached to the same locale it's probably not dangerous to the application, 
> only 
> potentially running the same code multiple times where only once would be 
> sufficient.

My knowledge of concurrency is pretty dated. I had an operating system course 
30 years ago that included a discussion of concurrency, and some parts of Glib 
are thread-safe, so I’ve bounced up against it doing maintenance a few times. 
Just mutex and semaphore stuff. 

I haven’t even yet read the concurrency chapter in Josuttis, but my 
understanding of std::atomic is that a std::atomic variable magically 
eliminates races and has certain operations (construction and operator= among 
them) that are guaranteed to be, well, atomic: The compiler will always create 
a contiguous uninterruptible sequence of machine instructions for atomic 
operations. There are also ways to order execution of some functions with 
std::memory_model that can make locks (i.e. mutexes and semaphores) 
unnecessary, and the compiler is able to figure out when that’s true and when 
it isn’t and to take care of the locking without the programmer needing to 
explicitly code it.

It’s not something I think worth worrying about now. GnuCash is very far away 
from being multi-threaded. Besides, the natural home for the instantiation of 
the cached C++ local is in main(), after all of the environment options are 
parsed but before the GUI is loaded or the first session started. This isn’t 
the function to worry about concurrent access.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 9, 2019, at 11:52 AM, Geert Janssens  
> wrote:
> 
> Op woensdag 9 januari 2019 18:06:49 CET schreef Geert Janssens:
>> Op woensdag 9 januari 2019 15:57:07 CET schreef John Ralls:
 On Jan 9, 2019, at 4:24 AM, Geert Janssens 
 wrote:>
 
 Op dinsdag 1 januari 2019 22:14:16 CET schreef John Ralls:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/3a105f07
>> 
>> (commit)
>> 
>via  https://github.com/Gnucash/gnucash/commit/95bee405 (commit)
>via  https://github.com/Gnucash/gnucash/commit/cc3bb4ef (commit)
>   
>   from  https://github.com/Gnucash/gnucash/commit/0f53b6c8 (commit)
> 
> commit 3a105f0728984df7f063110acc8390c93722d581
> Author: John Ralls 
> Date:   Tue Jan 1 13:12:39 2019 -0800
> 
>   Catch boost::locale character-conversion exceptions.
> 
>   Partial cause of the crash reported in Bug 797002.
 
 I suppose you meant 796996 ?
 
 Also it looks like you're really only catching the errors. The source of
 the conversion issue itself is not really determined yet ?
>>> 
>>> Yes, wrong bug.
>>> 
>>> Yes, in this one I’m only catching the exceptions because uncaught
>>> exceptions cause crashes. The root cause was libc’s not-quite-right
>>> creation of e.g. “Spanish_Spain.1252” locale strings, the .1252 part
>>> choking gen(“”) (and even Spanish_Spain chokes std::locale(“”). That’s
>>> addressed with the more thorough error handling and use of gen(“”) in
>>> b4fedff90e.
>>> 
>>> Regards,
>>> John Ralls
>> 
>> Oh right. I saw that commit later on, but didn't make the connection.
>> Thanks.
> 
> Returning to this: it occurred to me bug 796996 is about a crash on MacOS, 
> though the example you give looks like a Windows locale name.
> 
> Does libc MacOS also create improper locale names and does b4fedff90e fix 
> this 
> as well ?

Yes, and so do some Linux distros. What chokes gen(“”) is the appended 
encoding.  That’s why b4fedff90e strips everything after ‘.’ in the locale 
string and tries again.

The wider Windows case came up on IRC: 
https://wiki.gnucash.org/logs/2019/01/04.html#T15:58:41 
. That fail was from 
std::locale and is what led me to use gen(“”) for all C++ locale retrievals.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op woensdag 9 januari 2019 18:06:49 CET schreef Geert Janssens:
> Op woensdag 9 januari 2019 15:57:07 CET schreef John Ralls:
> > > On Jan 9, 2019, at 4:24 AM, Geert Janssens 
> > > wrote:>
> > > 
> > > Op dinsdag 1 januari 2019 22:14:16 CET schreef John Ralls:
> > >> Updated   via  https://github.com/Gnucash/gnucash/commit/3a105f07
> 
> (commit)
> 
> > >>   via  https://github.com/Gnucash/gnucash/commit/95bee405 (commit)
> > >>   via  https://github.com/Gnucash/gnucash/commit/cc3bb4ef (commit)
> > >>  
> > >>  from  https://github.com/Gnucash/gnucash/commit/0f53b6c8 (commit)
> > >> 
> > >> commit 3a105f0728984df7f063110acc8390c93722d581
> > >> Author: John Ralls 
> > >> Date:   Tue Jan 1 13:12:39 2019 -0800
> > >> 
> > >>Catch boost::locale character-conversion exceptions.
> > >>
> > >>Partial cause of the crash reported in Bug 797002.
> > > 
> > > I suppose you meant 796996 ?
> > > 
> > > Also it looks like you're really only catching the errors. The source of
> > > the conversion issue itself is not really determined yet ?
> > 
> > Yes, wrong bug.
> > 
> > Yes, in this one I’m only catching the exceptions because uncaught
> > exceptions cause crashes. The root cause was libc’s not-quite-right
> > creation of e.g. “Spanish_Spain.1252” locale strings, the .1252 part
> > choking gen(“”) (and even Spanish_Spain chokes std::locale(“”). That’s
> > addressed with the more thorough error handling and use of gen(“”) in
> > b4fedff90e.
> > 
> > Regards,
> > John Ralls
> 
> Oh right. I saw that commit later on, but didn't make the connection.
> Thanks.

Returning to this: it occurred to me bug 796996 is about a crash on MacOS, 
though the example you give looks like a Windows locale name.

Does libc MacOS also create improper locale names and does b4fedff90e fix this 
as well ?

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op woensdag 9 januari 2019 15:45:31 CET schreef John Ralls:
> > On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> > 
> > HI,
> > 
> > On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
> >> I like the idea of caching the system locale for future use. Too bad the
> >> std::locale is working so poorly on Windows :(
> >> 
> >> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> >>> +const std::locale&
> >>> +gnc_get_locale()
> >>> +{
> >>> +  static std::locale cached;
> >>> +  static bool tried_already = false;
> >> 
> >> If I understand it correctly using static variables makes the function
> >> unsuitable for multi-threading, right ?
> > 
> > Not necessarily.  There is a race condition on first-use, but beyond that
> > I don't see a MT issue here.  The data is read-only, right?  Multiple
> > threads could read from the same read-only data simultaneously, so that
> > should be fine.
> > 

It was this first-use race condition I was referring to.

Particularly, for thread safety I think setting tried_already = true should 
happen at the very end of the function, after we're sure cached has a proper 
value. Otherwise a competing thread could try to get the uninitialized cached 
value if thread execution of the first thread is interrupted right after 
tried_already is set true.

> > Static data is ont MT-unsafe if it's being changed on a per-call basis
> > (e.g. a time_t -> string API returning a static string buffer).
> > 
> >> Any idea how difficult would it be to fix that ?
> > 
> > You could add a mutex around the initialization.  That's all I think you
> > would need here.
> > 
> >> I know GnuCash is not thread-safe by a long shot and gtk itself is single
> >> threaded so it doesn't matter that much.
> >> 
> >> However I silently set a personal goal of changing that sometime. The C
> >> code
> >> is a lost cause IMO, but it might be worth to keep multi-threading in
> >> mind
> >> as
> >> we gradually convert to c++. In my basic understanding of threading this
> >> particular function should not be too hard to make tread safe.
> 
> Right, and I decided against making this the first introduction of mutex
> into GnuCash. I think std::atomic
> (https://en.cppreference.com/w/cpp/atomic/atomic
> ) is the correct modern
> C++ way for a simple case like this.

I was hoping indeed modern C++ has some answer to this. This is my first foray 
into multi-threading by the way so I'm pretty green in this area and wishing 
to learn more about it.

In this particular case would declaring the cached and tried_already as atomic 
be sufficient to make this thread safe ?

It seems to me this would still allow multiple threads to go in and run the 
initialization code for setting cached. Given they all should end up setting 
cached to the same locale it's probably not dangerous to the application, only 
potentially running the same code multiple times where only once would be 
sufficient.

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 9, 2019, at 4:24 AM, Geert Janssens  wrote:
> 
> Op dinsdag 1 januari 2019 22:14:16 CET schreef John Ralls:
>> Updated   via  https://github.com/Gnucash/gnucash/commit/3a105f07 
>> (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/95bee405 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/cc3bb4ef (commit)
>>  from  https://github.com/Gnucash/gnucash/commit/0f53b6c8 (commit)
>> 
>> 
>> 
>> commit 3a105f0728984df7f063110acc8390c93722d581
>> Author: John Ralls 
>> Date:   Tue Jan 1 13:12:39 2019 -0800
>> 
>>Catch boost::locale character-conversion exceptions.
>> 
>>Partial cause of the crash reported in Bug 797002.
>> 
> I suppose you meant 796996 ?
> 
> Also it looks like you're really only catching the errors. The source of the 
> conversion issue itself is not really determined yet ?

Yes, wrong bug.

Yes, in this one I’m only catching the exceptions because uncaught exceptions 
cause crashes. The root cause was libc’s not-quite-right creation of e.g. 
“Spanish_Spain.1252” locale strings, the .1252 part choking gen(“”) (and even 
Spanish_Spain chokes std::locale(“”). That’s addressed with the more thorough 
error handling and use of gen(“”) in b4fedff90e.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Jan 9, 2019, at 6:17 AM, Derek Atkins  wrote:
> 
> HI,
> 
> On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
>> I like the idea of caching the system locale for future use. Too bad the
>> std::locale is working so poorly on Windows :(
>> 
>> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
>>> +const std::locale&
>>> +gnc_get_locale()
>>> +{
>>> +  static std::locale cached;
>>> +  static bool tried_already = false;
>> 
>> If I understand it correctly using static variables makes the function
>> unsuitable for multi-threading, right ?
> 
> Not necessarily.  There is a race condition on first-use, but beyond that
> I don't see a MT issue here.  The data is read-only, right?  Multiple
> threads could read from the same read-only data simultaneously, so that
> should be fine.
> 
> Static data is ont MT-unsafe if it's being changed on a per-call basis
> (e.g. a time_t -> string API returning a static string buffer).
> 
>> Any idea how difficult would it be to fix that ?
> 
> You could add a mutex around the initialization.  That's all I think you
> would need here.
> 
>> I know GnuCash is not thread-safe by a long shot and gtk itself is single
>> threaded so it doesn't matter that much.
>> 
>> However I silently set a personal goal of changing that sometime. The C
>> code
>> is a lost cause IMO, but it might be worth to keep multi-threading in mind
>> as
>> we gradually convert to c++. In my basic understanding of threading this
>> particular function should not be too hard to make tread safe.

Right, and I decided against making this the first introduction of mutex into 
GnuCash.
I think std::atomic (https://en.cppreference.com/w/cpp/atomic/atomic 
) is the correct 
modern C++ way for a simple case like this. 

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Derek Atkins]

HI,

On Wed, January 9, 2019 9:12 am, Geert Janssens wrote:
> I like the idea of caching the system locale for future use. Too bad the
> std::locale is working so poorly on Windows :(
>
> Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
>> +const std::locale&
>> +gnc_get_locale()
>> +{
>> +  static std::locale cached;
>> +  static bool tried_already = false;
>
> If I understand it correctly using static variables makes the function
> unsuitable for multi-threading, right ?

Not necessarily.  There is a race condition on first-use, but beyond that
I don't see a MT issue here.  The data is read-only, right?  Multiple
threads could read from the same read-only data simultaneously, so that
should be fine.

Static data is ont MT-unsafe if it's being changed on a per-call basis
(e.g. a time_t -> string API returning a static string buffer).

> Any idea how difficult would it be to fix that ?

You could add a mutex around the initialization.  That's all I think you
would need here.

> I know GnuCash is not thread-safe by a long shot and gtk itself is single
> threaded so it doesn't matter that much.
>
> However I silently set a personal goal of changing that sometime. The C
> code
> is a lost cause IMO, but it might be worth to keep multi-threading in mind
> as
> we gradually convert to c++. In my basic understanding of threading this
> particular function should not be too hard to make tread safe.

-derek

-- 
   Derek Atkins 617-623-3745
   de...@ihtfp.com www.ihtfp.com
   Computer and Internet Security Consultant

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

I like the idea of caching the system locale for future use. Too bad the 
std::locale is working so poorly on Windows :(

Op zondag 6 januari 2019 19:13:28 CET schreef John Ralls:
> +const std::locale&
> +gnc_get_locale()
> +{
> +  static std::locale cached;
> +  static bool tried_already = false;

If I understand it correctly using static variables makes the function 
unsuitable for multi-threading, right ?

Any idea how difficult would it be to fix that ?

I know GnuCash is not thread-safe by a long shot and gtk itself is single 
threaded so it doesn't matter that much.

However I silently set a personal goal of changing that sometime. The C code 
is a lost cause IMO, but it might be worth to keep multi-threading in mind as 
we gradually convert to c++. In my basic understanding of threading this 
particular function should not be too hard to make tread safe.

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op dinsdag 1 januari 2019 22:14:16 CET schreef John Ralls:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/3a105f07 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/95bee405 (commit)
>via  https://github.com/Gnucash/gnucash/commit/cc3bb4ef (commit)
>   from  https://github.com/Gnucash/gnucash/commit/0f53b6c8 (commit)
> 
> 
> 
> commit 3a105f0728984df7f063110acc8390c93722d581
> Author: John Ralls 
> Date:   Tue Jan 1 13:12:39 2019 -0800
> 
> Catch boost::locale character-conversion exceptions.
> 
> Partial cause of the crash reported in Bug 797002.
> 
I suppose you meant 796996 ?

Also it looks like you're really only catching the errors. The source of the 
conversion issue itself is not really determined yet ?

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Nov 30, 2018, at 4:27 PM, Geert Janssens  
> wrote:
> 
> Op vrijdag 30 november 2018 08:12:02 CET schreef John Ralls:
>> Updated   via  https://github.com/Gnucash/gnucash/commit/3f09e5c6 
>> (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/e81bcf6e (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/bf55c30a (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/a9344841 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/f5260996 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/185787d7 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/8ed9a9c4 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/7e10b05c (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/7283c86f (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/876bfd19 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/24ce9205 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/8f22c4be (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/4ffeb3ef (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/43a30e1c (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/3d136275 (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/606d9cfe (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/faba7975 (commit)
>>  from  https://github.com/Gnucash/gnucash/commit/de6c173e (commit)
>> 
>> 
>> 
>> commit 3f09e5c6f1af66223503eca9adee84e9a346e42a
>> Author: John Ralls 
>> Date:   Fri Nov 30 16:11:42 2018 +0900
>> 
>>Only disable register warnings for SWIG 2.
>> 
>>SWIG 3 has removed the register storage class markers.
>> 
>> commit e81bcf6e33bc5bcf2af8aca6931e537889e1a17a
>> Author: John Ralls 
>> Date:   Fri Nov 30 14:44:49 2018 +0900
>> 
>>Fix the remaining static analysis warnings.
>> 
>>Except two incorrect leak warnings and one about mktemp
>> being insecure in the XML backend. See the respective
>>comments about those.
>> 
>> commit bf55c30aeb2a94a6bd29015278d8aa84e498011e
>> Author: John Ralls 
>> Date:   Fri Nov 30 13:56:08 2018 +0900
>> 
>>Fix most of the unused assignment errors from static analysis.
>> 
>>There are a very few left that need deeper study, but this gets
>>rid of most of the noise. For the most part it's just getting rid of
>>extra variables or removing an assignment that is always
>>replaced later but before any reads of the variable. A few are
>>discarded result variables.
>> 
>> commit a93448414f3e790e52a3f627f7f4b6c5df463a98
>> Author: John Ralls 
>> Date:   Fri Nov 30 13:52:21 2018 +0900
>> 
>>Fix another uninitialized variable in register.
>> 
>>Found by clang static analyzer.
>> 
>> commit f52609961e43b165f532ebe2d234626ceaa4372f
>> Author: John Ralls 
>> Date:   Thu Nov 29 21:49:54 2018 +0900
>> 
>>Fix uninitialized variables (and one leak) in gnome.
>> 
>>Found by clang static analyzer.
>> 
>> commit 185787d7be17b4927b4b0396f317ff2cc20e5eea
>> Author: John Ralls 
>> Date:   Wed Nov 28 22:24:41 2018 +0900
>> 
>>Initialize some gnc_numerics that could be returned uninitialized.
>> 
>>Found by clang static analyzer.
>> 
>> commit 8ed9a9c43af638276343dd886daea958079ef768
>> Author: John Ralls 
>> Date:   Wed Nov 28 22:22:28 2018 +0900
>> 
>>Initialize some variables that could be otherwise used uninitialized.
>> 
>>Found by clang static analyzer.
>> 
>> commit 7e10b05c494f262ca7c31fd8c8bc54d61a8fed98
>> Author: John Ralls 
>> Date:   Wed Nov 28 22:07:01 2018 +0900
>> 
>>Avoid over-ranging string storage.
>> 
>>Found by clang static analyzer.
>> 
>> commit 7283c86f6f0a20cd9bdbe7587273c2b625026cce
>> Author: John Ralls 
>> Date:   Wed Nov 28 21:58:05 2018 +0900
>> 
>>Fix various static analysis logic errors in gnome-utils.
>> 
>> commit 876bfd19ad2c7d80d8dae008241c9ef67f1655a2
>> Author: John Ralls 
>> Date:   Wed Nov 28 16:18:34 2018 +0900
>> 
>>Protect against nullptr dereference, remove unused GError.
>> 
>>Found by clang static analyzer.
>> 
>> commit 24ce92056ddf5f6137827467634ddb1ef7e2bc75
>> Author: John Ralls 
>> Date:   Wed Nov 28 16:11:32 2018 +0900
>> 
>>Protect from potential nullptr dereferences.
>> 
>>pmtsched is created in only one banch of the opening switch.
>>Found by clang static analyzer.
>> 
>> commit 8f22c4bed4a3da692cbfb042d5b671cd80fb00ec
>> Author: John Ralls 
>> Date:   Wed Nov 28 15:41:45 2018 +0900
>> 
>>Localize variables, ensure that val_imbalance is set, test txn_curr !=
>> commodity once.
>> 
>>Found by clang static analyzer.
>> 
>> commit 4ffeb3efac85cd1650f33fb4acd0665936307213
>> Author: John Ralls 
>> Date:   Wed Nov 28 15:40:21 2018 +0900
>> 
>>Ensure that a dereferenced variable isn't NULL.
>> 
>>Found by clang static analyzer.
>> 
>> commit 43a30e1c9799fba2c926f59d488fdfbcd9f6ff54
>> Author: John Ralls 
>> Date:   Wed Nov 28 15:39:07 2018 +0900
>> 
>>

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op vrijdag 30 november 2018 08:12:02 CET schreef John Ralls:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/3f09e5c6 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/e81bcf6e (commit)
>via  https://github.com/Gnucash/gnucash/commit/bf55c30a (commit)
>via  https://github.com/Gnucash/gnucash/commit/a9344841 (commit)
>via  https://github.com/Gnucash/gnucash/commit/f5260996 (commit)
>via  https://github.com/Gnucash/gnucash/commit/185787d7 (commit)
>via  https://github.com/Gnucash/gnucash/commit/8ed9a9c4 (commit)
>via  https://github.com/Gnucash/gnucash/commit/7e10b05c (commit)
>via  https://github.com/Gnucash/gnucash/commit/7283c86f (commit)
>via  https://github.com/Gnucash/gnucash/commit/876bfd19 (commit)
>via  https://github.com/Gnucash/gnucash/commit/24ce9205 (commit)
>via  https://github.com/Gnucash/gnucash/commit/8f22c4be (commit)
>via  https://github.com/Gnucash/gnucash/commit/4ffeb3ef (commit)
>via  https://github.com/Gnucash/gnucash/commit/43a30e1c (commit)
>via  https://github.com/Gnucash/gnucash/commit/3d136275 (commit)
>via  https://github.com/Gnucash/gnucash/commit/606d9cfe (commit)
>via  https://github.com/Gnucash/gnucash/commit/faba7975 (commit)
>   from  https://github.com/Gnucash/gnucash/commit/de6c173e (commit)
> 
> 
> 
> commit 3f09e5c6f1af66223503eca9adee84e9a346e42a
> Author: John Ralls 
> Date:   Fri Nov 30 16:11:42 2018 +0900
> 
> Only disable register warnings for SWIG 2.
> 
> SWIG 3 has removed the register storage class markers.
> 
> commit e81bcf6e33bc5bcf2af8aca6931e537889e1a17a
> Author: John Ralls 
> Date:   Fri Nov 30 14:44:49 2018 +0900
> 
> Fix the remaining static analysis warnings.
> 
> Except two incorrect leak warnings and one about mktemp
>  being insecure in the XML backend. See the respective
> comments about those.
> 
> commit bf55c30aeb2a94a6bd29015278d8aa84e498011e
> Author: John Ralls 
> Date:   Fri Nov 30 13:56:08 2018 +0900
> 
> Fix most of the unused assignment errors from static analysis.
> 
> There are a very few left that need deeper study, but this gets
> rid of most of the noise. For the most part it's just getting rid of
> extra variables or removing an assignment that is always
> replaced later but before any reads of the variable. A few are
> discarded result variables.
> 
> commit a93448414f3e790e52a3f627f7f4b6c5df463a98
> Author: John Ralls 
> Date:   Fri Nov 30 13:52:21 2018 +0900
> 
> Fix another uninitialized variable in register.
> 
> Found by clang static analyzer.
> 
> commit f52609961e43b165f532ebe2d234626ceaa4372f
> Author: John Ralls 
> Date:   Thu Nov 29 21:49:54 2018 +0900
> 
> Fix uninitialized variables (and one leak) in gnome.
> 
> Found by clang static analyzer.
> 
> commit 185787d7be17b4927b4b0396f317ff2cc20e5eea
> Author: John Ralls 
> Date:   Wed Nov 28 22:24:41 2018 +0900
> 
> Initialize some gnc_numerics that could be returned uninitialized.
> 
> Found by clang static analyzer.
> 
> commit 8ed9a9c43af638276343dd886daea958079ef768
> Author: John Ralls 
> Date:   Wed Nov 28 22:22:28 2018 +0900
> 
> Initialize some variables that could be otherwise used uninitialized.
> 
> Found by clang static analyzer.
> 
> commit 7e10b05c494f262ca7c31fd8c8bc54d61a8fed98
> Author: John Ralls 
> Date:   Wed Nov 28 22:07:01 2018 +0900
> 
> Avoid over-ranging string storage.
> 
> Found by clang static analyzer.
> 
> commit 7283c86f6f0a20cd9bdbe7587273c2b625026cce
> Author: John Ralls 
> Date:   Wed Nov 28 21:58:05 2018 +0900
> 
> Fix various static analysis logic errors in gnome-utils.
> 
> commit 876bfd19ad2c7d80d8dae008241c9ef67f1655a2
> Author: John Ralls 
> Date:   Wed Nov 28 16:18:34 2018 +0900
> 
> Protect against nullptr dereference, remove unused GError.
> 
> Found by clang static analyzer.
> 
> commit 24ce92056ddf5f6137827467634ddb1ef7e2bc75
> Author: John Ralls 
> Date:   Wed Nov 28 16:11:32 2018 +0900
> 
> Protect from potential nullptr dereferences.
> 
> pmtsched is created in only one banch of the opening switch.
> Found by clang static analyzer.
> 
> commit 8f22c4bed4a3da692cbfb042d5b671cd80fb00ec
> Author: John Ralls 
> Date:   Wed Nov 28 15:41:45 2018 +0900
> 
> Localize variables, ensure that val_imbalance is set, test txn_curr !=
> commodity once.
> 
> Found by clang static analyzer.
> 
> commit 4ffeb3efac85cd1650f33fb4acd0665936307213
> Author: John Ralls 
> Date:   Wed Nov 28 15:40:21 2018 +0900
> 
> Ensure that a dereferenced variable isn't NULL.
> 
> Found by clang static analyzer.
> 
> commit 43a30e1c9799fba2c926f59d488fdfbcd9f6ff54
> Author: John Ralls 
> Date:   Wed Nov 28 15:39:07 2018 +0900
> 
> Silence clang static analyzer complaint about potential div by 0.
> 
> It can't, because if b is 0 the function would have
> returned 

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Frank H. Ellenberger]

That is the reason, why I inserted the TODO. Because my search for URLs
already resulted in 2 different patches, I did not start it, too.

If you are ready with your other errands, you can go for it.

Regards
Frank

Am 03.11.18 um 10:36 schrieb Geert Janssens:
> Op zaterdag 3 november 2018 10:15:48 CET schreef Frank H. Ellenberger:
>> @@ -4892,8 +4892,8 @@ dgettext_swapped (const gchar *msgid,
>>   * This is copied into GnuCash from Gtk in order to fix problems when
>>   * empty msgids were passed through gettext().
>>   *
>> - * See https://bugs.gnucash.org/show_bug.cgi?id=326200 . If that bug
>> - * is fixed in the gtk that we can rely open, then
>> + * TODO: See https://bugzilla.gnome.org/show_bug.cgi?id=326200 . If that
>> bug + * is fixed in the gtk that we can rely on, then
>>   * gnc_gtk_action_group_set_translation_domain can be replaced by
>>   * gtk_action_group_set_translation_domain again.
>>   */
> If I follow the link to this bug, it suggests this has been fixed in 2006 
> already. So I suppose by now gnc_gtk_action_group_set_translation_domain can 
> be dropped again ?
> 
> Geert
> 
> 
> ___
> gnucash-devel mailing list
> gnucash-devel@gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-devel
> 

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op zaterdag 3 november 2018 10:15:48 CET schreef Frank H. Ellenberger:
> @@ -4892,8 +4892,8 @@ dgettext_swapped (const gchar *msgid,
>   * This is copied into GnuCash from Gtk in order to fix problems when
>   * empty msgids were passed through gettext().
>   *
> - * See https://bugs.gnucash.org/show_bug.cgi?id=326200 . If that bug
> - * is fixed in the gtk that we can rely open, then
> + * TODO: See https://bugzilla.gnome.org/show_bug.cgi?id=326200 . If that
> bug + * is fixed in the gtk that we can rely on, then
>   * gnc_gtk_action_group_set_translation_domain can be replaced by
>   * gtk_action_group_set_translation_domain again.
>   */
If I follow the link to this bug, it suggests this has been fixed in 2006 
already. So I suppose by now gnc_gtk_action_group_set_translation_domain can 
be dropped again ?

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op zondag 9 september 2018 23:04:26 CEST schreef Kevin Hale Boyes:
> The indentation (and logic) here:
> https://github.com/Gnucash/gnucash/commit/3845611f#diff-ee628819286d2dd65d45
> baf2c5ef4bbdR454 suggests that the if applies to both indented lines but
> there are no braces to form a block.
> 
> If gsr->reg is NULL then line line 456 will throw.
> 
> Kevin
> 
Oops, good catch.

Fixed and thanks!

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Kevin Hale Boyes]

The indentation (and logic) here:
https://github.com/Gnucash/gnucash/commit/3845611f#diff-ee628819286d2dd65d45baf2c5ef4bbdR454
suggests that the if applies to both indented lines but there are no braces
to form a block.

If gsr->reg is NULL then line line 456 will throw.

Kevin

On Sun, 9 Sep 2018 at 14:50, Geert Janssens 
wrote:

> Updated  via  https://github.com/Gnucash/gnucash/commit/d069b67d (commit)
>  via  https://github.com/Gnucash/gnucash/commit/3634e8f5 (commit)
>  via  https://github.com/Gnucash/gnucash/commit/3845611f (commit)
>  via  https://github.com/Gnucash/gnucash/commit/4cc61463 (commit)
> from  https://github.com/Gnucash/gnucash/commit/bfa6cd52 (commit)
>
>
>
> commit d069b67d48f99ef69d6a31c9dd428da67215b048
> Author: Geert Janssens 
> Date:   Sun Sep 9 22:49:52 2018 +0200
>
> Fix memory leak in xaccTransGetReadOnly
>
> In addition implement a cache for this value as suggested in the
> comments
> as this function is called on every transaction commit.
>
> commit 3634e8f59d576ce2c3b97fc6a817ada506d47989
> Author: Geert Janssens 
> Date:   Sun Sep 9 17:43:05 2018 +0200
>
> Fix memory leak using qof_instance_get on a GncGUID
>
> The underlying boxed type will return a copy so we should free this
> when no longer needed.
>
> commit 3845611f30848d6aca9b4764e24c050e801e0803
> Author: Geert Janssens 
> Date:   Sun Sep 9 12:35:56 2018 +0200
>
> Plug memory leak in register code
>
> The table storing cell dimensions was never freed. The size of this
> table is
> directly proportional to the number of cells in the register. So the
> more
> transactions/splits in a register, the more memory was leaked - each
> time
> a register was opened and closed. With my huge test book I saw leaks of
> 4Mb-10Mb per page that was opened/closed.
>
> commit 4cc61463abd60848e14543ec22adb8364b5a6cda
> Author: Geert Janssens 
> Date:   Sun Sep 9 12:32:04 2018 +0200
>
> Remove unused variable
>
>
>
> Summary of changes:
>  gnucash/gnome-utils/gnc-tree-util-split-reg.c  |  1 +
>  gnucash/gnome/dialog-payment.c |  1 +
>  gnucash/gnome/dialog-sx-editor.c   |  1 +
>  gnucash/gnome/dialog-sx-editor2.c  |  1 +
>  gnucash/gnome/gnc-plugin-page-register2.c  |  1 +
>  gnucash/gnome/gnc-split-reg.c  | 22 ++
>  gnucash/import-export/ofx/gnc-ofx-import.c | 13 +++---
>  .../register/ledger-core/split-register-model.c|  2 +-
>  gnucash/register/register-gnome/gnucash-sheet.c| 14 ---
>  gnucash/register/register-gnome/gnucash-style.c| 25 +++
>  gnucash/register/register-gnome/gnucash-style.h|  4 +-
>  libgnucash/app-utils/gnc-sx-instance-model.c   |  7 +++-
>  libgnucash/app-utils/gnc-ui-util.c |  7 +++-
>  libgnucash/core-utils/gnc-environment.c|  7 ++--
>  libgnucash/engine/SX-book.c|  4 +-
>  libgnucash/engine/Transaction.c| 49
> --
>  libgnucash/engine/Transaction.h|  2 +-
>  libgnucash/engine/TransactionP.h   |  9 
>  libgnucash/engine/cap-gains.c  |  2 +
>  libgnucash/engine/gnc-budget.c |  5 ++-
>  libgnucash/engine/gncInvoice.c | 10 -
>  libgnucash/engine/gncOwner.c   |  2 +
>  libgnucash/engine/qofbook.cpp  |  2 +-
>  libgnucash/engine/qofbook.h|  2 +-
>  .../engine/test/test-engine-kvp-properties.c   |  3 ++
>  25 files changed, 136 insertions(+), 60 deletions(-)
>
> ___
> gnucash-patches mailing list
> gnucash-patc...@gnucash.org
> https://lists.gnucash.org/mailman/listinfo/gnucash-patches
>
___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[John Ralls]

> On Apr 10, 2018, at 1:22 AM, Geert Janssens  
> wrote:
> 
> Op dinsdag 10 april 2018 00:18:07 CEST schreef John Ralls:
>> Updated   via  https://github.com/Gnucash/gnucash/commit/c3f528b8 
>> (commit)
>>   via  https://github.com/Gnucash/gnucash/commit/aea33ca5 (commit)
>>  from  https://github.com/Gnucash/gnucash/commit/972647d2 (commit)
>> 
>> 
>> 
>> commit c3f528b80c6cccdf9c139dc0f63a96ef513a0e62
>> Author: John Ralls 
>> Date:   Mon Apr 9 15:17:32 2018 -0700
>> 
>>Bug 795068 - src/optional/python-bindings missing several files
>> 
>> diff --git a/src/optional/python-bindings/CMakeLists.txt
>> b/src/optional/python-bindings/CMakeLists.txt index e87c194..96d92a1 100644
>> --- a/src/optional/python-bindings/CMakeLists.txt
>> +++ b/src/optional/python-bindings/CMakeLists.txt
>> @@ -1,7 +1,9 @@
>> ADD_SUBDIRECTORY(example_scripts)
>> ADD_SUBDIRECTORY(tests)
>> 
>> -IF (BUILDING_FROM_VCS)
>> +  SET(PYEXEC_FILES  __init__.py function_class.py gnucash_business.py
>> gnucash_core.py) +
>> +  IF (BUILDING_FROM_VCS)
>>   SET(SWIG_FILES ${CMAKE_CURRENT_SOURCE_DIR}/gnucash_core.i
>> ${CMAKE_CURRENT_SOURCE_DIR}/timespec.i) SET(GNUCASH_CORE_C_INCLUDES
>> ${CMAKE_BINARY_DIR}/src/config.h
>> @@ -63,8 +65,6 @@ IF(WITH_PYTHON)
>> ${PYTHON_INCLUDE_DIRS}
>>   )
>> 
>> -  SET(PYEXEC_FILES  __init__.py function_class.py gnucash_business.py
>> gnucash_core.py) -
>>   ADD_LIBRARY(gnucash_core_c MODULE ${SWIG_GNUCASH_CORE_C})
>>   TARGET_INCLUDE_DIRECTORIES(gnucash_core_c PRIVATE
>> ${gnucash_core_c_INCLUDE_DIRS})
>> 
>> 
>> commit aea33ca515aceb4f63a5cbc8165d4ec93f245d0c
>> Author: John Ralls 
>> Date:   Sun Apr 8 15:20:55 2018 -0700
>> 
>>Bug 795049 - GnuCash 2.6.20-1 (Fedora Linux package) is unable to
>> open...
>> 
>>MariaDB database.
>> 
>> diff --git a/src/backend/dbi/test/test-backend-dbi-basic.c
>> b/src/backend/dbi/test/test-backend-dbi-basic.c index 386a2c3..4a55a62
>> 100644
>> --- a/src/backend/dbi/test/test-backend-dbi-basic.c
>> +++ b/src/backend/dbi/test/test-backend-dbi-basic.c
>> @@ -666,15 +666,15 @@ test_suite_gnc_backend_dbi (void)
>> }
>> if (g_list_find_custom (drivers, "sqlite3", (GCompareFunc)g_strcmp0))
>> create_dbi_test_suite ("sqlite3", "sqlite3");
>> -if (strlen (TEST_MYSQL_URL) > 0 &&
>> -g_list_find_custom (drivers, "mysql", (GCompareFunc)g_strcmp0))
>> -create_dbi_test_suite ("mysql", TEST_MYSQL_URL);
>> if (strlen (TEST_PGSQL_URL) > 0 &&
>> g_list_find_custom (drivers, "pgsql", (GCompareFunc)g_strcmp0))
>> {
>> g_setenv ("PGOPTIONS", "-c client_min_messages=WARNING", FALSE);
>> create_dbi_test_suite ("postgres", TEST_PGSQL_URL);
>> }
>> +if (strlen (TEST_MYSQL_URL) > 0 &&
>> +g_list_find_custom (drivers, "mysql", (GCompareFunc)g_strcmp0))
>> +create_dbi_test_suite ("mysql", TEST_MYSQL_URL);
>> 
>> GNC_TEST_ADD_FUNC( suitename, "adjust sql options string localtime",
>> test_adjust_sql_options_string );
> 
> This is surprising: this commit suggests only the order of the DB test would 
> make a difference on whether a mariadb based book can be opened or not; 
> testing for mysql after testing for postgresql would fix this.
> I understood from our IRC conversation yesterday the issue was that there 
> were 
> two mysql statements in one dbi connection query. Did you forget to commit 
> that part of the fix or did we misunderstand the problem ?


Rats. Yes, the actual fix isn’t in the commit, so I’ll have to fix that and 
re-tag 2.6.21. Thanks for catching it!

The change to the order in the test program was to get the test to run the 
postgres test before getting stopped by the mysql one failing to see if 
postgres would work with the two-query statement.

Regards,
John Ralls

___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Re: [GNC-dev] gnucash maint: Multiple changes pushed

[Geert Janssens]

Op dinsdag 10 april 2018 00:18:07 CEST schreef John Ralls:
> Updatedvia  https://github.com/Gnucash/gnucash/commit/c3f528b8 
> (commit)
>via  https://github.com/Gnucash/gnucash/commit/aea33ca5 (commit)
>   from  https://github.com/Gnucash/gnucash/commit/972647d2 (commit)
> 
> 
> 
> commit c3f528b80c6cccdf9c139dc0f63a96ef513a0e62
> Author: John Ralls 
> Date:   Mon Apr 9 15:17:32 2018 -0700
> 
> Bug 795068 - src/optional/python-bindings missing several files
> 
> diff --git a/src/optional/python-bindings/CMakeLists.txt
> b/src/optional/python-bindings/CMakeLists.txt index e87c194..96d92a1 100644
> --- a/src/optional/python-bindings/CMakeLists.txt
> +++ b/src/optional/python-bindings/CMakeLists.txt
> @@ -1,7 +1,9 @@
>  ADD_SUBDIRECTORY(example_scripts)
>  ADD_SUBDIRECTORY(tests)
> 
> -IF (BUILDING_FROM_VCS)
> +  SET(PYEXEC_FILES  __init__.py function_class.py gnucash_business.py
> gnucash_core.py) +
> +  IF (BUILDING_FROM_VCS)
>SET(SWIG_FILES ${CMAKE_CURRENT_SOURCE_DIR}/gnucash_core.i
> ${CMAKE_CURRENT_SOURCE_DIR}/timespec.i) SET(GNUCASH_CORE_C_INCLUDES
>  ${CMAKE_BINARY_DIR}/src/config.h
> @@ -63,8 +65,6 @@ IF(WITH_PYTHON)
>  ${PYTHON_INCLUDE_DIRS}
>)
> 
> -  SET(PYEXEC_FILES  __init__.py function_class.py gnucash_business.py
> gnucash_core.py) -
>ADD_LIBRARY(gnucash_core_c MODULE ${SWIG_GNUCASH_CORE_C})
>TARGET_INCLUDE_DIRECTORIES(gnucash_core_c PRIVATE
> ${gnucash_core_c_INCLUDE_DIRS})
> 
> 
> commit aea33ca515aceb4f63a5cbc8165d4ec93f245d0c
> Author: John Ralls 
> Date:   Sun Apr 8 15:20:55 2018 -0700
> 
> Bug 795049 - GnuCash 2.6.20-1 (Fedora Linux package) is unable to
> open...
> 
> MariaDB database.
> 
> diff --git a/src/backend/dbi/test/test-backend-dbi-basic.c
> b/src/backend/dbi/test/test-backend-dbi-basic.c index 386a2c3..4a55a62
> 100644
> --- a/src/backend/dbi/test/test-backend-dbi-basic.c
> +++ b/src/backend/dbi/test/test-backend-dbi-basic.c
> @@ -666,15 +666,15 @@ test_suite_gnc_backend_dbi (void)
>  }
>  if (g_list_find_custom (drivers, "sqlite3", (GCompareFunc)g_strcmp0))
>  create_dbi_test_suite ("sqlite3", "sqlite3");
> -if (strlen (TEST_MYSQL_URL) > 0 &&
> -g_list_find_custom (drivers, "mysql", (GCompareFunc)g_strcmp0))
> -create_dbi_test_suite ("mysql", TEST_MYSQL_URL);
>  if (strlen (TEST_PGSQL_URL) > 0 &&
>  g_list_find_custom (drivers, "pgsql", (GCompareFunc)g_strcmp0))
> {
>  g_setenv ("PGOPTIONS", "-c client_min_messages=WARNING", FALSE);
>  create_dbi_test_suite ("postgres", TEST_PGSQL_URL);
>  }
> +if (strlen (TEST_MYSQL_URL) > 0 &&
> +g_list_find_custom (drivers, "mysql", (GCompareFunc)g_strcmp0))
> +create_dbi_test_suite ("mysql", TEST_MYSQL_URL);
> 
>  GNC_TEST_ADD_FUNC( suitename, "adjust sql options string localtime",
>  test_adjust_sql_options_string );

This is surprising: this commit suggests only the order of the DB test would 
make a difference on whether a mariadb based book can be opened or not; 
testing for mysql after testing for postgresql would fix this.
I understood from our IRC conversation yesterday the issue was that there were 
two mysql statements in one dbi connection query. Did you forget to commit 
that part of the fix or did we misunderstand the problem ?

Geert


___
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel