HostStubGen: Prevent conflicting command line args
Also remove unused option
Bug: 292141694
Test: run-all-tests.sh
Change-Id: I32c663100362c9bf19ed42c6453bdf097bb20b49
diff --git a/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh b/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh
index 85038be..91e6814 100755
--- a/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh
+++ b/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh
@@ -51,6 +51,8 @@
HOSTSTUBGEN_OUT=$TEMP/output.txt
+EXTRA_ARGS=""
+
# Because of `set -e`, we can't return non-zero from functions, so we store
# HostStubGen result in it.
HOSTSTUBGEN_RC=0
@@ -115,6 +117,7 @@
--keep-static-initializer-annotation \
android.hosttest.annotation.HostSideTestStaticInitializerKeep \
$filter_arg \
+ $EXTRA_ARGS \
|& tee $HOSTSTUBGEN_OUT
HOSTSTUBGEN_RC=${PIPESTATUS[0]}
echo "HostStubGen exited with $HOSTSTUBGEN_RC"
@@ -209,7 +212,6 @@
com.unsupported.*
"
-
run_hoststubgen_for_failure "One specific class disallowed" \
"TinyFrameworkClassAnnotations is not allowed to have Ravenwood annotations" \
"
@@ -229,6 +231,14 @@
STUB="" IMPL="" run_hoststubgen_for_success "No stub, no impl generation" ""
+EXTRA_ARGS="--in-jar abc" run_hoststubgen_for_failure "Duplicate arg" \
+ "Duplicate or conflicting argument found: --in-jar" \
+ ""
+
+EXTRA_ARGS="--quiet" run_hoststubgen_for_failure "Conflicting arg" \
+ "Duplicate or conflicting argument found: --quiet" \
+ ""
+
echo "All tests passed"
exit 0
\ No newline at end of file
diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt
index 3cdddc2..dbcf3a5 100644
--- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt
+++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt
@@ -22,7 +22,6 @@
import com.android.hoststubgen.filters.DefaultHookInjectingFilter
import com.android.hoststubgen.filters.FilterPolicy
import com.android.hoststubgen.filters.ImplicitOutputFilter
-import com.android.hoststubgen.filters.KeepAllClassesFilter
import com.android.hoststubgen.filters.OutputFilter
import com.android.hoststubgen.filters.StubIntersectingFilter
import com.android.hoststubgen.filters.createFilterFromTextPolicyFile
@@ -52,15 +51,15 @@
val errors = HostStubGenErrors()
// Load all classes.
- val allClasses = loadClassStructures(options.inJar)
+ val allClasses = loadClassStructures(options.inJar.get)
// Dump the classes, if specified.
- options.inputJarDumpFile?.let {
+ options.inputJarDumpFile.ifSet {
PrintWriter(it).use { pw -> allClasses.dump(pw) }
log.i("Dump file created at $it")
}
- options.inputJarAsKeepAllFile?.let {
+ options.inputJarAsKeepAllFile.ifSet {
PrintWriter(it).use {
pw -> allClasses.forEach {
classNode -> printAsTextPolicy(pw, classNode)
@@ -74,11 +73,11 @@
// Transform the jar.
convert(
- options.inJar,
- options.outStubJar,
- options.outImplJar,
+ options.inJar.get,
+ options.outStubJar.get,
+ options.outImplJar.get,
filter,
- options.enableClassChecker,
+ options.enableClassChecker.get,
allClasses,
errors,
)
@@ -153,7 +152,7 @@
// text-file based filter, which is handled by parseTextFilterPolicyFile.
// The first filter is for the default policy from the command line options.
- var filter: OutputFilter = ConstantFilter(options.defaultPolicy, "default-by-options")
+ var filter: OutputFilter = ConstantFilter(options.defaultPolicy.get, "default-by-options")
// Next, we need a filter that resolves "class-wide" policies.
// This is used when a member (methods, fields, nested classes) don't get any polices
@@ -163,16 +162,16 @@
// Inject default hooks from options.
filter = DefaultHookInjectingFilter(
- options.defaultClassLoadHook,
- options.defaultMethodCallHook,
+ options.defaultClassLoadHook.get,
+ options.defaultMethodCallHook.get,
filter
)
- val annotationAllowedClassesFilter = options.annotationAllowedClassesFile.let { filename ->
- if (filename == null) {
+ val annotationAllowedClassesFilter = options.annotationAllowedClassesFile.get.let { file ->
+ if (file == null) {
ClassFilter.newNullFilter(true) // Allow all classes
} else {
- ClassFilter.loadFromFile(filename, false)
+ ClassFilter.loadFromFile(file, false)
}
}
@@ -196,7 +195,7 @@
// Next, "text based" filter, which allows to override polices without touching
// the target code.
- options.policyOverrideFile?.let {
+ options.policyOverrideFile.ifSet {
filter = createFilterFromTextPolicyFile(it, allClasses, filter)
}
@@ -212,11 +211,6 @@
// Apply the implicit filter.
filter = ImplicitOutputFilter(errors, allClasses, filter)
- // Optionally keep all classes.
- if (options.keepAllClasses) {
- filter = KeepAllClassesFilter(filter)
- }
-
return filter
}
@@ -422,9 +416,9 @@
outVisitor = CheckClassAdapter(outVisitor)
}
val visitorOptions = BaseAdapter.Options(
- enablePreTrace = options.enablePreTrace,
- enablePostTrace = options.enablePostTrace,
- enableNonStubMethodCallDetection = options.enableNonStubMethodCallDetection,
+ enablePreTrace = options.enablePreTrace.get,
+ enablePostTrace = options.enablePostTrace.get,
+ enableNonStubMethodCallDetection = options.enableNonStubMethodCallDetection.get,
errors = errors,
)
outVisitor = BaseAdapter.getVisitor(classInternalName, classes, outVisitor, filter,
diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt
index 83f873d..0ae52af 100644
--- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt
+++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt
@@ -21,21 +21,60 @@
import java.io.FileReader
/**
+ * A single value that can only set once.
+ */
+class SetOnce<T>(
+ private var value: T,
+) {
+ class SetMoreThanOnceException : Exception()
+
+ private var set = false
+
+ fun set(v: T) {
+ if (set) {
+ throw SetMoreThanOnceException()
+ }
+ if (v == null) {
+ throw NullPointerException("This shouldn't happen")
+ }
+ set = true
+ value = v
+ }
+
+ val get: T
+ get() = this.value
+
+ val isSet: Boolean
+ get() = this.set
+
+ fun <R> ifSet(block: (T & Any) -> R): R? {
+ if (isSet) {
+ return block(value!!)
+ }
+ return null
+ }
+
+ override fun toString(): String {
+ return "$value"
+ }
+}
+
+/**
* Options that can be set from command line arguments.
*/
class HostStubGenOptions(
/** Input jar file*/
- var inJar: String = "",
+ var inJar: SetOnce<String> = SetOnce(""),
/** Output stub jar file */
- var outStubJar: String? = null,
+ var outStubJar: SetOnce<String?> = SetOnce(null),
/** Output implementation jar file */
- var outImplJar: String? = null,
+ var outImplJar: SetOnce<String?> = SetOnce(null),
- var inputJarDumpFile: String? = null,
+ var inputJarDumpFile: SetOnce<String?> = SetOnce(null),
- var inputJarAsKeepAllFile: String? = null,
+ var inputJarAsKeepAllFile: SetOnce<String?> = SetOnce(null),
var stubAnnotations: MutableSet<String> = mutableSetOf(),
var keepAnnotations: MutableSet<String> = mutableSetOf(),
@@ -51,27 +90,26 @@
var packageRedirects: MutableList<Pair<String, String>> = mutableListOf(),
- var annotationAllowedClassesFile: String? = null,
+ var annotationAllowedClassesFile: SetOnce<String?> = SetOnce(null),
- var defaultClassLoadHook: String? = null,
- var defaultMethodCallHook: String? = null,
+ var defaultClassLoadHook: SetOnce<String?> = SetOnce(null),
+ var defaultMethodCallHook: SetOnce<String?> = SetOnce(null),
var intersectStubJars: MutableSet<String> = mutableSetOf(),
- var policyOverrideFile: String? = null,
+ var policyOverrideFile: SetOnce<String?> = SetOnce(null),
- var defaultPolicy: FilterPolicy = FilterPolicy.Remove,
- var keepAllClasses: Boolean = false,
+ var defaultPolicy: SetOnce<FilterPolicy> = SetOnce(FilterPolicy.Remove),
- var logLevel: LogLevel = LogLevel.Info,
+ var logLevel: SetOnce<LogLevel> = SetOnce(LogLevel.Info),
- var cleanUpOnError: Boolean = false,
+ var cleanUpOnError: SetOnce<Boolean> = SetOnce(true),
- var enableClassChecker: Boolean = false,
- var enablePreTrace: Boolean = false,
- var enablePostTrace: Boolean = false,
+ var enableClassChecker: SetOnce<Boolean> = SetOnce(false),
+ var enablePreTrace: SetOnce<Boolean> = SetOnce(false),
+ var enablePostTrace: SetOnce<Boolean> = SetOnce(false),
- var enableNonStubMethodCallDetection: Boolean = false,
+ var enableNonStubMethodCallDetection: SetOnce<Boolean> = SetOnce(false),
) {
companion object {
@@ -111,110 +149,120 @@
break
}
- when (arg) {
- // TODO: Write help
- "-h", "--h" -> TODO("Help is not implemented yet")
+ // Define some shorthands...
+ fun nextArg(): String = ai.nextArgRequired(arg)
+ fun SetOnce<String>.setNextStringArg(): String = nextArg().also { this.set(it) }
+ fun SetOnce<String?>.setNextStringArg(): String = nextArg().also { this.set(it) }
+ fun MutableSet<String>.addUniqueAnnotationArg(): String =
+ nextArg().also { this += ensureUniqueAnnotation(it) }
- "-v", "--verbose" -> ret.logLevel = LogLevel.Verbose
- "-d", "--debug" -> ret.logLevel = LogLevel.Debug
- "-q", "--quiet" -> ret.logLevel = LogLevel.None
+ try {
+ when (arg) {
+ // TODO: Write help
+ "-h", "--help" -> TODO("Help is not implemented yet")
- "--in-jar" -> ret.inJar = ai.nextArgRequired(arg).ensureFileExists()
- "--out-stub-jar" -> ret.outStubJar = ai.nextArgRequired(arg)
- "--out-impl-jar" -> ret.outImplJar = ai.nextArgRequired(arg)
+ "-v", "--verbose" -> ret.logLevel.set(LogLevel.Verbose)
+ "-d", "--debug" -> ret.logLevel.set(LogLevel.Debug)
+ "-q", "--quiet" -> ret.logLevel.set(LogLevel.None)
- "--policy-override-file" ->
- ret.policyOverrideFile = ai.nextArgRequired(arg).ensureFileExists()
+ "--in-jar" -> ret.inJar.setNextStringArg().ensureFileExists()
+ "--out-stub-jar" -> ret.outStubJar.setNextStringArg()
+ "--out-impl-jar" -> ret.outImplJar.setNextStringArg()
- "--clean-up-on-error" -> ret.cleanUpOnError = true
- "--no-clean-up-on-error" -> ret.cleanUpOnError = false
+ "--policy-override-file" ->
+ ret.policyOverrideFile.setNextStringArg().ensureFileExists()
- "--default-remove" -> ret.defaultPolicy = FilterPolicy.Remove
- "--default-throw" -> ret.defaultPolicy = FilterPolicy.Throw
- "--default-keep" -> ret.defaultPolicy = FilterPolicy.Keep
- "--default-stub" -> ret.defaultPolicy = FilterPolicy.Stub
+ "--clean-up-on-error" -> ret.cleanUpOnError.set(true)
+ "--no-clean-up-on-error" -> ret.cleanUpOnError.set(false)
- "--keep-all-classes" -> ret.keepAllClasses = true
- "--no-keep-all-classes" -> ret.keepAllClasses = false
+ "--default-remove" -> ret.defaultPolicy.set(FilterPolicy.Remove)
+ "--default-throw" -> ret.defaultPolicy.set(FilterPolicy.Throw)
+ "--default-keep" -> ret.defaultPolicy.set(FilterPolicy.Keep)
+ "--default-stub" -> ret.defaultPolicy.set(FilterPolicy.Stub)
- "--stub-annotation" ->
- ret.stubAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--stub-annotation" ->
+ ret.stubAnnotations.addUniqueAnnotationArg()
- "--keep-annotation" ->
- ret.keepAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--keep-annotation" ->
+ ret.keepAnnotations.addUniqueAnnotationArg()
- "--stub-class-annotation" ->
- ret.stubClassAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--stub-class-annotation" ->
+ ret.stubClassAnnotations.addUniqueAnnotationArg()
- "--keep-class-annotation" ->
- ret.keepClassAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--keep-class-annotation" ->
+ ret.keepClassAnnotations.addUniqueAnnotationArg()
- "--throw-annotation" ->
- ret.throwAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--throw-annotation" ->
+ ret.throwAnnotations.addUniqueAnnotationArg()
- "--remove-annotation" ->
- ret.removeAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--remove-annotation" ->
+ ret.removeAnnotations.addUniqueAnnotationArg()
- "--substitute-annotation" ->
- ret.substituteAnnotations += ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--substitute-annotation" ->
+ ret.substituteAnnotations.addUniqueAnnotationArg()
- "--native-substitute-annotation" ->
- ret.nativeSubstituteAnnotations +=
- ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--native-substitute-annotation" ->
+ ret.nativeSubstituteAnnotations.addUniqueAnnotationArg()
- "--class-load-hook-annotation" ->
- ret.classLoadHookAnnotations +=
- ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--class-load-hook-annotation" ->
+ ret.classLoadHookAnnotations.addUniqueAnnotationArg()
- "--keep-static-initializer-annotation" ->
- ret.keepStaticInitializerAnnotations +=
- ensureUniqueAnnotation(ai.nextArgRequired(arg))
+ "--keep-static-initializer-annotation" ->
+ ret.keepStaticInitializerAnnotations.addUniqueAnnotationArg()
- "--package-redirect" ->
- ret.packageRedirects += parsePackageRedirect(ai.nextArgRequired(arg))
+ "--package-redirect" ->
+ ret.packageRedirects += parsePackageRedirect(ai.nextArgRequired(arg))
- "--annotation-allowed-classes-file" ->
- ret.annotationAllowedClassesFile = ai.nextArgRequired(arg)
+ "--annotation-allowed-classes-file" ->
+ ret.annotationAllowedClassesFile.setNextStringArg()
- "--default-class-load-hook" ->
- ret.defaultClassLoadHook = ai.nextArgRequired(arg)
+ "--default-class-load-hook" ->
+ ret.defaultClassLoadHook.setNextStringArg()
- "--default-method-call-hook" ->
- ret.defaultMethodCallHook = ai.nextArgRequired(arg)
+ "--default-method-call-hook" ->
+ ret.defaultMethodCallHook.setNextStringArg()
- "--intersect-stub-jar" ->
- ret.intersectStubJars += ai.nextArgRequired(arg).ensureFileExists()
+ "--intersect-stub-jar" ->
+ ret.intersectStubJars += nextArg().ensureFileExists()
- "--gen-keep-all-file" ->
- ret.inputJarAsKeepAllFile = ai.nextArgRequired(arg)
+ "--gen-keep-all-file" ->
+ ret.inputJarAsKeepAllFile.setNextStringArg()
- // Following options are for debugging.
- "--enable-class-checker" -> ret.enableClassChecker = true
- "--no-class-checker" -> ret.enableClassChecker = false
+ // Following options are for debugging.
+ "--enable-class-checker" -> ret.enableClassChecker.set(true)
+ "--no-class-checker" -> ret.enableClassChecker.set(false)
- "--enable-pre-trace" -> ret.enablePreTrace = true
- "--no-pre-trace" -> ret.enablePreTrace = false
+ "--enable-pre-trace" -> ret.enablePreTrace.set(true)
+ "--no-pre-trace" -> ret.enablePreTrace.set(false)
- "--enable-post-trace" -> ret.enablePostTrace = true
- "--no-post-trace" -> ret.enablePostTrace = false
+ "--enable-post-trace" -> ret.enablePostTrace.set(true)
+ "--no-post-trace" -> ret.enablePostTrace.set(false)
- "--enable-non-stub-method-check" -> ret.enableNonStubMethodCallDetection = true
- "--no-non-stub-method-check" -> ret.enableNonStubMethodCallDetection = false
+ "--enable-non-stub-method-check" ->
+ ret.enableNonStubMethodCallDetection.set(true)
- "--gen-input-dump-file" -> ret.inputJarDumpFile = ai.nextArgRequired(arg)
+ "--no-non-stub-method-check" ->
+ ret.enableNonStubMethodCallDetection.set(false)
- else -> throw ArgumentsException("Unknown option: $arg")
+ "--gen-input-dump-file" -> ret.inputJarDumpFile.setNextStringArg()
+
+ else -> throw ArgumentsException("Unknown option: $arg")
+ }
+ } catch (e: SetOnce.SetMoreThanOnceException) {
+ throw ArgumentsException("Duplicate or conflicting argument found: $arg")
}
}
- if (ret.inJar.isEmpty()) {
+ log.w(ret.toString())
+
+ if (!ret.inJar.isSet) {
throw ArgumentsException("Required option missing: --in-jar")
}
- if (ret.outStubJar == null && ret.outImplJar == null) {
+ if (!ret.outStubJar.isSet && !ret.outImplJar.isSet) {
log.w("Neither --out-stub-jar nor --out-impl-jar is set." +
" $COMMAND_NAME will not generate jar files.")
}
- if (ret.enableNonStubMethodCallDetection) {
+ if (ret.enableNonStubMethodCallDetection.get) {
log.w("--enable-non-stub-method-check is not fully implemented yet." +
" See the todo in doesMethodNeedNonStubCallCheck().")
}
@@ -329,7 +377,6 @@
intersectStubJars=$intersectStubJars,
policyOverrideFile=$policyOverrideFile,
defaultPolicy=$defaultPolicy,
- keepAllClasses=$keepAllClasses,
logLevel=$logLevel,
cleanUpOnError=$cleanUpOnError,
enableClassChecker=$enableClassChecker,
diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/Main.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/Main.kt
index 0321d9d..38ba0cc 100644
--- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/Main.kt
+++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/Main.kt
@@ -28,9 +28,9 @@
try {
// Parse the command line arguments.
val options = HostStubGenOptions.parseArgs(args)
- clanupOnError = options.cleanUpOnError
+ clanupOnError = options.cleanUpOnError.get
- log.level = options.logLevel
+ log.level = options.logLevel.get
log.v("HostStubGen started")
log.v("Options: $options")
diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/KeepAllClassesFilter.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/KeepAllClassesFilter.kt
deleted file mode 100644
index 45dd38d1..0000000
--- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/filters/KeepAllClassesFilter.kt
+++ /dev/null
@@ -1,30 +0,0 @@
-/*
- * Copyright (C) 2023 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package com.android.hoststubgen.filters
-
-/**
- * An [OutputFilter] that keeps all classes by default. (but none of its members)
- *
- * We're not currently using it, but using it *might* make certain things easier. For example, with
- * this, all classes would at least be loadable.
- */
-class KeepAllClassesFilter(fallback: OutputFilter) : DelegatingFilter(fallback) {
- override fun getPolicyForClass(className: String): FilterPolicyWithReason {
- // If the default visibility wouldn't keep it, change it to "keep".
- val f = super.getPolicyForClass(className)
- return f.promoteToKeep("keep-all-classes")
- }
-}
\ No newline at end of file