Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] correct paths in MarshalMethodsAssembly…
Browse files Browse the repository at this point in the history
…Rewriter

Context: dotnet/maui#15399 (comment)

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.
  • Loading branch information
jonathanpeppers committed Jun 27, 2023
1 parent dab495f commit 7f1f122
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 7f1f122

Please sign in to comment.