Skip to content

Commit 3a8c076

Browse files
ahumeskyGoogler
and
Googler
authored
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]>
1 parent cc83fdf commit 3a8c076

File tree

22 files changed

+619
-86
lines changed

22 files changed

+619
-86
lines changed

WORKSPACE

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
workspace(name = "io_bazel")
22

33
load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file", "http_jar")
4-
load("//:distdir.bzl", "dist_http_archive", "dist_http_file", "distdir_tar")
4+
load("//:distdir.bzl", "dist_http_archive", "dist_http_file", "dist_http_jar", "distdir_tar")
55
load("//:distdir_deps.bzl", "DIST_DEPS")
66

77
# These can be used as values for the patch_cmds and patch_cmds_win attributes
@@ -126,20 +126,20 @@ distdir_tar(
126126
archives = [
127127
"android_tools_pkg-0.27.0.tar.gz",
128128
# for android_gmaven_r8
129-
"r8-3.3.28.jar",
129+
"r8-8.0.40.jar",
130130
],
131131
dirname = "derived/distdir",
132132
dist_deps = {dep: attrs for dep, attrs in DIST_DEPS.items() if "additional_distfiles" in attrs["used_in"]},
133133
sha256 = {
134134
"android_tools_pkg-0.27.0.tar.gz": "1afa4b7e13c82523c8b69e87f8d598c891ec7e2baa41d9e24e08becd723edb4d",
135-
"r8-3.3.28.jar": "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972",
135+
"r8-8.0.40.jar": "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702",
136136
},
137137
urls = {
138138
"android_tools_pkg-0.27.0.tar.gz": [
139139
"https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz",
140140
],
141-
"r8-3.3.28.jar": [
142-
"https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar",
141+
"r8-8.0.40.jar": [
142+
"https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar",
143143
],
144144
},
145145
)
@@ -360,21 +360,16 @@ distdir_tar(
360360
name = "test_WORKSPACE_files",
361361
archives = [
362362
"android_tools_pkg-0.27.0.tar.gz",
363-
"r8-3.3.28.jar",
364363
],
365364
dirname = "test_WORKSPACE/distdir",
366365
dist_deps = {dep: attrs for dep, attrs in DIST_DEPS.items() if "test_WORKSPACE_files" in attrs["used_in"]},
367366
sha256 = {
368367
"android_tools_pkg-0.27.0.tar.gz": "1afa4b7e13c82523c8b69e87f8d598c891ec7e2baa41d9e24e08becd723edb4d",
369-
"r8-3.3.28.jar": "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972",
370368
},
371369
urls = {
372370
"android_tools_pkg-0.27.0.tar.gz": [
373371
"https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz",
374372
],
375-
"r8-3.3.28.jar": [
376-
"https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar",
377-
],
378373
},
379374
)
380375

@@ -413,12 +408,13 @@ http_archive(
413408
url = "https://mirror.bazel.build/bazel_android_tools/android_tools_pkg-0.27.0.tar.gz",
414409
)
415410

416-
# This must be kept in sync with src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE
417-
# and tools/android/android_extensions.bzl
418-
http_jar(
419-
name = "android_gmaven_r8_for_testing",
420-
sha256 = "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972",
421-
url = "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar",
411+
# This is here to override the android_gmaven_r8 rule from
412+
# src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE
413+
# so that tests like src/test/java/com/google/devtools/build/android/r8:AllTests
414+
# use the most recent version of R8 rather than the one might be referenced in a released
415+
# version of bazel that might have an outdated android_remote_tools.WORKSPACE relative to the tests.
416+
dist_http_jar(
417+
name = "android_gmaven_r8",
422418
)
423419

424420
dist_http_archive(

distdir.bzl

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
"""Defines a repository rule that generates an archive consisting of the specified files to fetch"""
1515

1616
load("//:distdir_deps.bzl", "DEPS_BY_NAME")
17-
load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file")
17+
load("//tools/build_defs/repo:http.bzl", "http_archive", "http_file", "http_jar")
1818

1919
_BUILD = """
2020
load("@rules_pkg//pkg:tar.bzl", "pkg_tar")
@@ -115,3 +115,21 @@ def dist_http_file(name, **kwargs):
115115
urls = info["urls"],
116116
**kwargs
117117
)
118+
119+
def dist_http_jar(name, **kwargs):
120+
"""Wraps http_jar, providing attributes like sha and urls from the central list.
121+
122+
dist_http_jar wraps an http_jar invocation, but looks up relevant attributes
123+
from distdir_deps.bzl so the user does not have to specify them.
124+
125+
Args:
126+
name: repo name
127+
**kwargs: see http_jar for allowed args.
128+
"""
129+
info = DEPS_BY_NAME[name]
130+
http_jar(
131+
name = name,
132+
sha256 = info["sha256"],
133+
urls = info["urls"],
134+
**kwargs
135+
)

distdir_deps.bzl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,17 @@ DIST_DEPS = {
261261
# Build time dependencies for testing and packaging
262262
#
263263
###################################################
264+
"android_gmaven_r8": {
265+
"archive": "r8-8.0.40.jar",
266+
"sha256": "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702",
267+
"urls": [
268+
"https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar",
269+
],
270+
"used_in": [
271+
"test_WORKSPACE_files",
272+
],
273+
"package_version": "8.0.40",
274+
},
264275
"bazel_skylib": {
265276
"archive": "bazel-skylib-1.0.3.tar.gz",
266277
"sha256": "1c531376ac7e5a180e0237938a2536de0c54d93f5c278634818e0efc952dd56c",

src/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ release_archive(
645645
filegroup(
646646
name = "test_repos",
647647
srcs = [
648-
"@android_gmaven_r8_for_testing//jar:file",
648+
"@android_gmaven_r8//jar:file",
649649
"@android_tools_for_testing//:WORKSPACE",
650650
"@bazel_skylib//:WORKSPACE",
651651
"@com_google_protobuf//:WORKSPACE",

src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_remote_tools.WORKSPACE

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@ maybe(
1313
maybe(
1414
http_jar,
1515
name = "android_gmaven_r8",
16-
sha256 = "8626ca32fb47aba7fddd2c897615e2e8ffcdb4d4b213572a2aefb3f838f01972",
17-
url = "https://maven.google.com/com/android/tools/r8/3.3.28/r8-3.3.28.jar",
16+
sha256 = "ab1379835c7d3e5f21f80347c3c81e2f762e0b9b02748ae5232c3afa14adf702",
17+
url = "https://maven.google.com/com/android/tools/r8/8.0.40/r8-8.0.40.jar",
1818
)

src/test/java/com/google/devtools/build/android/dexer/DexLimitTrackerTest.java

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,46 +42,61 @@ public void setUp() throws Exception {
4242
public void testUnderLimit() {
4343
DexLimitTracker tracker =
4444
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
45-
assertThat(tracker.track(dex)).isFalse();
45+
tracker.track(dex);
46+
assertThat(tracker.outsideLimits()).isFalse();
4647
}
4748

4849
@Test
4950
public void testOverLimit() throws Exception {
5051
DexLimitTracker tracker =
5152
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()) - 1);
52-
assertThat(tracker.track(dex)).isTrue();
53-
assertThat(tracker.track(dex)).isTrue();
54-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
53+
tracker.track(dex);
54+
assertThat(tracker.outsideLimits()).isTrue();
55+
tracker.track(dex);
56+
assertThat(tracker.outsideLimits()).isTrue();
57+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
58+
assertThat(tracker.outsideLimits()).isTrue();
5559
}
5660

5761
@Test
5862
public void testRepeatedReferencesDeduped() throws Exception {
5963
DexLimitTracker tracker =
6064
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
61-
assertThat(tracker.track(dex)).isFalse();
62-
assertThat(tracker.track(dex)).isFalse();
63-
assertThat(tracker.track(dex)).isFalse();
64-
assertThat(tracker.track(dex)).isFalse();
65-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
66-
assertThat(tracker.track(dex)).isTrue();
65+
tracker.track(dex);
66+
assertThat(tracker.outsideLimits()).isFalse();
67+
tracker.track(dex);
68+
assertThat(tracker.outsideLimits()).isFalse();
69+
tracker.track(dex);
70+
assertThat(tracker.outsideLimits()).isFalse();
71+
tracker.track(dex);
72+
assertThat(tracker.outsideLimits()).isFalse();
73+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
74+
assertThat(tracker.outsideLimits()).isTrue();
75+
tracker.track(dex);
76+
assertThat(tracker.outsideLimits()).isTrue();
6777
}
6878

6979
@Test
7080
public void testGoOverLimit() throws Exception {
7181
DexLimitTracker tracker =
7282
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
73-
assertThat(tracker.track(dex)).isFalse();
74-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
83+
tracker.track(dex);
84+
assertThat(tracker.outsideLimits()).isFalse();
85+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
86+
assertThat(tracker.outsideLimits()).isTrue();
7587
}
7688

7789
@Test
7890
public void testClear() throws Exception {
7991
DexLimitTracker tracker =
8092
new DexLimitTracker(Math.max(dex.methodIds().size(), dex.fieldIds().size()));
81-
assertThat(tracker.track(dex)).isFalse();
82-
assertThat(tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)))).isTrue();
93+
tracker.track(dex);
94+
assertThat(tracker.outsideLimits()).isFalse();
95+
tracker.track(DexFiles.toDex(convertClass(DexLimitTracker.class)));
96+
assertThat(tracker.outsideLimits()).isTrue();
8397
tracker.clear();
84-
assertThat(tracker.track(dex)).isFalse();
98+
tracker.track(dex);
99+
assertThat(tracker.outsideLimits()).isFalse();
85100
}
86101

87102
private static DexFile convertClass(Class<?> clazz) throws Exception {

src/test/java/com/google/devtools/build/android/r8/BUILD

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,15 @@ java_test(
5252
":barray",
5353
":dexmergersample",
5454
":naming001",
55+
":testdata_lambda_desugared.jar",
5556
":twosimpleclasses",
5657
],
5758
jvm_flags = [
5859
"-DCompatDexBuilderTests.twosimpleclasses=$(location :twosimpleclasses)",
5960
"-DCompatDexBuilderTests.naming001=$(location :naming001)",
6061
"-DCompatDxTests.arithmetic=$(location :arithmetic)",
6162
"-DCompatDxTests.barray=$(location :barray)",
63+
"-DCompatDexBuilderTests.lambda=$(location :testdata_lambda_desugared.jar)",
6264
],
6365
runtime_deps = [
6466
":tests",
@@ -114,3 +116,21 @@ java_library(
114116
"-target 8",
115117
],
116118
)
119+
120+
java_library(
121+
name = "testdata_lambda",
122+
srcs = glob(["testdata/lambda/*.java"]),
123+
)
124+
125+
genrule(
126+
name = "desugar_testdata_lambda",
127+
srcs = [
128+
":testdata_lambda",
129+
"@bazel_tools//tools/android:android_jar",
130+
],
131+
outs = ["testdata_lambda_desugared.jar"],
132+
cmd = "$(location //src/tools/android/java/com/google/devtools/build/android/r8:desugar) " +
133+
"-i $(location :testdata_lambda) -o $@ " +
134+
"--bootclasspath_entry $(location @bazel_tools//tools/android:android_jar)",
135+
tools = ["//src/tools/android/java/com/google/devtools/build/android/r8:desugar"],
136+
)

src/test/java/com/google/devtools/build/android/r8/CompatDexBuilderTest.java

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@
2121
import com.android.tools.r8.OutputMode;
2222
import com.google.common.collect.ImmutableList;
2323
import com.google.devtools.common.options.OptionsParsingException;
24+
import java.io.BufferedReader;
2425
import java.io.IOException;
26+
import java.io.InputStreamReader;
2527
import java.nio.file.Files;
2628
import java.nio.file.Path;
2729
import java.util.HashSet;
28-
import java.util.List;
2930
import java.util.Set;
3031
import java.util.concurrent.ExecutionException;
32+
import java.util.zip.ZipEntry;
3133
import java.util.zip.ZipFile;
3234
import org.junit.Rule;
3335
import org.junit.Test;
@@ -45,7 +47,7 @@ public void compileManyClasses()
4547
throws IOException, InterruptedException, ExecutionException, OptionsParsingException {
4648
// Random set of classes from the R8 example test directory naming001.
4749
final String inputJar = System.getProperty("CompatDexBuilderTests.naming001");
48-
final List<String> classNames =
50+
final ImmutableList<String> classNames =
4951
ImmutableList.of(
5052
"A",
5153
"B",
@@ -86,6 +88,40 @@ public void compileManyClasses()
8688
assertThat(expectedNames).isEmpty();
8789
}
8890

91+
@Test
92+
public void compileWithSyntheticLambdas() throws Exception {
93+
final String contextName = "com/google/devtools/build/android/r8/testdata/lambda/Lambda";
94+
final String inputJar = System.getProperty("CompatDexBuilderTests.lambda");
95+
final Path outputZip = temp.getRoot().toPath().resolve("out.zip");
96+
CompatDexBuilder.main(
97+
new String[] {"--input_jar", inputJar, "--output_zip", outputZip.toString()});
98+
assertThat(Files.exists(outputZip)).isTrue();
99+
100+
try (ZipFile zipFile = new ZipFile(outputZip.toFile(), UTF_8)) {
101+
assertThat(zipFile.getEntry(contextName + ".class.dex")).isNotNull();
102+
ZipEntry entry = zipFile.getEntry("META-INF/synthetic-contexts.map");
103+
assertThat(entry).isNotNull();
104+
try (BufferedReader reader =
105+
new BufferedReader(new InputStreamReader(zipFile.getInputStream(entry), UTF_8))) {
106+
String line = reader.readLine();
107+
assertThat(line).isNotNull();
108+
// Format of mapping is: <synthetic-binary-name>;<context-binary-name>\n
109+
int sep = line.indexOf(';');
110+
String syntheticNameInMap = line.substring(0, sep);
111+
String contextNameInMap = line.substring(sep + 1);
112+
// The synthetic will be prefixed by the context type. This checks the synthetic name
113+
// is larger than the context to avoid hardcoding the synthetic names, which may change.
114+
assertThat(syntheticNameInMap).startsWith(contextName);
115+
assertThat(syntheticNameInMap).isNotEqualTo(contextName);
116+
// Check expected context.
117+
assertThat(contextNameInMap).isEqualTo(contextName);
118+
// Only one synthetic and its context should be present.
119+
line = reader.readLine();
120+
assertThat(line).isNull();
121+
}
122+
}
123+
}
124+
89125
@Test
90126
public void compileTwoClassesAndRun() throws Exception {
91127
// Run CompatDexBuilder on dexMergeSample.jar
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright 2023 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.android.r8.testdata.lambda;
15+
16+
import java.util.function.Supplier;
17+
18+
/** Test class */
19+
public final class Lambda {
20+
21+
private Lambda() {}
22+
23+
private static <T> T foo(Supplier<T> fn) {
24+
return fn.get();
25+
}
26+
27+
public static void main(String[] args) {
28+
String unused = foo(() -> "Hello, world!");
29+
}
30+
}

src/test/shell/bazel/android/BUILD

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,3 +233,17 @@ android_sh_test(
233233
# See https://github.com/bazelbuild/bazel/issues/8235
234234
tags = ["no-remote"],
235235
)
236+
237+
android_sh_test(
238+
name = "DexFileSplitter_synthetic_classes_test",
239+
size = "medium",
240+
srcs = ["DexFileSplitter_synthetic_classes_test.sh"],
241+
data = [
242+
":android_helper",
243+
"//external:android_sdk_for_testing",
244+
"//src/test/shell/bazel:test-deps",
245+
],
246+
tags = [
247+
"no_windows",
248+
],
249+
)

0 commit comments

Comments
 (0)