Skip to content
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

[Xamarin.Android.Build.Tasks] Use Proguard config files from .aar files. #5310

Merged
merged 1 commit into from
Jul 7, 2022

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Nov 18, 2020

Fixes #3752

Add support for using proguard.txt file from aar files.
Also add support for including proguard.txt files in our generated aar files when we build a Library project
in .net 6. This will merge all ProguardConfiguration Items into one proguard.txt file and include that in the
root of the aar.

Update the Unit Tests to include test for this functionality.

Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

The changes here look good.

Do we need to be worried about proguard files getting included twice? I think some of the AndroidX/Support libraries extracted the proguard files and added them from .targets.

It seems like that might be OK, does it hurt anything?

@dellis1972
Copy link
Contributor Author

The changes here look good.

Do we need to be worried about proguard files getting included twice? I think some of the AndroidX/Support libraries extracted the proguard files and added them from .targets.

It seems like that might be OK, does it hurt anything?

If we end up with duplicate commands from multiple pro guard files that doesn't seem to cause any problems on the app side.

@dellis1972
Copy link
Contributor Author

@jonathanpeppers only the designer test is failing here. I think this one is good to go.

The only annoying bit I can see is if we reference C from B, B gets the proguard.txt from C in its .aar. As a result when the App A references B, it will also get C and will then have two of the same file during the R8/D8 compilation.

Should that be something I should take out do you think? It doesn't seem to cause any problems at the moment.

@dellis1972
Copy link
Contributor Author

I'm gonna rework this a bit so we don't include the additional proguard file if we are building a library project. The idea being that it will be included by the Reference by default in the final app.

@dellis1972 dellis1972 force-pushed the Issue3752 branch 2 times, most recently from b2906fd to dc5663e Compare July 4, 2022 10:09
@jonpryor
Copy link
Member

jonpryor commented Jul 5, 2022

In-progress commit message:

Fixes: https://github.com/xamarin/xamarin-android/issues/3752

`.aar` [can contain `proguard.txt` files][0]

> **Library dependencies**
> *Location*: AAR libraries: `<library-dir>/proguard.txt`
> *Description*: If an AAR library is published with its own ProGuard
> rules file, and you include that AAR as a compile-time dependency,
> R8 automatically applies its rules when compiling your project.

Update the `@(ProguardConfiguration)` item group to include
`proguard.cfg` and `proguard.txt` files in the project directory, and
for Library project builds add the contents of all
`@(ProguardConfiguration)` files into a `proguard.txt` entry within
the generated `.aar` file (fcd7cf8f).

Update the `_ResolveLibraryProjectImports` target and the
`<ResolveLibraryProjectImports/>` MSBuild task to look for
`proguard.txt` files contained within `.aar` packages, and update the
`<ReadLibraryProjectImportsCache/>` MSBuild task so that
`proguard.txt` files within `.aar` packages are added to the
`@(ProguardConfiguration)` item group of App builds, so that they are
appropriate passed to the `_CompileToDalvik` target and used when
converting Java bytecode into Dalvik bytecode.

TODO: 

The Android documentation mentions that `.jar` files can also contain
ProGuard files:

> *Location*: JAR libraries: `<library-dir>/META-INF/proguard/`

We do not currently use ProGuard files contained within `.jar` files.

[0]: https://developer.android.com/studio/build/shrink-code

@jonpryor
Copy link
Member

jonpryor commented Jul 5, 2022

The Android docs state that .jar files also have such a mechanism: https://developer.android.com/studio/build/shrink-code

Library dependencies
Location: AAR libraries: <library-dir>/proguard.txt
Location: JAR libraries: <library-dir>/META-INF/proguard/
Description: If an AAR library is published with its own ProGuard
rules file, and you include that AAR as a compile-time dependency,
R8 automatically applies its rules when compiling your project.

Do we handle mylib.jar!META-INF/proguard/proguard.txt files? (It doesn't look like it?)

Should we support mylib.jar!META-INF/proguard/proguard.txt files? (Maybe?)

Is there an easy way to find such a thing "in the wild"?

@dellis1972
Copy link
Contributor Author

The Android docs state that .jar files also have such a mechanism: https://developer.android.com/studio/build/shrink-code

Library dependencies
Location: AAR libraries: <library-dir>/proguard.txt
Location: JAR libraries: <library-dir>/META-INF/proguard/
Description: If an AAR library is published with its own ProGuard
rules file, and you include that AAR as a compile-time dependency,
R8 automatically applies its rules when compiling your project.

Do we handle mylib.jar!META-INF/proguard/proguard.txt files? (It doesn't look like it?)

Should we support mylib.jar!META-INF/proguard/proguard.txt files? (Maybe?)

Is there an easy way to find such a thing "in the wild"?

Well one issue there is we do not currently extract the contents of the .jar files into the lp directory. We do for .aar becuae we need to reference the .jar files within. So the proguard.txt will get extracted then, we do not do this for .jar.

So if we want to support that we would need to add another step to look for and extract the proguard files from that META-INF folder in the .jar itself.

Fixes dotnet#3752

Add support for using `proguard.txt` file from `aar` files.
Also add support for including `proguard.txt` files in our generated `aar` files when we build a Library project
in .net 6. This will merge all `ProguardConfiguration` Items into one `proguard.txt` file and include that in the
root of the `aar`.

Update the Unit Tests to include test for this functionality.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done & Blogged
Development

Successfully merging this pull request may close these issues.

Proguard configs should be included from .aar's
4 participants