diff --git a/WORKSPACE b/WORKSPACE index ff47163e724ec2..6b142957a35fa3 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1,7 +1,7 @@ workspace(name = "io_bazel") load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file", "http_jar") -load("//:distdir.bzl", "dist_http_archive", "dist_http_file", "distdir_tar") +load("//:distdir.bzl", "dist_http_archive", "dist_http_file", "dist_http_jar", "distdir_tar") load("//:distdir_deps.bzl", "DIST_DEPS") # These can be used as values for the patch_cmds and patch_cmds_win attributes @@ -126,20 +126,20 @@ distdir_tar( archives = [ "android_tools_pkg-0.27.0.tar.gz", # for android_gmaven_r8 - "r8-3.3.28.jar", + "r8-8.0.40.jar", ], dirname = "derived/distdir", dist_deps = {dep: attrs for dep, attrs in DIST_DEPS.items() if "additional_distfiles" in attrs["used_in"]}, sha256 = { "android_tools_pkg-0.27.0.tar.gz": "1afa4b7e13c82523c8b69e87f8d598c891ec7e2baa41d9e24e08becd723edb4d", - "r8-3.3.28.jar": "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972", + "r8-8.0.40.jar": "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702", }, urls = { "android_tools_pkg-0.27.0.tar.gz": [ "https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz", ], - "r8-3.3.28.jar": [ - "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar", + "r8-8.0.40.jar": [ + "https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar", ], }, ) @@ -360,21 +360,16 @@ distdir_tar( name = "test_WORKSPACE_files", archives = [ "android_tools_pkg-0.27.0.tar.gz", - "r8-3.3.28.jar", ], dirname = "test_WORKSPACE/distdir", dist_deps = {dep: attrs for dep, attrs in DIST_DEPS.items() if "test_WORKSPACE_files" in attrs["used_in"]}, sha256 = { "android_tools_pkg-0.27.0.tar.gz": "1afa4b7e13c82523c8b69e87f8d598c891ec7e2baa41d9e24e08becd723edb4d", - "r8-3.3.28.jar": "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972", }, urls = { "android_tools_pkg-0.27.0.tar.gz": [ "https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz", ], - "r8-3.3.28.jar": [ - "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar", - ], }, ) @@ -413,12 +408,13 @@ http_archive( url = "https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz", ) -# This must be kept in sync with src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE -# and tools/android/android_extensions.bzl -http_jar( - name = "android_gmaven_r8_for_testing", - sha256 = "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972", - url = "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar", +# This is here to override the android_gmaven_r8 rule from +# src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE +# so that tests like src/test/java/com/google/devtools/build/android/r8:AllTests +# use the most recent version of R8 rather than the one might be referenced in a released +# version of bazel that might have an outdated android_remote_tools.WORKSPACE relative to the tests. +dist_http_jar( + name = "android_gmaven_r8", ) dist_http_archive( diff --git a/distdir.bzl b/distdir.bzl index b4dab33424830d..d01467cc72cd1c 100644 --- a/distdir.bzl +++ b/distdir.bzl @@ -14,7 +14,7 @@ """Defines a repository rule that generates an archive consisting of the specified files to fetch""" load("//:distdir_deps.bzl", "DEPS_BY_NAME") -load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file") +load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file", "http_jar") _BUILD = """ load("@rules_pkg//pkg:tar.bzl", "pkg_tar") @@ -115,3 +115,21 @@ def dist_http_file(name, **kwargs): urls = info["urls"], **kwargs ) + +def dist_http_jar(name, **kwargs): + """Wraps http_jar, providing attributes like sha and urls from the central list. + + dist_http_jar wraps an http_jar invocation, but looks up relevant attributes + from distdir_deps.bzl so the user does not have to specify them. + + Args: + name: repo name + **kwargs: see http_jar for allowed args. + """ + info = DEPS_BY_NAME[name] + http_jar( + name = name, + sha256 = info["sha256"], + urls = info["urls"], + **kwargs + ) diff --git a/distdir_deps.bzl b/distdir_deps.bzl index e48b28c3e4b3e8..8df4c75e7887e7 100644 --- a/distdir_deps.bzl +++ b/distdir_deps.bzl @@ -261,6 +261,17 @@ DIST_DEPS = { # Build time dependencies for testing and packaging # ################################################### + "android_gmaven_r8": { + "archive": "r8-8.0.40.jar", + "sha256": "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702", + "urls": [ + "https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar", + ], + "used_in": [ + "test_WORKSPACE_files", + ], + "package_version": "8.0.40", + }, "bazel_skylib": { "archive": "bazel-skylib-1.0.3.tar.gz", "sha256": "1c531376ac7e5a180e0237938a2536de0c54d93f5c278634818e0efc952dd56c", diff --git a/src/BUILD b/src/BUILD index b8e62240ae641e..e0b9cc9cf4007a 100644 --- a/src/BUILD +++ b/src/BUILD @@ -645,7 +645,7 @@ release_archive( filegroup( name = "test_repos", srcs = [ - "@android_gmaven_r8_for_testing//jar:file", + "@android_gmaven_r8//jar:file", "@android_tools_for_testing//:WORKSPACE", "@bazel_skylib//:WORKSPACE", "@com_google_protobuf//:WORKSPACE", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE index 7dc69d00c2ca36..f567dafd842e48 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE @@ -13,6 +13,6 @@ maybe( maybe( http_jar, name = "android_gmaven_r8", - sha256 = "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972", - url = "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar", + sha256 = "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702", + url = "https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar", ) diff --git a/src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java b/src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java index f88ca36ab64709..9e93a0d9a67591 100644 --- a/src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java +++ b/src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java @@ -42,46 +42,61 @@ public void setUp() throws Exception { public void testUnderLimit() { DexLimitTracker tracker = new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size())); - assertThat(tracker.track(dex)).isFalse(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); } @Test public void testOverLimit() throws Exception { DexLimitTracker tracker = new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()) - 1); - assertThat(tracker.track(dex)).isTrue(); - assertThat(tracker.track(dex)).isTrue(); - assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isTrue(); + tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class))); + assertThat(tracker.outsideLimits()).isTrue(); } @Test public void testRepeatedReferencesDeduped() throws Exception { DexLimitTracker tracker = new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size())); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue(); - assertThat(tracker.track(dex)).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class))); + assertThat(tracker.outsideLimits()).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isTrue(); } @Test public void testGoOverLimit() throws Exception { DexLimitTracker tracker = new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size())); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class))); + assertThat(tracker.outsideLimits()).isTrue(); } @Test public void testClear() throws Exception { DexLimitTracker tracker = new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size())); - assertThat(tracker.track(dex)).isFalse(); - assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); + tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class))); + assertThat(tracker.outsideLimits()).isTrue(); tracker.clear(); - assertThat(tracker.track(dex)).isFalse(); + tracker.track(dex); + assertThat(tracker.outsideLimits()).isFalse(); } private static DexFile convertClass(Class clazz) throws Exception { diff --git a/src/test/java/com/google/devtools/build/android/r8/BUILD b/src/test/java/com/google/devtools/build/android/r8/BUILD index fde5a95bdceac2..5af3a24f6a4928 100644 --- a/src/test/java/com/google/devtools/build/android/r8/BUILD +++ b/src/test/java/com/google/devtools/build/android/r8/BUILD @@ -52,6 +52,7 @@ java_test( ":barray", ":dexmergersample", ":naming001", + ":testdata_lambda_desugared.jar", ":twosimpleclasses", ], jvm_flags = [ @@ -59,6 +60,7 @@ java_test( "-DCompatDexBuilderTests.naming001=$(location :naming001)", "-DCompatDxTests.arithmetic=$(location :arithmetic)", "-DCompatDxTests.barray=$(location :barray)", + "-DCompatDexBuilderTests.lambda=$(location :testdata_lambda_desugared.jar)", ], runtime_deps = [ ":tests", @@ -114,3 +116,21 @@ java_library( "-target 8", ], ) + +java_library( + name = "testdata_lambda", + srcs = glob(["testdata/lambda/*.java"]), +) + +genrule( + name = "desugar_testdata_lambda", + srcs = [ + ":testdata_lambda", + "@bazel_tools//tools/android:android_jar", + ], + outs = ["testdata_lambda_desugared.jar"], + cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/r8:desugar) " + + "-i $(location :testdata_lambda) -o $@ " + + "--bootclasspath_entry $(location @bazel_tools//tools/android:android_jar)", + tools = ["//src/tools/android/java/com/google/devtools/build/android/r8:desugar"], +) diff --git a/src/test/java/com/google/devtools/build/android/r8/CompatDexBuilderTest.java b/src/test/java/com/google/devtools/build/android/r8/CompatDexBuilderTest.java index 158200b7c78f07..8c88caedff91f9 100644 --- a/src/test/java/com/google/devtools/build/android/r8/CompatDexBuilderTest.java +++ b/src/test/java/com/google/devtools/build/android/r8/CompatDexBuilderTest.java @@ -21,13 +21,15 @@ import com.android.tools.r8.OutputMode; import com.google.common.collect.ImmutableList; import com.google.devtools.common.options.OptionsParsingException; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; import java.nio.file.Files; import java.nio.file.Path; import java.util.HashSet; -import java.util.List; import java.util.Set; import java.util.concurrent.ExecutionException; +import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import org.junit.Rule; import org.junit.Test; @@ -45,7 +47,7 @@ public void compileManyClasses() throws IOException, InterruptedException, ExecutionException, OptionsParsingException { // Random set of classes from the R8 example test directory naming001. final String inputJar = System.getProperty("CompatDexBuilderTests.naming001"); - final List classNames = + final ImmutableList classNames = ImmutableList.of( "A", "B", @@ -86,6 +88,40 @@ public void compileManyClasses() assertThat(expectedNames).isEmpty(); } + @Test + public void compileWithSyntheticLambdas() throws Exception { + final String contextName = "com/google/devtools/build/android/r8/testdata/lambda/Lambda"; + final String inputJar = System.getProperty("CompatDexBuilderTests.lambda"); + final Path outputZip = temp.getRoot().toPath().resolve("out.zip"); + CompatDexBuilder.main( + new String[] {"--input_jar", inputJar, "--output_zip", outputZip.toString()}); + assertThat(Files.exists(outputZip)).isTrue(); + + try (ZipFile zipFile = new ZipFile(outputZip.toFile(), UTF_8)) { + assertThat(zipFile.getEntry(contextName + ".class.dex")).isNotNull(); + ZipEntry entry = zipFile.getEntry("META-INF/synthetic-contexts.map"); + assertThat(entry).isNotNull(); + try (BufferedReader reader = + new BufferedReader(new InputStreamReader(zipFile.getInputStream(entry), UTF_8))) { + String line = reader.readLine(); + assertThat(line).isNotNull(); + // Format of mapping is: ;\n + int sep = line.indexOf(';'); + String syntheticNameInMap = line.substring(0, sep); + String contextNameInMap = line.substring(sep + 1); + // The synthetic will be prefixed by the context type. This checks the synthetic name + // is larger than the context to avoid hardcoding the synthetic names, which may change. + assertThat(syntheticNameInMap).startsWith(contextName); + assertThat(syntheticNameInMap).isNotEqualTo(contextName); + // Check expected context. + assertThat(contextNameInMap).isEqualTo(contextName); + // Only one synthetic and its context should be present. + line = reader.readLine(); + assertThat(line).isNull(); + } + } + } + @Test public void compileTwoClassesAndRun() throws Exception { // Run CompatDexBuilder on dexMergeSample.jar diff --git a/src/test/java/com/google/devtools/build/android/r8/testdata/lambda/Lambda.java b/src/test/java/com/google/devtools/build/android/r8/testdata/lambda/Lambda.java new file mode 100644 index 00000000000000..2fa5fedc6cb45c --- /dev/null +++ b/src/test/java/com/google/devtools/build/android/r8/testdata/lambda/Lambda.java @@ -0,0 +1,30 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// 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.google.devtools.build.android.r8.testdata.lambda; + +import java.util.function.Supplier; + +/** Test class */ +public final class Lambda { + + private Lambda() {} + + private static T foo(Supplier fn) { + return fn.get(); + } + + public static void main(String[] args) { + String unused = foo(() -> "Hello, world!"); + } +} diff --git a/src/test/shell/bazel/android/BUILD b/src/test/shell/bazel/android/BUILD index f0b63e53795237..4a4d14d1f4672b 100644 --- a/src/test/shell/bazel/android/BUILD +++ b/src/test/shell/bazel/android/BUILD @@ -233,3 +233,17 @@ android_sh_test( # See https://github.com/bazelbuild/bazel/issues/8235 tags = ["no-remote"], ) + +android_sh_test( + name = "DexFileSplitter_synthetic_classes_test", + size = "medium", + srcs = ["DexFileSplitter_synthetic_classes_test.sh"], + data = [ + ":android_helper", + "//external:android_sdk_for_testing", + "//src/test/shell/bazel:test-deps", + ], + 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 new file mode 100755 index 00000000000000..bd70e9f4533437 --- /dev/null +++ b/src/test/shell/bazel/android/DexFileSplitter_synthetic_classes_test.sh @@ -0,0 +1,142 @@ +#!/bin/bash +# +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# 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. + +# For these tests to run do the following: +# +# 1. Install an Android SDK from https://developer.android.com +# 2. Set the $ANDROID_HOME environment variable +# 3. Uncomment the line in WORKSPACE containing android_sdk_repository +# +# Note that if the environment is not set up as above android_integration_test +# will silently be ignored and will be shown as passing. + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +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 + + mkdir -p java/com/testapp + + cat > java/com/testapp/AndroidManifest.xml < + + + + + + + + + + + + +EOF + + cat > java/com/testapp/MainActivity.java < java/com/testapp/BigLib.java + + cat > java/com/testapp/BUILD < $i," + done + + echo " };" + echo " }" + echo " }" + echo "}" +} + +run_suite "Tests for DexFileSplitter with synthetic classes crossing dexfiles" \ No newline at end of file diff --git a/src/test/shell/testenv.sh.tmpl b/src/test/shell/testenv.sh.tmpl index 20d3a6205e316c..7ee2e772951205 100755 --- a/src/test/shell/testenv.sh.tmpl +++ b/src/test/shell/testenv.sh.tmpl @@ -272,7 +272,7 @@ EOF repos=( "android_tools_for_testing" - "android_gmaven_r8_for_testing" + "android_gmaven_r8" "bazel_skylib" "bazel_toolchains" "com_google_protobuf" diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java index e3fed0a1d1e3e9..c6f88b73d79d3f 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileAggregator.java @@ -82,7 +82,8 @@ public DexFileAggregator add(Dex dexFile) { if (multidex.isMultidexAllowed()) { // To determine whether currentShard is "full" we track unique field and method signatures, // which predicts precisely the number of field and method indices. - if (tracker.track(dexFile) && !currentShard.isEmpty()) { + tracker.track(dexFile); + if (tracker.outsideLimits() && !currentShard.isEmpty()) { // For simplicity just start a new shard to fit the given file. // Don't bother with waiting for a later file that might fit the old shard as in the extreme // we'd have to wait until the end to write all shards. diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileSplitter.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileSplitter.java index 4e684c87543b3d..3dd844d30ba485 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileSplitter.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexFileSplitter.java @@ -14,6 +14,7 @@ package com.google.devtools.build.android.dexer; import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; import static java.nio.charset.StandardCharsets.UTF_8; @@ -21,8 +22,8 @@ import com.android.dex.DexFormat; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicates; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.TreeMultimap; import com.google.common.io.ByteStreams; import com.google.common.io.Closer; import com.google.devtools.build.android.Converters.ExistingPathConverter; @@ -40,13 +41,17 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; -import java.util.Comparator; -import java.util.LinkedHashMap; +import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Scanner; +import java.util.Set; +import java.util.TreeMap; import java.util.function.Predicate; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import javax.annotation.Nullable; /** * Shuffles .class.dex files from input archives into 1 or more archives each to be merged into a @@ -159,32 +164,43 @@ static void splitIntoShards(Options options) throws IOException { try (Closer closer = Closer.create(); DexFileSplitter out = new DexFileSplitter(options.outputDirectory, options.maxNumberOfIdxPerDex)) { + // 1. Scan inputs in order and keep first occurrence of each class, keeping all zips open. // We don't process anything yet so we can shard in sorted order, which is what dx would do // if presented with a single jar containing all the given inputs. // TODO(kmb): Abandon alphabetic sorting to process each input fully before moving on (still // requires scanning inputs twice for main dex list). + Predicate inclusionFilter = ZipEntryPredicates.suffixes(".dex", ".class"); if (expected != null) { inclusionFilter = inclusionFilter.and(e -> expected.contains(e.getName())); } - LinkedHashMap deduped = new LinkedHashMap<>(); + + // Maps a dex file name to the zip file containing that dex file. + TreeMap dexFilesAndContainingZip = + new TreeMap<>(ZipEntryComparator::compareClassNames); + // Maps a class to its synthetic classes, if any. + TreeMultimap contextClassesToSyntheticClasses = TreeMultimap.create(); + for (Path inputArchive : options.inputArchives) { ZipFile zip = closer.register(new ZipFile(inputArchive.toFile())); + + // synthetic-contexts.map is generated by CompatDexBuilder. + ZipEntry syntheticContextsZipEntry = zip.getEntry("META-INF/synthetic-contexts.map"); + if (syntheticContextsZipEntry != null) { + parseSyntheticContextsMap( + zip.getInputStream(syntheticContextsZipEntry), contextClassesToSyntheticClasses); + } + zip.stream() .filter(inclusionFilter) - .forEach(e -> deduped.putIfAbsent(e.getName(), zip)); + .forEach(e -> dexFilesAndContainingZip.putIfAbsent(e.getName(), zip)); } - ImmutableList> files = - deduped - .entrySet() - .stream() - .sorted(Comparator.comparing(e -> e.getKey(), ZipEntryComparator::compareClassNames)) - .collect(ImmutableList.toImmutableList()); // 2. Process each class in desired order, rolling from shard to shard as needed. if (classesInMainDex == null || classesInMainDex.isEmpty()) { - out.processDexFiles(files, Predicates.alwaysTrue()); + out.processDexes( + dexFilesAndContainingZip, contextClassesToSyntheticClasses, Predicates.alwaysTrue()); } else { checkArgument(classesInMainDex.stream().noneMatch(s -> s.startsWith("j$/")), "%s lists classes in package 'j$', which can't be included in classes.dex and can " @@ -194,14 +210,15 @@ static void splitIntoShards(Options options) throws IOException { // 1. process only the classes listed in the given file // 2. process the remaining files Predicate mainDexFilter = ZipEntryPredicates.classFileNameFilter(classesInMainDex); - out.processDexFiles(files, mainDexFilter); + out.processDexes(dexFilesAndContainingZip, contextClassesToSyntheticClasses, mainDexFilter); // Fail if main_dex_list is too big, following dx's example checkState(out.shardsWritten() == 0, "Too many classes listed in main dex list file " + "%s, main dex capacity exceeded", options.mainDexListFile); if (options.minimalMainDex) { out.nextShard(); // Start new .dex file if requested } - out.processDexFiles(files, mainDexFilter.negate()); + out.processDexes( + dexFilesAndContainingZip, contextClassesToSyntheticClasses, mainDexFilter.negate()); } } } @@ -215,6 +232,23 @@ private static ImmutableSet expectedEntries(Path filterJar) throws IOExc } } + private static void parseSyntheticContextsMap( + InputStream inputStream, TreeMultimap syntheticClassContexts) { + Scanner scanner = new Scanner(inputStream, UTF_8); + scanner.useDelimiter("[;\n]"); + while (scanner.hasNext()) { + String syntheticClass = scanner.next(); + String context = scanner.next(); + // DexFileSplitter mostly expects filenames which all end in .class.dex, while the synthetic + // context map has class names, so add the extension here to make this easier to work with in + // the rest of the code. + syntheticClassContexts.put( + context + CLASS_DEX_EXTENSION, syntheticClass + CLASS_DEX_EXTENSION); + } + } + + private static final String CLASS_DEX_EXTENSION = ".class.dex"; + private final int maxNumberOfIdxPerDex; private final Path outputDirectory; /** Collect written zip files so we can conveniently wait for all of them to close when done. */ @@ -269,23 +303,31 @@ public void close() throws IOException { closer.close(); } - private void processDexFiles( - ImmutableList> filesToProcess, Predicate filter) + private void processDexes( + Map dexFilesAndContainingZip, + TreeMultimap contextClassesToSyntheticClasses, + Predicate filter) throws IOException { - for (Map.Entry entry : filesToProcess) { + + Set syntheticClasses = new HashSet<>(contextClassesToSyntheticClasses.values()); + for (Map.Entry entry : dexFilesAndContainingZip.entrySet()) { String filename = entry.getKey(); if (filter.test(filename)) { - ZipFile zipFile = entry.getValue(); - processDexEntry(zipFile, zipFile.getEntry(filename)); + // Synthetic classes will be gathered with their context classes and added to the dex file + // all together as a unit, so skip them here. + if (!syntheticClasses.contains(filename)) { + ZipFile zipFile = entry.getValue(); + processDex(zipFile, filename, contextClassesToSyntheticClasses.get(filename)); + } } } } - private void processDexEntry(ZipFile zip, ZipEntry entry) throws IOException { - String filename = entry.getName(); - checkState(filename.endsWith(".class.dex"), - "%s isn't a dex archive: %s", zip.getName(), filename); - checkState(entry.getMethod() == ZipEntry.STORED, "Expect to process STORED: %s", filename); + private void processDex(ZipFile zip, String filename, Set syntheticClasses) + throws IOException { + + // Synthetic classes base their names on their context classes, so this check only needs to be + // done for the context class. if (inCoreLib == null) { inCoreLib = filename.startsWith("j$/"); } else if (inCoreLib != filename.startsWith("j$/")) { @@ -299,6 +341,53 @@ private void processDexEntry(ZipFile zip, ZipEntry entry) throws IOException { filename); } + List zipEntryDexAndContents = new ArrayList<>(); + ZipEntryDexAndContent contextZdc = processDex(zip, filename); + checkNotNull(contextZdc, "Context class %s expected to be in %s", filename, zip.getName()); + zipEntryDexAndContents.add(contextZdc); + + for (String syntheticClass : syntheticClasses) { + ZipEntryDexAndContent syntheticClassZdc = processDex(zip, syntheticClass); + // Some synthetic classes are contained within the same dex as their enclosing class, + // so they won't be standalone dexes in the zip file, and some synthetic classes are present + // in synthetic-contexts.map but aren't standalone dexes in the zip nor are they in the + // dex with their enclosing class, so just skip these. + if (syntheticClassZdc != null) { + zipEntryDexAndContents.add(syntheticClassZdc); + } + } + + if (tracker.outsideLimits()) { + nextShard(); + for (ZipEntryDexAndContent zdc : zipEntryDexAndContents) { + tracker.track(zdc.dex); + } + checkState( + !tracker.outsideLimits(), + "Impossible to fit %s and all of its synthetic classes (count: %s) in a single shard", + filename, + syntheticClasses.size()); + } + + for (ZipEntryDexAndContent zdc : zipEntryDexAndContents) { + curOut.writeAsync(zdc.zipEntry, zdc.content); + } + } + + @Nullable + private ZipEntryDexAndContent processDex(ZipFile zip, String filename) throws IOException { + ZipEntry entry = zip.getEntry(filename); + if (entry == null) { + return null; + } + + checkState( + filename.endsWith(CLASS_DEX_EXTENSION), + "%s isn't a dex archive: %s", + zip.getName(), + filename); + checkState(entry.getMethod() == ZipEntry.STORED, "Expect to process STORED: %s", filename); + try (InputStream entryStream = zip.getInputStream(entry)) { // We don't want to use the Dex(InputStream) constructor because it closes the stream, // which will break the for loop, and it has its own bespoke way of reading the file into @@ -306,15 +395,27 @@ private void processDexEntry(ZipFile zip, ZipEntry entry) throws IOException { // TODO(kmb) since entry is stored, mmap content and give to Dex(ByteBuffer) and output zip byte[] content = new byte[(int) entry.getSize()]; ByteStreams.readFully(entryStream, content); // throws if file is smaller than expected - checkState(entryStream.read() == -1, - "Too many bytes in jar entry %s, expected %s", entry, entry.getSize()); + checkState( + entryStream.read() == -1, + "Too many bytes in jar entry %s, expected %s", + entry, + entry.getSize()); Dex dexFile = new Dex(content); - if (tracker.track(dexFile)) { - nextShard(); - tracker.track(dexFile); - } - curOut.writeAsync(entry, content); + tracker.track(dexFile); + return new ZipEntryDexAndContent(entry, content, dexFile); + } + } + + private static final class ZipEntryDexAndContent { + final ZipEntry zipEntry; + final byte[] content; + final Dex dex; + + ZipEntryDexAndContent(ZipEntry zipEntry, byte[] content, Dex dex) { + this.zipEntry = zipEntry; + this.content = content; + this.dex = dex; } } } diff --git a/src/tools/android/java/com/google/devtools/build/android/dexer/DexLimitTracker.java b/src/tools/android/java/com/google/devtools/build/android/dexer/DexLimitTracker.java index a0bfb51dfd054e..c8383e367d697e 100644 --- a/src/tools/android/java/com/google/devtools/build/android/dexer/DexLimitTracker.java +++ b/src/tools/android/java/com/google/devtools/build/android/dexer/DexLimitTracker.java @@ -38,14 +38,12 @@ public DexLimitTracker(int maxNumberOfIdxPerDex) { } /** - * Tracks the field and method references in the given file and returns whether we're within - * limits. + * Returns whether we're within limits. * - * @return {@code true} if method or field references are outside limits, {@code false} both - * are within limits. + * @return {@code true} if method or field references are outside limits, {@code false} both are + * within limits. */ - public boolean track(Dex dexFile) { - trackFieldsAndMethods(dexFile); + public boolean outsideLimits() { return fieldsSeen.size() > maxNumberOfIdxPerDex || methodsSeen.size() > maxNumberOfIdxPerDex; } @@ -55,7 +53,7 @@ public void clear() { methodsSeen.clear(); } - private void trackFieldsAndMethods(Dex dexFile) { + public void track(Dex dexFile) { int fieldCount = dexFile.fieldIds().size(); for (int fieldIndex = 0; fieldIndex < fieldCount; ++fieldIndex) { fieldsSeen.add(FieldDescriptor.fromDex(dexFile, fieldIndex)); diff --git a/src/tools/android/java/com/google/devtools/build/android/r8/BUILD b/src/tools/android/java/com/google/devtools/build/android/r8/BUILD index eb4ff9e3a936bd..55e6289887d281 100644 --- a/src/tools/android/java/com/google/devtools/build/android/r8/BUILD +++ b/src/tools/android/java/com/google/devtools/build/android/r8/BUILD @@ -47,7 +47,6 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/worker:work_request_handlers", "//src/main/java/com/google/devtools/common/options", - "//src/tools/android/java/com/google/devtools/build/android:android_builder_lib", "//third_party:asm", "//third_party:auto_value", "//third_party:guava", @@ -90,3 +89,25 @@ java_library( "//src/main/java/com/google/devtools/build/lib/worker:work_request_handlers", ], ) + +java_binary( + name = "desugar", + jvm_flags = [ + # b/71513487 + "-XX:+TieredCompilation", + "-XX:TieredStopAtLevel=1", + "-Xms8g", + "-Xmx8g", + # b/172508621 + "-Dcom.android.tools.r8.sortMethodsOnCfWriting", + "-Dcom.android.tools.r8.allowAllDesugaredInput", + "-Dcom.android.tools.r8.noCfMarkerForDesugaredCode", + "-Dcom.android.tools.r8.lambdaClassFieldsNotFinal", + "-Dcom.android.tools.r8.createSingletonsForStatelessLambdas", + ], + main_class = "com.google.devtools.build.android.r8.Desugar", + visibility = ["//src/test/java/com/google/devtools/build/android/r8:__pkg__"], + runtime_deps = [ + ":r8", + ], +) diff --git a/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java b/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java index 0de6e36c63c260..491f99b0580911 100644 --- a/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java +++ b/src/tools/android/java/com/google/devtools/build/android/r8/CompatDexBuilder.java @@ -24,8 +24,11 @@ import com.android.tools.r8.D8Command; import com.android.tools.r8.DexIndexedConsumer; import com.android.tools.r8.DiagnosticsHandler; +import com.android.tools.r8.SyntheticInfoConsumer; +import com.android.tools.r8.SyntheticInfoConsumerData; import com.android.tools.r8.origin.ArchiveEntryOrigin; import com.android.tools.r8.origin.PathOrigin; +import com.android.tools.r8.references.ClassReference; import com.google.auto.value.AutoValue; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; @@ -67,8 +70,47 @@ public class CompatDexBuilder { private static final long ONE_MEG = 1024 * 1024; + private static class ContextConsumer implements SyntheticInfoConsumer { + + // After compilation this will be non-null iff the compiled class is a D8 synthesized class. + ClassReference sythesizedPrimaryClass = null; + + // If the above is non-null then this will be the non-synthesized context class that caused + // D8 to synthesize the above class. + ClassReference contextOfSynthesizedClass = null; + + @Nullable + String getContextMapping() { + if (sythesizedPrimaryClass != null) { + return sythesizedPrimaryClass.getBinaryName() + + ";" + + contextOfSynthesizedClass.getBinaryName(); + } + return null; + } + + @Override + public synchronized void acceptSyntheticInfo(SyntheticInfoConsumerData data) { + verify( + sythesizedPrimaryClass == null || sythesizedPrimaryClass.equals(data.getSyntheticClass()), + "The single input classfile should ensure this has one value."); + verify( + contextOfSynthesizedClass == null + || contextOfSynthesizedClass.equals(data.getSynthesizingContextClass()), + "The single input classfile should ensure this has one value."); + sythesizedPrimaryClass = data.getSyntheticClass(); + contextOfSynthesizedClass = data.getSynthesizingContextClass(); + } + + @Override + public void finished() { + // Do nothing. + } + } + private static class DexConsumer implements DexIndexedConsumer { + final ContextConsumer contextConsumer = new ContextConsumer(); byte[] bytes; @Override @@ -86,6 +128,10 @@ void setBytes(byte[] byteCode) { this.bytes = byteCode; } + ContextConsumer getContextConsumer() { + return contextConsumer; + } + @Override public void finished(DiagnosticsHandler handler) { // Do nothing. @@ -269,10 +315,23 @@ private void dexEntries(@Nullable Cache dexCache, List { + + private final boolean mustExist; + + public PathConverter() { + this.mustExist = false; + } + + protected PathConverter(boolean mustExist) { + this.mustExist = mustExist; + } + + @Override + public Path convert(String input) throws OptionsParsingException { + try { + Path path = FileSystems.getDefault().getPath(input); + if (mustExist && !Files.exists(path)) { + throw new OptionsParsingException( + String.format("%s is not a valid path: it does not exist.", input)); + } + return path; + } catch (InvalidPathException e) { + throw new OptionsParsingException( + String.format("%s is not a valid path: %s.", input, e.getMessage()), e); + } + } + + @Override + public String getTypeDescription() { + return "a valid filesystem path"; + } + } + + /** + * Validating converter for Paths. A Path is considered valid if it resolves to a file and exists. + */ + public static class ExistingPathConverter extends PathConverter { + public ExistingPathConverter() { + super(true); + } + } + + private OptionsConverters() {} +} diff --git a/tools/android/android_extensions.bzl b/tools/android/android_extensions.bzl index ea84807d8487e7..7358edfb38e7c7 100644 --- a/tools/android/android_extensions.bzl +++ b/tools/android/android_extensions.bzl @@ -24,8 +24,8 @@ def _remote_android_tools_extensions_impl(_ctx): ) http_jar( name = "android_gmaven_r8", - sha256 = "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972", - url = "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar", + sha256 = "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702", + url = "https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar", ) remote_android_tools_extensions = module_extension(