-
Notifications
You must be signed in to change notification settings - Fork 566
Migrate GenerateProguardConfiguration from ILLink step to MSBuild task #10694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sbomer
wants to merge
6
commits into
main
Choose a base branch
from
dev/sbomer/proguard-step
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+169
−98
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d42b163
Migrate GenerateProguardConfiguration from ILLink step to MSBuild task
sbomer 5dded3d
Make _GenerateProguardConfiguration target incremental
sbomer 169bcf0
Add debug log message for BadImageFormatException
sbomer f809f92
Merge branch 'main' into dev/sbomer/proguard-step
sbomer 553ec86
Fix proguard generation for NativeAOT by using ResolvedFileToPublish
sbomer a918512
Merge remote-tracking branch 'origin/main' into proguard-step
sbomer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
87 changes: 0 additions & 87 deletions
87
src/Microsoft.Android.Sdk.ILLink/GenerateProguardConfiguration.cs
This file was deleted.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
136 changes: 136 additions & 0 deletions
136
src/Xamarin.Android.Build.Tasks/Tasks/GenerateProguardConfiguration.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,136 @@ | ||||||
| #nullable enable | ||||||
|
|
||||||
| using System; | ||||||
| using System.IO; | ||||||
| using System.Reflection.Metadata; | ||||||
| using System.Reflection.PortableExecutable; | ||||||
| using Microsoft.Build.Framework; | ||||||
| using Microsoft.Android.Build.Tasks; | ||||||
|
|
||||||
| namespace Xamarin.Android.Tasks | ||||||
| { | ||||||
| public class GenerateProguardConfiguration : AndroidTask | ||||||
| { | ||||||
| public override string TaskPrefix => "GPC"; | ||||||
|
|
||||||
| [Required] | ||||||
| public ITaskItem[] LinkedAssemblies { get; set; } = []; | ||||||
|
|
||||||
| [Required] | ||||||
| public string OutputFile { get; set; } = ""; | ||||||
|
|
||||||
| public override bool RunTask () | ||||||
| { | ||||||
| var dir = Path.GetDirectoryName (OutputFile); | ||||||
| if (!Directory.Exists (dir)) | ||||||
| Directory.CreateDirectory (dir); | ||||||
| using var writer = File.CreateText (OutputFile); | ||||||
|
|
||||||
| foreach (var assembly in LinkedAssemblies) { | ||||||
| ProcessAssembly (assembly.ItemSpec, writer); | ||||||
| } | ||||||
|
|
||||||
| return !Log.HasLoggedErrors; | ||||||
| } | ||||||
|
|
||||||
| void ProcessAssembly (string assemblyPath, TextWriter writer) | ||||||
| { | ||||||
| try { | ||||||
| using var stream = File.OpenRead (assemblyPath); | ||||||
| using var pe = new PEReader (stream); | ||||||
|
|
||||||
| if (!pe.HasMetadata) | ||||||
| return; | ||||||
|
|
||||||
| var reader = pe.GetMetadataReader (); | ||||||
|
|
||||||
| // Those assemblies that do not reference Mono.Android.dll (such as System.* | ||||||
| // assemblies and Mono.Android.dll itself) can be skipped. | ||||||
| // (Mono.Android.dll is special; android.jar is not part of classes.dex). | ||||||
| // | ||||||
| // FIXME: Those non-embedded jar bindings could visit here too, and they don't have to | ||||||
| // be part of proguard configuration. But they don't break (they will be NOTEd though). | ||||||
| if (!ReferencesMonoAndroid (reader)) | ||||||
| return; | ||||||
|
|
||||||
| var assemblyName = reader.GetString (reader.GetAssemblyDefinition ().Name); | ||||||
| writer.WriteLine ($"# ACW for {assemblyName}"); | ||||||
|
|
||||||
| foreach (var typeHandle in reader.TypeDefinitions) { | ||||||
| var type = reader.GetTypeDefinition (typeHandle); | ||||||
| ProcessType (reader, type, writer); | ||||||
| } | ||||||
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } catch (BadImageFormatException ex) { | ||||||
| // Skip non-managed assemblies | ||||||
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| Log.LogDebugMessage ($"Could not read assembly '{assemblyPath}': {ex.Message}"); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| static bool ReferencesMonoAndroid (MetadataReader reader) | ||||||
| { | ||||||
| foreach (var refHandle in reader.AssemblyReferences) { | ||||||
| var reference = reader.GetAssemblyReference (refHandle); | ||||||
| if (reader.GetString (reference.Name) == "Mono.Android") | ||||||
| return true; | ||||||
| } | ||||||
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| void ProcessType (MetadataReader reader, TypeDefinition type, TextWriter writer) | ||||||
| { | ||||||
| // RegisterAttribute can be applied to interfaces, but proguard rules are only needed for classes. | ||||||
| // Structs don't need to be checked because RegisterAttribute cannot be applied to them. | ||||||
| if ((type.Attributes & System.Reflection.TypeAttributes.Interface) != 0) | ||||||
| return; | ||||||
|
|
||||||
| string? javaTypeName = null; | ||||||
| foreach (var attrHandle in type.GetCustomAttributes ()) { | ||||||
| var attr = reader.GetCustomAttribute (attrHandle); | ||||||
| var attrName = reader.GetCustomAttributeFullName (attr, Log); | ||||||
| if (attrName == "Android.Runtime.RegisterAttribute") { | ||||||
| var args = attr.GetCustomAttributeArguments (); | ||||||
| if (args.FixedArguments.Length > 0 && args.FixedArguments[0].Value is string jtype) { | ||||||
| javaTypeName = jtype.Replace ('/', '.'); | ||||||
| } | ||||||
| break; | ||||||
| } | ||||||
| } | ||||||
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| if (javaTypeName == null) | ||||||
| return; | ||||||
|
|
||||||
| writer.WriteLine ($"-keep class {javaTypeName}"); | ||||||
| writer.WriteLine ($"-keepclassmembers class {javaTypeName} {{"); | ||||||
|
|
||||||
| foreach (var methodHandle in type.GetMethods ()) { | ||||||
| ProcessMethod (reader, methodHandle, writer); | ||||||
| } | ||||||
|
|
||||||
| writer.WriteLine ("}"); | ||||||
| writer.WriteLine (); | ||||||
| } | ||||||
|
|
||||||
| void ProcessMethod (MetadataReader reader, MethodDefinitionHandle methodHandle, TextWriter writer) | ||||||
| { | ||||||
| var method = reader.GetMethodDefinition (methodHandle); | ||||||
|
|
||||||
| foreach (var attrHandle in method.GetCustomAttributes ()) { | ||||||
| var attr = reader.GetCustomAttribute (attrHandle); | ||||||
| var attrName = reader.GetCustomAttributeFullName (attr, Log); | ||||||
| if (attrName == "Android.Runtime.RegisterAttribute") { | ||||||
| var args = attr.GetCustomAttributeArguments (); | ||||||
| if (args.FixedArguments.Length >= 2 && | ||||||
| args.FixedArguments[0].Value is string jname && | ||||||
| args.FixedArguments[1].Value is string jargs) { | ||||||
|
||||||
| args.FixedArguments[1].Value is string jargs) { | |
| args.FixedArguments[1].Value is string) { |
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
jonathanpeppers marked this conversation as resolved.
Show resolved
Hide resolved
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.