cli: wait for all wifi scans to finish before displaying the result

Otherwise devices are displayed in an inconsistent order.

Replaces debian/patches/Ignore-client-tests-failures.patch.

6 files changed, 322 insertions, 27 deletions

diff --git a/debian/patches/Ignore-client-tests-failures.patch b/debian/patches/Ignore-client-tests-failures.patch
deleted file mode 100644
index a56af7d7..00000000
--- a/debian/patches/Ignore-client-tests-failures.patch
+++ /dev/null
@@ -1,26 +0,0 @@
-From: Michael Biebl <biebl@debian.org>
-Date: Thu, 25 Oct 2018 00:09:31 +0200
-Subject: Ignore client/tests failures
-
-check-local-clients-tests-test-client appears to be flaky and prone to
-fail on slower architectures. Ignore failures of this test until this
-has been properly investigated.
-
-https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/39
----
- Makefile.am | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/Makefile.am b/Makefile.am
-index 204f1a0..9181ad1 100644
---- a/Makefile.am
-+++ b/Makefile.am
-@@ -4056,7 +4056,7 @@ check-local-clients-tests-test-client: clients/cli/nmcli clients/tests/test-clie
- 	"$(PYTHON)" \
- 	$(srcdir)/clients/tests/test-client.py -v &> "$(builddir)/clients/tests/test-client.log" && r=ok; \
- 	cat "$(builddir)/clients/tests/test-client.log"; \
--	test "$$r" == ok
-+	true
- 
- check_local += check-local-clients-tests-test-client
- 
diff --git a/debian/patches/cli-fix-memory-leaks.patch b/debian/patches/cli-fix-memory-leaks.patch
new file mode 100644
index 00000000..457a7f85
--- /dev/null
+++ b/debian/patches/cli-fix-memory-leaks.patch
@@ -0,0 +1,37 @@
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Wed, 31 Oct 2018 10:10:10 +0100
+Subject: cli: fix memory leaks
+
+(cherry picked from commit a985efaf934cedd37a69f94cd35990d7e2c43f0b)
+(cherry picked from commit e22602096fe60fd73586bf041863b63aad84bbee)
+---
+ clients/cli/devices.c | 2 +-
+ clients/cli/utils.c   | 2 ++
+ 2 files changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/clients/cli/devices.c b/clients/cli/devices.c
+index 38e24e7..b7ba50f 100644
+--- a/clients/cli/devices.c
++++ b/clients/cli/devices.c
+@@ -2795,7 +2795,7 @@ show_access_point_info (NMDeviceWifi *wifi, NmCli *nmc, NmcOutputData *out)
+ 
+ 		aps = sort_access_points (nm_device_wifi_get_access_points (wifi));
+ 		g_ptr_array_foreach (aps, fill_output_access_point, &info);
+-		g_ptr_array_free (aps, FALSE);
++		g_ptr_array_free (aps, TRUE);
+ 	}
+ 
+ 	print_data_prepare_width (out->output_data);
+diff --git a/clients/cli/utils.c b/clients/cli/utils.c
+index e21c108..feb6722 100644
+--- a/clients/cli/utils.c
++++ b/clients/cli/utils.c
+@@ -919,6 +919,8 @@ nmc_empty_output_fields (NmcOutputData *output_data)
+ 	/* Empty output_data array */
+ 	if (output_data->output_data->len > 0)
+ 		g_ptr_array_remove_range (output_data->output_data, 0, output_data->output_data->len);
++
++	g_ptr_array_unref (output_data->output_data);
+ }
+ 
+ /*****************************************************************************/
diff --git a/debian/patches/cli-tests-fix-output-on-failure.patch b/debian/patches/cli-tests-fix-output-on-failure.patch
new file mode 100644
index 00000000..4a213a92
--- /dev/null
+++ b/debian/patches/cli-tests-fix-output-on-failure.patch
@@ -0,0 +1,25 @@
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Wed, 31 Oct 2018 11:29:26 +0100
+Subject: cli/tests: fix output on failure
+
+(cherry picked from commit 5665f67bae25ee9f96e057c2a993b8cf69cbc127)
+(cherry picked from commit d1afd1c4206d3720de7776ed93ae61c48a7cc71d)
+---
+ clients/tests/test-client.py | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/clients/tests/test-client.py b/clients/tests/test-client.py
+index e8f1f7f..d92d499 100755
+--- a/clients/tests/test-client.py
++++ b/clients/tests/test-client.py
+@@ -744,8 +744,8 @@ class TestNmcli(NmTestBase):
+                     skip_test_for_l10n_diff.append(n['test_name'])
+                     continue
+                 print("\n\n\nThe file '%s' does not have the expected content:" % (filename))
+-                print("ACTUAL OUTPUT:\n[[%s]]\n" % (results_expect[i]))
+-                print("EXPECT OUTPUT:\n[[%s]]\n" % (n['content']))
++                print("ACTUAL OUTPUT:\n[[%s]]\n" % (n['content']))
++                print("EXPECT OUTPUT:\n[[%s]]\n" % (results_expect[i]))
+                 print("Let the test write the file by rerunning with NM_TEST_REGENERATE=1")
+                 print("See howto in %s for details.\n" % (PathConfiguration.canonical_script_filename()))
+                 self.fail("Unexpected output of command, expected %s. Rerun test with NM_TEST_REGENERATE=1 to regenerate files" % (filename))
diff --git a/debian/patches/cli-wait-for-all-wifi-scans-to-finish-before-displaying-t.patch b/debian/patches/cli-wait-for-all-wifi-scans-to-finish-before-displaying-t.patch
new file mode 100644
index 00000000..aaf4f28b
--- /dev/null
+++ b/debian/patches/cli-wait-for-all-wifi-scans-to-finish-before-displaying-t.patch
@@ -0,0 +1,219 @@
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Wed, 31 Oct 2018 10:00:22 +0100
+Subject: cli: wait for all wifi scans to finish before displaying the result
+
+Otherwise devices are displayed in a inconsistent order.
+
+https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/39
+(cherry picked from commit c0138cdb35b51bf268c361a2dc7f4c4f6f36ef52)
+(cherry picked from commit c1bf574ea5b191bf081dbb31eee7f709e0b654a1)
+---
+ clients/cli/devices.c | 140 ++++++++++++++++++++++++++++----------------------
+ 1 file changed, 78 insertions(+), 62 deletions(-)
+
+diff --git a/clients/cli/devices.c b/clients/cli/devices.c
+index 029ca67..38e24e7 100644
+--- a/clients/cli/devices.c
++++ b/clients/cli/devices.c
+@@ -2870,28 +2870,38 @@ wifi_print_aps (NMDeviceWifi *wifi,
+ 
+ typedef struct {
+ 	NmCli *nmc;
+-	NMDeviceWifi *wifi;
+-	const NMMetaAbstractInfo *const*tmpl;
+-
++	NMDevice **devices;
++	const NMMetaAbstractInfo *const *tmpl;
+ 	const char *bssid_user;
++	GArray *out_indices;
++} ScanInfo;
++
++typedef struct {
++	ScanInfo *scan_info;
++	NMDeviceWifi *wifi;
+ 	gulong last_scan_id;
+ 	guint  timeout_id;
+ 	GCancellable *scan_cancellable;
+-	GArray *out_indices;
+ } WifiListData;
+ 
+ static void
+ wifi_list_finish (WifiListData *data)
+ {
+-	NmCli *nmc = data->nmc;
+-
+-	wifi_print_aps (data->wifi, data->nmc, data->out_indices,
+-	                data->tmpl, data->bssid_user);
++	ScanInfo *info = data->scan_info;
++	NmCli *nmc = info->nmc;
++	guint i;
+ 
+-	if (--nmc->should_wait == 0) {
++	if (--info->nmc->should_wait == 0) {
++		for (i = 0; info->devices[i]; i++) {
++			wifi_print_aps (NM_DEVICE_WIFI (info->devices[i]),
++			                info->nmc,
++			                info->out_indices,
++			                info->tmpl,
++			                info->bssid_user);
++		}
+ 		if (nmc->return_value == NMC_RESULT_ERROR_NOT_FOUND) {
+ 			g_string_printf (nmc->return_text, _("Error: Access point with bssid '%s' not found."),
+-			                 data->bssid_user);
++			                 data->scan_info->bssid_user);
+ 		}
+ 		g_main_loop_quit (loop);
+ 	}
+@@ -2899,9 +2909,15 @@ wifi_list_finish (WifiListData *data)
+ 	g_signal_handler_disconnect (data->wifi, data->last_scan_id);
+ 	nm_clear_g_source (&data->timeout_id);
+ 	nm_clear_g_cancellable (&data->scan_cancellable);
+-	g_array_unref (data->out_indices);
+-	g_object_unref (data->wifi);
+ 	g_slice_free (WifiListData, data);
++
++	if (info->nmc->should_wait == 0) {
++		for (i = 0; info->devices[i]; i++)
++			g_object_unref (info->devices[i]);
++		g_free (info->devices);
++		g_array_unref (info->out_indices);
++		g_free (info);
++	}
+ }
+ 
+ static void
+@@ -2945,49 +2961,6 @@ wifi_list_scan_timeout (gpointer user_data)
+ 	return G_SOURCE_REMOVE;
+ }
+ 
+-static void
+-wifi_list_aps (NMDeviceWifi *wifi,
+-               NmCli *nmc,
+-               GArray *out_indices,
+-               const NMMetaAbstractInfo *const*tmpl,
+-               const char *bssid_user,
+-               gint64 rescan_cutoff)
+-{
+-	gboolean needs_rescan;
+-	WifiListData *data;
+-
+-	needs_rescan = rescan_cutoff < 0 || (rescan_cutoff > 0 && nm_device_wifi_get_last_scan (wifi) < rescan_cutoff);
+-
+-	/* FIXME: nmcli should either
+-	 *  - don't request any new scan for any device and print the full AP list right
+-	 *    away.
+-	 *  - or, when requesting a scan on one or more devices, don't print the result
+-	 *    before all requests complete.
+-	 *
+-	 *  Otherwise:
+-	 *    - the printed output is not self consistent. E.g. it will print the result
+-	 *      on one device at a certain time, while printing the result for another
+-	 *      device at a later point in time.
+-	 *    - the order in which we print the AP list per-device, is unstable. */
+-	if (needs_rescan) {
+-		data = g_slice_new0 (WifiListData);
+-		data->nmc = nmc;
+-		data->wifi = g_object_ref (wifi);
+-		data->tmpl = tmpl;
+-		data->out_indices = g_array_ref (out_indices);;
+-		data->bssid_user = bssid_user;
+-		data->last_scan_id = g_signal_connect (wifi, "notify::" NM_DEVICE_WIFI_LAST_SCAN,
+-		                                       G_CALLBACK (wifi_last_scan_updated), data);
+-		data->scan_cancellable = g_cancellable_new ();
+-		data->timeout_id = g_timeout_add_seconds (15, wifi_list_scan_timeout, data);
+-		nm_device_wifi_request_scan_async (wifi, data->scan_cancellable, wifi_list_rescan_cb, data);
+-
+-		nmc->should_wait++;
+-	} else {
+-		wifi_print_aps (wifi, nmc, out_indices, tmpl, bssid_user);
+-	}
+-}
+-
+ static void
+ complete_aps (NMDevice **devices, const char *ifname,
+               const char *bssid_prefix, const char *ssid_prefix)
+@@ -3017,12 +2990,15 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
+ 	const char *bssid_user = NULL;
+ 	const char *rescan = NULL;
+ 	gs_free NMDevice **devices = NULL;
+-	guint i;
+ 	const char *fields_str = NULL;
+ 	const NMMetaAbstractInfo *const*tmpl;
+ 	gs_unref_array GArray *out_indices = NULL;
+ 	int option;
+ 	guint64 rescan_cutoff;
++	NMDeviceWifi *wifi;
++	ScanInfo *scan_info = NULL;
++	WifiListData *data;
++	guint i, j;
+ 
+ 	devices = nmc_get_devices_sorted (nmc->client);
+ 
+@@ -3110,7 +3086,8 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
+ 		}
+ 
+ 		if (NM_IS_DEVICE_WIFI (device)) {
+-			wifi_list_aps (NM_DEVICE_WIFI (device), nmc, out_indices, tmpl, bssid_user, rescan_cutoff);
++			devices[0] = device;
++			devices[1] = NULL;
+ 		} else {
+ 			if (   nm_device_get_device_type (device) == NM_DEVICE_TYPE_GENERIC
+ 			    && g_strcmp0 (nm_device_get_type_description (device), "wifi") == 0) {
+@@ -3124,13 +3101,52 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv)
+ 			}
+ 			return NMC_RESULT_ERROR_UNKNOWN;
+ 		}
++	}
++
++	/* Filter out non-wifi devices */
++	for (i = 0, j = 0; devices[i]; i++) {
++		if (NM_IS_DEVICE_WIFI (devices[i]))
++			devices[j++] = devices[i];
++	}
++	devices[j] = NULL;
++
++	/* Start a new scan for devices that need it */
++	for (i = 0; devices[i]; i++) {
++		wifi = (NMDeviceWifi *) devices[i];
++		g_object_ref (wifi);
++
++		if (   rescan_cutoff == 0
++		    || (rescan_cutoff > 0 && nm_device_wifi_get_last_scan (wifi) >= rescan_cutoff))
++			continue;
++
++		if (!scan_info) {
++			scan_info = g_new0 (ScanInfo, 1);
++			scan_info->out_indices = g_array_ref (out_indices);
++			scan_info->tmpl = tmpl;
++			scan_info->bssid_user = bssid_user;
++			scan_info->nmc = nmc;
++		}
++
++		nmc->should_wait++;
++		data = g_slice_new0 (WifiListData);
++		data->wifi = wifi;
++		data->scan_info = scan_info;
++		data->last_scan_id = g_signal_connect (wifi, "notify::" NM_DEVICE_WIFI_LAST_SCAN,
++		                                       G_CALLBACK (wifi_last_scan_updated), data);
++		data->scan_cancellable = g_cancellable_new ();
++		data->timeout_id = g_timeout_add_seconds (15, wifi_list_scan_timeout, data);
++		nm_device_wifi_request_scan_async (wifi, data->scan_cancellable, wifi_list_rescan_cb, data);
++	}
++
++	if (scan_info) {
++		scan_info->devices = g_steal_pointer (&devices);
+ 	} else {
++		/* Print results right away if no scan is pending */
+ 		for (i = 0; devices[i]; i++) {
+-			NMDevice *dev = devices[i];
+-
+-			if (NM_IS_DEVICE_WIFI (dev)) {
+-				wifi_list_aps (NM_DEVICE_WIFI (dev), nmc, out_indices, tmpl, bssid_user, rescan_cutoff);
+-			}
++			wifi_print_aps (NM_DEVICE_WIFI (devices[i]),
++			                nmc, out_indices,
++			                tmpl, bssid_user);
++			g_object_unref (devices[i]);
+ 		}
+ 	}
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 5be1814f..a5646381 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,10 +1,13 @@
 Force-online-state-with-unmanaged-devices.patch
 Don-t-setup-Sleep-Monitor-if-not-booted-with-systemd.patch
 Don-t-make-NetworkManager-D-Bus-activatable.patch
