diff --git a/src/test/java/com/google/devtools/build/android/ziputils/SplitterTest.java b/src/test/java/com/google/devtools/build/android/ziputils/SplitterTest.java index 3f24eda65efeaf..568db636ccb0ca 100644 --- a/src/test/java/com/google/devtools/build/android/ziputils/SplitterTest.java +++ b/src/test/java/com/google/devtools/build/android/ziputils/SplitterTest.java @@ -51,7 +51,7 @@ public void testAssign() { filter.add("dir" + i + ARCHIVE_FILE_SEPARATOR + "file" + i + CLASS_SUFFIX); } Splitter splitter = new Splitter(size + 1, input.size()); - splitter.assign(filter); + splitter.assignAllToCurrentShard(filter); splitter.nextShard(); output = new LinkedHashMap<>(); for (String path : input) { diff --git a/src/test/shell/BUILD b/src/test/shell/BUILD index f3054b8c6c8bf4..6ddf5da707e68f 100644 --- a/src/test/shell/BUILD +++ b/src/test/shell/BUILD @@ -29,6 +29,7 @@ gen_workspace_stanza( "rules_cc", "rules_java", "rules_license", + "rules_pkg", "rules_proto", ], template = "testenv.sh.tmpl", diff --git a/src/test/shell/bazel/android/BUILD b/src/test/shell/bazel/android/BUILD index 4a4d14d1f4672b..9921da218dd1f7 100644 --- a/src/test/shell/bazel/android/BUILD +++ b/src/test/shell/bazel/android/BUILD @@ -238,11 +238,14 @@ android_sh_test( name = "DexFileSplitter_synthetic_classes_test", size = "medium", srcs = ["DexFileSplitter_synthetic_classes_test.sh"], + # Fixes to DexMapper are not released yet. + create_test_with_released_tools = False, data = [ ":android_helper", "//external:android_sdk_for_testing", "//src/test/shell/bazel:test-deps", ], + shard_count = 2, tags = [ "no_windows", ], diff --git a/src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh b/src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh index bd70e9f4533437..44c4a00020f2ee 100755 --- a/src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh +++ b/src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh @@ -26,18 +26,20 @@ # Load the test setup defined in the parent directory CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + source "${CURRENT_DIR}/android_helper.sh" \ || { echo "android_helper.sh not found!" >&2; exit 1; } fail_if_no_android_sdk -source "${CURRENT_DIR}/../../integration_test_setup.sh" \ - || { echo "integration_test_setup.sh not found!" >&2; exit 1; } - resolve_android_toolchains "$1" -function test_DexFileSplitter_synthetic_classes_crossing_dexfiles() { - create_new_workspace - setup_android_sdk_support +function create_test_app() { + + inner_class_count="$1" + lambda_count="$2" + dex_shard_count="$3" mkdir -p java/com/testapp @@ -81,7 +83,7 @@ public class MainActivity extends Activity { } EOF - generate_java_file_with_many_synthetic_classes > java/com/testapp/BigLib.java + generate_java_file_with_many_synthetic_classes "$1" "$2" > java/com/testapp/BigLib.java cat > java/com/testapp/BUILD < $i," done @@ -139,4 +137,42 @@ function generate_java_file_with_many_synthetic_classes() { echo "}" } +function test_DexFileSplitter_synthetic_classes_crossing_dexfiles() { + create_new_workspace + setup_android_sdk_support + + # dex_shards default is 1 + create_test_app 21400 6000 1 + + bazel build java/com/testapp || fail "Test app should have built succesfully" + + dex_file_count=$(unzip -l bazel-bin/java/com/testapp/testapp.apk | grep "classes[0-9]*.dex" | wc -l) + if [[ ! "$dex_file_count" -ge "2" ]]; then + echo "Expected at least 2 dexes in app, found: $dex_file_count" + exit 1 + fi +} + +function test_DexMapper_synthetic_classes_crossing_dexfiles() { + create_new_workspace + setup_android_sdk_support + + # 3 inner classes, 6 lambdas (and thus 6 synthetics from D8) and 5 dex_shards + # is one magic combination to repro synthetics being separated from their + # context / enclosing classes. + create_test_app 3 6 5 + + echo here2 + echo $TEST_TMPDIR/bazelrc + cat $TEST_TMPDIR/bazelrc + + bazel build java/com/testapp || fail "Test app should have built succesfully" + + dex_file_count=$(unzip -l bazel-bin/java/com/testapp/testapp.apk | grep "classes[0-9]*.dex" | wc -l) + if [[ ! "$dex_file_count" -eq "5" ]]; then + echo "Expected 5 dexes in app, found: $dex_file_count" + exit 1 + fi +} + run_suite "Tests for DexFileSplitter with synthetic classes crossing dexfiles" \ No newline at end of file diff --git a/src/test/shell/bazel/android/android_sh_test.bzl b/src/test/shell/bazel/android/android_sh_test.bzl index 40f21648a2f939..8c0bbc626bc40e 100644 --- a/src/test/shell/bazel/android/android_sh_test.bzl +++ b/src/test/shell/bazel/android/android_sh_test.bzl @@ -29,20 +29,36 @@ CHECK_FOR_ANDROID_SDK = select( no_match_error = "This test requires an android SDK, and one isn't present. Make sure to uncomment the android rules in the WORKSPACE.", ) -def android_sh_test(**kwargs): +def android_sh_test(create_test_with_released_tools = True, **kwargs): + """Creates versions of the test with and without platforms and head android tools. + + Args: + create_test_with_released_tools: Whether to create a version of the test with the released + android tools, for when the code under test relies on not-yet-released code. + **kwargs: Args to sh_test + """ name = kwargs.pop("name") data = kwargs.pop("data") if not data: data = [] data = data + CHECK_FOR_ANDROID_SDK - # Test with released android_tools version. - native.sh_test( - name = name, - args = ["--without_platforms"], - data = data, - **kwargs - ) + if create_test_with_released_tools: + # Test with released android_tools version. + native.sh_test( + name = name, + args = ["--without_platforms"], + data = data, + **kwargs + ) + + # Test with platform-based toolchain resolution. + native.sh_test( + name = name + "_with_platforms", + data = data, + args = ["--with_platforms"], + **kwargs + ) # Test with android_tools version that's built at the same revision # as the test itself. @@ -54,11 +70,3 @@ def android_sh_test(**kwargs): ], **kwargs ) - - # Test with platform-based toolchain resolution. - native.sh_test( - name = name + "_with_platforms", - data = data, - args = ["--with_platforms"], - **kwargs - ) diff --git a/src/test/shell/testenv.sh.tmpl b/src/test/shell/testenv.sh.tmpl index 7ee2e772951205..7a9aac74580a1c 100755 --- a/src/test/shell/testenv.sh.tmpl +++ b/src/test/shell/testenv.sh.tmpl @@ -330,6 +330,7 @@ EOF "rules_license" "rules_proto" "rules_python" + "rules_pkg" ) for repo in "${repos[@]}"; do reponame="${repo%"_for_testing"}" @@ -554,6 +555,14 @@ load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") EOF } +function add_rules_pkg_to_workspace() { + cat >> "$1"<> "$1"< syntheticClassContexts = HashMultimap.create(); + for (ZipIn in : inputs) { CentralDirectory cdir = centralDirectories.get(in.getFilename()); + + for (DirectoryEntry entry : cdir.list()) { + if (entry.getFilename().equals("META-INF/synthetic-contexts.map")) { + parseSyntheticContextsMap(in.entryFor(entry).getContent(), syntheticClassContexts); + break; + } + } + for (DirectoryEntry entry : cdir.list()) { String filename = normalizedFilename(entry.getFilename()); if (!inputFilter.apply(filename)) { @@ -438,17 +454,31 @@ private void split() { } } } + Splitter splitter = new Splitter(outputs.size(), classes.size()); + if (filter != null) { // Assign files in the filter to the first output file. - splitter.assign(Sets.filter(filter, inputFilter)); + splitter.assignAllToCurrentShard(Sets.filter(filter, inputFilter)); splitter.nextShard(); // minimal initial shard } + + Set allSyntheticClasses = new HashSet<>(syntheticClassContexts.values()); + for (String path : classes) { - // Use normalized filename so the filter file doesn't have to change - int assignment = splitter.assign(path); - Preconditions.checkState(assignment >= 0 && assignment < zipOuts.length); - assignments.put(path, zipOuts[assignment]); + if (!allSyntheticClasses.contains(path)) { + + // Use normalized filename so the filter file doesn't have to change + int assignment = splitter.assign(path); + Set syntheticClasses = syntheticClassContexts.get(path); + splitter.assignAllToCurrentShard(syntheticClasses); + Preconditions.checkState(assignment >= 0 && assignment < zipOuts.length); + + assignments.put(path, zipOuts[assignment]); + for (String syntheticClass : syntheticClasses) { + assignments.put(syntheticClass, zipOuts[assignment]); + } + } } } @@ -494,6 +524,23 @@ private String fixPath(String path) { return path; } + private static void parseSyntheticContextsMap( + ByteBuffer byteBuffer, Multimap syntheticClassContexts) { + // The ByteBuffer returned from the Splitter's zip library is not backed by an accessible array, + // so ByteBuffer.array() is not supported, so we must go the long way. + byte[] bytes = new byte[byteBuffer.remaining()]; + byteBuffer.get(bytes); + Scanner scanner = new Scanner(new ByteArrayInputStream(bytes), UTF_8); + scanner.useDelimiter("[;\n]"); + while (scanner.hasNext()) { + String syntheticClass = scanner.next(); + String context = scanner.next(); + // The context map uses class names, whereas SplitZip uses class file names, so add ".class" + // here to make it easier to work with the map in the rest of the code. + syntheticClassContexts.put(context + ".class", syntheticClass + ".class"); + } + } + private void verbose(String msg) { if (verbose) { System.out.println(msg); diff --git a/src/tools/android/java/com/google/devtools/build/android/ziputils/Splitter.java b/src/tools/android/java/com/google/devtools/build/android/ziputils/Splitter.java index 7f1a469f10368d..2c03b74cacac09 100644 --- a/src/tools/android/java/com/google/devtools/build/android/ziputils/Splitter.java +++ b/src/tools/android/java/com/google/devtools/build/android/ziputils/Splitter.java @@ -24,10 +24,10 @@ class Splitter { private final int numberOfShards; private final Map assigned; - private int size = 0; - private int shard = 0; + private int size = 0; // Number of classes in the current shard + private int shard = 0; // Current shard number private String prevPath = null; - private int remaining; + private int remaining; // Number of classes remaining to be assigned shards private int idealSize; private int almostFull; @@ -51,15 +51,15 @@ public Splitter(int numberOfShards, int expectedSize) { * remaining entries to process is adjusted, by subtracting the number of as-of-yet unassigned * entries from the filter. */ - public void assign(Collection filter) { - if (filter != null) { - for (String s : filter) { - if (!assigned.containsKey(s)) { + public void assignAllToCurrentShard(Collection entries) { + if (entries != null) { + for (String e : entries) { + if (!assigned.containsKey(e)) { remaining--; } - assigned.put(s, shard); + assigned.put(e, shard); } - size = filter.size(); + size += entries.size(); } } diff --git a/tools/android/runtime_deps/BUILD.bazel b/tools/android/runtime_deps/BUILD.bazel index 9c50eb42dc2978..b56610231034a7 100644 --- a/tools/android/runtime_deps/BUILD.bazel +++ b/tools/android/runtime_deps/BUILD.bazel @@ -4,7 +4,7 @@ # extract all Android rules and tools out of Bazel and into rules_android # and tools_android. -load("@bazel_tools//tools/build_defs/pkg:pkg.bzl", "pkg_tar") +load("@rules_pkg//pkg:tar.bzl", "pkg_tar") filegroup( name = "srcs", @@ -84,5 +84,9 @@ pkg_tar( "//src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps:ImportDepsChecker_deploy.jar", "//src/tools/android/java/com/google/devtools/build/android:all_android_tools_deploy.jar", ], + remap_paths = { + "src/java_tools/import_deps_checker/java/com/google/devtools/build/importdeps:ImportDepsChecker_deploy.jar": "ImportDepsChecker_deploy.jar", + "src/tools/android/java/com/google/devtools/build/android:all_android_tools_deploy.jar": "all_android_tools_deploy.jar", + }, visibility = ["//src/test/shell/bazel:__subpackages__"], )