From 9422b22207cd067a51cf369840acf550d64ff8a4 Mon Sep 17 00:00:00 2001
From: Jonathan Peppers <jonathan.peppers@microsoft.com>
Date: Tue, 27 Aug 2019 22:42:55 -0500
Subject: [PATCH] [Xamarin.Android.Build.Tasks] improve incremental deploy in
 IDEs

When doing some testing in the IDE, I noticed something odd for an
incremental deploy:

    Building target "_BuildApkFastDev" completely.
    Input file "obj\Debug\90\adb.props" is newer than output file "obj\Debug\90\android\bin\com.companyname.app14.apk".

This target took 2.470s in this case.

This was the Android App template in VS 2019 (not Xamarin.Forms).

I just modified a `.cs` file, how did `adb.props` change???

It turns out during `Build`, `adb.props` is blank and during `Install`
it is properly set. That means that `Install` is currently always
running `_BuildApkFastDev` on incremental deployments.

I let the IDE team know, so they can see about sending `$(AdbTarget)`
during `Build`. However, they can't do this in every case, due to the
way the UI is setup. Depending on the user's selection, current
document, focus, etc. the play button in VS may not be enabled or even
*know* the current attached device.

This leads me to think we should really change something in our
MSBuild targets, which currently does:

    <WriteLinesToFile
        Condition=" '$(DesignTimeBuild)' != 'True' "
        File="$(_AdbPropertiesCache)"
        Lines="AdbTarget=$(AdbTarget);AdbOptions=$(AdbOptions)"
        Overwrite="true"
        WriteOnlyWhenDifferent="true"
    />

If we didn't write this file when `$(AdbTarget)` is blank, the problem
would be avoided. However, we also should write the file if it does
not exist. If an `Input` to a target doesn't exist, it will run every
time. `adb.props` is used as `Inputs` for some targets.

I added a test for this scenario in the `MSBuildDeviceIntegration`
tests.
---
 .../Utilities/DeviceTest.cs                   | 28 +++++++++++++++++--
 .../Xamarin.Android.Common.targets            |  2 +-
 .../Tests/InstallTests.cs                     | 25 ++++++++++++++++-
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs
index b119272d36f..4b0306e3f5a 100644
--- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs
+++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/DeviceTest.cs
@@ -140,5 +140,29 @@ protected static void ClickButton (string packageName, string buttonName, string
 			var bounds = GetControlBounds (packageName, buttonName, buttonText);
 			RunAdbInput ("input tap", bounds.x + ((bounds.w - bounds.x) / 2), bounds.y + ((bounds.h - bounds.y) / 2));
 		}
-    }
-}
\ No newline at end of file
+
+		/// <summary>
+		/// Returns the first device listed via `adb devices`
+		/// 
+		/// Output is:
+		/// > adb devices
+		/// List of devices attached
+		/// 89RY0AEFA device
+		/// </summary>
+		/// <returns></returns>
+		protected static string GetAttachedDeviceSerial ()
+		{
+			var text = RunAdbCommand ("devices");
+			var lines = text.Split ('\n');
+			if (lines.Length < 2) {
+				Assert.Fail ($"Unexpected `adb devices` output: {text}");
+			}
+			var serial = lines [1];
+			var index = serial.IndexOf ('\t');
+			if (index != -1) {
+				serial = serial.Substring (0, index);
+			}
+			return serial.Trim ();
+		}
+	}
+}
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
index 9498eab2d79..daefc61640b 100755
--- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
+++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -1262,7 +1262,7 @@ because xbuild doesn't support framework reference assemblies.
 			WriteOnlyWhenDifferent="true"
 	/>
 	<WriteLinesToFile
-			Condition=" '$(DesignTimeBuild)' != 'True' "
+			Condition=" '$(DesignTimeBuild)' != 'True' And ('$(AdbTarget)' != '' Or !Exists('$(_AdbPropertiesCache)')) "
 			File="$(_AdbPropertiesCache)"
 			Lines="AdbTarget=$(AdbTarget);AdbOptions=$(AdbOptions)"
 			Overwrite="true"
diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs
index f4299d421fa..e7b51e914c4 100644
--- a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs
+++ b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs
@@ -12,7 +12,7 @@ namespace Xamarin.Android.Build.Tests
 	[TestFixture]
 	[NonParallelizable] //These tests deploy to devices
 	[Category ("Commercial")]
-	public class InstallTests : BaseTest
+	public class InstallTests : DeviceTest
 	{
 		[Test]
 		public void ReInstallIfUserUninstalled ([Values (false, true)] bool isRelease)
@@ -298,5 +298,28 @@ public void ToggleFastDev ()
 				Assert.IsTrue (builder.Install (proj), "Third install should have succeeded.");
 			}
 		}
+
+		[Test]
+		public void BlankAdbTarget ()
+		{
+			if (!CommercialBuildAvailable) {
+				Assert.Ignore ("Not required on Open Source Builds");
+			}
+			if (!HasDevices) {
+				Assert.Ignore ("Test Skipped no devices or emulators found.");
+			}
+
+			var serial = GetAttachedDeviceSerial ();
+			var proj = new XamarinAndroidApplicationProject ();
+			proj.SetProperty (proj.DebugProperties, "AndroidUseSharedRuntime", true);
+			proj.SetProperty (proj.DebugProperties, "EmbedAssembliesIntoApk", false);
+
+			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
+				b.Build (proj, parameters: new [] { $"AdbTarget=\"-e {serial}\"" });
+				// Build again, no $(AdbTarget)
+				b.Build (proj);
+				Assert.IsTrue (b.Output.IsTargetSkipped ("_BuildApkFastDev"), "_BuildApkFastDev should be skipped!");
+			}
+		}
 	}
 }