Master - saving to xml broken

[John Ralls]

> On Dec 3, 2016, at 9:38 AM, Geert Janssens <geert.gnucash at kobaltwit.be> wrote:
> 
> Op zaterdag 3 december 2016 18:32:44 CET schreef Geert Janssens:
>> Op zaterdag 3 december 2016 17:24:27 CET schreef Wm via gnucash-devel:
>>> On 03/12/2016 16:05, John Ralls wrote:
>>>>> On Dec 3, 2016, at 7:46 AM, Geert Janssens <geert.gnucash at kobaltwit.be>
>>>>> wrote:>>
>>>>> 
>>>>> Op zaterdag 3 december 2016 16:34:13 CET schreef Geert Janssens:
>>>>>> Op vrijdag 2 december 2016 14:30:19 CET schreef John Ralls:
>>>>>>>> On Dec 2, 2016, at 12:53 PM, John Ralls <jralls at ceridwen.us> wrote:
>>>>>>>>> On Dec 2, 2016, at 12:47 PM, Robert Fewell <14ubobit at gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> Just built master from
>>>>>>>>> https://github.com/Gnucash/gnucash/commit/dd4b8a104d0f7ad2205407e8b
>>>>>>>>> f1
>>>>>>>>> 0f
>>>>>>>>> ee
>>>>>>>>> c364c8127 and I still do not see the errors Geert is having. Added
>>>>>>>>> a
>>>>>>>>> Customer and a Vendor and still only have one of each.
>>>>>>>> 
>>>>>>>> Bob,
>>>>>>>> 
>>>>>>>> Yes, I'm also not yet able to reproduce the duplicate vendor issue.
>>>>>>>> Geert
>>>>>>>> mentioned on IRC that he's using Fedora-25 so I'm building a new VM
>>>>>>>> with
>>>>>>>> that to test.
>>>>>>> 
>>>>>>> I can't reproduce it on Fedora-25 either.
>>>>>>> 
>>>>>>> The save problems and crashes on master I *can* reproduce, so I'm
>>>>>>> working
>>>>>>> on that.
>>>>>>> 
>>>>>>> Regards,
>>>>>>> John Ralls
>>>>>> 
>>>>>> John,
>>>>>> 
>>>>>> I  can confirm your commits fix the save problems and crashes. Thanks.
>>>>>> 
>>>>>> As I appear to be the only one experiencing the other part of
>>>>>> duplicate
>>>>>> objects, I am digging further locally.
>>>>>> 
>>>>>> Using gdb, I found that all non-core objects are registered twice.
>>>>>> Once they are registered as part of loading the app-utils module,
>>>>>> which
>>>>>> in
>>>>>> turn loads the engine module which then loads the business modules.
>>>>>> 
>>>>>> The second time they are loaded because the python bindings load the
>>>>>> engine
>>>>>> module, triggering loading of the business modules again apparently.
>>>>>> 
>>>>>> I have no idea (yet) why this didn't happen before the backend rewrite
>>>>>> was
>>>>>> merged back in.
>>>>>> 
>>>>>> I suppose none of you have the python bindings enabled, which would
>>>>>> explain
>>>>>> why only I'm seeing this.
>>>>>> 
>>>>>> Geert
>>>>> 
>>>>> The easy one-line fix would be to test for engine_is_initialized == 1
>>>>> in
>>>>> gnc_engine_init_part2 just like in gnc_engine_init_part1.
>>>>> 
>>>>> I wonder though whether we'd want to move this up to gnc_engine_init in
>>>>> general though. Do we want the init hooks to be run each time some code
>>>>> calls gnc_engine_init or should they be called only once also ?
>>>> 
>>>> IMO we want to get rid of all of the dlopening and just link the
>>>> convenience libraries like a normal program. That's a lot of work,
>>>> though, so for now we should be loading only once.
>>>> 
>>>> Is the problem really the python bindings (src/optional/python-bindings)
>>>> or the python console (src/python) that loads the python bindings?
>>> 
>>> Open Q: has anyone tried this on Win where the python stuff is known not
>>> to work?
>>> 
>>> I think python is red herring in this for me and agree there are (at
>>> least) two issues
>> 
>> No, python is not a red herring here. This thread is becoming increasingly
>> confusing because I report many different issues at once.
>> 
>> There were the save failure and crash which John fixed yesterday.
>> There is still the issue of all commodities being saved instead of only
>> those that are actually in use.
>> And there's the one I alone was seeing (business ojbects being saved twice
>> resulting in a corrupt xml file). This last one only appeared when gnucash
>> is built with python bindings and I have just pushed a fix for a a couple
>> of minutes ago.
>> 
>> So the only thing left is the abundance of commodities being saved by xml.
>> 
> And I should have added that this indeed probably doesn't have anything to do 
> with the python bindings. So we can ignore those again from now on :)

I found it. When I abstracted KVP to make it private-ish to QofInstance I removed some tests in the course of changing kvp_frame_to_dom_tree() into qof_instance_slots_to_dom_tree() with the result that it returns an empty element instead of nullptr on the currencies so that the commodity gets saved when it shouldn't.

Regards,
John Ralls