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

…he IDE

Context: https://github.com/xamarin/xamarin-android/wiki/IDE-Performance-Results
Context: https://github.com/jonathanpeppers/HelloWorld

When doing some performance measurements *inside* of Visual Studio, I
noticed we seem to have significant overhead in a build with no
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 `Install` depends on `SignAndroidPackage`,
which depends on `Build`. We are doing a lot of MSBuild work here
twice, since MSBuild needs to run through certain targets twice and
decide they can be skipped. This work is "not free" and mostly
involved MSBuild evaluating properties and time stamps on files.

What I found we could do here is skip `Build` on the `Install` target
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

This is a ~2 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 `CreateApkBuilder` method
run `Build,SignAndroidPackage` 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 `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/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

@@ -3163,7 +3163,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>