Revert "Report errors when options -outjars/adaptresourcefilecontents are used." This reverts commit 1e21890a7acfce3e6e59b0352453ee3ce4a4321b. Reason for revert: breaks performance benchmarks. Change-Id: I3ab36facdf6e506dc20f32e264a0ba29203685a9
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java index f3605d9..564aa9c 100644 --- a/src/main/java/com/android/tools/r8/R8Command.java +++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -137,14 +137,6 @@ /** * Add proguard configuration. */ - public Builder addProguardConfiguration(ProguardConfigurationSource configuration) { - proguardConfigs.add(configuration); - return self(); - } - - /** - * Add proguard configuration as strings. - */ public Builder addProguardConfiguration(List<String> lines) { proguardConfigs.add(new ProguardConfigurationSourceStrings(lines)); return self();
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java index 092cce6..36f7efd 100644 --- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java +++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationParser.java
@@ -40,12 +40,12 @@ private final DiagnosticsHandler diagnosticsHandler; - private static final List<String> IGNORED_SINGLE_ARG_OPTIONS = ImmutableList + private static final List<String> ignoredSingleArgOptions = ImmutableList .of("protomapping", "target"); - private static final List<String> IGNORED_OPTIONAL_SINGLE_ARG_OPTIONS = ImmutableList + private static final List<String> ignoredOptionalSingleArgOptions = ImmutableList .of("keepdirectories", "runtype", "laststageoutput"); - private static final List<String> IGNORED_FLAG_OPTIONS = ImmutableList + private static final List<String> ignoredFlagOptions = ImmutableList .of("forceprocessing", "dontusemixedcaseclassnames", "dontpreverify", "experimentalshrinkunusedprotofields", "filterlibraryjarswithorginalprogramjars", @@ -53,22 +53,24 @@ "dontskipnonpubliclibraryclassmembers", "overloadaggressively", "invokebasemethod"); - private static final List<String> IGNORED_CLASS_DESCRIPTOR_OPTIONS = ImmutableList + private static final List<String> ignoredClassDescriptorOptions = ImmutableList .of("isclassnamestring", "whyarenotsimple"); - private static final List<String> WARNED_SINGLE_ARG_OPTIONS = ImmutableList + private static final List<String> warnedSingleArgOptions = ImmutableList .of("dontnote", - "printconfiguration"); - private static final List<String> WARNED_FLAG_OPTIONS = ImmutableList + "printconfiguration", + // TODO -outjars (http://b/37137994) and -adaptresourcefilecontents (http://b/37139570) + // should be reported as errors, not just as warnings! + "outjars", + "adaptresourcefilecontents"); + private static final List<String> warnedFlagOptions = ImmutableList .of(); // Those options are unsupported and are treated as compilation errors. // Just ignoring them would produce outputs incompatible with user expectations. - public static final List<String> UNSUPPORTED_FLAG_OPTIONS = ImmutableList - .of("adaptresourcefilecontents", - "outjars", - "skipnonpubliclibraryclasses"); + private static final List<String> unsupportedFlagOptions = ImmutableList + .of("skipnonpubliclibraryclasses"); public ProguardConfigurationParser( DexItemFactory dexItemFactory, DiagnosticsHandler diagnosticsHandler) { @@ -105,26 +107,17 @@ public void parse(List<ProguardConfigurationSource> sources) throws ProguardRuleParserException, IOException { for (ProguardConfigurationSource source : sources) { - new ProguardConfigurationSourceParser(source).parse(); + new ProguardFileParser(source, diagnosticsHandler).parse(); } } - private void warnIgnoringOptions(String optionName) { - diagnosticsHandler.warning(new StringDiagnostic("Ignoring option: -" + optionName)); - } - - private void warnOverridingOptions(String optionName, String victim) { - diagnosticsHandler.warning( - new StringDiagnostic("Option -" + optionName + " overrides -" + victim)); - } - - private class ProguardConfigurationSourceParser { + private class ProguardFileParser { private final String name; private final String contents; private int position = 0; private Path baseDirectory; - ProguardConfigurationSourceParser(ProguardConfigurationSource source) + ProguardFileParser(ProguardConfigurationSource source, DiagnosticsHandler diagnosticsHandler) throws IOException { contents = source.get(); baseDirectory = source.getBaseDirectory(); @@ -147,20 +140,18 @@ } expectChar('-'); String option; - if (Iterables.any(IGNORED_SINGLE_ARG_OPTIONS, this::skipOptionWithSingleArg) - || Iterables.any( - IGNORED_OPTIONAL_SINGLE_ARG_OPTIONS, this::skipOptionWithOptionalSingleArg) - || Iterables.any(IGNORED_FLAG_OPTIONS, this::skipFlag) - || Iterables.any(IGNORED_CLASS_DESCRIPTOR_OPTIONS, this::skipOptionWithClassSpec) + if (Iterables.any(ignoredSingleArgOptions, this::skipOptionWithSingleArg) + || Iterables.any(ignoredOptionalSingleArgOptions, this::skipOptionWithOptionalSingleArg) + || Iterables.any(ignoredFlagOptions, this::skipFlag) + || Iterables.any(ignoredClassDescriptorOptions, this::skipOptionWithClassSpec) || parseOptimizationOption()) { // Intentionally left empty. } else if ( - (option = Iterables.find(WARNED_SINGLE_ARG_OPTIONS, + (option = Iterables.find(warnedSingleArgOptions, this::skipOptionWithSingleArg, null)) != null - || (option = Iterables.find(WARNED_FLAG_OPTIONS, this::skipFlag, null)) != null) { + || (option = Iterables.find(warnedFlagOptions, this::skipFlag, null)) != null) { warnIgnoringOptions(option); - } else if ( - (option = Iterables.find(UNSUPPORTED_FLAG_OPTIONS, this::skipFlag, null)) != null) { + } else if ((option = Iterables.find(unsupportedFlagOptions, this::skipFlag, null)) != null) { throw parseError("Unsupported option: -" + option); } else if (acceptString("renamesourcefileattribute")) { skipWhitespace(); @@ -305,11 +296,19 @@ return true; } + private void warnIgnoringOptions(String optionName) { + diagnosticsHandler.warning(new StringDiagnostic("Ignoring option: -" + optionName)); + } + + private void warnOverridingOptions(String optionName, String victim) { + diagnosticsHandler.warning( + new StringDiagnostic("Option -" + optionName + " overrides -" + victim)); + } private void parseInclude() throws ProguardRuleParserException { Path included = parseFileName(); try { - new ProguardConfigurationSourceParser(new ProguardConfigurationSourceFile(included)) + new ProguardFileParser(new ProguardConfigurationSourceFile(included), diagnosticsHandler) .parse(); } catch (FileNotFoundException | NoSuchFileException e) { throw parseError("Included file '" + included.toString() + "' not found", e);
diff --git a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationSourceStrings.java b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationSourceStrings.java index 4a62485..a4c89e0 100644 --- a/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationSourceStrings.java +++ b/src/main/java/com/android/tools/r8/shaking/ProguardConfigurationSourceStrings.java
@@ -11,31 +11,20 @@ import joptsimple.internal.Strings; public class ProguardConfigurationSourceStrings implements ProguardConfigurationSource { - private final Path baseDirectory; private final List<String> config; public ProguardConfigurationSourceStrings(List<String> config) { - this(Paths.get("."), config); - } - - /** - * Creates {@link ProguardConfigurationSource} with raw {@param config}, along with - * {@param baseDirectory}, which allows all other options that use a relative path to reach out - * to desired paths appropriately. - */ - public ProguardConfigurationSourceStrings(Path baseDirectory, List<String> config) { - this.baseDirectory = baseDirectory; this.config = config; } @Override public String get() throws IOException{ - return Strings.join(config, System.lineSeparator()); + return Strings.join(config, "\n"); } @Override public Path getBaseDirectory() { - return baseDirectory; + return Paths.get("."); } @Override
diff --git a/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java b/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java index cf56c9e..3e4d129 100644 --- a/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java +++ b/src/test/java/com/android/tools/r8/internal/CompilationTestBase.java
@@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. package com.android.tools.r8.internal; -import static com.android.tools.r8.shaking.ProguardConfigurationParser.UNSUPPORTED_FLAG_OPTIONS; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -15,8 +14,7 @@ import com.android.tools.r8.R8RunArtTestsTest.CompilerUnderTest; import com.android.tools.r8.Resource; import com.android.tools.r8.ToolHelper; -import com.android.tools.r8.shaking.ProguardConfigurationSourceFile; -import com.android.tools.r8.shaking.ProguardConfigurationSourceStrings; +import com.android.tools.r8.dex.Constants; import com.android.tools.r8.shaking.ProguardRuleParserException; import com.android.tools.r8.utils.AndroidApiLevel; import com.android.tools.r8.utils.AndroidApp; @@ -27,7 +25,6 @@ import com.android.tools.r8.utils.InternalOptions; import com.android.tools.r8.utils.ListUtils; import com.android.tools.r8.utils.OutputMode; -import com.google.common.collect.ImmutableList; import com.google.common.io.ByteStreams; import com.google.common.io.Closer; import java.io.File; @@ -39,16 +36,12 @@ import java.util.List; import java.util.concurrent.ExecutionException; import java.util.function.Consumer; -import java.util.function.Predicate; import org.junit.ComparisonFailure; import org.junit.Rule; import org.junit.rules.TemporaryFolder; public abstract class CompilationTestBase { - private static final Predicate<String> BYPASSING_OPTIONS_FOR_TESTING = - line -> UNSUPPORTED_FLAG_OPTIONS.stream().noneMatch(line::contains); - @Rule public TemporaryFolder temp = ToolHelper.getTemporaryFolderForTest(); @@ -81,24 +74,16 @@ assertTrue(referenceApk == null || new File(referenceApk).exists()); AndroidApp outputApp; if (compiler == CompilerUnderTest.R8) { - R8Command.Builder builder = - R8Command.builder() - .addProgramFiles(ListUtils.map(inputs, Paths::get)) - .setMode(mode) - .setMinApiLevel(AndroidApiLevel.L.getLevel()); + R8Command.Builder builder = R8Command.builder(); + builder.addProgramFiles(ListUtils.map(inputs, Paths::get)); if (pgMap != null) { builder.setProguardMapFile(Paths.get(pgMap)); } if (pgConf != null) { - ProguardConfigurationSourceFile pgFile = - new ProguardConfigurationSourceFile(Paths.get(pgConf)); - builder.addProguardConfiguration( - new ProguardConfigurationSourceStrings( - Paths.get(pgConf).getParent(), - Arrays.stream(pgFile.get().split(System.lineSeparator())) - .filter(BYPASSING_OPTIONS_FOR_TESTING) - .collect(ImmutableList.toImmutableList()))); + builder.addProguardConfigurationFiles(Paths.get(pgConf)); } + builder.setMode(mode); + builder.setMinApiLevel(AndroidApiLevel.L.getLevel()); builder.addProguardConfigurationConsumer(b -> { b.setPrintSeeds(false); });