Merge pull request #844 from ispyisail/fix/save-reproducible-ordering

Make saving a project reproducible: sort four hash-ordered XML writes

sources/diagram.cpp

@@ -56,7 +56,20 @@ namespace {
 	/// non-deterministic across process runs for any legacy file.
 	QString elementSortKey(Element *elmt)
 	{
-		return positionKey(elmt->pos());
+			//Position alone is not a total order: two elements can sit at the
+			//same x/y (lmdg.qet has a pair of text elements both at 780,350).
+			//With equal keys std::stable_sort falls back to the order the
+			//scene handed us, which varies per run, so those two swapped
+			//places on every save. The uuid breaks the tie.
+			//
+			//For an element with a persisted uuid attribute this is fully
+			//deterministic. For a legacy element without one, fromXml()
+			//invents a fresh uuid per load, so a collision between two such
+			//elements is no better ordered than before -- but no worse
+			//either, and the tiebreaker is only consulted when the positions
+			//are equal.
+		return positionKey(elmt->pos())
+				+ QLatin1Char(':') + elmt->uuid().toString();
 	}
 
 	/// Sort key for a terminal: its parent element's position, then the

sources/qetgraphicsitem/ViewItem/projectdbmodel.cpp

@@ -17,6 +17,8 @@
 */
 #include "projectdbmodel.h"
 
+#include <algorithm>
+
 #include "../../dataBase/projectdatabase.h"
 #include "../../qetapp.h"
 #include "../../qetinformation.h"
@@ -266,7 +268,13 @@ QDomElement ProjectDBModel::toXml(QDomDocument &document) const
 		//We save all data except the display role, because he was generated in the fly
 		auto list = m_header_data.value(key).keys();
 		list.removeAll(Qt::DisplayRole);
-		
+			//Sorted: m_header_data's inner container is a QHash too, so its
+			//key order is randomised per process. modelHeaderDataToXml()
+			//writes the roles of a section in the order given here, so an
+			//unsorted list reordered the <data> children of that section on
+			//every save and kept the save irreproducible.
+		std::sort(list.begin(), list.end());
+
 		horizontal_.insert(key, list);
 	}
 	

sources/qetproject.cpp

@@ -1910,7 +1910,14 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 
 		// export default XRef properties
 	QDomElement xrefs_elmt = xml_document.createElement("xrefs");
-	for (QString key : defaultXRefProperties().keys())
+		//Sorted, because defaultXRefProperties() is a QHash and its key order
+		//is randomised per process. Writing it unsorted made two saves of an
+		//unchanged project differ only in the order of these <xref> children,
+		//so a save was not reproducible and diffing two saved files showed
+		//spurious changes.
+	QStringList xref_keys = defaultXRefProperties().keys();
+	xref_keys.sort();
+	for (QString &key : xref_keys)
 	{
 		auto xrp = defaultXRefProperties(key);
 		xrp.setKey(key);
@@ -1924,7 +1931,12 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 	conductor_autonums.setAttribute("current_autonum", m_current_conductor_autonum);
 	conductor_autonums.setAttribute("freeze_new_conductors", m_freeze_new_conductors ? "true" : "false");
 	conductor_autonums.setAttribute("auto_break_conductors", m_auto_break_conductor ? "true" : "false");
-	foreach (QString key, conductorAutoNum().keys()) {
+		//Sorted for the same reason as the xrefs above: these three
+		//collections are QHash, whose key order is randomised per process,
+		//so an unsorted write reorders these children on every save.
+	QStringList conductor_autonum_keys = conductorAutoNum().keys();
+	conductor_autonum_keys.sort();
+	for (const QString &key : std::as_const(conductor_autonum_keys)) {
 	QDomElement conductor_autonum = conductorAutoNum(key).toXml(xml_document, "conductor_autonum");
 		if (key != "" && conductorAutoNumFormula(key) != "") {
 			conductor_autonum.setAttribute("title", key);
@@ -1936,7 +1948,9 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 
 	//Export Folio Autonums
 	QDomElement folio_autonums = xml_document.createElement("folio_autonums");
-	foreach (QString key, folioAutoNum().keys()) {
+	QStringList folio_autonum_keys = folioAutoNum().keys();
+	folio_autonum_keys.sort();
+	for (const QString &key : std::as_const(folio_autonum_keys)) {
 	QDomElement folio_autonum = folioAutoNum(key).toXml(xml_document, "folio_autonum");
 		folio_autonum.setAttribute("title", key);
 		folio_autonums.appendChild(folio_autonum);
@@ -1947,7 +1961,9 @@ void QETProject::writeDefaultPropertiesXml(QDomElement &xml_element)
 	QDomElement element_autonums = xml_document.createElement("element_autonums");
 	element_autonums.setAttribute("current_autonum", m_current_element_autonum);
 	element_autonums.setAttribute("freeze_new_elements", m_freeze_new_elements ? "true" : "false");
-	foreach (QString key, elementAutoNum().keys()) {
+	QStringList element_autonum_keys = elementAutoNum().keys();
+	element_autonum_keys.sort();
+	for (const QString &key : std::as_const(element_autonum_keys)) {
 	QDomElement element_autonum = elementAutoNum(key).toXml(xml_document, "element_autonum");
 		if (key != "" && elementAutoNumFormula(key) != "") {
 			element_autonum.setAttribute("title", key);

sources/qetxml.cpp

@@ -17,6 +17,8 @@
 */
 #include "qetxml.h"
 
+#include <algorithm>
+
 #include "NameList/nameslist.h"
 #include "utils/qetutils.h"
 
@@ -454,7 +456,14 @@ QDomElement QETXML::modelHeaderDataToXml(
 		//Iterate twice, first for horizontal header and second to vertical header
 	while (true)
 	{
-		for (auto section : data_hash.keys())
+			//Sorted: data_hash is a QHash, whose key order is randomised per
+			//process, so writing the sections in hash order reordered these
+			//<data> children on every save and made the save irreproducible.
+			//The roles within a section keep their given order, which is a
+			//QList and therefore already stable.
+		QList<int> sections = data_hash.keys();
+		std::sort(sections.begin(), sections.end());
+		for (auto section : std::as_const(sections))
 		{
 			for (auto role : data_hash.value(section))
 			{