[HSG] Allow class directive without policy
In some cases, we want to add a policy for a member (field/method) in
the policy file for a class that already has an annotation.
Previously, we had to always have some policy in the class line, which
could cause confusion because this will essentially override the
annotation, so even when you change the class annotation, that wouldn't
be picked up.
Now update the parser so that a class directive is allowed to omit
a policy.
Also fix b/388562869. But I still need to ignore certain lines in diff
because the merged golden files are already missing the expected lines.
Flag: EXEMPT host test change only
Bug: 292141694
Bug: 388562869
Test: $ANDROID_BUILD_TOP/frameworks/base/ravenwood/scripts/run-ravenwood-tests.sh -s
Change-Id: Ifa37cd0ff74026bc1ba2c7e8599b159fa0a07412
diff --git a/ravenwood/texts/ravenwood-framework-policies.txt b/ravenwood/texts/ravenwood-framework-policies.txt
index 80126df..26b6fe3 100644
--- a/ravenwood/texts/ravenwood-framework-policies.txt
+++ b/ravenwood/texts/ravenwood-framework-policies.txt
@@ -52,7 +52,7 @@
class android.content.Context keep
method <init> ()V keep
method getSystemService (Ljava/lang/Class;)Ljava/lang/Object; keep
-class android.content.pm.PackageManager keep
+class android.content.pm.PackageManager
method <init> ()V keep
class android.text.ClipboardManager keep
method <init> ()V keep
diff --git a/ravenwood/tools/hoststubgen/scripts/dump-jar b/ravenwood/tools/hoststubgen/scripts/dump-jar
index 998357b..02a6d25 100755
--- a/ravenwood/tools/hoststubgen/scripts/dump-jar
+++ b/ravenwood/tools/hoststubgen/scripts/dump-jar
@@ -89,14 +89,33 @@
# - Some other transient lines
# - Sometimes the javap shows mysterious warnings, so remove them too.
#
- # `/PATTERN-1/,/PATTERN-2/{//!d}` is a trick to delete lines between two patterns, without
- # the start and the end lines.
+ # Most conversion are simple per-line deletion or simple inline replacement with a regex.
+ #
+ # But removing the constant pool is a bit tricky. It looks like this in the output:
+ #---------------------------
+ #Constant pool:
+ # #1 = Methodref #31.#88 // java/lang/Object."<init>":()V
+ # #2 = Class #89 // java/lang/UnsupportedOperationException
+ # :
+ #{ // Or something, I'm not sure if it always ends with a "{".
+ #---------------------------
+ # i.e. we want to delete all lines from "Constant pool:" as long as the first character
+ # is a space.
+ #
+ # If we simply use '/^Constant pool:/,/^[^ ]/d', then it'll delete the "Constant pool:"
+ # line and "{" line too, but again the last line might be important, so we don't want to
+ # delete it.
+ #
+ # So we instead, use '/^Constant pool:/,/^[^ ]/{/^ /d}', which mean:
+ # between lines matching '/^Constant pool:/' and '/^[^ ]/', delete lines that start with
+ # a space. (=='/^ /d').
+ #
sed -e 's/#[0-9][0-9]*/#x/g' \
-e 's/^\( *\)[0-9][0-9]*:/\1x:/' \
- -e '/^Constant pool:/,/^[^ ]/{//!d}' \
+ -e '/^Constant pool:/,/^[^ ]/{/^ /d}' \
-e '/^ *line *[0-9][0-9]*: *[0-9][0-9]*$/d' \
- -e '/SHA-256 checksum/d' \
- -e '/Last modified/d' \
+ -e '/^ *SHA-256 checksum/d' \
+ -e '/^ *Last modified/d' \
-e '/^Classfile jar/d' \
-e '/\[warning\]/d'
else
diff --git a/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt b/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
index be1b6ca..c550083 100644
--- a/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
+++ b/ravenwood/tools/hoststubgen/src/com/android/hoststubgen/filters/TextFileFilterPolicyParser.kt
@@ -413,8 +413,8 @@
}
private fun parseClass(fields: Array<String>) {
- if (fields.size < 3) {
- throw ParseException("Class ('c') expects 2 fields.")
+ if (fields.size <= 1) {
+ throw ParseException("Class ('c') expects 1 or 2 fields.")
}
val className = fields[1]
@@ -424,7 +424,9 @@
// :aidl, etc?
val classType = resolveSpecialClass(className)
- if (fields[2].startsWith("!")) {
+ val policyStr = if (fields.size > 2) { fields[2] } else { "" }
+
+ if (policyStr.startsWith("!")) {
if (classType != SpecialClass.NotSpecial) {
// We could support it, but not needed at least for now.
throw ParseException(
@@ -432,10 +434,10 @@
)
}
// It's a redirection class.
- val toClass = fields[2].substring(1)
+ val toClass = policyStr.substring(1)
processor.onRedirectionClass(className, toClass)
- } else if (fields[2].startsWith("~")) {
+ } else if (policyStr.startsWith("~")) {
if (classType != SpecialClass.NotSpecial) {
// We could support it, but not needed at least for now.
throw ParseException(
@@ -443,11 +445,23 @@
)
}
// It's a class-load hook
- val callback = fields[2].substring(1)
+ val callback = policyStr.substring(1)
processor.onClassLoadHook(className, callback)
} else {
- val policy = parsePolicy(fields[2])
+ // Special case: if it's a class directive with no policy, then it encloses
+ // members, but we don't apply any policy to the class itself.
+ // This is only allowed in a not-special case.
+ if (policyStr == "") {
+ if (classType == SpecialClass.NotSpecial && superClass == null) {
+ currentClassName = className
+ processor.onSimpleClassStart(className)
+ return
+ }
+ throw ParseException("Special class or subclass directive must have a policy")
+ }
+
+ val policy = parsePolicy(policyStr)
if (!policy.isUsableWithClasses) {
throw ParseException("Class can't have policy '$policy'")
}
diff --git a/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh b/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
index 8408a18..23699fd 100755
--- a/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
+++ b/ravenwood/tools/hoststubgen/test-tiny-framework/diff-and-update-golden.sh
@@ -37,8 +37,7 @@
GOLDEN_DIR=${GOLDEN_DIR:-golden-output}
mkdir -p $GOLDEN_DIR
-# TODO(b/388562869) We shouldn't need `--ignore-matching-lines`, but the golden files may not have the "Constant pool:" lines.
-DIFF_CMD=${DIFF_CMD:-diff -u --ignore-blank-lines --ignore-space-change --ignore-matching-lines='^\(Constant.pool:\|{\)$'}
+DIFF_CMD=${DIFF_CMD:-./tiny-framework-dump-test.py run-diff}
update=0
three_way=0
@@ -63,12 +62,10 @@
shift $(($OPTIND - 1))
# Build the dump files, which are the input of this test.
-run ${BUILD_CMD:=m} dump-jar tiny-framework-dump-test
-
+run ${BUILD_CMD:-m} dump-jar tiny-framework-dump-test
# Get the path to the generate text files. (not the golden files.)
# We get them from $OUT/module-info.json
-
files=(
$(python3 -c '
import sys
@@ -78,7 +75,7 @@
with open(sys.argv[1], "r") as f:
data = json.load(f)
- # Equivalent to: jq -r '.["tiny-framework-dump-test"]["installed"][]'
+ # Equivalent to: jq -r '.["tiny-framework-dump-test"]["installed"][]'
for path in data["tiny-framework-dump-test"]["installed"]:
if "golden-output" in path:
diff --git a/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py b/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
index c35d6d1..7617482 100755
--- a/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
+++ b/ravenwood/tools/hoststubgen/test-tiny-framework/tiny-framework-dump-test.py
@@ -28,9 +28,16 @@
# Run diff.
def run_diff(file1, file2):
- # TODO(b/388562869) We shouldn't need `--ignore-matching-lines`, but the golden files may not have the "Constant pool:" lines.
- command = ['diff', '-u', '--ignore-blank-lines',
+ command = ['diff', '-u',
+ '--ignore-blank-lines',
'--ignore-space-change',
+
+ # Ignore the class file version.
+ '--ignore-matching-lines=^ *\(major\|minor\) version:$',
+
+ # We shouldn't need `--ignore-matching-lines`, but somehow
+ # the golden files were generated without these lines for b/388562869,
+ # so let's just ignore them.
'--ignore-matching-lines=^\(Constant.pool:\|{\)$',
file1, file2]
print(' '.join(command))
@@ -85,4 +92,13 @@
if __name__ == "__main__":
+ args = sys.argv
+
+ # This script is used by diff-and-update-golden.sh too.
+ if len(args) > 1 and args[1] == "run-diff":
+ if run_diff(args[2], args[3]):
+ sys.exit(0)
+ else:
+ sys.exit(1)
+
unittest.main(verbosity=2)