[Xamarin.Android.Build.Tasks] remove Xamarin.Android.Windows.targets (#…

…2561)

* [Xamarin.Android.Build.Tasks] remove Xamarin.Android.Windows.targets

Fixes: http://work.devdiv.io/707557

In 539954c, I was aiming to remove the
`_RegisterAndroidFilesWithFileWrites` MSBuild target (runs only on
Windows). I was having trouble getting everything to work, so I undid
this change to make the PR smaller.

Reasons to remove `_RegisterAndroidFilesWithFileWrites`:

- It looks like it *may* be a source of incremental build bugs. It
  causes `$(CleanFile)` to grow on incremental builds indefinitely...
- We should instead use the `FileWrites` item group appropriately.
- We can avoid ~81ms of build time.

    81 ms  _RegisterAndroidFilesWithFileWrites        1 calls

Revisiting it now, it looks like we can remove
`Xamarin.Android.Windows.targets` completely!

- `_RegisterAndroidFilesWithFileWrites` is the only target.
- The `$(_IsRunningXBuild)` property is only used in this file.
- I moved the `$(Debugger)` property to
  `Xamarin.Android.Common.props`. I believe this value is only used in
  VS Windows, but it make more sense to be here.

~~ CopyGeneratedJavaResourceClasses ~~

The `<CopyGeneratedJavaResourceClasses/>` MSBuild task was copying an
`R.java` file from `%TEMP%` to:

    $(IntermediateOutputPath)android\unnamedproject\unnamedproject\R.java

We only use this file in one place, since the rest of the build uses:

    $(IntermediateOutputPath)android\src\unnamedproject\unnamedproject\R.java

We can avoid copying this extra file and adding it to `FileWrites`:

- `<CopyGeneratedJavaResourceClasses/>` now optionally uses the
  `DestinationTopDirectory` property
- If `DestinationTopDirectory` is blank, the original source file is
  returned as the `PrimaryJavaResgenFile` output property.

We now use the `R.java` from `%TEMP%` directly, and avoid copying a
file to `$(IntermediateOutputPath)`.

~~ Other Changes ~~

I modified the `CheckNothingIsDeletedByIncrementalClean` test to
verify that `$(CleanFile)` isn't growing on incremental builds.

Anything added to `FileWrites` during the `_CompileDex` MSBuild target
(or its dependent targets) wasn't showing up in `$(CleanFile)`! I
adjusted its `BeforeTargets` to match what I did in 539954c for
`_PrepareAssemblies`.

Two files needed to be added to `FileWrites`:

- `$(_PackagedResources)`
- `$(_GeneratedPrimaryJavaResgenFile)`

* [tests] AndroidUpdateResourcesTest.DesignTimeBuild changes

`obj/Debug/android/**/R.java` no longer exists after a design-time build.

We can still look for `obj/Debug/android/src/**/R.java` after a full build.

src/Xamarin.Android.Build.Tasks/Tasks/CopyGeneratedJavaResourceClasses.cs

