Cherrypick fixes for #16368 into Bazel 6.3.0 (#18808)

* Update D8/R8 dependency in bazel to 8.0.40 PiperOrigin-RevId: 520331100 Change-Id: Ia150c637747c980fc5cdfa07c80edd3a9e30e04b * Add options converters specific to R8 to avoid depending on android_builder_lib which causes classpath conflicts with D8 classes. RELNOTE: None PiperOrigin-RevId: 526149555 Change-Id: Iacfce24cc3d2327ef63c47f3bfd76813beaefc7a * Add android_gmaven_r8 to bazel's WORKSPACE file so that it overrides the version of R8 that might come from android_remote_tools.WORKSPACE in a released version of bazel used to run the test (which would be potentially out of date with respect to the source code). Also merge this with android_gmaven_r8_for_testing since they would be redundant. RELNOTES: None. PiperOrigin-RevId: 529830525 Change-Id: Ic5c18ce5beb7fb915b084dc19590a05b04675548 * Collect the contexts of D8 compiler-synthesized classes. This completes steps 2 and 3 in b/241351268#comment9 towards resolving #16368 PiperOrigin-RevId: 530238344 Change-Id: I569bf8e3bfa81f7005f3e9b79338d5a5b868e339 * Group synthetic classes with their context classes in DexFileSplitter so that synthetic classes don't end up without their context classes in dex shards, which will causing merging to fail in DexFileMerger. Fixes #16368. RELNOTES: None PiperOrigin-RevId: 544134712 Change-Id: Ib29f6659f18dd71be96a7985bc25cfb44e719ae5 --------- Co-authored-by: Googler <[email protected]>
bazelbuild · Jun 29, 2023 · 3a8c076 · 3a8c076
1 parent cc83fdf
commit 3a8c076
Show file tree

Hide file tree

Showing 22 changed files with 619 additions and 86 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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/...ain/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE b/...ain/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
@@ -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
@@ -52,13 +52,15 @@ java_test(
         ":barray",
         ":dexmergersample",
         ":naming001",
+        ":testdata_lambda_desugared.jar",
         ":twosimpleclasses",
     ],
     jvm_flags = [
         "-DCompatDexBuilderTests.twosimpleclasses=$(location :twosimpleclasses)",
         "-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
@@ -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<String> classNames =
+    final ImmutableList<String> 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: <synthetic-binary-name>;<context-binary-name>\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
@@ -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> T foo(Supplier<T> 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
@@ -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",
+    ],
+)