From a30dd214484892645f0a579de349715bfc434a8b Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Thu, 3 Jan 2019 10:31:19 -0600 Subject: [PATCH] [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 `` 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`: - `` 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. --- .../Xamarin.Android.Windows.targets | 26 ----------- .../Tasks/CopyGeneratedJavaResourceClasses.cs | 19 ++++---- .../AndroidUpdateResourcesTest.cs | 16 ++----- .../IncrementalBuildTest.cs | 45 ++++++++++++++----- .../Xamarin.Android.Build.Tasks.targets | 5 --- .../Xamarin.Android.Common.props.in | 1 + .../Xamarin.Android.Common.targets | 21 +++++++-- 7 files changed, 65 insertions(+), 68 deletions(-) delete mode 100644 src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Common/ImportAfter/Xamarin.Android.Windows.targets diff --git a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Common/ImportAfter/Xamarin.Android.Windows.targets b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Common/ImportAfter/Xamarin.Android.Windows.targets deleted file mode 100644 index 623dda9c056..00000000000 --- a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Common/ImportAfter/Xamarin.Android.Windows.targets +++ /dev/null @@ -1,26 +0,0 @@ - - - - - Xamarin - <_IsRunningXBuild Condition=" '$(MSBuildRuntimeVersion)' == '' ">true - - - - - - - - - - - diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/CopyGeneratedJavaResourceClasses.cs b/src/Xamarin.Android.Build.Tasks/Tasks/CopyGeneratedJavaResourceClasses.cs index fff273d99b9..d3c8b8fdc70 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/CopyGeneratedJavaResourceClasses.cs +++ b/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 (); 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 (); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs index 636aa72022c..c709f16cdea 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AndroidUpdateResourcesTest.cs +++ b/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 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 ().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 (); 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 ().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"); } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs index f562126e390..8cc23b15088 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/IncrementalBuildTest.cs +++ b/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 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!"); } } diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets index 53ff1d33987..f40f107f4a1 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.targets @@ -52,11 +52,6 @@ PreserveNewest Xamarin.Android.Common\ImportAfter\Microsoft.Cpp.Android.targets - - PreserveNewest - \Xamarin.Android.Common\ImportAfter\Xamarin.Android.Windows.targets - PreserveNewest diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in index 4126d86a4f8..3d5d3601231 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.props.in @@ -2,6 +2,7 @@ @PACKAGE_VERSION@-@PACKAGE_VERSION_BUILD@ <_JavaInteropReferences>Java.Interop;System.Runtime + Xamarin true false true diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 94b2c69b6e6..5674548136a 100755 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1794,7 +1794,6 @@ because xbuild doesn't support framework reference assemblies. @@ -2464,6 +2463,11 @@ because xbuild doesn't support framework reference assemblies. + + + + + @@ -2690,9 +2694,20 @@ because xbuild doesn't support framework reference assemblies. - + + <_CompileDexDependsOn> + _CompileToDalvikWithDx; + _CompileToDalvikWithD8; + + <_CompileDexBeforeTargets Condition=" '$(AndroidApplication)' == 'True' "> + IncrementalClean; + _CleanGetCurrentAndPriorFileWrites; + + + <_DexFile Include="$(IntermediateOutputPath)android\bin\dex\*.dex" /> <_DexFile Include="$(IntermediateOutputPath)android\bin\*.dex" />