AUDIT: r15205 - gnucash/trunk - Load and store a commodity's KVP-frame (IFF it's non-empty).

[Derek Atkins]

Quoting Christian Stimming <stimming at tuhh.de>:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Derek Atkins schrieb:
>> 1) This patch doesn't actually change any data formats, because there's
>>    no data in those fields.  So without additional changes (which I agree
>>    should not get backported), there's no chance of a data format change.
>
> Does that mean this code doesn't write any data (in the KVP fields) if
> and only if there isn't any data written during runtime? If yes, then
> I'd happily accept that code, BUT we'd have to be really sure we don't
> accidentally back-port any of the code that would introduce data into
> those kvp fields.

Correct, if the KVP fields are empty then there is no change to the XML.
Indeed, I tested this by reading my data file and re-writing it out, and
the gnc:commodity entries in the XML file did not change.  So the only
time it will write extra data to the XML file is if there was already
extra data in the file, or if somehow during runtime something adds
data to that KVP.

Right now nothing writes to that KVP, so I think it's safe to say that
we wont backport anything that would...  So what we're doing is making
2.0.4 backwards- and forwards-compatible with potential changes in 2.2.

>> Keep in mind that just making the XML parser not blow chunks on
>> unrecognized tags isn't sufficient -- we also need to re-write out
>> those unrecognized tags or we destroy data.   Reading and silently
>> discarding is worse than erroring out.
>
> *sigh* You're probably right, although a crashing gnucash is always not
> a good thing. We should have some different method available if users
> want to read a non-backward-compatible file, with the clear notion that
> this will almost certainly mean data loss. Something like "Import
> gnucash file from other version" or similar.

Well, I dont think it would crash, per se..  It should just output an
error.   HOWEVER in this particular case the gnc-commodity parser WILL
ignore anything it doesn't understand, so we already have the 'data loss'
state..   So chris' worry isn't really there; this change IS safe and
just makes it safer -- 2.0.[0123] will read a data file with this new
object and just ignore it (and drop it on the floor) without erroring out.

>> So, I argue that this change still is safe because it's effectively
>> unused code, but it would let us add additional information in a 2.2
>> release and not destroy that data in 2.0, which is why I think we should
>> backport this change.
>
> I agree on the backport.

Thanks,

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available