-Ignore-client-tests-failures.patch
 sd-dhcp-lease-fix-memleaks.patch
 dhcp6-fix-buffer-size-checking.patch
 sd-dhcp6-fix-argument-and-error-handling-of-dhcp6_option_.patch
 sd-dhcp6-make-dhcp6_option_parse_domainname-not-store-emp.patch
 sd-dhcp-remove-unreachable-route-after-rebinding-return-N.patch
 dhcp6-make-sure-we-have-enough-space-for-the-DHCP6-option.patch
+cli-wait-for-all-wifi-scans-to-finish-before-displaying-t.patch
+cli-fix-memory-leaks.patch
+cli-tests-fix-output-on-failure.patch
+tests-simulate-old-LastScan-wifi-value-in-test-networkman.patch
diff --git a/debian/patches/tests-simulate-old-LastScan-wifi-value-in-test-networkman.patch b/debian/patches/tests-simulate-old-LastScan-wifi-value-in-test-networkman.patch
new file mode 100644
index 00000000..31ec7ee6
--- /dev/null
+++ b/debian/patches/tests-simulate-old-LastScan-wifi-value-in-test-networkman.patch
@@ -0,0 +1,37 @@
+From: Beniamino Galvani <bgalvani@redhat.com>
+Date: Fri, 2 Nov 2018 13:49:25 +0100
+Subject: tests: simulate old LastScan wifi value in
+ test-networkmanager-service.py
+
+In this way clients will randomly find that the AP list is older than
+30 seconds and they will issue a new scan.
+
+(cherry picked from commit 5ba301f4eb652a208a500a39d59dadf427a64590)
+(cherry picked from commit 9b0735f6fd9d69b486d54a7ed99b595c111e54f7)
+---
+ tools/test-networkmanager-service.py | 12 ++----------
+ 1 file changed, 2 insertions(+), 10 deletions(-)
+
+diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py
+index 8a37fb1..9aa44a9 100755
+--- a/tools/test-networkmanager-service.py
++++ b/tools/test-networkmanager-service.py
+@@ -916,16 +916,8 @@ class WifiDevice(Device):
+         self.aps = []
+         self.scan_cb_id = None
+ 
+-        # Note: we would like to simulate how nmcli calls RequestScan() and we could
+-        # do so by using an older timestamp. However, that makes the client tests
+-        # racy, because if a bunch of nmcli instances run in parallel against this
+-        # service, earlier instances will issue a RequestScan(), while later instances
+-        # won't do that (because the LastScan timestamp is already updated). That means,
+-        # the later instances will print the scan result immediately, and in another sort
+-        # order. That should be fixed, by nmcli not starting to print anything, before
+-        # all RequestScan() requests complete, and thus, always print a consistent list
+-        # of results.
+-        ts = NM.utils_get_timestamp_msec()
++        # Use a randomly older timestamp to trigger RequestScan() from the client
++        ts = max(0, NM.utils_get_timestamp_msec() - Util.random_int(self.path, 20000, 40000))
+ 
+         props = {
+             PRP_WIFI_HW_ADDRESS:            mac,