Version support #59
Version support #59
Conversation
| implementation 'com.google.mug:mug-guava:6.6' | ||
| implementation 'commons-cli:commons-cli:1.5.0' | ||
| implementation 'com.android.tools.apkparser:binary-resources:31.4.0' | ||
| implementation 'com.android.tools.apkparser:binary-resources:31.7.3' |
There was a problem hiding this comment.
Should move to toml in a follow up
| @@ -102,8 +103,9 @@ class EvaluationSettings( | |||
| var estimateOptimization: Boolean = false | |||
| private set | |||
|
|
|||
| val isHoneyfaceMode | |||
| get() = schemaVersion == HONEYFACE_VERSION | |||
| @get:JvmName("isHoneyfaceMode") | |||
There was a problem hiding this comment.
Is backwards compatible required?
There was a problem hiding this comment.
Not sure, I thought it safest to leave it as an option, @alexclarke-g will know best.
| @@ -268,6 +267,9 @@ class EvaluationSettings( | |||
| if (line.hasOption(reportModeOption)) { | |||
| evaluationSettings.reportMode = true | |||
| } | |||
| if (line.hasOption(honeyfaceOption)) { | |||
There was a problem hiding this comment.
Is honey face explained somewhere?
There was a problem hiding this comment.
Not to my knowledge, it predates WFF.
There was a problem hiding this comment.
Yes, honeyface predates WFF. We keep support for it because we wanted to perform some estimations on older watch faces and we may need it in the future.
...tions/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/AndroidManifest.kt
Outdated
Show resolved
Hide resolved
...tions/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/AndroidManifest.kt
Outdated
Show resolved
Hide resolved
...tions/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/AndroidManifest.kt
Outdated
Show resolved
Hide resolved
...tions/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/AndroidManifest.kt
Outdated
Show resolved
Hide resolved
...ns/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/EvaluationSettings.kt
Outdated
Show resolved
Hide resolved
...ns/memory-footprint/src/main/java/com/google/wear/watchface/dfx/memory/EvaluationSettings.kt
Show resolved
Hide resolved
...ry-footprint/src/main/java/com/google/wear/watchface/dfx/memory/ResourceMemoryEvaluator.java
Outdated
Show resolved
Hide resolved
|
Thanks @lucianbc, I've addressed those comments, PTAL. |
| val wffVersion = | ||
| getAttribute(doc, manifestProto, "//property/@android:value").toInt() |
There was a problem hiding this comment.
The manifest may contain more than one , so you'll have to filter by name. Also, technically, there's no guarantee that such a property will be there and indeed that's not the case for honeyface watch faces. To fix this, I suggest this approach:
- throw an IllegalArgumentException if no such property exists
- only parse the AndroidManifest.xml if necessary (next comment).
To filter by name, you can use:
val wffVersion = getAttribute(doc, "//property[@android:name=\"com.google.wear.watchface.format.version\"]/@android:value").toIntOrNull()
requireNotNull(wffVersion) {
"Watch Face Manifest must have a property with name \"com.google.wear.watchface.format.version\" that specifies the version of the Watch Face Format to be used."
}
You may save the XPath in a constant and use it in the loadFromPlainXml method as well.
| @@ -112,11 +112,25 @@ private static void evaluateInHumanReadableMode(EvaluationSettings settings) { | |||
| */ | |||
| static List<MemoryFootprint> evaluateMemoryFootprint(EvaluationSettings evaluationSettings) { | |||
| try (InputPackage inputPackage = InputPackage.open(evaluationSettings.getWatchFacePath())) { | |||
|
|
|||
| AndroidManifest manifest = inputPackage.getManifest(); | |||
There was a problem hiding this comment.
Move this to line 122, so that it executes only if the --schema-version is not homeyface. If it is honeyface, then we won't throw in case the is not defined.
What
Adds support for extracting WFF schema version from
AndroidManifest.xmlfor use in validating the watch face package.Why
The current version of the memory footprint evaluator relies on the
--schema-versioncommand-line argument in order to know which version of the schema to validate against.However, the actual schema used in the watch face is specified in the
AndroidManifest.xmlfile in thecom.google.wear.watchface.format.versionproperty.A user could mistakenly be specifying a command-line value for the version, which is not the version of the schema in the manifest, and the resulting evaluation could therefore be incorrect.
How
Adds a
getManifest()method toInputPackagewhich returns a new classAndroidManifest, containing useful details such as WFF version.AndroidManifestcan be created from AAB, APK and plain file-based sources.--schema-versionis changed from a required to an optional argument, and where a mismatch is detected between CLI and manifest version, a warning is printed, but the CLI version is used.