From 4647127aca84e6315bae76d73de535a356dbcf9f Mon Sep 17 00:00:00 2001 From: Brendan Zagaeski Date: Thu, 6 Aug 2020 21:00:15 -0700 Subject: [PATCH] [Xamarin.Android.Build.Tasks] Deploy changes to transitive reference assembly dependencies Fixes: https://developercommunity.visualstudio.com/content/problem/1086457/index.html Context: 8f2ae24813ec3cdab56f688c4c3b611a33759333 Context: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/938468 If a Xamarin.Android app referenced a .NET Standard library that had `$(ProduceReferenceAssembly)`=`true`, and that library in turn referenced a second .NET Standard library, then changes to the second library would not be deployed during incremental deployments in Visual Studio unless the app project also directly referenced the second library. For additional context, note that as discussed in internal work item 938468, if a .NET Framework console application references a .NET Standard library that has `$(ProduceReferenceAssembly)`=`true`, then changes to the library are similarly missing in the running console app after incremental builds in Visual Studio. Although the problem in Xamarin.Android apps was similar to the .NET Framework behavior, an important difference was that Xamarin.Android apps did reflect changes correctly for libraries referenced directly by the app. Only transitive references caused trouble in Xamarin.Android. The reason direct references worked was that Visual Studio always ran Xamarin.Android's `Install` target on each incremental deployment, and the `` task in that target correctly located the up-to-date assemblies for direct references. For example, for the included build test, the `` task resolved the `Library1` reference to: C:\Source\xamarin-android\bin\TestDebug\temp\TransitiveDependencyProduceReferenceAssembly\Library1\bin\Debug\netstandard2.0\Library1.dll In contrast, the `` task resolved the `Library2` reference to: C:\Source\xamarin-android\bin\TestDebug\temp\TransitiveDependencyProduceReferenceAssembly\App\bin\Debug\Library2.dll This was problematic because that copy of the library was left over from the previous deployment. In a sense, this was a circular reference: The input for the current deployment was the output from the previous deployment. To fix this behavior, add the `@(ReferenceDependencyPaths)` items that are found by the `` task to the `@(FilteredAssemblies)` input. For the included build test, this means the following additional item is now included in `@(FilteredAssemblies)`: C:\Source\xamarin-android\bin\TestDebug\temp\TransitiveDependencyProduceReferenceAssembly\Library1\bin\Debug\netstandard2.0\Library2.dll This path is then treated as a top-level reference in ``, so the task no longer needs to try to resolve it from elsewhere. In theory, another way to fix the problem could be to change how `` resolves transitive dependencies, but `` is only used in the legacy build targets, so adding more sophistication now would have limited value. Other changes: * The `BuildWithResolveAssembliesFailure` test using `packages.config` gets a slightly different error message due to the head project knowing about `Microsoft.Azure.EventHubs`. I think the new behavior is OK. * `@(FilteredAssemblies)` can now have duplicates, so we need to use `->Distinct()`. Note that MSBuild preserves ordering: https://github.com/microsoft/msbuild/blob/9c2d922959e80ba72fefb7addec21ede227639c3/src/Build/Evaluation/Expander.cs#L2484-L2513 Co-authored-by: Jonathan Peppers --- .../release-notes/transitive-refasm.md | 8 +++ .../Xamarin.Android.Build.Tests/BuildTest.cs | 2 +- .../IncrementalBuildTest.cs | 62 +++++++++++++++++++ .../Xamarin.Android.Legacy.targets | 6 +- 4 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 Documentation/release-notes/transitive-refasm.md diff --git a/Documentation/release-notes/transitive-refasm.md b/Documentation/release-notes/transitive-refasm.md new file mode 100644 index 00000000000..225c8ea0e4b --- /dev/null +++ b/Documentation/release-notes/transitive-refasm.md @@ -0,0 +1,8 @@ +#### Application and library build and deployment + +- [Developer Community 1086457](https://developercommunity.visualstudio.com/content/problem/1086457/index.html): + Changes to libraries referenced by the .NET Standard library in a default + Xamarin.Forms project were not reflected in the running app without a clean + rebuild. More generally, this issue affected any library referenced + indirectly via a .NET Standard library that had the + `ProduceReferenceAssembly` MSBuild property set to `true`. 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 3a5ca58a8e9..59a6ab1952a 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 @@ -2692,7 +2692,7 @@ public void BuildWithResolveAssembliesFailure ([Values (true, false)] bool usePa Assert.IsTrue (appBuilder.LastBuildOutput.ContainsText ($"{error} `Microsoft.Azure.EventHubs`, referenced by `MyLibrary`. Please add a NuGet package or assembly reference for `Microsoft.Azure.EventHubs`, or remove the reference to `MyLibrary`."), $"Should recieve '{error}' regarding `Microsoft.Azure.EventHubs`!"); } else { - Assert.IsTrue (appBuilder.LastBuildOutput.ContainsText ($"{error} `Microsoft.Azure.Amqp`, referenced by `MyLibrary` > `Microsoft.Azure.EventHubs`. Please add a NuGet package or assembly reference for `Microsoft.Azure.Amqp`, or remove the reference to `MyLibrary`."), + Assert.IsTrue (appBuilder.LastBuildOutput.ContainsText ($"{error} `Microsoft.Azure.Amqp`, referenced by `Microsoft.Azure.EventHubs`. Please add a NuGet package or assembly reference for `Microsoft.Azure.Amqp`, or remove the reference to `Microsoft.Azure.EventHubs`"), $"Should recieve '{error}' regarding `Microsoft.Azure.Amqp`!"); } //Now add the PackageReference to the app to see a different error message 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 349a5374bbd..40fd6dc0017 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 @@ -516,6 +516,68 @@ public void ProduceReferenceAssembly () } } + [Test] + public void TransitiveDependencyProduceReferenceAssembly () + { + var path = Path.Combine (Root, "temp", TestName); + var app = new XamarinAndroidApplicationProject { + ProjectName = "App", + Sources = { + new BuildItem.Source ("Class1.cs") { + TextContent = () => "public class Class1 : Library1.Class1 { }" + }, + } + }; + var lib1 = new DotNetStandard { + ProjectName = "Library1", + Sdk = "Microsoft.NET.Sdk", + TargetFramework = "netstandard2.0", + Sources = { + new BuildItem.Source ("Class1.cs") { + TextContent = () => "namespace Library1 { public class Class1 { } }" + }, + new BuildItem.Source ("Class2.cs") { + TextContent = () => "namespace Library1 { public class Class2 : Library2.Class1 { } }" + } + } + }; + lib1.SetProperty ("ProduceReferenceAssembly", "True"); + var lib2 = new DotNetStandard { + ProjectName = "Library2", + Sdk = "Microsoft.NET.Sdk", + TargetFramework = "netstandard2.0", + Sources = { + new BuildItem.Source ("Class1.cs") { + TextContent = () => "namespace Library2 { public class Class1 { } }" + }, + } + }; + lib2.SetProperty ("ProduceReferenceAssembly", "True"); + lib1.OtherBuildItems.Add (new BuildItem.ProjectReference ($"..\\{lib2.ProjectName}\\{lib2.ProjectName}.csproj", lib2.ProjectName, lib2.ProjectGuid)); + app.References.Add (new BuildItem.ProjectReference ($"..\\{lib1.ProjectName}\\{lib1.ProjectName}.csproj", lib1.ProjectName, lib1.ProjectGuid)); + + using (var lib2Builder = CreateDllBuilder (Path.Combine (path, lib2.ProjectName), cleanupAfterSuccessfulBuild: false)) + using (var lib1Builder = CreateDllBuilder (Path.Combine (path, lib1.ProjectName), cleanupAfterSuccessfulBuild: false)) + using (var appBuilder = CreateApkBuilder (Path.Combine (path, app.ProjectName))) { + Assert.IsTrue (lib2Builder.Build (lib2), "first Library2 build should have succeeded."); + Assert.IsTrue (lib1Builder.Build (lib1), "first Library1 build should have succeeded."); + Assert.IsTrue (appBuilder.Build (app), "first app build should have succeeded."); + + lib2.Sources.Add (new BuildItem.Source ("Class2.cs") { + TextContent = () => "namespace Library2 { public class Class2 { } }" + }); + + Assert.IsTrue (lib2Builder.Build (lib2, doNotCleanupOnUpdate: true), "second Library2 build should have succeeded."); + Assert.IsTrue (lib1Builder.Build (lib1, doNotCleanupOnUpdate: true, saveProject: false), "second Library1 build should have succeeded."); + appBuilder.Target = "SignAndroidPackage"; + Assert.IsTrue (appBuilder.Build (app, doNotCleanupOnUpdate: true, saveProject: false), "app SignAndroidPackage build should have succeeded."); + + var lib2Output = Path.Combine (path, lib2.ProjectName, "bin", "Debug", "netstandard2.0", $"{lib2.ProjectName}.dll"); + var lib2InAppOutput = Path.Combine (path, app.ProjectName, app.IntermediateOutputPath, "android", "assets", $"{lib2.ProjectName}.dll"); + FileAssert.AreEqual (lib2Output, lib2InAppOutput, "new Library2 should have been copied to app output directory"); + } + } + [Test] [Category ("dotnet")] public void LinkAssembliesNoShrink () diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets index 5cce13cbd7c..1cefd0b1c98 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets @@ -258,6 +258,10 @@ projects. .NET 5 projects will not import this file. Include="@(ReferenceCopyLocalPaths)" Condition="'%(ReferenceCopyLocalPaths.ResolvedFrom)' != 'ImplicitlyExpandDesignTimeFacades' And '%(ReferenceCopyLocalPaths.Extension)' == '.dll' And '%(ReferenceCopyLocalPaths.RelativeDir)' == '' And Exists('%(ReferenceCopyLocalPaths.Identity)') " /> +