Desugared library: fail on incompatible desugared library version
- add marker property for desugared library
Bug: 154106502
Bug: 155051133
Change-Id: I6cac0d0c4cf292ee4226ede3cd86420f851a068d
diff --git a/src/library_desugar/desugar_jdk_libs.json b/src/library_desugar/desugar_jdk_libs.json
index aadb671..07765bb 100644
--- a/src/library_desugar/desugar_jdk_libs.json
+++ b/src/library_desugar/desugar_jdk_libs.json
@@ -1,6 +1,8 @@
{
- "configuration_format_version": 3,
- "version": "0.11.1",
+ "configuration_format_version": 4,
+ "group_id" : "com.tools.android",
+ "artifact_id" : "desugar_jdk_libs",
+ "version": "0.11.2",
"required_compilation_api_level": 26,
"synthesized_library_classes_package_prefix": "j$.",
"library_flags": [
diff --git a/src/library_desugar/desugar_jdk_libs_comments.md b/src/library_desugar/desugar_jdk_libs_comments.md
index f08da27..2ba5908 100644
--- a/src/library_desugar/desugar_jdk_libs_comments.md
+++ b/src/library_desugar/desugar_jdk_libs_comments.md
@@ -1,4 +1,4 @@
-# Description of the core library configuration file
+# Description of the desugared library configuration file
## Version
@@ -7,27 +7,33 @@
Non-backward compatible changes to the desugared library increase the version number, and such
library cannot be compiled without upgrading R8/D8 to the latest version.
-The second field `version` holds the version of the content for the configuration. This number
+The fields `group_id` and `artifact_id` are maven-coordinated ids for the desugared library
+configuration file.
+
+The field `version` holds the version of the content for the configuration. This number
must be updated each time the configuration is changed.
+A unique identifier is generated for the desugared library configuration using
+`group_id:artifact_id:version`.
+
## Required compilation API level
-The third field `required_compilation_api_level` encodes the minimal Android API level required for
+The field `required_compilation_api_level` encodes the minimal Android API level required for
the desugared library to be compiled correctly. If the API of library used for compilation of the
library or a program using the library is lower than this level, one has to upgrade the SDK version
used to be able to use desugared libraries.
## Library and program flags
-The fourth and fifth fields are `library_flags` and `program_flags`. They include the set of flags
-required for respectively the library and the program using the desugared library compilation. The
-sets of flags are different depending on the min API level used. The flags are in a list, where
-each list entry specifies up to which min API level the set of flags should be applied. During
-compilation, R8/D8 adds up all the required flags for the min API level specified at compilation.
+The fields `library_flags` and `program_flags` include the set of flags required for respectively
+the library and the program using the desugared library compilation. The sets of flags are
+different depending on the min API level used. The flags are in a list, where each list entry
+specifies up to which min API level the set of flags should be applied. During compilation,
+R8/D8 adds up all the required flags for the min API level specified at compilation.
-For example, let's say the `program_flags` have entries for `api_level_below_or_equal` 20, 24 and 26.
-If compiling the program for min API 24, R8/D8 will use both the set of flags for API 24 and 26
-(24 <= 24, 24 <= 26 but !(24 <= 20)).
+For example, let's say the `program_flags` have entries for `api_level_below_or_equal` 20, 24 and
+26. If compiling the program for min API 24, R8/D8 will use both the set of flags for API 24 and
+26 (24 <= 24, 24 <= 26 but !(24 <= 20)).
## Extra keep rules
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index ea07dfc..e8c6bb1 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -239,6 +239,7 @@
if (marker != null && hasClassResources) {
markers.add(marker);
}
+ Marker.checkCompatibleDesugaredLibrary(markers, options.reporter);
InspectorImpl.runInspections(options.outputInspections, app);
if (options.isGeneratingClassFiles()) {
diff --git a/src/main/java/com/android/tools/r8/dex/Marker.java b/src/main/java/com/android/tools/r8/dex/Marker.java
index f8e7a07..1484f02 100644
--- a/src/main/java/com/android/tools/r8/dex/Marker.java
+++ b/src/main/java/com/android/tools/r8/dex/Marker.java
@@ -5,25 +5,30 @@
import com.android.tools.r8.CompilationMode;
import com.android.tools.r8.graph.DexString;
+import com.android.tools.r8.utils.Reporter;
+import com.android.tools.r8.utils.StringDiagnostic;
+import com.google.gson.JsonArray;
import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import com.google.gson.JsonSyntaxException;
import java.util.Comparator;
+import java.util.HashSet;
import java.util.Map.Entry;
+import java.util.Set;
-/**
- * Abstraction for hidden dex marker intended for the main dex file.
- */
+/** Abstraction for hidden dex marker intended for the main dex file. */
public class Marker {
public static final String VERSION = "version";
public static final String MIN_API = "min-api";
+ public static final String DESUGARED_LIBRARY_IDENTIFIERS = "desugared-library-identifiers";
public static final String SHA1 = "sha-1";
public static final String COMPILATION_MODE = "compilation-mode";
public static final String HAS_CHECKSUMS = "has-checksums";
public static final String PG_MAP_ID = "pg-map-id";
public static final String R8_MODE = "r8-mode";
+ private static final String NO_LIBRARY_DESUGARING = "<no-library-desugaring>";
public enum Tool {
D8,
@@ -54,6 +59,48 @@
this.jsonObject = jsonObject;
}
+ public static void checkCompatibleDesugaredLibrary(Set<Marker> markers, Reporter reporter) {
+ if (markers.size() <= 1) {
+ return;
+ }
+ // In L8 compilation, the compilation has two markers, a L8 marker, which has a desugared
+ // library property, and either a D8 or a R8 marker, which has no desugared library property.
+ // In other compilations, the desugared library versions have to be consistent.
+ Set<String> desugaredLibraryIdentifiers = new HashSet<>();
+ for (Marker marker : markers) {
+ if (marker.tool == Tool.L8) {
+ assert marker.getDesugaredLibraryIdentifiers().length > 0;
+ assert markers.stream()
+ .allMatch(m -> m.tool == Tool.L8 || m.getDesugaredLibraryIdentifiers().length == 0);
+ } else {
+ String[] identifiers = marker.getDesugaredLibraryIdentifiers();
+ String identifier;
+ switch (identifiers.length) {
+ case 0:
+ identifier = NO_LIBRARY_DESUGARING;
+ break;
+ case 1:
+ identifier = identifiers[0];
+ break;
+ default:
+ // To be implemented once D8/R8 compilation supports multiple desugared libraries.
+ throw reporter.fatalError(
+ new StringDiagnostic(
+ "Merging program compiled with multiple desugared libraries."));
+ }
+ desugaredLibraryIdentifiers.add(identifier);
+ }
+ }
+
+ if (desugaredLibraryIdentifiers.size() > 1) {
+ reporter.error(
+ new StringDiagnostic(
+ "The compilation is merging inputs with different desugared library desugaring "
+ + desugaredLibraryIdentifiers
+ + ", which may lead to unexpected runtime errors."));
+ }
+ }
+
public Tool getTool() {
return tool;
}
@@ -94,6 +141,28 @@
return this;
}
+ public String[] getDesugaredLibraryIdentifiers() {
+ if (jsonObject.has(DESUGARED_LIBRARY_IDENTIFIERS)) {
+ JsonArray array = jsonObject.get(DESUGARED_LIBRARY_IDENTIFIERS).getAsJsonArray();
+ String[] identifiers = new String[array.size()];
+ for (int i = 0; i < array.size(); i++) {
+ identifiers[i] = array.get(i).getAsString();
+ }
+ return identifiers;
+ }
+ return new String[0];
+ }
+
+ public Marker setDesugaredLibraryIdentifiers(String... identifiers) {
+ assert !jsonObject.has(DESUGARED_LIBRARY_IDENTIFIERS);
+ JsonArray jsonIdentifiers = new JsonArray();
+ for (String identifier : identifiers) {
+ jsonIdentifiers.add(identifier);
+ }
+ jsonObject.add(DESUGARED_LIBRARY_IDENTIFIERS, jsonIdentifiers);
+ return this;
+ }
+
public String getSha1() {
return jsonObject.get(SHA1).getAsString();
}
@@ -148,8 +217,7 @@
public String toString() {
// In order to make printing of markers deterministic we sort the entries by key.
final JsonObject sortedJson = new JsonObject();
- jsonObject.entrySet()
- .stream()
+ jsonObject.entrySet().stream()
.sorted(Comparator.comparing(Entry::getKey))
.forEach(entry -> sortedJson.add(entry.getKey(), entry.getValue()));
return PREFIX + tool + sortedJson;
@@ -191,7 +259,7 @@
private static Marker internalParse(Tool tool, String str) {
try {
- JsonElement result = new JsonParser().parse(str);
+ JsonElement result = new JsonParser().parse(str);
if (result.isJsonObject()) {
return new Marker(tool, result.getAsJsonObject());
}
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
index baa6317..1590484 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfiguration.java
@@ -32,6 +32,7 @@
private final AndroidApiLevel requiredCompilationAPILevel;
private final boolean libraryCompilation;
private final String synthesizedLibraryClassesPackagePrefix;
+ private final String identifier;
private final Map<String, String> rewritePrefix;
private final Map<DexType, DexType> emulateLibraryInterface;
private final Map<DexString, Map<DexType, DexType>> retargetCoreLibMember;
@@ -50,6 +51,7 @@
AndroidApiLevel.B,
true,
FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX,
+ "testingOnlyVersion",
prefix,
ImmutableMap.of(),
ImmutableMap.of(),
@@ -64,6 +66,7 @@
AndroidApiLevel.B,
false,
FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX,
+ null,
ImmutableMap.of(),
ImmutableMap.of(),
ImmutableMap.of(),
@@ -77,6 +80,7 @@
AndroidApiLevel requiredCompilationAPILevel,
boolean libraryCompilation,
String packagePrefix,
+ String identifier,
Map<String, String> rewritePrefix,
Map<DexType, DexType> emulateLibraryInterface,
Map<DexString, Map<DexType, DexType>> retargetCoreLibMember,
@@ -87,6 +91,7 @@
this.requiredCompilationAPILevel = requiredCompilationAPILevel;
this.libraryCompilation = libraryCompilation;
this.synthesizedLibraryClassesPackagePrefix = packagePrefix;
+ this.identifier = identifier;
this.rewritePrefix = rewritePrefix;
this.emulateLibraryInterface = emulateLibraryInterface;
this.retargetCoreLibMember = retargetCoreLibMember;
@@ -114,6 +119,10 @@
return synthesizedLibraryClassesPackagePrefix;
}
+ public String getIdentifier() {
+ return identifier;
+ }
+
public Map<String, String> getRewritePrefix() {
return rewritePrefix;
}
@@ -164,6 +173,7 @@
private boolean libraryCompilation = false;
private String synthesizedLibraryClassesPackagePrefix =
FALL_BACK_SYNTHESIZED_CLASSES_PACKAGE_PREFIX;
+ private String identifier;
private Map<String, String> rewritePrefix = new HashMap<>();
private Map<DexType, DexType> emulateLibraryInterface = new HashMap<>();
private Map<DexString, Map<DexType, DexType>> retargetCoreLibMember = new IdentityHashMap<>();
@@ -177,8 +187,12 @@
}
public Builder setSynthesizedLibraryClassesPackagePrefix(String prefix) {
- String replace = prefix.replace('.', '/');
- this.synthesizedLibraryClassesPackagePrefix = replace;
+ this.synthesizedLibraryClassesPackagePrefix = prefix.replace('.', '/');
+ return this;
+ }
+
+ public Builder setDesugaredLibraryIdentifier(String identifier) {
+ this.identifier = identifier;
return this;
}
@@ -268,6 +282,7 @@
requiredCompilationAPILevel,
libraryCompilation,
synthesizedLibraryClassesPackagePrefix,
+ identifier,
ImmutableMap.copyOf(rewritePrefix),
ImmutableMap.copyOf(emulateLibraryInterface),
ImmutableMap.copyOf(retargetCoreLibMember),
diff --git a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
index b66432e..a659cc6 100644
--- a/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
+++ b/src/main/java/com/android/tools/r8/ir/desugar/DesugaredLibraryConfigurationParser.java
@@ -19,7 +19,7 @@
public class DesugaredLibraryConfigurationParser {
- private static final int MAX_SUPPORTED_VERSION = 3;
+ private static final int MAX_SUPPORTED_VERSION = 4;
private final DesugaredLibraryConfiguration.Builder configurationBuilder;
private final Reporter reporter;
@@ -53,14 +53,14 @@
JsonParser parser = new JsonParser();
JsonObject jsonConfig = parser.parse(jsonConfigString).getAsJsonObject();
- int version = jsonConfig.get("configuration_format_version").getAsInt();
- if (version > MAX_SUPPORTED_VERSION) {
+ int formatVersion = jsonConfig.get("configuration_format_version").getAsInt();
+ if (formatVersion > MAX_SUPPORTED_VERSION) {
throw reporter.fatalError(
new StringDiagnostic(
"Unsupported desugared library configuration version, please upgrade the D8/R8"
+ " compiler."));
}
- if (version == 1) {
+ if (formatVersion == 1) {
reporter.warning(
new StringDiagnostic(
"You are using an experimental version of the desugared library configuration, "
@@ -68,6 +68,19 @@
+ "production releases and to fix this warning."));
}
+ String version = jsonConfig.get("version").getAsString();
+ String groupID;
+ String artifactID;
+ if (formatVersion < 4) {
+ groupID = "com.tools.android";
+ artifactID = "desugar_jdk_libs";
+ } else {
+ groupID = jsonConfig.get("group_id").getAsString();
+ artifactID = jsonConfig.get("artifact_id").getAsString();
+ }
+ String identifier = String.join(":", groupID, artifactID, version);
+ configurationBuilder.setDesugaredLibraryIdentifier(identifier);
+
if (jsonConfig.has("synthesized_library_classes_package_prefix")) {
configurationBuilder.setSynthesizedLibraryClassesPackagePrefix(
jsonConfig.get("synthesized_library_classes_package_prefix").getAsString());
diff --git a/src/main/java/com/android/tools/r8/utils/InternalOptions.java b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
index a42ed5d..7e34093 100644
--- a/src/main/java/com/android/tools/r8/utils/InternalOptions.java
+++ b/src/main/java/com/android/tools/r8/utils/InternalOptions.java
@@ -334,6 +334,9 @@
if (!isGeneratingClassFiles()) {
marker.setMinApi(minApiLevel);
}
+ if (desugaredLibraryConfiguration.getIdentifier() != null) {
+ marker.setDesugaredLibraryIdentifiers(desugaredLibraryConfiguration.getIdentifier());
+ }
if (Version.isDevelopmentVersion()) {
marker.setSha1(VersionProperties.INSTANCE.getSha());
}
diff --git a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/MergingWithDesugaredLibraryTest.java b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/MergingWithDesugaredLibraryTest.java
index eefe0f7..03052cc 100644
--- a/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/MergingWithDesugaredLibraryTest.java
+++ b/src/test/java/com/android/tools/r8/desugar/desugaredlibrary/MergingWithDesugaredLibraryTest.java
@@ -4,10 +4,13 @@
package com.android.tools.r8.desugar.desugaredlibrary;
-import static org.hamcrest.core.StringContains.containsString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import com.android.tools.r8.CompilationFailedException;
import com.android.tools.r8.D8TestCompileResult;
+import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.TestDiagnosticMessages;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.TestParametersCollection;
@@ -16,6 +19,7 @@
import com.android.tools.r8.utils.AndroidApiLevel;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.List;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -39,39 +43,53 @@
@Test
public void testMergeDesugaredAndNonDesugared() throws Exception {
- D8TestCompileResult compileResult =
- testForD8()
- .addProgramFiles(buildPart1DesugaredLibrary(), buildPart2NoDesugaredLibrary())
- .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
- .setMinApi(parameters.getApiLevel())
- .enableCoreLibraryDesugaring(parameters.getApiLevel())
- .compile()
- .addDesugaredCoreLibraryRunClassPath(
- this::buildDesugaredLibrary, parameters.getApiLevel());
- // TODO(b/154106502): This should raise a proper warning. The dex files are incompatible,
- // so the behavior is undefined regarding desugared types.
- if (parameters.getApiLevel().getLevel() < AndroidApiLevel.N.getLevel()) {
- compileResult
- .run(parameters.getRuntime(), Part1.class)
- .assertSuccessWithOutputLines(J$_RESULT);
- } else {
- compileResult
- .run(parameters.getRuntime(), Part1.class)
- .assertSuccessWithOutputLines(JAVA_RESULT);
+ D8TestCompileResult compileResult;
+ try {
+ compileResult =
+ testForD8()
+ .addProgramFiles(buildPart1DesugaredLibrary(), buildPart2NoDesugaredLibrary())
+ .addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.P))
+ .setMinApi(parameters.getApiLevel())
+ .enableCoreLibraryDesugaring(parameters.getApiLevel())
+ .compileWithExpectedDiagnostics(this::assertError);
+ assertFalse(expectError());
+ } catch (CompilationFailedException e) {
+ assertTrue(expectError());
+ return;
}
- if (parameters.getRuntime().asDex().getMinApiLevel().getLevel()
- < AndroidApiLevel.N.getLevel()) {
- compileResult
- .run(parameters.getRuntime(), Part2.class)
- .assertFailureWithErrorThatMatches(containsString("java.lang.NoSuchMethodError"));
- } else {
+ assert !expectError();
+ assert compileResult != null;
+ compileResult.addDesugaredCoreLibraryRunClassPath(
+ this::buildDesugaredLibrary, parameters.getApiLevel());
+ compileResult
+ .run(parameters.getRuntime(), Part1.class)
+ .assertSuccessWithOutputLines(JAVA_RESULT);
+ compileResult
+ .run(parameters.getRuntime(), Part2.class)
+ .assertSuccessWithOutputLines(JAVA_RESULT);
+ }
- compileResult
- .run(parameters.getRuntime(), Part2.class)
- .assertSuccessWithOutputLines(JAVA_RESULT);
+ private void assertError(TestDiagnosticMessages m) {
+ List<Diagnostic> errors = m.getErrors();
+ if (expectError()) {
+ assertEquals(1, errors.size());
+ assertTrue(
+ errors.stream()
+ .anyMatch(
+ w ->
+ w.getDiagnosticMessage()
+ .contains(
+ "The compilation is merging inputs with different"
+ + " desugared library desugaring")));
+ } else {
+ assertEquals(0, errors.size());
}
}
+ private boolean expectError() {
+ return parameters.getApiLevel().getLevel() <= AndroidApiLevel.N.getLevel();
+ }
+
@Test
public void testMergeDesugaredAndClassFile() throws Exception {
D8TestCompileResult compileResult =
