Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] MarshalMethodsAssemblyRewriter+new file (
Browse files Browse the repository at this point in the history
…#8151)

Context: dotnet/maui#15399 (comment)

During a PR updating AndroidX dependencies in .NET MAUI, we saw:

	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()

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`?  Where is  `.PoolingContainer`?

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,
	});

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

By 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.
  • Loading branch information
jonathanpeppers authored Jun 28, 2023
1 parent 27078a5 commit efa14e2
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,12 @@ public void Rewrite (DirectoryAssemblyResolver resolver, List<string> 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
Expand All @@ -137,36 +138,23 @@ public void Rewrite (DirectoryAssemblyResolver resolver, List<string> 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)
Expand All @@ -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}'");
Expand Down

0 comments on commit efa14e2

Please sign in to comment.