Correctly order devices when using logical parents

If the device parent is added using fu_device_add_parent_guid() or ParentGuid
from a quirk file then the child is not returned in fu_device_get_children() by
design as the physical ID will be likely different.

This means we cannot reliably 'depsolve' the order in FuDevice as we need the
full list of devices that might be parents. The existing algorithm had several
logical problems when dealing with more than a single parent and child.

The FuDeviceList object is the right place to do this as it knows about all
added devices on the system.

This means the addition of a logical device causes the root logical device
(and all it's children, and grandchildren) to be re-ordered. This allows
firmware on deeply nested composite devices like hubs to be installed in the
correct order.

libfwupdplugin/fu-device.c

@@ -653,13 +653,6 @@ fu_device_set_parent (FuDevice *self, FuDevice *parent)
 
 	g_return_if_fail (FU_IS_DEVICE (self));
 
-	/* if unspecified, always child before parent */
-	if (parent != NULL &&
-	    fu_device_get_order (parent) == fu_device_get_order (self)) {
-		g_debug ("auto-setting %s order", fu_device_get_id (parent));
-		fu_device_set_order (parent, fu_device_get_order (self) + 1);
-	}
-
 	/* if the parent has quirks, make the child inherit it */
 	if (parent != NULL) {
 		if (fu_device_get_quirks (self) == NULL &&
@@ -802,15 +795,6 @@ fu_device_add_child (FuDevice *self, FuDevice *child)
 
 	/* ensure the parent is also set on the child */
 	fu_device_set_parent (child, self);
-
-	/* order devices so they are updated in the correct sequence */
-	if (fu_device_has_flag (child, FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST)) {
-		if (priv->order >= fu_device_get_order (child))
-			fu_device_set_order (child, priv->order + 1);
-	} else {
-		if (priv->order <= fu_device_get_order (child))
-			priv->order = fu_device_get_order (child) + 1;
-	}
 }
 
 /**

src/fu-device-list.c

@@ -81,6 +81,60 @@ fu_device_list_emit_device_changed (FuDeviceList *self, FuDevice *device)
 	g_signal_emit (self, signals[SIGNAL_CHANGED], 0, device);
 }
 
+/* we cannot use fu_device_get_children() as this will not find "parent-only"
+ * logical relationships added using fu_device_add_parent_guid() */
+static GPtrArray *
+fu_device_list_get_children (FuDeviceList *self, FuDevice *device)
+{
+	GPtrArray *devices = g_ptr_array_new_with_free_func ((GDestroyNotify) g_object_unref);
+	g_rw_lock_reader_lock (&self->devices_mutex);
+	for (guint i = 0; i < self->devices->len; i++) {
+		FuDeviceItem *item = g_ptr_array_index (self->devices, i);
+		if (device == fu_device_get_parent (item->device))
+			g_ptr_array_add (devices, g_object_ref (item->device));
+	}
+	g_rw_lock_reader_unlock (&self->devices_mutex);
+	return devices;
+}
+
+static void
+fu_device_list_depsolve_order_full (FuDeviceList *self, FuDevice *device, guint depth)
+{
+	g_autoptr(GPtrArray) children = NULL;
+
+	/* ourself */
+	fu_device_set_order (device, depth);
+
+	/* optional children */
+	children = fu_device_list_get_children (self, device);
+	for (guint i = 0; i < children->len; i++) {
+		FuDevice *child = g_ptr_array_index (children, i);
+		if (fu_device_has_flag (child, FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST)) {
+			fu_device_list_depsolve_order_full (self, child, depth + 1);
+		} else {
+			fu_device_list_depsolve_order_full (self, child, depth - 1);
+		}
+	}
+}
+
+/**
+ * fu_device_list_depsolve_order:
+ * @self: A #FuDeviceList
+ * @device: A #FuDevice
+ *
+ * Sets the device order using the logical parent->child relationships -- by default
+ * the child is updated first, unless the device has set flag
+ * %FWUPD_DEVICE_FLAG_INSTALL_PARENT_FIRST.
+ *
+ * Since: 1.5.0
+ **/
+void
+fu_device_list_depsolve_order (FuDeviceList *self, FuDevice *device)
+{
+	g_autoptr(FuDevice) root = fu_device_get_root (device);
+	fu_device_list_depsolve_order_full (self, root, 0);
+}
+
 /**
  * fu_device_list_get_all:
  * @self: A #FuDeviceList

src/fu-device-list.h

@@ -31,3 +31,5 @@ FuDevice	*fu_device_list_get_by_guid		(FuDeviceList	*self,
 gboolean	 fu_device_list_wait_for_replug		(FuDeviceList	*self,
 							 FuDevice	*device,
 							 GError		**error);
+void		 fu_device_list_depsolve_order		(FuDeviceList	*self,
+							 FuDevice	*device);

src/fu-engine.c

@@ -5208,6 +5208,9 @@ fu_engine_add_device (FuEngine *self, FuDevice *device)
 	/* create new device */
 	fu_device_list_add (self->device_list, device);
 
+	/* fix order */
+	fu_device_list_depsolve_order (self->device_list, device);
+
 	/* fixup the name and format as needed from cached metadata */
 	if (component != NULL)
 		fu_engine_md_refresh_device_from_component (self, device, component);

src/fu-self-test.c

@@ -1076,15 +1076,18 @@ fu_engine_device_parent_func (gconstpointer user_data)
 	/* add two together */
 	fu_engine_add_device (engine, device2);
 
+	/* this is normally done by fu_plugin_device_add() */
+	fu_engine_add_device (engine, device3);
+
 	/* verify both children were adopted */
 	g_assert (fu_device_get_parent (device3) == device2);
 	g_assert (fu_device_get_parent (device1) == device2);
 	g_assert_cmpstr (fu_device_get_vendor (device3), ==, "oem");
 
 	/* verify order */
-	g_assert_cmpint (fu_device_get_order (device1), ==, 0);
-	g_assert_cmpint (fu_device_get_order (device2), ==, 1);
-	g_assert_cmpint (fu_device_get_order (device3), ==, 0);
+	g_assert_cmpint (fu_device_get_order (device1), ==, -1);
+	g_assert_cmpint (fu_device_get_order (device2), ==, 0);
+	g_assert_cmpint (fu_device_get_order (device3), ==, -1);
 }
 
 static void