Backport or not ?

[John Ralls]

On Jun 22, 2012, at 9:41 AM, Geert Janssens <janssens-geert at telenet.be> wrote:

> Thanks all for the feedback.
> 
> On 22-06-12 09:46, John Ralls wrote:
>> On Jun 22, 2012, at 8:09 AM, David Carlson<carlson.dl at sbcglobal.net>  wrote:
>> 
>>> Gentlemen,
>>> 
>>> What would Releases 2.4.10 and previous do if they encounter an unfamiliar format?  Do they throw an error that the file needs an updated program?
>>> 
>>> The answer may help your decision.
>> Yes, exactly. That's Geert's point.
>> 
>> Regards,
>> John Ralls
>> 
>> 
>> 
> I wasn't very awake apparently yesterday when I wrote my first message. David's mail triggered me to actually *test* how 2.4.10 would react on the new data format:
> - It opens the file without any warning
> - The scheduled transaction is missing the parameter (weekly adjust)
> - But if you don't change the scheduled transaction, the parameter won't get lost either: reopening the file again in trunk shows the parameter is still saved.
> - Back to 2.4.10: when exiting the application, it spews a few assertion failures in g_hash_for_each:
> CRIT <GLib> g_hash_table_foreach: assertion `version == hash_table->version' failed (3 times)
> - Trying to modify or create a scheduled transaction in 2.4.10 with the new data fails with a vague error that the database transaction failed.
> 
>> I think the rule is that 2.4.11 should be able to read the new format, but shouldn't write it, but that seems a bit stupid because it doesn't fix the bug. I suppose we could do something with KVP where 2.4.11 writes (and reads) the weekend_adjust value to KVP and trunk knows to look there, but it would be an ugly, complex hack.
> I can see how that rule is useful for new features, but I think it doesn't make sense for a bugfix. GnuCash is supposed to save that parameter. The fact that it doesn't is a bug, reported against 2.4.10.
> 
> Also given that
> - 2.4.10 opens the new format just fine (except for an assertion when closing)
> - 2.4.10 doesn't lose data (it ignores the parameter, but it's still there and it prevents the user from doing data destroying actions)
> 
> I'm pretty tempted to backport this as is.
> 
> The only useful addition I can think of would be a sensible warning, instead of the vague error.
> 
> Geert


The SQL backend doesn't ever look at the table version, just writes it. The same is true of GNUCASH_RESAVE_VERSION. The only mechanism that stops loading an updated database would seem to be in the XML backend. Seems Phil and I wrote the first part (store the version) before shipping 2.4 and never wrote the checks afterward. 

A qof_session_safe_save() would force a rewrite of the database and would therefore lose the new weekly_adjust field... but that's only triggered by a GNC_BACKEND_TOO_OLD, and there's no code to emit that in the SQL backend.

Yeah, just backport it.

Regards,
John Ralls