@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using System.Xml.Linq;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 
@@ -12,7 +11,6 @@ public class CopyGeneratedJavaResourceClasses : Task
 	{
 		[Required]
 		public string SourceTopDirectory { get; set; }
-		[Required]
 		public string DestinationTopDirectory { get; set; }
 		[Required]
 		public string PrimaryPackageName { get; set; }
@@ -23,23 +21,22 @@ public class CopyGeneratedJavaResourceClasses : Task
 
 		public override bool Execute ()
 		{
-			Log.LogDebugMessage ("SourceTopDirectory: {0}", SourceTopDirectory);
-			Log.LogDebugMessage ("DestinationTopDirectory: {0}", DestinationTopDirectory);
-			Log.LogDebugMessage ("PrimaryPackageName: {0}", PrimaryPackageName);
-			Log.LogDebugMessage ("ExtraPackages: {0}", ExtraPackages);
-
 			var list = new List<string> ();
 			foreach (var pkg in GetPackages ()) {
 				string subpath = Path.Combine (pkg.Split ('.'));
 				string src = Path.Combine (SourceTopDirectory, subpath, "R.java");
-				string dst = Path.Combine (DestinationTopDirectory, subpath, "R.java");
 
 				if (!File.Exists (src))
 					continue;
 
-				var date = File.GetLastWriteTimeUtc (src);
-				MonoAndroidHelper.CopyIfChanged (src, dst);
-				list.Add (dst);
+				//NOTE: DestinationTopDirectory is optional, and we can just use the file in SourceTopDirectory
+				if (!string.IsNullOrEmpty (DestinationTopDirectory)) {
+					string dst = Path.Combine (DestinationTopDirectory, subpath, "R.java");
+					MonoAndroidHelper.CopyIfChanged (src, dst);
+					list.Add (dst);
+				} else {
+					list.Add (src);
+				}
 			}
 			// so far we only need the package's R.java for GenerateResourceDesigner input.
 			PrimaryJavaResgenFile = list.FirstOrDefault ();

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

@@ -92,16 +92,6 @@ public void DesignTimeBuild ([Values(false, true)] bool isRelease, [Values (fals
 					Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, parameters: new string [] { "DesignTimeBuild=true" }, environmentVariables: envVar),
 						"first build failed");
 					Assert.IsTrue (b.Output.IsTargetSkipped ("_BuildAdditionalResourcesCache"), "Target `_BuildAdditionalResourcesCache` should be skipped!");
-					var items = new List<string> ();
-					string first = null;
-					if (!useManagedParser) {
-						foreach (var file in Directory.EnumerateFiles (Path.Combine (intermediateOutputPath, "android"), "R.java", SearchOption.AllDirectories)) {
-							var matches = regEx.Matches (File.ReadAllText (file));
-							items.AddRange (matches.Cast<System.Text.RegularExpressions.Match> ().Select (x => x.Groups ["value"].Value));
-						}
-						first = items.First ();
-						Assert.IsTrue (items.All (x => x == first), "All Items should have matching values");
-					}
 					var designTimeDesigner = Path.Combine (intermediateOutputPath, "designtime", "Resource.designer.cs");
 					if (useManagedParser) {
 						FileAssert.Exists (designTimeDesigner, $"{designTimeDesigner} should have been created.");
@@ -117,13 +107,13 @@ public void DesignTimeBuild ([Values(false, true)] bool isRelease, [Values (fals
 					if (useManagedParser) {
 						FileAssert.Exists (designTimeDesigner, $"{designTimeDesigner} should not have been deleted.");
 					}
-					items.Clear ();
+					var items = new List<string> ();
 					if (!useManagedParser) {
-						foreach (var file in Directory.EnumerateFiles (Path.Combine (intermediateOutputPath, "android"), "R.java", SearchOption.AllDirectories)) {
+						foreach (var file in Directory.EnumerateFiles (Path.Combine (intermediateOutputPath, "android", "src"), "R.java", SearchOption.AllDirectories)) {
 							var matches = regEx.Matches (File.ReadAllText (file));
 							items.AddRange (matches.Cast<System.Text.RegularExpressions.Match> ().Select (x => x.Groups ["value"].Value));
 						}
-						first = items.First ();
+						var first = items.First ();
 						Assert.IsTrue (items.All (x => x == first), "All Items should have matching values");
 					}
 				}

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

@@ -1,12 +1,11 @@
-using System;
-using System.Collections.Generic;
+using Microsoft.Build.Framework;
 using NUnit.Framework;
-using Xamarin.ProjectTools;
+using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using Microsoft.Build.Framework;
 using System.Text;
-using System.Xml.Linq;
+using Xamarin.ProjectTools;
 
 namespace Xamarin.Android.Build.Tests
 {
@@ -16,9 +15,6 @@ public class IncrementalBuildTest : BaseTest
 		[Test]
 		public void CheckNothingIsDeletedByIncrementalClean ([Values (true, false)] bool enableMultiDex, [Values (true, false)] bool useAapt2)
 		{
-			// do a release build
-			// change one of the properties (say AotAssemblies) 
-			// do another build. it should NOT hose the resource directory.
 			var path = Path.Combine ("temp", TestName);
 			var proj = new XamarinFormsAndroidApplicationProject () {
 				ProjectName = "App1",
@@ -29,12 +25,41 @@ public void CheckNothingIsDeletedByIncrementalClean ([Values (true, false)] bool
 			if (useAapt2)
 				proj.SetProperty ("AndroidUseAapt2", "True");
 			using (var b = CreateApkBuilder (path)) {
+				//To be sure we are at a clean state
+				var projectDir = Path.Combine (Root, b.ProjectDirectory);
+				if (Directory.Exists (projectDir))
+					Directory.Delete (projectDir, true);
+
 				Assert.IsTrue (b.Build (proj), "First should have succeeded" );
-				IEnumerable<string> files = Directory.EnumerateFiles (Path.Combine (Root, path, proj.IntermediateOutputPath), "*.*", SearchOption.AllDirectories);
-				Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, parameters: null, saveProject: false), "Second should have succeeded");
+				var intermediate = Path.Combine (projectDir, proj.IntermediateOutputPath);
+				var output = Path.Combine (projectDir, proj.OutputPath);
+				var fileWrites = Path.Combine (intermediate, $"{proj.ProjectName}.csproj.FileListAbsolute.txt");
+				FileAssert.Exists (fileWrites);
+				var expected = File.ReadAllText (fileWrites);
+				var files = Directory.EnumerateFiles (intermediate, "*", SearchOption.AllDirectories).ToList ();
+				files.AddRange (Directory.EnumerateFiles (output, "*", SearchOption.AllDirectories));
+
+				//Touch a few files, do an incremental build
+				var filesToTouch = new [] {
+ 					Path.Combine (intermediate, "build.props"),
+ 					Path.Combine (intermediate, $"{proj.ProjectName}.pdb"),
+ 				};
+				foreach (var file in filesToTouch) {
+					FileAssert.Exists (file);
+					File.SetLastWriteTimeUtc (file, DateTime.UtcNow);
+					File.SetLastAccessTimeUtc (file, DateTime.UtcNow);
+				}
+				Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Second should have succeeded");
+
+				//No changes
+				Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Third should have succeeded");
+				Assert.IsFalse (b.Output.IsTargetSkipped ("IncrementalClean"), "`IncrementalClean` should have run!");
 				foreach (var file in files) {
 					FileAssert.Exists (file, $"{file} should not have been deleted!" );
 				}
+				FileAssert.Exists (fileWrites);
+				var actual = File.ReadAllText (fileWrites);
+				Assert.AreEqual (expected, actual, $"`{fileWrites}` has changes!");
 			}
 		}
 

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

@@ -52,11 +52,6 @@
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
       <Link>Xamarin.Android.Common\ImportAfter\Microsoft.Cpp.Android.targets</Link>
     </None>
-    <None
-        Include="MSBuild\Xamarin\Android\Xamarin.Android.Common\ImportAfter\Xamarin.Android.Windows.targets">
-      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
-      <Link>\Xamarin.Android.Common\ImportAfter\Xamarin.Android.Windows.targets</Link>
-    </None>
     <None
         Include="MSBuild\Xamarin\Android\Xamarin.Android.Common.After.targets">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in

@@ -2,6 +2,7 @@
 	<PropertyGroup>
 		<XamarinAndroidVersion>@PACKAGE_VERSION@-@PACKAGE_VERSION_BUILD@</XamarinAndroidVersion>
 		<_JavaInteropReferences>Java.Interop;System.Runtime</_JavaInteropReferences>
+		<Debugger Condition=" '$(Debugger)' == '' ">Xamarin</Debugger>
 		<DependsOnSystemRuntime Condition=" '$(DependsOnSystemRuntime)' == '' ">true</DependsOnSystemRuntime>
 		<ImplicitlyExpandNETStandardFacades>false</ImplicitlyExpandNETStandardFacades>
 		<CopyNuGetImplementations Condition=" '$(CopyNuGetImplementations)' == ''">true</CopyNuGetImplementations>

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

@@ -1794,7 +1794,6 @@ because xbuild doesn't support framework reference assemblies.
 
 	<CopyGeneratedJavaResourceClasses
 		SourceTopDirectory="$(ResgenTemporaryDirectory)"
-		DestinationTopDirectory="$(IntermediateOutputPath)android"
 		PrimaryPackageName="$(_AndroidPackage)"
 		ExtraPackages="$(AaptExtraPackages)">
 		<Output TaskParameter="PrimaryJavaResgenFile" PropertyName="_GeneratedPrimaryJavaResgenFile" />
@@ -2464,6 +2463,11 @@ because xbuild doesn't support framework reference assemblies.
 
   <!-- Delete our temporary directory -->
   <RemoveDirFixed Directories="$(AaptTemporaryDirectory)" />
+
+  <ItemGroup>
+    <FileWrites Include="$(_PackagedResources)" />
+    <FileWrites Include="$(_GeneratedPrimaryJavaResgenFile)" />
+  </ItemGroup>
 </Target>
 
 <Target Name="_FindJavaStubFiles" DependsOnTargets="_GetAddOnPlatformLibraries">
@@ -2690,9 +2694,20 @@ because xbuild doesn't support framework reference assemblies.
 
 </Target>
 
-<Target Name="_CompileDex"
-		DependsOnTargets="_CompileToDalvikWithDx;_CompileToDalvikWithD8">
+<PropertyGroup>
+	<_CompileDexDependsOn>
+		_CompileToDalvikWithDx;
+		_CompileToDalvikWithD8;
+	</_CompileDexDependsOn>
+	<_CompileDexBeforeTargets Condition=" '$(AndroidApplication)' == 'True' ">
+		IncrementalClean;
+		_CleanGetCurrentAndPriorFileWrites;
+	</_CompileDexBeforeTargets>
+</PropertyGroup>
 
+<Target Name="_CompileDex"
+		DependsOnTargets="$(_CompileDexDependsOn)"
+		BeforeTargets="$(_CompileDexBeforeTargets)">
 	<ItemGroup>
 		<_DexFile Include="$(IntermediateOutputPath)android\bin\dex\*.dex" />
 		<_DexFile Include="$(IntermediateOutputPath)android\bin\*.dex" />