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

Export proguard specs from aar_import #12749

Conversation

benjaminRomano
Copy link
Contributor

Background
#3778

proguard specs from the aar_import rule do not get bubbled up to android_binary. In this PR, I wire up a ProguardSpecProvider from this rule that exports the proguard.txt within an AAR if it exists and any transitive proguard specs from the exports attribute.

Changes

  • Add an aar_embedded_proguard_extractor script to extract proguard.txt from an AAR if it exists otherwise generate an empty proguard specs file
  • In AarImport, wire up the proguard extractor action and export results through a ProguardSpecProvider.

Once this lands, the android rules would need to be bumped.

Test Plan

  • Added tests for the extraction python script
  • Added tests for the aar_import rule changes

@benjaminRomano
Copy link
Contributor Author

I think the test failures are related to android_tools version need to be bumped?

@jin jin added the team-Android Issues for Android team label Jan 15, 2021
@nkoroste
Copy link
Contributor

Any updates on this?

Copy link
Contributor

@ahumesky ahumesky left a comment

Choose a reason for hiding this comment

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

Thank you, sorry for the delay here. Looks good, I'll start importing this change today

@ahumesky
Copy link
Contributor

ahumesky commented Feb 24, 2021

Also, could you clarify what "the android rules would need to be bumped" means? If you're referring to the remote android tools (the dependencies automatically included by android_remote_tools.WORKSPACE), right now those don't contain these python tools. These python tools are still embedded within bazel.

@ahumesky
Copy link
Contributor

Gotcha, so these python tools aren't part of the remote android tools. These get embedded in bazel. So these changes will be part of the next bazel release.

@bazel-io bazel-io closed this in c8c0d94 Feb 26, 2021
katre pushed a commit that referenced this pull request Jul 12, 2021
**Background**
#3778

proguard specs from the `aar_import` rule do not get bubbled up to `android_binary`. In this PR, I wire up a `ProguardSpecProvider` from this rule that exports the `proguard.txt` within an AAR if it exists and any transitive proguard specs from the `exports` attribute.

**Changes**
* Add an `aar_embedded_proguard_extractor` script to extract `proguard.txt` from an AAR if it exists otherwise generate an empty proguard specs file
* In AarImport, wire up the proguard extractor action and export results through a `ProguardSpecProvider`.

Once this lands, the android rules would need to be bumped.

**Test Plan**
* Added tests for the extraction python script
* Added tests for the `aar_import` rule changes

Closes #12749.

PiperOrigin-RevId: 359667674
cpsauer added a commit to cpsauer/bazel that referenced this pull request Mar 4, 2022
Background:
JAR files can bundle ProGuard specs under `META-INF/proguard/`
[See https://developer.android.com/studio/build/shrink-code]

Problem:
Bazel previously erroniously ignored these ProGuard specs, leading to failures with, for example, androidx.annotation.Keep. Bad times.

There was previously a parallel issue with aar_import.
[Fixed in bazelbuild#12749]

Solution:
This change causes the previously ignored, embedded proguards to be extracted, validated, and then bubbled up correctly via the ProguardSpecProvider.

There's also a minor fix to aar_import, adding proguard validation and slightly simplifying the resulting code. For reasoning behind why library proguards should be validated, see the module docstring of proguard_whitelister.py

Remaining issues:
JAR files brought down from Maven via rules_jvm_external bypass java_import in favor of rolling their own jvm_import, since java_import apparently been broken for Kotlin for years. That'll need a subsequent fix, since this only fixes java_import. For context on the Kotlin breakage, see bazelbuild#4549. For the status on fixes in rules_jvm_external, see bazel-contrib/rules_jvm_external#672
cpsauer added a commit to cpsauer/bazel that referenced this pull request Mar 5, 2022
Background:
JAR files can bundle ProGuard specs under `META-INF/proguard/`
[See https://developer.android.com/studio/build/shrink-code]

Problem:
Bazel previously erroniously ignored these ProGuard specs, leading to failures with, for example, androidx.annotation.Keep. Bad times.

There was previously a parallel issue with aar_import.
[Fixed in bazelbuild#12749]

Solution:
This change causes the previously ignored, embedded proguards to be extracted, validated, and then bubbled up correctly via the ProguardSpecProvider.

There's also a minor fix to aar_import, adding proguard validation and slightly simplifying the resulting code. For reasoning behind why library proguards should be validated, see the module docstring of proguard_whitelister.py

Remaining issues:
JAR files brought down from Maven via rules_jvm_external bypass java_import in favor of rolling their own jvm_import, since java_import apparently been broken for Kotlin for years. That'll need a subsequent fix, since this only fixes java_import. For context on the Kotlin breakage, see bazelbuild#4549. For the status on fixes in rules_jvm_external, see bazel-contrib/rules_jvm_external#672
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants