Add extend support for embedded rules in library providers
Read embedded rules from library dependencies from -libraryjars
in configuration files as well.
Also remove the fixpoint which was not needed.
Bug: b/289087274
Fixes: b/370665838
Change-Id: I7c613724f2ef5d107ce11cf5182b3560a6344328
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index 6a7475f..b6c9ae4 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -63,19 +63,19 @@
import java.io.InputStream;
import java.nio.file.Path;
import java.nio.file.Paths;
-import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
-import java.util.Deque;
import java.util.List;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiPredicate;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Supplier;
+import java.util.stream.Collectors;
/**
* Immutable command structure for an invocation of the {@link R8} compiler.
@@ -875,43 +875,48 @@
}
return compilerVersion;
});
- // TODO(b/370665838): Remove the fixpoint, as -injars are not supported in embedded keep
- // rules. The comment below is not true.
- // Since -injars can itself reference archives with rules and that in turn have -injars the
- // completion of amending rules and providers must run in a fixed point. The fixed point is
- // reached once the injars set is stable.
- Set<FilteredClassPath> seenInjars = SetUtils.newIdentityHashSet();
- Deque<ProgramResourceProvider> providers =
- new ArrayDeque<>(getAppBuilder().getProgramResourceProviders());
- while (true) {
- for (FilteredClassPath injar : parser.getConfigurationBuilder().getInjars()) {
- if (seenInjars.add(injar)) {
- ArchiveResourceProvider provider = getAppBuilder().createAndAddProvider(injar);
+ Set<FilteredClassPath> seen = SetUtils.newIdentityHashSet();
+ // Find resources in program providers. Both from API and added through legacy -injars in
+ // configuration files.
+ List<DataResourceProvider> providers =
+ new ArrayList<>(
+ getAppBuilder().getProgramResourceProviders().stream()
+ .map(ProgramResourceProvider::getDataResourceProvider)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList()));
+ for (FilteredClassPath injar : parser.getConfigurationBuilder().getInjars()) {
+ if (seen.add(injar)) {
+ ArchiveResourceProvider provider = getAppBuilder().createAndAddProvider(injar);
+ if (provider != null) {
+ providers.add(provider);
+ }
+ }
+ }
+
+ if (readEmbeddedRulesFromClasspathAndLibrary) {
+ // Find resources in classpath providers. No legacy configuration file option for classpath.
+ getAppBuilder().getClasspathResourceProviders().stream()
+ .map(ClassFileResourceProvider::getDataResourceProvider)
+ .filter(Objects::nonNull)
+ .forEach(providers::add);
+ // Find resources in library providers. Both from API and added through legacy -libraryjars
+ // in configuration files.
+ getAppBuilder().getLibraryResourceProviders().stream()
+ .map(ClassFileResourceProvider::getDataResourceProvider)
+ .filter(Objects::nonNull)
+ .forEach(providers::add);
+ for (FilteredClassPath libraryjar :
+ parser.getConfigurationBuilder().build().getLibraryjars()) {
+ if (seen.add(libraryjar)) {
+ ArchiveResourceProvider provider = getAppBuilder().createAndAddProvider(libraryjar);
if (provider != null) {
providers.add(provider);
}
}
}
- if (providers.isEmpty()) {
- break;
- }
-
- while (!providers.isEmpty()) {
- DataResourceProvider dataResourceProvider = providers.pop().getDataResourceProvider();
- parseEmbeddedRules(reporter, parser, semanticVersionSupplier, dataResourceProvider);
- }
- // TODO(b/370665838): Remove the fixpoint. For now assert that it is not needed.
- assert seenInjars.containsAll(parser.getConfigurationBuilder().getInjars());
}
- if (readEmbeddedRulesFromClasspathAndLibrary) {
- for (ClassFileResourceProvider provider : getAppBuilder().getClasspathResourceProviders()) {
- parseEmbeddedRules(
- reporter, parser, semanticVersionSupplier, provider.getDataResourceProvider());
- }
- for (ClassFileResourceProvider provider : getAppBuilder().getLibraryResourceProviders()) {
- parseEmbeddedRules(
- reporter, parser, semanticVersionSupplier, provider.getDataResourceProvider());
- }
+ for (DataResourceProvider provider : providers) {
+ parseEmbeddedRules(reporter, parser, semanticVersionSupplier, provider);
}
}
diff --git a/src/test/java/com/android/tools/r8/shaking/LibraryProvidedProguardRulesFromClasspathOrLibraryTest.java b/src/test/java/com/android/tools/r8/shaking/LibraryProvidedProguardRulesFromClasspathOrLibraryTest.java
index d9631f1..552e81d 100644
--- a/src/test/java/com/android/tools/r8/shaking/LibraryProvidedProguardRulesFromClasspathOrLibraryTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/LibraryProvidedProguardRulesFromClasspathOrLibraryTest.java
@@ -16,6 +16,7 @@
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertThrows;
+import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import com.android.tools.r8.ClassFileResourceProvider;
@@ -23,13 +24,13 @@
import com.android.tools.r8.DataResourceProvider;
import com.android.tools.r8.ProgramResource;
import com.android.tools.r8.ProgramResource.Kind;
+import com.android.tools.r8.R8FullTestBuilder;
import com.android.tools.r8.ResourceException;
import com.android.tools.r8.TestParameters;
import com.android.tools.r8.ToolHelper;
import com.android.tools.r8.origin.ArchiveEntryOrigin;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApiLevel;
-import com.android.tools.r8.utils.BooleanUtils;
import com.android.tools.r8.utils.DescriptorUtils;
import com.android.tools.r8.utils.codeinspector.CodeInspector;
import com.google.common.collect.ImmutableList;
@@ -53,6 +54,12 @@
public interface Interface {}
+ private enum HowToAdd {
+ API_CLASSPATH,
+ API_LIBRARY,
+ RULES_LIBRARYJARS
+ }
+
@Parameter(0)
public TestParameters parameters;
@@ -60,30 +67,36 @@
public LibraryType libraryType;
@Parameter(2)
- public boolean isClasspath;
+ public HowToAdd howToAdd;
- @Parameters(name = "{0}, libraryType: {1}, isClasspath {2}")
+ @Parameters(name = "{0}, libraryType: {1}, howToAdd: {2}")
public static List<Object[]> data() {
return buildParameters(
getTestParameters().withDefaultRuntimes().withApiLevel(AndroidApiLevel.B).build(),
LibraryType.values(),
- BooleanUtils.values());
+ HowToAdd.values());
}
private Path buildLibrary(List<String> rules) throws Exception {
return buildLibrary(libraryType, ImmutableList.of(Interface.class, Keep.class), rules);
}
+ private void addClasspathOrLibrary(Path library, R8FullTestBuilder builder) {
+ builder.applyIf(
+ howToAdd == HowToAdd.API_CLASSPATH,
+ b -> b.addClasspathFiles(library),
+ howToAdd == HowToAdd.API_LIBRARY,
+ b ->
+ b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST))
+ .addLibraryFiles(library),
+ b -> b.addKeepRules("-libraryjars " + library.toAbsolutePath()));
+ }
+
private CodeInspector runTest(List<String> rules) throws Exception {
Path library = buildLibrary(rules);
return testForR8(parameters.getBackend())
.addProgramClasses(A.class, B.class)
- .applyIf(
- isClasspath,
- b -> b.addClasspathFiles(library),
- b ->
- b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST))
- .addLibraryFiles(library))
+ .apply(b -> addClasspathOrLibrary(library, b))
.setMinApi(parameters)
.apply(b -> ToolHelper.setReadEmbeddedRulesFromClasspathAndLibrary(b.getBuilder(), true))
.compile()
@@ -155,12 +168,7 @@
() -> {
Path library = buildLibrary(ImmutableList.of("-injars some.jar"));
testForR8(parameters.getBackend())
- .applyIf(
- isClasspath,
- b -> b.addClasspathFiles(library),
- b ->
- b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST))
- .addLibraryFiles(library))
+ .apply(b -> addClasspathOrLibrary(library, b))
.setMinApi(parameters)
.apply(
b -> ToolHelper.setReadEmbeddedRulesFromClasspathAndLibrary(b.getBuilder(), true))
@@ -181,12 +189,7 @@
() -> {
Path library = buildLibrary(ImmutableList.of("-include other.rules"));
testForR8(parameters.getBackend())
- .applyIf(
- isClasspath,
- b -> b.addClasspathFiles(library),
- b ->
- b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST))
- .addLibraryFiles(library))
+ .apply(b -> addClasspathOrLibrary(library, b))
.setMinApi(parameters)
.apply(
b -> ToolHelper.setReadEmbeddedRulesFromClasspathAndLibrary(b.getBuilder(), true))
@@ -233,6 +236,7 @@
@Test
public void throwingDataResourceProvider() {
+ assumeFalse(howToAdd == HowToAdd.RULES_LIBRARYJARS);
// TODO(b/228319861): Read Proguard rules from AAR's.
assumeTrue(!libraryType.isAar());
assertThrows(
@@ -240,7 +244,7 @@
() ->
testForR8(parameters.getBackend())
.applyIf(
- isClasspath,
+ howToAdd == HowToAdd.API_CLASSPATH,
b -> b.addClasspathResourceProviders(new TestProvider()),
b ->
b.addLibraryFiles(ToolHelper.getAndroidJar(AndroidApiLevel.LATEST))
