[Xamarin.Android.Build.Tasks] Install should skip Build when inside t…

…he IDE (#2626)

Context: http://work.devdiv.io/765447
Context: https://github.com/jonathanpeppers/HelloWorld
Context: https://github.com/xamarin/xamarin-android/wiki/IDE-Performance-Results

When doing some performance measurements *inside* of Visual Studio,
I noticed some significant overhead in a build without changes.

In a "Hello World" Xamarin.Forms app:

	Preparation Time: 00:03.9
	Launch Time:      00:02.5

Where "Preparation Time" is everything MSBuild, and "Launch Time"
is the time it takes to start the Android application and attach the
debugger.

"Preparation Time" is effectively:

	msbuild Foo.Android.csproj /p:BuildingInsideVisualStudio=True /t:Build
	msbuild Foo.Android.csproj /p:BuildingInsideVisualStudio=True /t:Install

One concern here is that the `Install` target depends on the
`SignAndroidPackage` target, which depends on the `Build` target.
We are doing a lot of MSBuild work here *twice*, since MSBuild needs
to run through certain targets twice and decide whether they can be
skipped.  This work is "not free" and mostly involves MSBuild
evaluating properties and time stamps on files.

What I found we could do here is skip the `Build` on the `Install`
targets when `$(BuildingInsideVisualStudio)` is `True`.  Due to the
dependency chain, this also affects `SignAndroidPackage`.

The minimal list of targets for `SignAndroidPackage` that still work:

  * `_CreatePropertiesCache`
  * `ResolveReferences`
  * `_CopyPackage`
  * `_Sign`

Initial results from the IDE show:

	Preparation Time: 00:02.06s
	Launch Time:      00:02.5

This is a ~1.8 second saving on the inner dev loop!

~~ MSBuild Tests ~~

Since our MSBuild tests set `$(BuildingInsideVisualStudio)`, a lot of
our tests might break.  We might have to add an additional call to
`Build` in each failing test.

The way I worked around this was to make the
`BuildHelper.CreateApkBuilder()` method run the
`Build,SignAndroidPackage` target by default.  Originally there were
248 test failures.

I also did some other cleanup:

  - Added a `ProjectBuilder.RunTarget()` method, so tests can more
    easily run custom targets and not modify the `Target` property:
    `builder.RunTarget ("Foo")`.
  - Added `ProjectBuilder.DesignTimeBuild`().
  - `Builder` now has a `BuildingInsideVisualStudio` property to
    toggle the value as a command-line global property.  We were
    putting this in the `csproj`, which seems a bit different than
    what IDEs are actually doing...

Also added a `BuildTest.BuildOutsideVisualStudio()` test to verify
the `SignAndroidPackage` target works by itself *outside* of IDEs.

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs

@@ -80,8 +80,6 @@ public void DesignTimeBuild ([Values(false, true)] bool isRelease, [Values (fals
 			var intermediateOutputPath = Path.Combine (path, proj.ProjectName, proj.IntermediateOutputPath);
 			proj.SetProperty ("AndroidUseManagedDesignTimeResourceGenerator", useManagedParser.ToString ());
 			proj.SetProperty ("AndroidUseAapt2", useAapt2.ToString ());
-			if (useManagedParser)
-				proj.SetProperty ("BuildingInsideVisualStudio", "True");
 			using (var l = CreateDllBuilder (Path.Combine (path, lib.ProjectName), false, false)) {
 				using (var b = CreateApkBuilder (Path.Combine (path, proj.ProjectName), false, false)) {
 					l.Verbosity = LoggerVerbosity.Diagnostic;
@@ -1080,10 +1078,7 @@ public void BuildAppWithManagedResourceParser()
 			};
 			appProj.SetProperty ("AndroidUseManagedDesignTimeResourceGenerator", "True");
 			using (var appBuilder = CreateApkBuilder (Path.Combine (path, appProj.ProjectName))) {
-				appBuilder.Verbosity = LoggerVerbosity.Diagnostic;
-				appBuilder.Target = "Compile";
-				Assert.IsTrue (appBuilder.Build (appProj, parameters: new string[] { "DesignTimeBuild=true", "BuildingInsideVisualStudio=true" } ),
-					"DesignTime Application Build should have succeeded.");
+				Assert.IsTrue (appBuilder.DesignTimeBuild (appProj), "DesignTime Application Build should have succeeded.");
 				Assert.IsFalse (appProj.CreateBuildOutput (appBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
 					"Target '_ManagedUpdateAndroidResgen' should have run.");
 				var designerFile = Path.Combine (Root, path, appProj.ProjectName, appProj.IntermediateOutputPath, "designtime", "Resource.designer.cs");
@@ -1095,7 +1090,6 @@ public void BuildAppWithManagedResourceParser()
 				StringAssert.Contains ("myButton", designerContents, $"{designerFile} should contain Resources.Id.myButton");
 				StringAssert.Contains ("Icon", designerContents, $"{designerFile} should contain Resources.Drawable.Icon");
 				StringAssert.Contains ("Main", designerContents, $"{designerFile} should contain Resources.Layout.Main");
-				appBuilder.Target = "SignAndroidPackage";
 				Assert.IsTrue (appBuilder.Build (appProj),
 					"Normal Application Build should have succeeded.");
 				Assert.IsTrue (appProj.CreateBuildOutput (appBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
@@ -1156,15 +1150,12 @@ public void BuildAppWithManagedResourceParserAndLibraries ()
 				libBuilder.Verbosity = LoggerVerbosity.Diagnostic;
 				libBuilder.ThrowOnBuildFailure = false;
 				using (var appBuilder = CreateApkBuilder (Path.Combine (path, appProj.ProjectName), false, false)) {
-					appBuilder.Verbosity = LoggerVerbosity.Diagnostic;
 					appBuilder.ThrowOnBuildFailure = false;
-					libBuilder.Target = "Compile";
-					Assert.IsTrue (libBuilder.Build (libProj, parameters: new string [] { "DesignTimeBuild=true", "BuildingInsideVisualStudio=true" }), "Library project should have built");
+					Assert.IsTrue (libBuilder.DesignTimeBuild (libProj), "Library project should have built");
 					Assert.LessOrEqual (libBuilder.LastBuildTime.TotalMilliseconds, maxBuildTimeMs, $"DesignTime build should be less than {maxBuildTimeMs} milliseconds.");
 					Assert.IsFalse (libProj.CreateBuildOutput (libBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
 						"Target '_ManagedUpdateAndroidResgen' should have run.");
-					appBuilder.Target = "Compile";
-					Assert.AreEqual (!appBuilder.RunningMSBuild, appBuilder.Build (appProj, parameters: new string [] { "DesignTimeBuild=true", "BuildingInsideVisualStudio=true" }), "Application project should have built");
+					Assert.AreEqual (!appBuilder.RunningMSBuild, appBuilder.DesignTimeBuild (appProj), "Application project should have built");
 					Assert.LessOrEqual (appBuilder.LastBuildTime.TotalMilliseconds, maxBuildTimeMs, $"DesignTime build should be less than {maxBuildTimeMs} milliseconds.");
 					Assert.IsFalse (appProj.CreateBuildOutput (appBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
 						"Target '_ManagedUpdateAndroidResgen' should have run.");
@@ -1180,12 +1171,10 @@ public void BuildAppWithManagedResourceParserAndLibraries ()
 					StringAssert.Contains ("material_grey_50", designerContents, $"{designerFile} should contain Resources.Color.material_grey_50");
 					StringAssert.DoesNotContain ("main_text_item_size", designerContents, $"{designerFile} should not contain Resources.Dimension.main_text_item_size");
 					StringAssert.DoesNotContain ("theme_devicedefault_background", designerContents, $"{designerFile} should not contain Resources.Color.theme_devicedefault_background");
-					libBuilder.Target = "Build";
 					Assert.IsTrue (libBuilder.Build (libProj), "Library project should have built");
 					Assert.IsTrue (libProj.CreateBuildOutput (libBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
 						"Target '_ManagedUpdateAndroidResgen' should not have run.");
-					appBuilder.Target = "Compile";
-					Assert.IsTrue (appBuilder.Build (appProj, parameters: new string [] { "DesignTimeBuild=true", "BuildingInsideVisualStudio=true" }), "App project should have built");
+					Assert.IsTrue (appBuilder.DesignTimeBuild (appProj), "App project should have built");
 					Assert.LessOrEqual (appBuilder.LastBuildTime.TotalMilliseconds, maxBuildTimeMs, $"DesignTime build should be less than {maxBuildTimeMs} milliseconds.");
 					Assert.IsFalse (appProj.CreateBuildOutput (appBuilder).IsTargetSkipped ("_ManagedUpdateAndroidResgen"),
 					"Target '_ManagedUpdateAndroidResgen' should have run.");
@@ -1202,18 +1191,13 @@ public void BuildAppWithManagedResourceParserAndLibraries ()
 					StringAssert.Contains ("theme_devicedefault_background", designerContents, $"{designerFile} should contain Resources.Color.theme_devicedefault_background");
 					StringAssert.Contains ("main_text_item_size", designerContents, $"{designerFile} should contain Resources.Dimension.main_text_item_size");
 					StringAssert.Contains ("SomeColor", designerContents, $"{designerFile} should contain Resources.Color.SomeColor");
-
-					appBuilder.Target = "SignAndroidPackage";
 					Assert.IsTrue (appBuilder.Build (appProj), "App project should have built");
 
-
 					Assert.IsTrue (appBuilder.Clean (appProj), "Clean should have succeeded");
 					Assert.IsTrue (File.Exists (designerFile), $"'{designerFile}' should not have been cleaned.");
 					designerFile = Path.Combine (Root, path, libProj.ProjectName, libProj.IntermediateOutputPath, "designtime", "Resource.designer.cs");
 					Assert.IsTrue (libBuilder.Clean (libProj), "Clean should have succeeded");
 					Assert.IsTrue (File.Exists (designerFile), $"'{designerFile}' should not have been cleaned.");
-
-
 				}
 			}
 		}
@@ -1521,12 +1505,9 @@ public void SetupDependenciesForDesigner ()
 			using (var libb = CreateDllBuilder (Path.Combine (path, lib.ProjectName)))
 			using (var appb = CreateApkBuilder (Path.Combine (path, proj.ProjectName))) {
 				libb.Save (lib);
-				appb.Target = "SetupDependenciesForDesigner";
-				Assert.IsTrue (appb.Build (proj, parameters: new [] { "DesignTimeBuild=True" }), "design-time build should have succeeded.");
-
+				Assert.IsTrue (appb.RunTarget (proj, "SetupDependenciesForDesigner", parameters: new [] { "DesignTimeBuild=True" }), "design-time build should have succeeded.");
 				//Now a full build
 				Assert.IsTrue (libb.Build (lib), "library build should have succeeded.");
-				appb.Target = "SignAndroidPackage";
 				Assert.IsTrue (appb.Build (proj), "app build should have succeeded.");
 			}
 		}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

@@ -3104,7 +3104,7 @@ public void BuildAMassiveApp()
 				},
 			};
 			//NOTE: BuildingInsideVisualStudio prevents the projects from being built as dependencies
-			app1.SetProperty ("BuildingInsideVisualStudio", "False");
+			sb.BuildingInsideVisualStudio = false;
 			app1.Imports.Add (new Import ("foo.targets") {
 				TextContent = () => @"<?xml version=""1.0"" encoding=""utf-16""?>
 <Project ToolsVersion=""4.0"" xmlns=""http://schemas.microsoft.com/developer/msbuild/2003"">
@@ -3452,6 +3452,38 @@ public void DuplicateManagedNames ()
 				}
 			}
 		}
+
+		[Test]
+		public void BuildOutsideVisualStudio ()
+		{
+			var path = Path.Combine ("temp", TestName);
+			var lib = new XamarinAndroidLibraryProject {
+				ProjectName = "Library1",
+				Sources = {
+					new BuildItem.Source ("Foo.cs") {
+						TextContent = () => "public class Foo { }",
+					}
+				},
+			};
+			var proj = new XamarinFormsAndroidApplicationProject {
+				ProjectName = "App1",
+				References = { new BuildItem ("ProjectReference", "..\\Library1\\Library1.csproj") },
+				Sources = {
+					new BuildItem.Source ("Bar.cs") {
+						TextContent = () => "public class Bar : Foo { }",
+					}
+				},
+			};
+			using (var libb = CreateDllBuilder (Path.Combine (path, lib.ProjectName)))
+			using (var appb = CreateApkBuilder (Path.Combine (path, proj.ProjectName))) {
+				libb.BuildingInsideVisualStudio =
+					appb.BuildingInsideVisualStudio = false;
+				appb.Target = "SignAndroidPackage";
+				//Save, but don't build
+				libb.Save (lib);
+				Assert.IsTrue (appb.Build (proj), "build should have succeeded.");
+			}
+		}
 	}
 }
 

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs

@@ -97,7 +97,6 @@ public void IncrementalCleanDuringClean ()
 				IsRelease = true,
 			};
 			proj.SetProperty ("AndroidUseManagedDesignTimeResourceGenerator", "True");
-			proj.SetProperty ("BuildingInsideVisualStudio", "True");
 			using (var b = CreateApkBuilder (path)) {
 				b.Target = "Compile";
 				Assert.IsTrue(b.Build (proj), "DesignTime Build should have succeeded");

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs

@@ -498,15 +498,9 @@ protected override void OnCreate (Bundle bundle)
 				if (!Directory.Exists (builder.MicrosoftNetSdkDirectory))
 					Assert.Ignore ("Microsoft.NET.Sdk not found.");
 				using (var ab = CreateApkBuilder (Path.Combine (path, app.ProjectName), cleanupOnDispose: false)) {
-					builder.RequiresMSBuild = true;
-					builder.Target = "Restore";
-					Assert.IsTrue (builder.Build (netStandardProject), "XamFormsSample Nuget packages should have been restored.");
-					builder.Target = "Build";
+					builder.RequiresMSBuild =
+						ab.RequiresMSBuild = true;
 					Assert.IsTrue (builder.Build (netStandardProject), "XamFormsSample should have built.");
-					ab.RequiresMSBuild = true;
-					ab.Target = "Restore";
-					Assert.IsTrue (ab.Build (app), "App should have built.");
-					ab.Target = "SignAndroidPackage";
 					Assert.IsTrue (ab.Build (app), "App should have built.");
 					var apk = Path.Combine (Root, ab.ProjectDirectory,
 						app.IntermediateOutputPath, "android", "bin", "UnnamedProject.UnnamedProject.apk");
@@ -523,8 +517,7 @@ protected override void OnCreate (Bundle bundle)
 					}
 					if (!HasDevices)
 						Assert.Ignore ("Skipping Installation. No devices available.");
-					ab.Target = "Install";
-					Assert.IsTrue (ab.Build (app), "App should have installed.");
+					Assert.IsTrue (ab.RunTarget (app, "Install"), "App should have installed.");
 				}
 			}
 		}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/BuildHelper.cs

@@ -14,7 +14,8 @@ public static class BuildHelper
 		public static ProjectBuilder CreateApkBuilder (string directory, bool cleanupAfterSuccessfulBuild = false, bool cleanupOnDispose = true)
 		{
 			var ret = CreateDllBuilder (directory, cleanupAfterSuccessfulBuild, cleanupOnDispose);
-			ret.Target = "SignAndroidPackage";
+			//NOTE: since $(BuildingInsideVisualStudio) is set, Build will not happen by default
+			ret.Target = "Build,SignAndroidPackage";
 			return ret;
 		}
 

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/Builder.cs

@@ -24,6 +24,10 @@ public class Builder : IDisposable
 		string buildLogFullPath;
 		public bool IsUnix { get; set; }
 		public bool RunningMSBuild { get; set; }
+		/// <summary>
+		/// This passes /p:BuildingInsideVisualStudio=True, command-line to MSBuild
+		/// </summary>
+		public bool BuildingInsideVisualStudio { get; set; } = true;
 		public LoggerVerbosity Verbosity { get; set; }
 		public IEnumerable<string> LastBuildOutput {
 			get {
@@ -341,10 +345,10 @@ protected bool BuildInternal (string projectOrSolution, string target, string []
 			var psi   = new ProcessStartInfo (XABuildExe);
 			args.AppendFormat ("{0} /t:{1} /restore {2}",
 				QuoteFileName(Path.Combine (XABuildPaths.TestOutputDirectory, projectOrSolution)), target, logger);
-			if (RunningMSBuild)
+			args.Append ($" /p:BuildingInsideVisualStudio={BuildingInsideVisualStudio}");
+			if (BuildingInsideVisualStudio && RunningMSBuild) {
 				args.Append (" /p:BuildingOutOfProcess=true");
-			else
-				args.Append (" /p:UseHostCompilerIfAvailable=false /p:BuildingInsideVisualStudio=true");
+			}
 			if (!string.IsNullOrEmpty (AndroidSdkDirectory)) {
 				args.AppendFormat (" /p:AndroidSdkDirectory=\"{0}\" ", AndroidSdkDirectory);
 			}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetXamarinProject.cs

@@ -30,7 +30,6 @@ protected DotNetXamarinProject (string debugConfigurationName = "Debug", string
 			SetProperty ("ConsolePause", "false");
 			SetProperty ("RootNamespace", () => RootNamespace ?? ProjectName);
 			SetProperty ("AssemblyName", () => AssemblyName ?? ProjectName);
-			SetProperty ("BuildingInsideVisualStudio", "True");
 			SetProperty ("BaseIntermediateOutputPath", "obj\\", " '$(BaseIntermediateOutputPath)' == '' ");
 
 			SetProperty (DebugProperties, "DebugSymbols", "true");

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/ProjectBuilder.cs

@@ -76,35 +76,31 @@ public bool Build (XamarinProject project, bool doNotCleanupOnUpdate = false, st
 
 		public bool Restore (XamarinProject project, bool doNotCleanupOnUpdate = false)
 		{
-			var oldTarget = Target;
-			Target = "Restore";
-			try {
-				return Build (project, doNotCleanupOnUpdate);
-			} finally {
-				Target = oldTarget;
-			}
+			return RunTarget (project, "Restore", doNotCleanupOnUpdate);
 		}
 
 		public bool Clean (XamarinProject project, bool doNotCleanupOnUpdate = false)
 		{
-			var oldTarget = Target;
-			Target = "Clean";
-			try {
-				return Build (project, doNotCleanupOnUpdate);
-			}
-			finally {
-				Target = oldTarget;
-			}
+			return RunTarget (project, "Clean", doNotCleanupOnUpdate);
 		}
 
 		public bool UpdateAndroidResources (XamarinProject project, bool doNotCleanupOnUpdate = false, string [] parameters = null, Dictionary<string, string> environmentVariables = null)
+		{
+			return RunTarget (project, "UpdateAndroidResources", doNotCleanupOnUpdate, parameters, environmentVariables);
+		}
+
+		public bool DesignTimeBuild (XamarinProject project, bool doNotCleanupOnUpdate = false)
+		{
+			return RunTarget (project, "Compile", doNotCleanupOnUpdate, parameters: new string [] { "DesignTimeBuild=True" });
+		}
+
+		public bool RunTarget (XamarinProject project, string target, bool doNotCleanupOnUpdate = false, string [] parameters = null, Dictionary<string, string> environmentVariables = null)
 		{
 			var oldTarget = Target;
-			Target = "UpdateAndroidResources";
+			Target = target;
 			try {
 				return Build (project, doNotCleanupOnUpdate: doNotCleanupOnUpdate, parameters: parameters, environmentVariables: environmentVariables);
-			}
-			finally {
+			} finally {
 				Target = oldTarget;
 			}
 		}

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -3169,7 +3169,21 @@ because xbuild doesn't support framework reference assemblies.
 	<Delete Files="%(ApkAbiFilesUnaligned.FullPath)" />
 </Target>
 
-<Target Name="SignAndroidPackage" DependsOnTargets="Build;Package;_Sign">
+<PropertyGroup>
+  <SignAndroidPackageDependsOn Condition=" '$(BuildingInsideVisualStudio)' != 'True' ">
+    Build;
+    Package;
+    _Sign;
+  </SignAndroidPackageDependsOn>
+  <!-- When inside an IDE, Build has just been run. This is a minimal list of targets for SignAndroidPackage. -->
+  <SignAndroidPackageDependsOn Condition=" '$(BuildingInsideVisualStudio)' == 'True' ">
+    _CreatePropertiesCache;
+    ResolveReferences;
+    _CopyPackage;
+    _Sign;
+  </SignAndroidPackageDependsOn>
+</PropertyGroup>
+<Target Name="SignAndroidPackage" DependsOnTargets="$(SignAndroidPackageDependsOn)">
 </Target>
 
 <PropertyGroup>

tests/CodeBehind/UnitTests/BuildTests.cs

@@ -16,6 +16,11 @@ namespace CodeBehindUnitTests
 {
 	sealed class LocalBuilder : Builder
 	{
+		public LocalBuilder ()
+		{
+			BuildingInsideVisualStudio = false;
+		}
+
 		public bool Build (string projectOrSolution, string target, string[] parameters = null, Dictionary<string, string> environmentVariables = null)
 		{
 			return BuildInternal (projectOrSolution, target, parameters, environmentVariables);
@@ -533,6 +538,7 @@ void RunTest (string testName, bool many, bool dtb, Action<TestProjectInfo, bool
 		{
 			string temporaryProjectDir = PrepareProject (testName);
 			LocalBuilder builder = GetBuilder ($"{ProjectName}.{testName}");
+			builder.BuildingInsideVisualStudio = dtb;
 			var testInfo = new TestProjectInfo (ProjectName, testName, temporaryProjectDir, TestOutputDir);
 
 			try {

tests/CodeGen-MkBundle/Xamarin.Android.MakeBundle-UnitTests/BuildTests.cs

@@ -20,6 +20,11 @@ namespace Xamarin.Android.MakeBundle.UnitTests
 {
 	sealed class LocalBuilder : Builder
 	{
+		public LocalBuilder ()
+		{
+			BuildingInsideVisualStudio = false;
+		}
+
 		public bool Build (string projectOrSolution, string target, string[] parameters = null, Dictionary<string, string> environmentVariables = null)
 		{
 			return BuildInternal (projectOrSolution, target, parameters, environmentVariables);

tests/EmbeddedDSOs/EmbeddedDSO-UnitTests/BuildTests.cs

@@ -20,6 +20,11 @@ namespace EmbeddedDSOUnitTests
 {
 	sealed class LocalBuilder : Builder
 	{
+		public LocalBuilder ()
+		{
+			BuildingInsideVisualStudio = false;
+		}
+
 		public bool Build (string projectOrSolution, string target, string[] parameters = null, Dictionary<string, string> environmentVariables = null)
 		{
 			return BuildInternal (projectOrSolution, target, parameters, environmentVariables);