From dd1f25bb009d9812c332e8db8486cce2c58d74ed Mon Sep 17 00:00:00 2001 From: Dean Ellis Date: Wed, 7 Feb 2024 22:05:07 +0000 Subject: [PATCH 1/3] Add test for Binding Projects Fixes https://github.com/xamarin/xamarin-android/issues/8658 Fixes https://github.com/xamarin/xamarin-android/issues/8698 The PR fixes an issue where the bindings files in intermediate `generated` folder get deleted on subsequent builds. This causes the entire binding process to run again, slowing down builds. The root fo the problem is the `_ClearGeneratedManagedBindings` target. It was designed to clean out the `generated` folder in the case where no binding libraries were present. However it turns out if was running during a design time build! During design time builds the binding library item groups are not evaluated, so the `_ClearGeneratedManagedBindings` would run.. deleting everything. To fix this issue we only run the `_ClearGeneratedManagedBindings` in a standard build. The good thing is this allowed us time to take a look at our incremental build process for bindings. The binding generator does produce a `.projitems` file which lists all the files it generated. We can use that data to incrementally clean up the `generated` folder of old files. This way code that is no longer needed can be removed. While we know the linker will eventually remove unused classes and types, it is better to not have to compile this stuff in the first place. --- .../Xamarin.Android.Bindings.Core.targets | 35 ++++++++-- .../Android/Xamarin.Android.Javac.targets | 24 ++++++- .../Microsoft.Android.Sdk.BuildOrder.targets | 10 ++- .../Tasks/Generator.cs | 21 +++++- .../BindingBuildTest.cs | 24 ++++++- .../Xamarin.Android.Build.Tests/BuildTest.cs | 8 +-- .../Xamarin.Android.Build.Tests/BuildTest2.cs | 8 +-- .../Common/BuildOutput.cs | 6 +- .../Common/DotNetXamarinProject.cs | 3 + .../Tests/InstallTests.cs | 68 +++++++++++++++++++ 10 files changed, 182 insertions(+), 25 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets index 37a058a8ded..25ea0d166e8 100644 --- a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets +++ b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets @@ -44,23 +44,31 @@ It is shared between "legacy" binding projects and .NET 5 projects. /> + + + <_LibrariesToBind Include="@(EmbeddedJar)" /> + <_LibrariesToBind Include="@(InputJar)" /> + <_LibrariesToBind Include="@(LibraryProjectZip)" /> + <_LibrariesToBind Include="@(_JavaBindingSource)" Condition=" '%(_JavaBindingSource.Bind)' == 'true' "/> + + + + Condition=" '@(_LibrariesToBind->Count())' != '0' "> <_AndroidGenerateManagedBindings>true - + <_GeneratedManagedBindingFiles Include="$(GeneratedOutputPath)**\*.cs" /> @@ -75,7 +83,6 @@ It is shared between "legacy" binding projects and .NET 5 projects. @@ -128,21 +135,35 @@ It is shared between "legacy" binding projects and .NET 5 projects. Nullable="$(Nullable)" UseJavaLegacyResolver="$(_AndroidUseJavaLegacyResolver)" NamespaceTransforms="@(AndroidNamespaceReplacement)" - /> + GeneratedFileListFile="$(GeneratedOutputPath)src\$(AssemblyName).projitems" + > + + + + <_ExistingGeneratedFiles Include="$(GeneratedOutputPath)**\*.cs" /> + <_ExistingGeneratedFiles Remove="@(_GeneratedBindingFiles)" /> + + + + + + + + + > true diff --git a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Javac.targets b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Javac.targets index b932c578336..4c2a8ed5173 100644 --- a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Javac.targets +++ b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Javac.targets @@ -21,7 +21,9 @@ It is shared between "legacy" binding projects and .NET 7+ projects. <_AndroidIntermediateBindingClassesZip>$(IntermediateOutputPath)binding\bin\$(MSBuildProjectName).jar <_AndroidIntermediateBindingClassesDocs>$(IntermediateOutputPath)binding\bin\$(MSBuildProjectName)-docs.xml <_AndroidCompileJavaStampFile>$(_AndroidStampDirectory)_CompileJava.stamp + <_AndroidCompileJavaFileList>$(IntermediateOutputPath)_CompileJava.FileList.txt <_AndroidCompileBindingJavaStampFile>$(_AndroidStampDirectory)_CompileBindingJava.stamp + <_AndroidCompileBindingJavaFileList>$(IntermediateOutputPath)_CompileBindingJava.FileList.txt @@ -74,18 +76,36 @@ It is shared between "legacy" binding projects and .NET 7+ projects. <_JavaBindingSource Include="@(AndroidJavaSource)" Condition=" '%(AndroidJavaSource.Bind)' == 'True' " /> + + + + <_JavaSource Include="@(AndroidJavaSource)" Condition=" '%(AndroidJavaSource.Bind)' != 'True' " /> + + + + @@ -132,7 +152,7 @@ It is shared between "legacy" binding projects and .NET 7+ projects. diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets index 31c9442e8e9..df4dab646d8 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.BuildOrder.targets @@ -120,9 +120,6 @@ properties that determine build ordering. UpdateAndroidResources; _BuildResourceDesigner; UpdateAndroidInterfaceProxies; - _SetAndroidGenerateManagedBindings; - _ClearGeneratedManagedBindings; - AddBindingsToCompile; _CheckForInvalidDesignerConfig; @@ -144,6 +141,13 @@ properties that determine build ordering. _AddAndroidDefines; _IncludeLayoutBindingSources; AddLibraryJarsToBind; + _CollectLibrariesToBind; + _SetAndroidGenerateManagedBindings; + ExportJarToXml; + GenerateBindings; + _CollectGeneratedManagedBindingFiles; + _ClearGeneratedManagedBindings; + AddBindingsToCompile; _BuildResourceDesigner; _AddResourceDesignerFiles; $(CompileDependsOn); diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs index bb76b585a1d..a3ee0e21175 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs @@ -6,7 +6,9 @@ using System.Linq; using System.Xml; using System.Xml.Linq; +using System.Xml.XPath; using Microsoft.Build.Framework; +using Microsoft.Build.Utilities; using Microsoft.Android.Build.Tasks; namespace Xamarin.Android.Tasks @@ -67,6 +69,10 @@ public class BindingsGenerator : AndroidDotnetToolTask public bool UseJavaLegacyResolver { get; set; } + public string GeneratedFileListFile { get; set; } + [Output] + public ITaskItem [] GeneratedFiles { get; set; } = Array.Empty (); + private List> transform_files = new List> (); public override bool RunTask () @@ -133,7 +139,20 @@ public override bool RunTask () if (Log.HasLoggedErrors) return false; - return base.RunTask (); + var result = base.RunTask (); + List files = new List (); + if (result && GeneratedFileListFile != null && File.Exists (GeneratedFileListFile)) { + var doc = XDocument.Load (GeneratedFileListFile); + var compileItems = doc.XPathSelectElements ("//Project/ItemGroup/Compile"); + foreach (var item in compileItems) { + var file = item.Attribute ("Include"); + if (file != null && File.Exists (file.Value)) { + files.Add (new TaskItem (file.Value)); + } + } + } + GeneratedFiles = files.ToArray (); + return result; } void WriteLine (StreamWriter sw, string line) diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BindingBuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BindingBuildTest.cs index 58d8012352a..81acffd0fb1 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BindingBuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BindingBuildTest.cs @@ -39,7 +39,7 @@ public void DotNetBuildBinding () proj.OtherBuildItems.Add (new BuildItem ("JavaSourceJar", "javaclasses-sources.jar") { BinaryContent = () => ResourceData.JavaSourceJarTestSourcesJar, }); - proj.OtherBuildItems.Add (new AndroidItem.AndroidJavaSource ("JavaSourceTestExtension.java") { + proj.AndroidJavaSources.Add (new AndroidItem.AndroidJavaSource ("JavaSourceTestExtension.java") { Encoding = Encoding.ASCII, TextContent = () => ResourceData.JavaSourceTestExtension, Metadata = { { "Bind", "True"} }, @@ -75,6 +75,7 @@ public void BindingLibraryIncremental (string classParser) "_ResolveLibraryProjectImports", "CoreCompile", "_CreateAar", + "_ClearGeneratedManagedBindings", }; var proj = new XamarinAndroidBindingProject () { @@ -112,6 +113,23 @@ public void BindingLibraryIncremental (string classParser) foreach (var target in targets) { Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should be skipped on second build!"); } + + Assert.IsTrue (b.DesignTimeBuild (proj, target: "UpdateGeneratedFiles"), "DTB should have succeeded."); + var cs_file = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "generated", "src", "Com.Larvalabs.Svgandroid.SVGParser.cs"); + FileAssert.Exists (cs_file); + Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "third build should succeed"); + foreach (var target in targets) { + Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should be skipped on second build!"); + } + // Fast Update Check Build + Assert.IsTrue (b.DesignTimeBuild (proj, target: "PrepareResources;_GenerateCompileInputs"), "DTB should have succeeded."); + cs_file = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, "generated", "src", "Com.Larvalabs.Svgandroid.SVGParser.cs"); + FileAssert.Exists (cs_file); + Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "forth build should succeed"); + foreach (var target in targets) { + Assert.IsTrue (b.Output.IsTargetSkipped (target), $"`{target}` should be skipped on second build!"); + } + } } @@ -671,6 +689,10 @@ public void BindingWithAndroidJavaSource () StringAssertEx.ContainsText (File.ReadAllLines (generatedIface), "string GreetWithQuestion (string name, global::Java.Util.Date date, string question);"); Assert.IsTrue (libBuilder.Build (lib), "Library build should have succeeded."); Assert.IsTrue (libBuilder.Output.IsTargetSkipped ("_CompileBindingJava"), $"`_CompileBindingJava` should be skipped on second build!"); + Assert.IsTrue (libBuilder.Output.IsTargetSkipped ("_ClearGeneratedManagedBindings"), $"`_ClearGeneratedManagedBindings` should be skipped on second build!"); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have not be deleted on second build."); + Assert.IsTrue (libBuilder.DesignTimeBuild (lib, target: "UpdateGeneratedFiles"), "DTB should have succeeded."); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have not be deleted on DTB build."); Assert.IsTrue (appBuilder.Build (app), "App build should have succeeded."); appBuilder.Target = "SignAndroidPackage"; Assert.IsTrue (appBuilder.Build (app), "App SignAndroidPackage should have succeeded."); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs index b0b69ff759b..fce460a2c6d 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs @@ -70,7 +70,7 @@ public void DotNetBuild (string runtimeIdentifiers, bool isRelease, bool aot, bo proj.OtherBuildItems.Add (new BuildItem ("JavaSourceJar", "javaclasses-sources.jar") { BinaryContent = () => ResourceData.JavaSourceJarTestSourcesJar, }); - proj.OtherBuildItems.Add (new AndroidItem.AndroidJavaSource ("JavaSourceTestExtension.java") { + proj.AndroidJavaSources.Add (new AndroidItem.AndroidJavaSource ("JavaSourceTestExtension.java") { Encoding = Encoding.ASCII, TextContent = () => ResourceData.JavaSourceTestExtension, Metadata = { { "Bind", "True"} }, @@ -1511,7 +1511,7 @@ public void BuildApplicationWithJavaSourceUsingAndroidX ([Values(true, false)] b { var proj = new XamarinAndroidApplicationProject () { IsRelease = isRelease, - OtherBuildItems = { + AndroidJavaSources = { new BuildItem (AndroidBuildActions.AndroidJavaSource, "ToolbarEx.java") { TextContent = () => @"package com.unnamedproject.unnamedproject; import android.content.Context; @@ -1556,11 +1556,11 @@ public void BuildApplicationCheckThatAddStaticResourcesTargetDoesNotRerun () public void CheckJavaError () { var proj = new XamarinAndroidApplicationProject (); - proj.OtherBuildItems.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "TestMe.java") { + proj.AndroidJavaSources.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "TestMe.java") { TextContent = () => "public classo TestMe { }", Encoding = Encoding.ASCII }); - proj.OtherBuildItems.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "TestMe2.java") { + proj.AndroidJavaSources.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "TestMe2.java") { TextContent = () => "public class TestMe2 {" + "public vod Test ()" + "}", diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs index 8e625dc3ab7..83d47a85bdc 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest2.cs @@ -1000,14 +1000,14 @@ public void BuildProguardEnabledProject (string rid) XamarinAndroidApplicationProject CreateMultiDexRequiredApplication (string debugConfigurationName = "Debug", string releaseConfigurationName = "Release") { var proj = new XamarinAndroidApplicationProject (debugConfigurationName, releaseConfigurationName); - proj.OtherBuildItems.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "ManyMethods.java") { + proj.AndroidJavaSources.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "ManyMethods.java") { TextContent = () => "public class ManyMethods { \n" + string.Join (Environment.NewLine, Enumerable.Range (0, 32768).Select (i => "public void method" + i + "() {}")) + "}", Encoding = Encoding.ASCII, Metadata = { { "Bind", "False "}}, }); - proj.OtherBuildItems.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "ManyMethods2.java") { + proj.AndroidJavaSources.Add (new BuildItem (AndroidBuildActions.AndroidJavaSource, "ManyMethods2.java") { TextContent = () => "public class ManyMethods2 { \n" + string.Join (Environment.NewLine, Enumerable.Range (0, 32768).Select (i => "public void method" + i + "() {}")) + "}", @@ -1064,8 +1064,8 @@ public void BuildAfterMultiDexIsNotRequired () } //Now build project again after it no longer requires multidex, remove the *HUGE* AndroidJavaSource build items - while (proj.OtherBuildItems.Count > 1) - proj.OtherBuildItems.RemoveAt (proj.OtherBuildItems.Count - 1); + while (proj.AndroidJavaSources.Count > 1) + proj.AndroidJavaSources.RemoveAt (proj.AndroidJavaSources.Count - 1); proj.SetProperty ("AndroidEnableMultiDex", "False"); Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true), "Build should have succeeded."); diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs index 853003056f7..a5692e315ba 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/BuildOutput.cs @@ -52,9 +52,9 @@ public List GetAssemblyMapCache () return File.ReadLines (path).ToList (); } - public bool IsTargetSkipped (string target) => IsTargetSkipped (Builder.LastBuildOutput, target); + public bool IsTargetSkipped (string target, bool defaultIfNotUsed = false) => IsTargetSkipped (Builder.LastBuildOutput, target, defaultIfNotUsed); - public static bool IsTargetSkipped (IEnumerable output, string target) + public static bool IsTargetSkipped (IEnumerable output, string target, bool defaultIfNotUsed = false) { bool found = false; foreach (var line in output) { @@ -69,7 +69,7 @@ public static bool IsTargetSkipped (IEnumerable output, string target) if (found) return true; } - return false; + return defaultIfNotUsed; } /// diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetXamarinProject.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetXamarinProject.cs index 32425e363c0..30ae44a3628 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetXamarinProject.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.ProjectTools/Common/DotNetXamarinProject.cs @@ -17,10 +17,12 @@ protected DotNetXamarinProject (string debugConfigurationName = "Debug", string Sources = new List (); OtherBuildItems = new List (); + AndroidJavaSources = new List (); ItemGroupList.Add (References); ItemGroupList.Add (OtherBuildItems); ItemGroupList.Add (Sources); + ItemGroupList.Add (AndroidJavaSources); SetProperty ("RootNamespace", () => RootNamespace ?? ProjectName); SetProperty ("AssemblyName", () => AssemblyName ?? ProjectName); @@ -40,6 +42,7 @@ protected DotNetXamarinProject (string debugConfigurationName = "Debug", string public IList OtherBuildItems { get; private set; } public IList Sources { get; private set; } + public IList AndroidJavaSources { get; private set; } public IList ActiveConfigurationProperties { get { return IsRelease ? ReleaseProperties : DebugProperties; } diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs index 20f9de4f0e6..f981b3f091d 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs @@ -611,5 +611,73 @@ public void AdbTargetChangesAppBundle () var after = File.GetLastWriteTimeUtc (apkset); Assert.AreNotEqual (before, after, $"{apkset} should change!"); } + + [Test] + public void AppWithAndroidJavaSource () + { + var path = Path.Combine ("temp", TestName); + var itemToDelete = new AndroidItem.AndroidJavaSource ("TestJavaClass2.java") { + Encoding = Encoding.ASCII, + TextContent = () => @"package com.test.java; + +public class TestJavaClass2 { + + public String test(){ + + return ""Java is called""; + } +}", + Metadata = { + { "Bind", "True" }, + }, + }; + var proj = new XamarinAndroidApplicationProject { + EnableDefaultItems = true, + AndroidJavaSources = { + new AndroidItem.AndroidJavaSource ("TestJavaClass.java") { + Encoding = Encoding.ASCII, + TextContent = () => @"package com.test.java; + +public class TestJavaClass { + + public String test(){ + + return ""Java is called""; + } +}", + Metadata = { + { "Bind", "True" }, + }, + }, + itemToDelete, + } + }; + using (var b = CreateApkBuilder ()) { + b.ThrowOnBuildFailure = false; + Assert.IsTrue (b.Build (proj), "Build should have succeeded."); + b.AssertHasNoWarnings (); + var generatedCode = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, + "generated", "src", "Com.Test.Java.TestJavaClass.cs"); + var generatedCode2 = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath, + "generated", "src", "Com.Test.Java.TestJavaClass2.cs"); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have been generated."); + FileAssert.Exists (generatedCode2, $"'{generatedCode2}' should have been generated."); + Assert.IsTrue (b.DesignTimeBuild (proj, target: "UpdateGeneratedFiles"), "DTB should have succeeded."); + Assert.IsTrue (b.Output.IsTargetSkipped ("_ClearGeneratedManagedBindings", defaultIfNotUsed: true), $"`_ClearGeneratedManagedBindings` should be skipped on DTB build!"); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have not be deleted on DTB build."); + FileAssert.Exists (generatedCode2, $"'{generatedCode2}' should have not be deleted on DTB build."); + proj.AndroidJavaSources.Remove (itemToDelete); + File.Delete (Path.Combine (Root, b.ProjectDirectory, itemToDelete.Include ())); + Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Second build should have succeeded."); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have not be deleted on second build."); + FileAssert.DoesNotExist (generatedCode2, $"'{generatedCode2}' should have be deleted on second build."); + Assert.IsFalse (b.Output.IsTargetSkipped ("_CompileBindingJava"), $"`_CompileBindingJava` should run on second build!"); + Assert.IsTrue (b.Output.IsTargetSkipped ("_ClearGeneratedManagedBindings"), $"`_ClearGeneratedManagedBindings` should be skipped on second build!"); + // Call Install directly so Build does not get called automatically + Assert.IsTrue (b.RunTarget (proj, "Install", doNotCleanupOnUpdate: true, saveProject: false), "Install build should have succeeded."); + FileAssert.Exists (generatedCode, $"'{generatedCode}' should have not be deleted on Install build."); + Assert.IsTrue (b.Output.IsTargetSkipped ("_ClearGeneratedManagedBindings", defaultIfNotUsed: true), $"`_ClearGeneratedManagedBindings` should be skipped on Install build!"); + } + } } } From 8af42441a121e9549903caeafdc826e634616f6a Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 25 Mar 2024 19:52:32 -0400 Subject: [PATCH 2/3] Fix indentation --- .../MSBuildDeviceIntegration/Tests/InstallTests.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs index f981b3f091d..c7e625c0707 100644 --- a/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs +++ b/tests/MSBuildDeviceIntegration/Tests/InstallTests.cs @@ -617,8 +617,8 @@ public void AppWithAndroidJavaSource () { var path = Path.Combine ("temp", TestName); var itemToDelete = new AndroidItem.AndroidJavaSource ("TestJavaClass2.java") { - Encoding = Encoding.ASCII, - TextContent = () => @"package com.test.java; + Encoding = Encoding.ASCII, + TextContent = () => @"package com.test.java; public class TestJavaClass2 { @@ -627,10 +627,10 @@ public String test(){ return ""Java is called""; } }", - Metadata = { - { "Bind", "True" }, - }, - }; + Metadata = { + { "Bind", "True" }, + }, + }; var proj = new XamarinAndroidApplicationProject { EnableDefaultItems = true, AndroidJavaSources = { @@ -650,7 +650,7 @@ public String test(){ }, }, itemToDelete, - } + }, }; using (var b = CreateApkBuilder ()) { b.ThrowOnBuildFailure = false; From 10e3e66c57b166c41c2436e362b4881b77e83113 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Mon, 25 Mar 2024 20:00:23 -0400 Subject: [PATCH 3/3] Remove dead code. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: https://github.com/xamarin/xamarin-android/pull/8706/files#r1538222261 https://github.com/xamarin/xamarin-android/pull/8706/commits/dd1f25bb009d9812c332e8db8486cce2c58d74ed ("initial" commit) had code within the `` task to parse the `generator`-emitted `.projitems` file, to read out the `@(Compile)` group: List files = new List (); var doc = XDocument.Load (GeneratedFileListFile); var compileItems = doc.XPathSelectElements ("//Project/ItemGroup/Compile"); foreach (var item in compileItems) { var file = item.Attribute ("Include"); if (file != null && File.Exists (file.Value)) { files.Add (new TaskItem (file.Value)); } } GeneratedFiles = files.ToArray (); There were two fundamental problems with this code: 1. No XML namespaces 2. The `File.Exists()` check. `GeneratedFileListFile` would be the `.projitems` file, which is a normal MSBuild file with a default xmlns: $(DefineConstants);ANDROID_1;ANDROID_2;ANDROID_3;ANDROID_4;ANDROID_5;ANDROID_6;ANDROID_7;ANDROID_8;ANDROID_9;ANDROID_10;ANDROID_11;ANDROID_12;ANDROID_13;ANDROID_14;ANDROID_15;ANDROID_16;ANDROID_17;ANDROID_18;ANDROID_19;ANDROID_20;ANDROID_21;ANDROID_22;ANDROID_23;ANDROID_24;ANDROID_25;ANDROID_26;ANDROID_27;ANDROID_28;ANDROID_29;ANDROID_30;ANDROID_31;ANDROID_32;ANDROID_33;ANDROID_34 The original `doc.XPathSelectElements()` expression didn't have any XML namespaces present, so `compileItems` would always be *empty*. This can be fixed by using an `XmlNamespaceManager`: List files = new List (); Log.LogDebugMessage ($"# jonp: SHOULD process {GeneratedFileListFile}"); var doc = XDocument.Load (GeneratedFileListFile); var nsmgr = new XmlNamespaceManager(new NameTable()); nsmgr.AddNamespace ("msb", "http://schemas.microsoft.com/developer/msbuild/2003"); var compileItems = doc.XPathSelectElements ("//msb:Project/msb:ItemGroup/msb:Compile", nsmgr); foreach (var item in compileItems) { var file = item.Attribute ("Include"); if (file != null && File.Exists (file.Value)) { Log.LogDebugMessage ($"# jonp: found //Compile/@Include value: {file.Value}"); files.Add (new TaskItem (file.Value)); } } GeneratedFiles = files.ToArray (); Now `compileItems` has elements! Then we encounter the `File.Exists()` problem: *we* are parsing the `.projitems` file, ***not MSBuild***, so if (when) the file contains MSBuild-isms such as `$(MSBuildThisFileDirectory)`, those pass through unchanged! For example, `compileItems.ElementAt(0)` would be: `item.Attribute("Include").Value` would be: $(MSBuildThisFileDirectory)E.Example.cs which means we're trying to *literally* do: File.Exists("$(MSBuildThisFileDirectory)E.Example.cs") No expansion is occurring. This has (approximately) ***no*** chance of working. At minimum we'd have to `string.Replace()` the sub-string `$(MSBuildThisFileDirectory)` with `OutputDirectory`. However, the fact that all the unit tests pass means one of two things: 1. We need more unit tests ;-), or 2. We don't actually need to parse the `.projitems` file. Assume that (2) is the case: update `` et al to *remove* the processing of `.projitems`. Does It Work™? --- .../Xamarin.Android.Bindings.Core.targets | 13 +------------ .../Tasks/Generator.cs | 19 +------------------ 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets index 25ea0d166e8..5d3ee2ba295 100644 --- a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets +++ b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Bindings.Core.targets @@ -135,22 +135,11 @@ It is shared between "legacy" binding projects and .NET 5 projects. Nullable="$(Nullable)" UseJavaLegacyResolver="$(_AndroidUseJavaLegacyResolver)" NamespaceTransforms="@(AndroidNamespaceReplacement)" - GeneratedFileListFile="$(GeneratedOutputPath)src\$(AssemblyName).projitems" - > - - + /> - - <_ExistingGeneratedFiles Include="$(GeneratedOutputPath)**\*.cs" /> - <_ExistingGeneratedFiles Remove="@(_GeneratedBindingFiles)" /> - - - - - diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs index a3ee0e21175..15f04ccf5e6 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/Generator.cs @@ -69,10 +69,6 @@ public class BindingsGenerator : AndroidDotnetToolTask public bool UseJavaLegacyResolver { get; set; } - public string GeneratedFileListFile { get; set; } - [Output] - public ITaskItem [] GeneratedFiles { get; set; } = Array.Empty (); - private List> transform_files = new List> (); public override bool RunTask () @@ -139,20 +135,7 @@ public override bool RunTask () if (Log.HasLoggedErrors) return false; - var result = base.RunTask (); - List files = new List (); - if (result && GeneratedFileListFile != null && File.Exists (GeneratedFileListFile)) { - var doc = XDocument.Load (GeneratedFileListFile); - var compileItems = doc.XPathSelectElements ("//Project/ItemGroup/Compile"); - foreach (var item in compileItems) { - var file = item.Attribute ("Include"); - if (file != null && File.Exists (file.Value)) { - files.Add (new TaskItem (file.Value)); - } - } - } - GeneratedFiles = files.ToArray (); - return result; + return base.RunTask (); } void WriteLine (StreamWriter sw, string line)