From c8a04dca66f78307db54c187f236d1b5eefc92ba Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Tue, 21 Nov 2023 09:52:24 -0800 Subject: HostStubGen: Stub generation is now optional, etc - Now both --out-stub-jar and --out-impl-jar are optional - Disable --enable-non-stub-method-check by default (it's not fully implemented anyway) - Delete the script `run-ravenwood-test` since now atest just works. Test: run-all-tests.sh Bug: 292141694 Change-Id: I3b7d63600139425e5fffc12930ee860edf2acd7f --- Ravenwood.bp | 2 - .../nativesubstitution/Parcel_host.java | 7 +- .../invoketest/hoststubgen-invoke-test.sh | 51 +++++++- .../src/com/android/hoststubgen/HostStubGen.kt | 37 +++--- .../com/android/hoststubgen/HostStubGenOptions.kt | 17 +-- .../hoststubgen/test-framework/README.md | 6 - .../hoststubgen/test-tiny-framework/README.md | 6 - tools/hoststubgen/scripts/Android.bp | 6 - tools/hoststubgen/scripts/run-all-tests.sh | 6 +- tools/hoststubgen/scripts/run-ravenwood-test | 129 --------------------- 10 files changed, 83 insertions(+), 184 deletions(-) delete mode 100755 tools/hoststubgen/scripts/run-ravenwood-test diff --git a/Ravenwood.bp b/Ravenwood.bp index 03a23ba15273..ca73378c8e81 100644 --- a/Ravenwood.bp +++ b/Ravenwood.bp @@ -32,7 +32,6 @@ java_genrule { cmd: "$(location hoststubgen) " + "@$(location ravenwood/ravenwood-standard-options.txt) " + - "--out-stub-jar $(location ravenwood_stub.jar) " + "--out-impl-jar $(location ravenwood.jar) " + "--gen-keep-all-file $(location hoststubgen_keep_all.txt) " + @@ -49,7 +48,6 @@ java_genrule { ], out: [ "ravenwood.jar", - "ravenwood_stub.jar", // It's not used. TODO: Update hoststubgen to make it optional. // Following files are created just as FYI. "hoststubgen_keep_all.txt", diff --git a/tools/hoststubgen/hoststubgen/helper-framework-runtime-src/framework/com/android/hoststubgen/nativesubstitution/Parcel_host.java b/tools/hoststubgen/hoststubgen/helper-framework-runtime-src/framework/com/android/hoststubgen/nativesubstitution/Parcel_host.java index 4a3a79803b65..668c94c0f91c 100644 --- a/tools/hoststubgen/hoststubgen/helper-framework-runtime-src/framework/com/android/hoststubgen/nativesubstitution/Parcel_host.java +++ b/tools/hoststubgen/hoststubgen/helper-framework-runtime-src/framework/com/android/hoststubgen/nativesubstitution/Parcel_host.java @@ -27,9 +27,10 @@ import java.util.concurrent.atomic.AtomicLong; /** * Tentative, partial implementation of the Parcel native methods, using Java's - * {@link ByteBuffer}. It turned out there's enough semantics differences between Parcel - * and {@link ByteBuffer}, so it didn't actually work. - * (e.g. Parcel seems to allow moving the data position to be beyond its size? Which + * {@code byte[]}. + * (We don't use a {@link ByteBuffer} because there's enough semantics differences between Parcel + * and {@link ByteBuffer}, and it didn't work out. + * e.g. Parcel seems to allow moving the data position to be beyond its size? Which * {@link ByteBuffer} wouldn't allow...) */ public class Parcel_host { diff --git a/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh b/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh index 89daa2084420..85038be80c51 100755 --- a/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh +++ b/tools/hoststubgen/hoststubgen/invoketest/hoststubgen-invoke-test.sh @@ -13,6 +13,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +# This command is expected to be executed with: atest hoststubgen-invoke-test + set -e # Exit when any command files # This script runs HostStubGen directly with various arguments and make sure @@ -35,6 +37,12 @@ if [[ "$TEMP" == "" ]] ; then mkdir -p $TEMP fi +cleanup_temp() { + rm -fr $TEMP/* +} + +cleanup_temp + JAR=hoststubgen-test-tiny-framework.jar STUB=$TEMP/stub.jar IMPL=$TEMP/impl.jar @@ -47,12 +55,10 @@ HOSTSTUBGEN_OUT=$TEMP/output.txt # HostStubGen result in it. HOSTSTUBGEN_RC=0 -# Define the functions to - - # Note, because the build rule will only install hoststubgen.jar, but not the wrapper script, # we need to execute it manually with the java command. hoststubgen() { + echo "Running hoststubgen with: $*" java -jar ./hoststubgen.jar "$@" } @@ -62,7 +68,7 @@ run_hoststubgen() { echo "# Test: $test_name" - rm -f $HOSTSTUBGEN_OUT + cleanup_temp local filter_arg="" @@ -73,11 +79,21 @@ run_hoststubgen() { cat $ANNOTATION_FILTER fi + local stub_arg="" + local impl_arg="" + + if [[ "$STUB" != "" ]] ; then + stub_arg="--out-stub-jar $STUB" + fi + if [[ "$IMPL" != "" ]] ; then + impl_arg="--out-impl-jar $IMPL" + fi + hoststubgen \ --debug \ --in-jar $JAR \ - --out-stub-jar $STUB \ - --out-impl-jar $IMPL \ + $stub_arg \ + $impl_arg \ --stub-annotation \ android.hosttest.annotation.HostSideTestStub \ --keep-annotation \ @@ -105,6 +121,21 @@ run_hoststubgen() { return 0 } +assert_file_generated() { + local file="$1" + if [[ "$file" == "" ]] ; then + if [[ -f "$file" ]] ; then + echo "HostStubGen shouldn't have generated $file" + return 1 + fi + else + if ! [[ -f "$file" ]] ; then + echo "HostStubGen didn't generate $file" + return 1 + fi + fi +} + run_hoststubgen_for_success() { run_hoststubgen "$@" @@ -112,6 +143,9 @@ run_hoststubgen_for_success() { echo "HostStubGen expected to finish successfully, but failed with $rc" return 1 fi + + assert_file_generated "$STUB" + assert_file_generated "$IMPL" } run_hoststubgen_for_failure() { @@ -189,6 +223,11 @@ run_hoststubgen_for_success "One specific class disallowed, but it doesn't use a * # All other classes allowed " +STUB="" run_hoststubgen_for_success "No stub generation" "" + +IMPL="" run_hoststubgen_for_success "No impl generation" "" + +STUB="" IMPL="" run_hoststubgen_for_success "No stub, no impl generation" "" echo "All tests passed" diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt index 07bd2dc7c867..3cdddc23b332 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGen.kt @@ -237,8 +237,8 @@ class HostStubGen(val options: HostStubGenOptions) { */ private fun convert( inJar: String, - outStubJar: String, - outImplJar: String, + outStubJar: String?, + outImplJar: String?, filter: OutputFilter, enableChecker: Boolean, classes: ClassNodes, @@ -254,8 +254,8 @@ class HostStubGen(val options: HostStubGenOptions) { log.withIndent { // Open the input jar file and process each entry. ZipFile(inJar).use { inZip -> - ZipOutputStream(FileOutputStream(outStubJar)).use { stubOutStream -> - ZipOutputStream(FileOutputStream(outImplJar)).use { implOutStream -> + maybeWithZipOutputStream(outStubJar) { stubOutStream -> + maybeWithZipOutputStream(outImplJar) { implOutStream -> val inEntries = inZip.entries() while (inEntries.hasMoreElements()) { val entry = inEntries.nextElement() @@ -265,22 +265,29 @@ class HostStubGen(val options: HostStubGenOptions) { log.i("Converted all entries.") } } - log.i("Created stub: $outStubJar") - log.i("Created impl: $outImplJar") + outStubJar?.let { log.i("Created stub: $it") } + outImplJar?.let { log.i("Created impl: $it") } } } val end = System.currentTimeMillis() log.v("Done transforming the jar in %.1f second(s).", (end - start) / 1000.0) } + private fun maybeWithZipOutputStream(filename: String?, block: (ZipOutputStream?) -> T): T { + if (filename == null) { + return block(null) + } + return ZipOutputStream(FileOutputStream(filename)).use(block) + } + /** * Convert a single ZIP entry, which may or may not be a class file. */ private fun convertSingleEntry( inZip: ZipFile, entry: ZipEntry, - stubOutStream: ZipOutputStream, - implOutStream: ZipOutputStream, + stubOutStream: ZipOutputStream?, + implOutStream: ZipOutputStream?, filter: OutputFilter, packageRedirector: PackageRedirectRemapper, enableChecker: Boolean, @@ -316,8 +323,8 @@ class HostStubGen(val options: HostStubGenOptions) { // Unknown type, we just copy it to both output zip files. // TODO: We probably shouldn't do it for stub jar? log.v("Copying: %s", entry.name) - copyZipEntry(inZip, entry, stubOutStream) - copyZipEntry(inZip, entry, implOutStream) + stubOutStream?.let { copyZipEntry(inZip, entry, it) } + implOutStream?.let { copyZipEntry(inZip, entry, it) } } } @@ -346,8 +353,8 @@ class HostStubGen(val options: HostStubGenOptions) { private fun processSingleClass( inZip: ZipFile, entry: ZipEntry, - stubOutStream: ZipOutputStream, - implOutStream: ZipOutputStream, + stubOutStream: ZipOutputStream?, + implOutStream: ZipOutputStream?, filter: OutputFilter, packageRedirector: PackageRedirectRemapper, enableChecker: Boolean, @@ -361,7 +368,7 @@ class HostStubGen(val options: HostStubGenOptions) { return } // Generate stub first. - if (classPolicy.policy.needsInStub) { + if (stubOutStream != null && classPolicy.policy.needsInStub) { log.v("Creating stub class: %s Policy: %s", classInternalName, classPolicy) log.withIndent { BufferedInputStream(inZip.getInputStream(entry)).use { bis -> @@ -374,8 +381,8 @@ class HostStubGen(val options: HostStubGenOptions) { } } } - log.v("Creating impl class: %s Policy: %s", classInternalName, classPolicy) - if (classPolicy.policy.needsInImpl) { + if (implOutStream != null && classPolicy.policy.needsInImpl) { + log.v("Creating impl class: %s Policy: %s", classInternalName, classPolicy) log.withIndent { BufferedInputStream(inZip.getInputStream(entry)).use { bis -> val newEntry = ZipEntry(entry.name) diff --git a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt index da5348707528..83f873d38f1b 100644 --- a/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt +++ b/tools/hoststubgen/hoststubgen/src/com/android/hoststubgen/HostStubGenOptions.kt @@ -28,10 +28,10 @@ class HostStubGenOptions( var inJar: String = "", /** Output stub jar file */ - var outStubJar: String = "", + var outStubJar: String? = null, /** Output implementation jar file */ - var outImplJar: String = "", + var outImplJar: String? = null, var inputJarDumpFile: String? = null, @@ -71,7 +71,7 @@ class HostStubGenOptions( var enablePreTrace: Boolean = false, var enablePostTrace: Boolean = false, - var enableNonStubMethodCallDetection: Boolean = true, + var enableNonStubMethodCallDetection: Boolean = false, ) { companion object { @@ -209,11 +209,14 @@ class HostStubGenOptions( if (ret.inJar.isEmpty()) { throw ArgumentsException("Required option missing: --in-jar") } - if (ret.outStubJar.isEmpty()) { - throw ArgumentsException("Required option missing: --out-stub-jar") + if (ret.outStubJar == null && ret.outImplJar == null) { + log.w("Neither --out-stub-jar nor --out-impl-jar is set." + + " $COMMAND_NAME will not generate jar files.") } - if (ret.outImplJar.isEmpty()) { - throw ArgumentsException("Required option missing: --out-impl-jar") + + if (ret.enableNonStubMethodCallDetection) { + log.w("--enable-non-stub-method-check is not fully implemented yet." + + " See the todo in doesMethodNeedNonStubCallCheck().") } return ret diff --git a/tools/hoststubgen/hoststubgen/test-framework/README.md b/tools/hoststubgen/hoststubgen/test-framework/README.md index 20e2f873b152..f616ad61d219 100644 --- a/tools/hoststubgen/hoststubgen/test-framework/README.md +++ b/tools/hoststubgen/hoststubgen/test-framework/README.md @@ -14,12 +14,6 @@ tests. $ atest --no-bazel-mode HostStubGenTest-framework-test-host-test ``` -- With `run-ravenwood-test` - -``` -$ run-ravenwood-test HostStubGenTest-framework-test-host-test -``` - - Advanced option: `run-test-without-atest.sh` runs the test without using `atest` or `run-ravenwood-test` ``` diff --git a/tools/hoststubgen/hoststubgen/test-tiny-framework/README.md b/tools/hoststubgen/hoststubgen/test-tiny-framework/README.md index f3c0450d42a3..3bfad9bd673b 100644 --- a/tools/hoststubgen/hoststubgen/test-tiny-framework/README.md +++ b/tools/hoststubgen/hoststubgen/test-tiny-framework/README.md @@ -13,12 +13,6 @@ This test doesn't use the actual android framework code. $ atest hoststubgen-test-tiny-test ``` -- With `run-ravenwood-test` should work too. This is the proper way to run it. - -``` -$ run-ravenwood-test hoststubgen-test-tiny-test -``` - - `run-test-manually.sh` also run the test, but it builds the stub/impl jars and the test without using the build system. This is useful for debugging the tool. diff --git a/tools/hoststubgen/scripts/Android.bp b/tools/hoststubgen/scripts/Android.bp index 5da805e5640e..b1ba07ec540d 100644 --- a/tools/hoststubgen/scripts/Android.bp +++ b/tools/hoststubgen/scripts/Android.bp @@ -18,9 +18,3 @@ genrule_defaults { tools: ["dump-jar"], cmd: "$(location dump-jar) -s -o $(out) $(in)", } - -sh_binary_host { - name: "run-ravenwood-test", - src: "run-ravenwood-test", - visibility: ["//visibility:public"], -} diff --git a/tools/hoststubgen/scripts/run-all-tests.sh b/tools/hoststubgen/scripts/run-all-tests.sh index 2dac08969d44..82faa91e2cac 100755 --- a/tools/hoststubgen/scripts/run-all-tests.sh +++ b/tools/hoststubgen/scripts/run-all-tests.sh @@ -22,10 +22,10 @@ cd .. READY_TEST_MODULES=( HostStubGenTest-framework-all-test-host-test hoststubgen-test-tiny-test + CtsUtilTestCasesRavenwood ) MUST_BUILD_MODULES=( - run-ravenwood-test "${NOT_READY_TEST_MODULES[*]}" HostStubGenTest-framework-test ) @@ -51,8 +51,6 @@ run ./scripts/build-framework-hostside-jars-and-extract.sh # run ./scripts/build-framework-hostside-jars-without-genrules.sh # These tests should all pass. -run-ravenwood-test ${READY_TEST_MODULES[*]} - -run atest CtsUtilTestCasesRavenwood +run atest ${READY_TEST_MODULES[*]} echo ""${0##*/}" finished, with no unexpected failures. Ready to submit!" \ No newline at end of file diff --git a/tools/hoststubgen/scripts/run-ravenwood-test b/tools/hoststubgen/scripts/run-ravenwood-test deleted file mode 100755 index 9bbb859f5c3d..000000000000 --- a/tools/hoststubgen/scripts/run-ravenwood-test +++ /dev/null @@ -1,129 +0,0 @@ -#!/bin/bash -# 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. - -set -e - -# Script to run a "Ravenwood" host side test. -# -# A proper way to run these tests is to use `atest`, but `atest` has a known issue of loading -# unrelated jar files as the class path, so for now we use this script to run host side tests. - -# Copy (with some changes) some functions from ../common.sh, so this script can be used without it. - -m() { - if (( $SKIP_BUILD )) ; then - echo "Skipping build: $*" 1>&2 - return 0 - fi - run ${ANDROID_BUILD_TOP}/build/soong/soong_ui.bash --make-mode "$@" -} - -run() { - echo "Running: $*" 1>&2 - "$@" -} - -run_junit_test_jar() { - local jar="$1" - echo "Starting test: $jar ..." - run cd "${jar%/*}" - - run ${JAVA:-java} $JAVA_OPTS \ - -cp $jar \ - org.junit.runner.JUnitCore \ - com.android.hoststubgen.hosthelper.HostTestSuite || return 1 - return 0 -} - -help() { - cat <<'EOF' - - run-ravenwood-test -- Run Ravenwood host tests - - Usage: - run-ravenwood-test [options] MODULE-NAME ... - - Options: - -h: Help - -t: Run test only, without building - -b: Build only, without running - - Example: - run-ravenwood-test HostStubGenTest-framework-test-host-test - -EOF -} - -#------------------------------------------------------------------------- -# Parse options -#------------------------------------------------------------------------- -build=0 -test=0 - -while getopts "htb" opt; do - case "$opt" in - h) help; exit 0 ;; - t) - test=1 - ;; - b) - build=1 - ;; - esac -done -shift $(($OPTIND - 1)) - -# If neither -t nor -b is provided, then build and run./ -if (( ( $build + $test ) == 0 )) ; then - build=1 - test=1 -fi - - -modules=("${@}") - -if (( "${#modules[@]}" == 0 )); then - help - exit 1 -fi - -#------------------------------------------------------------------------- -# Build -#------------------------------------------------------------------------- -if (( $build )) ; then - run m "${modules[@]}" -fi - -#------------------------------------------------------------------------- -# Run -#------------------------------------------------------------------------- - -failures=0 -if (( test )) ; then - for module in "${modules[@]}"; do - if run_junit_test_jar "$ANDROID_BUILD_TOP/out/host/linux-x86/testcases/${module}/${module}.jar"; then - : # passed. - else - failures=$(( failures + 1 )) - fi - done - - if (( $failures > 0 )) ; then - echo "$failures test jar(s) failed." 1>&2 - exit 2 - fi -fi - -exit 0 \ No newline at end of file -- cgit v1.2.3-59-g8ed1b