Bug 370373 - Please sort <member-groups> on saving index.xml
Summary: Please sort <member-groups> on saving index.xml
Status: RESOLVED FIXED
Alias: None
Product: kphotoalbum
Classification: Applications
Component: XML backend (other bugs)
Version First Reported In: 4.7.2
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: KPhotoAlbum Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-09 20:21 UTC by Martin Jost
Modified: 2016-12-15 01:31 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Jost 2016-10-09 20:21:21 UTC
Currently it seems the <member-groups> isn't sorted in any way in the index.xml.
It seems in addition that it gets rearranged on saving.
I keep my index.xml versioned using RCS and I usually do a rcsdiff before committing an updated version.
For this purpose it is good to have as few changes as possible and above all only "real" changes.
While i see that the order is irrelevant for KPA (so it probably doesn't care) it is annoying for diffing versions.
(I do this to avoid screwing up my DB)
So I would like to see <member-groups> sorted reasonably. My suggestion would be to sort by
category/group-name/member (in this order of precedence)

Reproducible: Always

Steps to Reproduce:
1. Add a new tag
2. rcsdiff the newly created index.xml
3. It not only differs in the newly added tag, but the order within <member-groups> changed in many places

Actual Results:  
see above

Expected Results:  
Only changes in <member-groups> for actually added/changed/removed tags

Example:
>         <member category="Inhalt" group-name="ATE" member="sampling Scope"/>
>         <member category="Inhalt" group-name="ATE" member="Waferprober"/>
304535d304530
<         <member category="Inhalt" group-name="ATE" member="Waferprober"/>
304537,304539d304531
<         <member category="Inhalt" group-name="ATE" member="sampling Scope"/>
<         <member category="Inhalt" group-name="Affe" member="Gorilla"/>
<         <member category="Inhalt" group-name="Affe" member="Orang Utan"/>
304540a304533,304534
>         <member category="Inhalt" group-name="Affe" member="Orang Utan"/>
>         <member category="Inhalt" group-name="Affe" member="Gorilla"/>
304543,304544d304536
<         <member category="Inhalt" group-name="Baum" member="Apfel"/>
<         <member category="Inhalt" group-name="Baum" member="Eiche"/>
Comment 1 Johannes Zarl-Zierl 2016-10-10 18:51:59 UTC
The issue is known (and also a problem with the kpa-backup.sh script). I just didn't have time to fix it, yet.
Expect a fix in version 5.1 (my apologies for not including it with 5.0)...
Comment 2 Johannes Zarl-Zierl 2016-12-07 00:37:35 UTC
Git commit fb60b9965e353e938507e1dd6c3c0ea636824235 by Johannes Zarl-Zierl.
Committed on 07/12/2016 at 00:32.
Pushed by johanneszarl into branch 'master'.

Try harder to get minimal changes in the index.xml

Scripts like kpa-backup require diff-able files if one wants
human-readable change descriptions. This tweaks XMLCategory::addItem()
so that it doesn't always change all numerical category ids, and sorts
the blocklist before writing it to a file.

M  +4    -1    XMLDB/FileWriter.cpp
M  +2    -2    XMLDB/XMLCategory.cpp

https://commits.kde.org/kphotoalbum/fb60b9965e353e938507e1dd6c3c0ea636824235
Comment 3 Johannes Zarl-Zierl 2016-12-07 00:43:04 UTC
How is fb60b996 for a change?

I've refrained from sorting the tags and member-groups in the way you describe, because this would mean that numerical tag id's change every time I add a tag (and thus almost every image entry would change as well).
Comment 4 Johannes Zarl-Zierl 2016-12-07 01:19:09 UTC
Git commit d5f1ac71af2217f33df56b3c9739030ea2b60ae0 by Johannes Zarl-Zierl.
Committed on 07/12/2016 at 01:16.
Pushed by johanneszarl into branch 'master'.

Make writing index.xml more deterministic.

There were still a few QSet uses left that introduce non-determinism in
file-writing. There is still some work to be done, but getting back
diffable files seems viable.
(Performance will also have to be checked, afterwards...

M  +9    -2    XMLDB/FileWriter.cpp
M  +2    -3    XMLDB/XMLCategory.cpp

https://commits.kde.org/kphotoalbum/d5f1ac71af2217f33df56b3c9739030ea2b60ae0
Comment 5 Johannes Zarl-Zierl 2016-12-09 00:20:29 UTC
Git commit 728df5bb6073cb8d2d854daec1ecb26bfadd4f96 by Johannes Zarl-Zierl.
Committed on 09/12/2016 at 00:15.
Pushed by johanneszarl into branch 'master'.

Make saving the DB deterministic

M  +6    -1    XMLDB/FileWriter.cpp

https://commits.kde.org/kphotoalbum/728df5bb6073cb8d2d854daec1ecb26bfadd4f96
Comment 6 Johannes Zarl-Zierl 2016-12-09 00:20:29 UTC
Git commit 69420290638781721128f2fd27b209829b73cd57 by Johannes Zarl-Zierl.
Committed on 09/12/2016 at 00:16.
Pushed by johanneszarl into branch 'master'.

Add benchmarking code for deterministic saving of DB.

On my system, deterministic saving costs about 34 msec with a database of
just below 25k images.

Saving time, non-deterministic: 1473.2 msec
Saving time, deterministic: 1506.9 msec

M  +34   -0    XMLDB/FileWriter.cpp

https://commits.kde.org/kphotoalbum/69420290638781721128f2fd27b209829b73cd57
Comment 7 Johannes Zarl-Zierl 2016-12-09 00:25:45 UTC
(In reply to Johannes Zarl-Zierl from comment #6)
> Git commit 69420290638781721128f2fd27b209829b73cd57 by Johannes Zarl-Zierl.
> Committed on 09/12/2016 at 00:16.
> Pushed by johanneszarl into branch 'master'.
> 
> Add benchmarking code for deterministic saving of DB.
> 
> On my system, deterministic saving costs about 34 msec with a database of
> just below 25k images.
> 
> Saving time, non-deterministic: 1473.2 msec
> Saving time, deterministic: 1506.9 msec
> 
> M  +34   -0    XMLDB/FileWriter.cpp
> 
> https://commits.kde.org/kphotoalbum/69420290638781721128f2fd27b209829b73cd57

I forgot to mention in the commit message: the numbers were averages of 10 consecutive save operations...
Comment 8 Johannes Zarl-Zierl 2016-12-09 18:55:52 UTC
Fix was confirmed in pm to me...
Comment 9 Unknown 2016-12-10 23:43:02 UTC
Git commit 61fe41652be693d213ebbc3465bf970ce52a4f76 by Johannes Zarl-Zierl.
Committed on 10/12/2016 at 23:37.
Pushed by johanneszarl into branch 'master'.

Fix the SortLastUsed functionality in ListSelect

Unfortunately, sorting the ListSelect entries in the annotation dialog
in a most-recently-used first order currently requires shuffling the tag
list in the XMLCategory.
That means that the numerical id's in the database invariably change to
reflect the most-recently-used order...

M  +4    -2    XMLDB/XMLCategory.cpp

https://commits.kde.org/kphotoalbum/61fe41652be693d213ebbc3465bf970ce52a4f76
Comment 10 Johannes Zarl-Zierl 2016-12-15 01:31:31 UTC
Git commit 61b306eaa1723ccd42fa926be1c2b2d7748f205e by Johannes Zarl-Zierl.
Committed on 15/12/2016 at 01:27.
Pushed by johanneszarl into branch 'master'.

Reuse numerical tag ids.

This patch makes KPA distinguish between tag order in the xml file and
tag id. In most cases, tags will now get the same id when saving as they
had when the db was loaded from disk.
This allows us to reorder tags (as required by the SortLastUsed option
of the ListSelect) and still have diffable xml files.

M  +1    -1    AnnotationDialog/ListSelect.cpp
M  +20   -5    XMLDB/XMLCategory.cpp

https://commits.kde.org/kphotoalbum/61b306eaa1723ccd42fa926be1c2b2d7748f205e