"-keepattributes Deprecated" is not honored [130421335]

Fixed

Bug

Reviewed (L1)

Status Update

No update yet.

Description

pe...@gmail.com

created issue #1

Apr 12, 2019 06:59PM

Build: 3.4 RC 3, AI-183.5429.30.34.5400832, 201903230519,

AI-183.5429.30.34.5400832, JRE 1.8.0_152-release-1343-b01x64 JetBrains s.r.o, OS Mac OS X(x86_64) v10.14.3, screens 1280x800, 1920x1080

Android Gradle Plugin: 3.4.0-rc03
Gradle: 5.1.1
NDK: from local.properties: 18.1.5063045; latest from SDK: 18.1.5063045;
LLDB: LLDB 3.1 (revision: 3.1.4508709)
CMake: from local.properties: (not specified); latest from SDK: 3.6.0-rc2; from PATH: (not found);

R8 will not keep the "Deprecated" attribute if instructed to do so with -keepattributes Deprecated. This will prevent compiler warnings when using deprecated features of an R8-processed AAR, for instance. (Note that there is a difference between the Deprecated attribute and the java.lang.Deprecated annotation).

To reproduce:
1. Start with an Android Library project.
2. Enable minification on the library project for release. I.e.:

android {
buildTypes {
release {
minifyEnabled true
}
}
}

3. Add a custom proguardFile with "-keepattributes Deprecated" in addition to the default proguard-android.txt file.
I.e.,

android {
buildTypes {
minifyEnabled true
proguardFiles getDefaultProguardFile('proguard-android.txt'), "custom-rules.txt"
}
}

Where custom-rules.txt is a file in the same directory as the library's build.gradle containing the following:

-keepattributes Deprecated

4. Add the @Deprecated annotation to a method in the Library project that will be kept by R8.
5. Build the library.
6. Extract the classes.jar from both the debug AAR and the release AAR.
7. Run javap -v -cp <debug classes.jar> <name of class in which you added the @Deprecated annotation>
The method on which you added the @Deprecated annotation will have the Deprecated attribute listed with it (in addition to the RuntimeVisibleAnnotations attribute for the @Deprecated annotation).
8. Run javap -v -cp <release classes.jar> <name of class in which you added the @Deprecated annotation>
The method on which you added the @Deprecated annotation will not have the Deprecated attribute.

I would expect R8 to leave the Deprecated attribute intact if "-keepattributes Deprecated" is specified.

IMPORTANT: Please read

https://developer.android.com/studio/report-bugs.html carefully and supply all required information.

Comments

uc...@google.com <uc...@google.com> #2Apr 12, 2019 08:56PM

Assigned to ag...@google.com.

Thank you for the report, and sorry for any inconvenience. While reproducing the issue, one workaround I found is: -keeppackagenames com.example.customview.extra** (i.e., double asterisks, instead of single).

https://www.guardsquare.com/en/products/proguard/manual/usage#filters

