Skip to content

Migrate GenerateProguardConfiguration from ILLink step to MSBuild task#10694

Open
sbomer wants to merge 4 commits intomainfrom
dev/sbomer/proguard-step
Open

Migrate GenerateProguardConfiguration from ILLink step to MSBuild task#10694
sbomer wants to merge 4 commits intomainfrom
dev/sbomer/proguard-step

Conversation

@sbomer
Copy link
Member

@sbomer sbomer commented Jan 14, 2026

The new task runs AfterTargets="ILLink" and uses System.Reflection.Metadata instead of Mono.Cecil to scan linked assemblies.

The new task runs AfterTargets="ILLink" and uses System.Reflection.Metadata
instead of Mono.Cecil to scan linked assemblies.
@jonathanpeppers
Copy link
Member

Looking at the test failures, I think this might have broken Proguard rules on NativeAOT:

image

I'll rerun to verify.

Copilot AI review requested due to automatic review settings February 3, 2026 23:25
@sbomer sbomer requested a review from simonrozsival as a code owner February 3, 2026 23:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates the GenerateProguardConfiguration functionality from an ILLink custom step (using Mono.Cecil) to an MSBuild task (using System.Reflection.Metadata). The task runs after the ILLink target and scans linked assemblies to generate ProGuard configuration rules for Android callable wrapper (ACW) types.

Changes:

  • Added new MSBuild task GenerateProguardConfiguration using System.Reflection.Metadata for assembly inspection
  • Updated MSBuild targets to invoke the new task after ILLink instead of as a linker custom step
  • Removed the old ILLink step implementation that used Mono.Cecil

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/Xamarin.Android.Build.Tasks/Tasks/GenerateProguardConfiguration.cs New MSBuild task that scans linked assemblies for RegisterAttribute and generates ProGuard keep rules
src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.ILLink.targets Added targets to prepare linked assemblies and invoke the new task; removed old linker custom step configuration
src/Microsoft.Android.Sdk.ILLink/GenerateProguardConfiguration.cs Deleted old ILLink step implementation

Comment on lines +25 to +26
if (!Directory.Exists (dir))
Directory.CreateDirectory (dir);
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path.GetDirectoryName can return null when the path doesn't contain directory information. The variable 'dir' should be checked for null before being passed to Directory.Exists and Directory.CreateDirectory. Consider using a pattern like the one in KeyTool.cs line 42 where the result is checked with IsNullOrEmpty before use.

Suggested change
if (!Directory.Exists (dir))
Directory.CreateDirectory (dir);
if (!dir.IsNullOrEmpty () && !Directory.Exists (dir)) {
Directory.CreateDirectory (dir);
}

Copilot uses AI. Check for mistakes.
var args = attr.GetCustomAttributeArguments ();
if (args.FixedArguments.Length >= 2 &&
args.FixedArguments[0].Value is string jname &&
args.FixedArguments[1].Value is string jargs) {
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment to jargs is useless, since its value is never read.

Suggested change
args.FixedArguments[1].Value is string jargs) {
args.FixedArguments[1].Value is string) {

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants