From 7f1f122aeb7c1ef518716d9d5160dc46b08eaba1 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Tue, 27 Jun 2023 12:05:01 -0500 Subject: [PATCH] [Xamarin.Android.Build.Tasks] correct paths in MarshalMethodsAssemblyRewriter Context: https://github.com/dotnet/maui/pull/15399#issuecomment-1609576331 During a PR updating AndroidX dependencies in .NET MAUI, we got the error: Task GenerateJavaStubs ... System.IO.IOException: The process cannot access the file 'D:\a\_work\1\s\src\Compatibility\ControlGallery\src\Android\obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.pdb' because it is being used by another process. at System.IO.FileSystem.CopyFile(String sourceFullPath, String destFullPath, Boolean overwrite) at Xamarin.Android.Tasks.MarshalMethodsAssemblyRewriter.Rewrite(DirectoryAssemblyResolver resolver, List`1 targetAssemblyPaths, Boolean brokenExceptionTransitions) at Xamarin.Android.Tasks.GenerateJavaStubs.Run(DirectoryAssemblyResolver res, Boolean useMarshalMethods) at Xamarin.Android.Tasks.GenerateJavaStubs.RunTask() at Microsoft.Android.Build.Tasks.AndroidTask.Execute() in /Users/runner/work/1/s/xamarin-android/external/xamarin-android-tools/src/Microsoft.Android.Build.BaseTasks/AndroidTask.cs:line 25 Which has some very odd logging right before the failure: Copying rewritten assembly: obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.PoolingContainer.dll.new -> obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.PoolingContainer.dll Copying rewritten assembly: obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.PoolingContainer.pdb -> obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.pdb Copying `Xamarin.AndroidX.CustomView.PoolingContainer.pdb` to `Xamarin.AndroidX.CustomView.pdb`? I could reproduce the issue in a test with `PublishTrimmed=false`: [Test] public void SimilarAndroidXAssemblyNames ([Values(true, false)] bool publishTrimmed) { var proj = new XamarinAndroidApplicationProject { IsRelease = true, AotAssemblies = publishTrimmed, PackageReferences = { new Package { Id = "Xamarin.AndroidX.CustomView", Version = "1.1.0.17" }, new Package { Id = "Xamarin.AndroidX.CustomView.PoolingContainer", Version = "1.0.0.4" }, } }; proj.SetProperty (KnownProperties.PublishTrimmed, publishTrimmed.ToString()); proj.MainActivity = proj.DefaultMainActivity.Replace ("//${AFTER_ONCREATE}", "AndroidX.CustomView.PoolingContainer.PoolingContainer.IsPoolingContainer (null);"); using var builder = CreateApkBuilder (); Assert.IsTrue (builder.Build (proj), "Build should have succeeded."); } In `MarshalMethodsAssemblyRewriter` we write temporary assembly files to `foo.new` and copy to the original path at `foo.dll`. We next copy any symbol files if found. I found two underlying issues here. First, this `Mono.Cecil` API: AssemblyDefinition.Write("foo.new", new WriterParameters { WriteSymbols = true }); It would write a `foo.pdb` instead of `foo.new.pdb`, and so we have no way for this to write symbols to a temporary location. I put the temporary location in a `new` subdirectory instead of appending `.new` to the path. The second problem is this code: target = Path.ChangeExtension (Path.Combine (targetPath, Path.GetFileNameWithoutExtension (pdb)), ".pdb"); CopyFile (pdb, target); It appears to lose `.PoolingContainer` from the path, and so it uses the destination of: obj\Release\net8.0-android\android\assets\Xamarin.AndroidX.CustomView.pdb But using a `new` subdirectory instead, we bypass this issue. After these changes, we instead get: Copying rewritten assembly: obj\Release\android\assets\new\Xamarin.AndroidX.CustomView.PoolingContainer.dll -> obj\Release\android\assets\Xamarin.AndroidX.CustomView.PoolingContainer.dll Copying rewritten assembly: obj\Release\android\assets\new\Xamarin.AndroidX.CustomView.PoolingContainer.pdb -> obj\Release\android\assets\Xamarin.AndroidX.CustomView.PoolingContainer.pdb Deleting: obj\Release\android\assets\new\Xamarin.AndroidX.CustomView.PoolingContainer.dll Deleting: obj\Release\android\assets\new\Xamarin.AndroidX.CustomView.PoolingContainer.pdb Lastly, I removed `.mdb` file support -- but that is completely unrelated to the issue. --- .../Xamarin.Android.Build.Tests/BuildTest.cs | 17 ++++++++++++ .../MarshalMethodsAssemblyRewriter.cs | 27 ++++++------------- 2 files changed, 25 insertions(+), 19 deletions(-) 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 952c5c9d374..b17f7beafbb 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 @@ -2560,5 +2560,22 @@ public void CheckLintResourceFileReferencesAreFixed () StringAssertEx.DoesNotContain (errorFilePath, b.LastBuildOutput, $"Path {errorFilePath} should have been replaced."); } } + + [Test] + public void SimilarAndroidXAssemblyNames ([Values(true, false)] bool publishTrimmed) + { + var proj = new XamarinAndroidApplicationProject { + IsRelease = true, + AotAssemblies = publishTrimmed, + PackageReferences = { + new Package { Id = "Xamarin.AndroidX.CustomView", Version = "1.1.0.17" }, + new Package { Id = "Xamarin.AndroidX.CustomView.PoolingContainer", Version = "1.0.0.4" }, + } + }; + proj.SetProperty (KnownProperties.PublishTrimmed, publishTrimmed.ToString()); + proj.MainActivity = proj.DefaultMainActivity.Replace ("//${AFTER_ONCREATE}", "AndroidX.CustomView.PoolingContainer.PoolingContainer.IsPoolingContainer (null);"); + using var builder = CreateApkBuilder (); + Assert.IsTrue (builder.Build (proj), "Build should have succeeded."); + } } } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs index acd69f2d7ea..4131349d150 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs @@ -118,11 +118,12 @@ public void Rewrite (DirectoryAssemblyResolver resolver, List targetAsse foreach (AssemblyDefinition asm in uniqueAssemblies) { foreach (string path in GetAssemblyPaths (asm)) { var writerParams = new WriterParameters { - WriteSymbols = (File.Exists (path + ".mdb") || File.Exists (Path.ChangeExtension (path, ".pdb"))), + WriteSymbols = File.Exists (Path.ChangeExtension (path, ".pdb")), }; - - string output = $"{path}.new"; + string directory = Path.Combine (Path.GetDirectoryName (path), "new"); + Directory.CreateDirectory (directory); + string output = Path.Combine (directory, Path.GetFileName (path)); log.LogDebugMessage ($"Writing new version of assembly: {output}"); // TODO: this should be used eventually, but it requires that all the types are reloaded from the assemblies before typemaps are generated @@ -137,36 +138,23 @@ public void Rewrite (DirectoryAssemblyResolver resolver, List targetAsse // versions around. foreach (string path in newAssemblyPaths) { string? pdb = null; - string? mdb = null; - string source = Path.ChangeExtension (Path.Combine (Path.GetDirectoryName (path), Path.GetFileNameWithoutExtension (path)), ".pdb"); + string source = Path.ChangeExtension (path, ".pdb"); if (File.Exists (source)) { pdb = source; } - source = $"{path}.mdb"; - if (File.Exists (source)) { - mdb = source; - } - foreach (string targetPath in targetAssemblyPaths) { - string target = Path.Combine (targetPath, Path.GetFileNameWithoutExtension (path)); + string target = Path.Combine (targetPath, Path.GetFileName (path)); CopyFile (path, target); if (!String.IsNullOrEmpty (pdb)) { - target = Path.ChangeExtension (Path.Combine (targetPath, Path.GetFileNameWithoutExtension (pdb)), ".pdb"); - CopyFile (pdb, target); - } - - if (!String.IsNullOrEmpty (mdb)) { - target = Path.Combine (targetPath, Path.ChangeExtension (Path.GetFileName (path), ".mdb")); - CopyFile (mdb, target); + CopyFile (pdb, Path.ChangeExtension (target, ".pdb")); } } RemoveFile (path); RemoveFile (pdb); - RemoveFile (mdb); } void CopyFile (string source, string target) @@ -182,6 +170,7 @@ void RemoveFile (string? path) } try { + log.LogDebugMessage ($"Deleting: {path}"); File.Delete (path); } catch (Exception ex) { log.LogWarning ($"Unable to delete source file '{path}'");