R8 currently regards <package_filter> in that directive as <class_filter>. That's why single asterisk---a class name not containing the package separator---doesn't work for now, whereas double asterisks---matches a class name, possibly containing any number of package separators---works. The caveat is, use of, e.g., extra**, will keep all the sub packages as well. :( We'll prepare the fix soon.

ag...@google.com <ag...@google.com> #3Apr 15, 2019 08:09AM

Status: Won't Fix (Intended Behavior)

Project: r8
Branch: master

commit a9a9e7d1231726094c6e16c2cef1bad887da3bba
Author: Jinseong Jeon <jsjeon@google.com>
Date: Tue Apr 09 11:49:28 2019

Reproduce

b/130135768

: keeppackagenames with asterisks

Bug: 130135768
Change-Id: I5ab1cdb5148a570a63bdd488358759bb33da700a

M src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
A src/test/java/com/android/tools/r8/naming/KeepPackageNamesTest.java
A src/test/java/com/android/tools/r8/naming/keeppackagenames/Top.java
A src/test/java/com/android/tools/r8/naming/keeppackagenames/sub/SubClass.java

https://r8-review.googlesource.com/36860

pe...@gmail.com <pe...@gmail.com> #4Apr 15, 2019 01:13PM

Project: r8
Branch: master

commit 391e2992aee375adf46edbb5e7b098556c8604ed
Author: Søren Gjesse <sgjesse@google.com>
Date: Fri Apr 12 11:05:43 2019

New implementation of -keeppackagenames

This implementation collects the list of package names given for
-keeppackagenames and matches against that in the minifier.

The previous implementation created class matching rules to piggybag
on the existing matching.

Bug: 130135768
Change-Id: I5a341e145747a5dec8788a066a0c67d4e259ec77

M src/main/java/com/android/tools/r8/graph/DexType.java
M src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
M src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
M src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
D src/main/java/com/android/tools/r8/shaking/ProguardKeepPackageNamesRule.java
A src/main/java/com/android/tools/r8/shaking/ProguardPackageMatcher.java
A src/main/java/com/android/tools/r8/shaking/ProguardPackageNameList.java
M src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
M src/test/java/com/android/tools/r8/naming/KeepPackageNamesTest.java
M src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
A src/test/java/com/android/tools/r8/shaking/ProguardPackageNameMatcherTest.java

https://r8-review.googlesource.com/36944

Message last modified on Apr 15, 2019 01:14PM

ag...@google.com <ag...@google.com> #5Apr 15, 2019 01:15PM

Status: Assigned (reopened)

Project: r8
Branch: d8-1.4

commit b6ca9ee953e43aa029411bb4db030e4694180063
Author: Christoffer Quist Adamsen <christofferqa@google.com>
Date: Fri Apr 12 12:51:40 2019

Version 1.4.86

Cherry pick: New implementation of -keeppackagenames
CL:

https://r8-review.googlesource.com/c/r8/+/36944

Cherry pick: Reproduce

b/130135768

: keeppackagenames with asterisks
CL:

https://r8-review.googlesource.com/c/r8/+/36860

Bug: 130135768
Change-Id: I8a340afe4811ec3f8826956c99b94346f4c2c8b4

M src/main/java/com/android/tools/r8/Version.java
M src/main/java/com/android/tools/r8/graph/DexType.java
M src/main/java/com/android/tools/r8/naming/ClassNameMinifier.java
M src/main/java/com/android/tools/r8/shaking/ProguardConfiguration.java
M src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
D src/main/java/com/android/tools/r8/shaking/ProguardKeepPackageNamesRule.java
A src/main/java/com/android/tools/r8/shaking/ProguardPackageMatcher.java
A src/main/java/com/android/tools/r8/shaking/ProguardPackageNameList.java
M src/main/java/com/android/tools/r8/shaking/RootSetBuilder.java
M src/test/java/com/android/tools/r8/TestShrinkerBuilder.java
A src/test/java/com/android/tools/r8/naming/KeepPackageNamesTest.java
A src/test/java/com/android/tools/r8/naming/keeppackagenames/Top.java
A src/test/java/com/android/tools/r8/naming/keeppackagenames/sub/SubClass.java
M src/test/java/com/android/tools/r8/shaking/ProguardConfigurationParserTest.java
A src/test/java/com/android/tools/r8/shaking/ProguardPackageNameMatcherTest.java

https://r8-review.googlesource.com/36959

sg...@google.com <sg...@google.com> #6Apr 15, 2019 03:32PM

Yes this is an oversight on our side, and something we need to fix for library projects.

After a bit of research I found that the Deprecated attribute is from before annotations where added to Java, and deprecation was only supported through javadoc. Given the following class

/**
* @deprecated ...
*/
class Deprecated {
}

Compiled with javac will add the deprecated attribute (and that also compiles with "javac -target 1.2 -source 1.2").

ap...@google.com <ap...@google.com> #7Sep 23, 2020 10:46AM

Project: r8
Branch: master

commit 2348e43018ae0a9255ff972929c9e3e0953c4b74
Author: Søren Gjesse <sgjesse@google.com>
Date: Tue Sep 22 18:13:56 2020

Preserve the "Deprecated" attribute for D8 cf-to-cf desugar

Bug: 130421335
Bug: 147485959
Change-Id: Ibf21f54bf829d3bd8aff76efa0be28c612cf67d9

M src/main/java/com/android/tools/r8/GenerateLintFiles.java
M src/main/java/com/android/tools/r8/graph/DexEncodedField.java
M src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
M src/main/java/com/android/tools/r8/graph/DexProgramClass.java
M src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
M src/main/java/com/android/tools/r8/jar/CfApplicationWriter.java
A src/main/java/com/android/tools/r8/utils/AsmUtils.java
A src/test/java/com/android/tools/r8/desugar/DesugarToClassFileDeprecatedAttribute.java

https://r8-review.googlesource.com/54004

ap...@google.com <ap...@google.com> #8Sep 23, 2020 11:26AM

Project: r8
Branch: master

commit e39812b81d944d7f70a39b6903aaf81a7a2b1618
Author: Søren Gjesse <sgjesse@google.com>
Date: Wed Sep 23 13:21:39 2020

Fix signatures to satisfy errorprone

Bug: 130421335
Bug: 147485959
Change-Id: I183bbe144c56549cd9028eca8c6d9fcbe0d7601e

M src/main/java/com/android/tools/r8/GenerateLintFiles.java
M src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
M src/main/java/com/android/tools/r8/graph/JarClassFileReader.java
M src/main/java/com/android/tools/r8/horizontalclassmerging/ConstructorMerger.java
M src/main/java/com/android/tools/r8/horizontalclassmerging/VirtualMethodMerger.java
M src/main/java/com/android/tools/r8/ir/optimize/enums/EnumUnboxingRewriter.java
M src/main/java/com/android/tools/r8/shaking/VerticalClassMerger.java

https://r8-review.googlesource.com/54062

sg...@google.com <sg...@google.com> Oct 18, 2021 08:56AM

Marked as fixed.