Report errors for non-existant flag files correctly.
Additionally, fix the contents of the flags file for the tests.
This worked by accident on Linux but is incorrect and doesn't
work on Windows.
R=sgjesse@google.com, zerny@google.com
Change-Id: I1e67db038d65c739808daff927a13aabdbba32d0
diff --git a/src/main/java/com/android/tools/r8/D8.java b/src/main/java/com/android/tools/r8/D8.java
index 250f2e9..1db187e 100644
--- a/src/main/java/com/android/tools/r8/D8.java
+++ b/src/main/java/com/android/tools/r8/D8.java
@@ -17,7 +17,6 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.ExceptionUtils;
-import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.ThreadUtils;
import com.android.tools.r8.utils.Timing;
@@ -94,9 +93,8 @@
});
}
- private static void run(String[] args) throws CompilationFailedException, IOException {
- String[] expandedArgs = FlagFile.expandFlagFiles(args);
- D8Command command = D8Command.parse(expandedArgs, CommandLineOrigin.INSTANCE).build();
+ private static void run(String[] args) throws CompilationFailedException {
+ D8Command command = D8Command.parse(args, CommandLineOrigin.INSTANCE).build();
if (command.isPrintHelp()) {
System.out.println(USAGE_MESSAGE);
return;
diff --git a/src/main/java/com/android/tools/r8/D8Command.java b/src/main/java/com/android/tools/r8/D8Command.java
index 4c65134..6f956db 100644
--- a/src/main/java/com/android/tools/r8/D8Command.java
+++ b/src/main/java/com/android/tools/r8/D8Command.java
@@ -7,6 +7,7 @@
import com.android.tools.r8.graph.DexItemFactory;
import com.android.tools.r8.origin.Origin;
import com.android.tools.r8.utils.AndroidApp;
+import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.Reporter;
import com.android.tools.r8.utils.StringDiagnostic;
@@ -215,9 +216,10 @@
Path outputPath = null;
OutputMode outputMode = null;
boolean hasDefinedApiLevel = false;
+ String[] expandedArgs = FlagFile.expandFlagFiles(args, builder.getReporter());
try {
- for (int i = 0; i < args.length; i++) {
- String arg = args[i].trim();
+ for (int i = 0; i < expandedArgs.length; i++) {
+ String arg = expandedArgs[i].trim();
if (arg.length() == 0) {
continue;
} else if (arg.equals("--help")) {
@@ -243,7 +245,7 @@
} else if (arg.equals("--file-per-class")) {
outputMode = OutputMode.DexFilePerClassFile;
} else if (arg.equals("--output")) {
- String output = args[++i];
+ String output = expandedArgs[++i];
if (outputPath != null) {
builder.getReporter().error(new StringDiagnostic(
"Cannot output both to '" + outputPath.toString() + "' and '" + output + "'",
@@ -252,15 +254,15 @@
}
outputPath = Paths.get(output);
} else if (arg.equals("--lib")) {
- builder.addLibraryFiles(Paths.get(args[++i]));
+ builder.addLibraryFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--classpath")) {
- builder.addClasspathFiles(Paths.get(args[++i]));
+ builder.addClasspathFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--main-dex-list")) {
- builder.addMainDexListFiles(Paths.get(args[++i]));
+ builder.addMainDexListFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--optimize-multidex-for-linearalloc")) {
builder.setOptimizeMultidexForLinearAlloc(true);
} else if (arg.equals("--min-api")) {
- hasDefinedApiLevel = parseMinApi(builder, args[++i], hasDefinedApiLevel, origin);
+ hasDefinedApiLevel = parseMinApi(builder, expandedArgs[++i], hasDefinedApiLevel, origin);
} else if (arg.equals("--intermediate")) {
builder.setIntermediate(true);
} else if (arg.equals("--no-desugaring")) {
diff --git a/src/main/java/com/android/tools/r8/R8.java b/src/main/java/com/android/tools/r8/R8.java
index 8e1722e..16c9de8 100644
--- a/src/main/java/com/android/tools/r8/R8.java
+++ b/src/main/java/com/android/tools/r8/R8.java
@@ -51,7 +51,6 @@
import com.android.tools.r8.utils.CfgPrinter;
import com.android.tools.r8.utils.ExceptionUtils;
import com.android.tools.r8.utils.FileUtils;
-import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.android.tools.r8.utils.LineNumberOptimizer;
@@ -534,9 +533,8 @@
}
}
- private static void run(String[] args) throws CompilationFailedException, IOException {
- String[] expandedArgs = FlagFile.expandFlagFiles(args);
- R8Command command = R8Command.parse(expandedArgs, CommandLineOrigin.INSTANCE).build();
+ private static void run(String[] args) throws CompilationFailedException {
+ R8Command command = R8Command.parse(args, CommandLineOrigin.INSTANCE).build();
if (command.isPrintHelp()) {
System.out.println(USAGE_MESSAGE);
return;
diff --git a/src/main/java/com/android/tools/r8/R8Command.java b/src/main/java/com/android/tools/r8/R8Command.java
index ced17d4..4fc0590 100644
--- a/src/main/java/com/android/tools/r8/R8Command.java
+++ b/src/main/java/com/android/tools/r8/R8Command.java
@@ -18,6 +18,7 @@
import com.android.tools.r8.utils.AndroidApp;
import com.android.tools.r8.utils.ExceptionDiagnostic;
import com.android.tools.r8.utils.FileUtils;
+import com.android.tools.r8.utils.FlagFile;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.InternalOptions.LineNumberOptimization;
import com.android.tools.r8.utils.Reporter;
@@ -513,8 +514,9 @@
Origin argsOrigin,
Builder builder,
ParseState state) {
- for (int i = 0; i < args.length; i++) {
- String arg = args[i].trim();
+ String[] expandedArgs = FlagFile.expandFlagFiles(args, builder.getReporter());
+ for (int i = 0; i < expandedArgs.length; i++) {
+ String arg = expandedArgs[i].trim();
if (arg.length() == 0) {
continue;
} else if (arg.equals("--help")) {
@@ -546,7 +548,7 @@
}
state.outputMode = OutputMode.ClassFile;
} else if (arg.equals("--output")) {
- String outputPath = args[++i];
+ String outputPath = expandedArgs[++i];
if (state.outputPath != null) {
builder.getReporter().error(new StringDiagnostic(
"Cannot output both to '"
@@ -558,10 +560,10 @@
}
state.outputPath = Paths.get(outputPath);
} else if (arg.equals("--lib")) {
- builder.addLibraryFiles(Paths.get(args[++i]));
+ builder.addLibraryFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--min-api")) {
state.hasDefinedApiLevel =
- parseMinApi(builder, args[++i], state.hasDefinedApiLevel, argsOrigin);
+ parseMinApi(builder, expandedArgs[++i], state.hasDefinedApiLevel, argsOrigin);
} else if (arg.equals("--no-tree-shaking")) {
builder.setDisableTreeShaking(true);
} else if (arg.equals("--no-minification")) {
@@ -569,17 +571,17 @@
} else if (arg.equals("--no-desugaring")) {
builder.setDisableDesugaring(true);
} else if (arg.equals("--main-dex-rules")) {
- builder.addMainDexRulesFiles(Paths.get(args[++i]));
+ builder.addMainDexRulesFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--main-dex-list")) {
- builder.addMainDexListFiles(Paths.get(args[++i]));
+ builder.addMainDexListFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--main-dex-list-output")) {
- builder.setMainDexListOutputPath(Paths.get(args[++i]));
+ builder.setMainDexListOutputPath(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--optimize-multidex-for-linearalloc")) {
builder.setOptimizeMultidexForLinearAlloc(true);
} else if (arg.equals("--pg-conf")) {
- builder.addProguardConfigurationFiles(Paths.get(args[++i]));
+ builder.addProguardConfigurationFiles(Paths.get(expandedArgs[++i]));
} else if (arg.equals("--pg-map-output")) {
- builder.setProguardMapOutputPath(Paths.get(args[++i]));
+ builder.setProguardMapOutputPath(Paths.get(expandedArgs[++i]));
} else {
if (arg.startsWith("--")) {
builder.getReporter().error(new StringDiagnostic("Unknown option: " + arg, argsOrigin));
diff --git a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
index 577632a..9c7f525 100644
--- a/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
+++ b/src/main/java/com/android/tools/r8/utils/ExceptionUtils.java
@@ -74,7 +74,7 @@
}
public interface MainAction {
- void run() throws CompilationFailedException, IOException;
+ void run() throws CompilationFailedException;
}
public static void withMainProgramHandler(MainAction action) {
@@ -84,7 +84,7 @@
// Detail of the errors were already reported
System.err.println("Compilation failed");
System.exit(STATUS_ERROR);
- } catch (RuntimeException | IOException e) {
+ } catch (RuntimeException e) {
System.err.println("Compilation failed with an internal error.");
Throwable cause = e.getCause() == null ? e : e.getCause();
cause.printStackTrace();
diff --git a/src/main/java/com/android/tools/r8/utils/FlagFile.java b/src/main/java/com/android/tools/r8/utils/FlagFile.java
index 2354066..4791217 100644
--- a/src/main/java/com/android/tools/r8/utils/FlagFile.java
+++ b/src/main/java/com/android/tools/r8/utils/FlagFile.java
@@ -4,18 +4,41 @@
package com.android.tools.r8.utils;
+import com.android.tools.r8.origin.Origin;
import java.io.IOException;
import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
public class FlagFile {
- public static String[] expandFlagFiles(String[] args) throws IOException {
+
+ private static class FlagFileOrigin extends Origin {
+ private final Path path;
+
+ protected FlagFileOrigin(Path path) {
+ super(Origin.root());
+ this.path = path;
+ }
+
+ @Override
+ public String part() {
+ return "flag file argument: '@" + path + "'";
+ }
+ }
+
+ public static String[] expandFlagFiles(String[] args, Reporter reporter) {
List<String> flags = new ArrayList<>(args.length);
for (String arg : args) {
if (arg.startsWith("@")) {
- flags.addAll(Files.readAllLines(Paths.get(arg.substring(1))));
+ Path flagFilePath = Paths.get(arg.substring(1));
+ try {
+ flags.addAll(Files.readAllLines(flagFilePath));
+ } catch (IOException e) {
+ Origin origin = new FlagFileOrigin(flagFilePath);
+ reporter.error(new ExceptionDiagnostic(e, origin));
+ }
} else {
flags.add(arg);
}
diff --git a/src/test/java/com/android/tools/r8/D8CommandTest.java b/src/test/java/com/android/tools/r8/D8CommandTest.java
index 5576058..1b846a5 100644
--- a/src/test/java/com/android/tools/r8/D8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/D8CommandTest.java
@@ -9,6 +9,7 @@
import static java.nio.file.StandardOpenOption.CREATE_NEW;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
import com.android.sdklib.AndroidVersion;
@@ -99,7 +100,7 @@
FileUtils.writeTextFile(
flagsFile,
"--output",
- output.toString(),
+ "output.zip",
"--min-api",
"24",
input.toString());
@@ -110,6 +111,19 @@
assertEquals(Tool.D8, marker.getTool());
}
+ @Test(expected=CompilationFailedException.class)
+ public void nonExistingFlagsFile() throws Throwable {
+ Path working = temp.getRoot().toPath();
+ Path flags = working.resolve("flags.txt").toAbsolutePath();
+ assertNotEquals(0, ToolHelper.forkR8(working, "@flags.txt").exitCode);
+ DiagnosticsChecker.checkErrorsContains("File not found", handler ->
+ D8.run(
+ D8Command.parse(
+ new String[] { "@" + flags.toString() },
+ EmbeddedOrigin.INSTANCE,
+ handler).build()));
+ }
+
@Test
public void printsHelpOnNoInput() throws Throwable {
ProcessResult result = ToolHelper.forkD8(temp.getRoot().toPath());
diff --git a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
index 2265c3e..2f45ae2 100644
--- a/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
+++ b/src/test/java/com/android/tools/r8/DiagnosticsChecker.java
@@ -49,13 +49,14 @@
try {
runner.run(handler);
} catch (CompilationFailedException e) {
+ List<String> messages = ListUtils.map(handler.errors, Diagnostic::getDiagnosticMessage);
System.out.println("Expecting match for '" + snippet + "'");
- System.out.println("StdErr:\n" + handler.errors);
+ System.out.println("StdErr:\n" + messages);
assertTrue(
"Expected to find snippet '"
+ snippet
+ "' in error messages:\n"
- + String.join("\n", ListUtils.map(handler.errors, Diagnostic::getDiagnosticMessage)),
+ + String.join("\n", messages),
handler.errors.stream().anyMatch(d -> d.getDiagnosticMessage().contains(snippet)));
throw e;
}
diff --git a/src/test/java/com/android/tools/r8/R8CommandTest.java b/src/test/java/com/android/tools/r8/R8CommandTest.java
index c1072bd..fbd88be 100644
--- a/src/test/java/com/android/tools/r8/R8CommandTest.java
+++ b/src/test/java/com/android/tools/r8/R8CommandTest.java
@@ -101,7 +101,7 @@
FileUtils.writeTextFile(
flagsFile,
"--output",
- output.toString(),
+ "output.zip",
"--min-api",
"24",
"--lib",
@@ -114,6 +114,20 @@
assertEquals(Tool.R8, marker.getTool());
}
+
+ @Test(expected=CompilationFailedException.class)
+ public void nonExistingFlagsFile() throws Throwable {
+ Path working = temp.getRoot().toPath();
+ Path flags = working.resolve("flags.txt").toAbsolutePath();
+ assertNotEquals(0, ToolHelper.forkR8(working, "@flags.txt").exitCode);
+ DiagnosticsChecker.checkErrorsContains("File not found", handler ->
+ R8.run(
+ R8Command.parse(
+ new String[] { "@" + flags.toString() },
+ EmbeddedOrigin.INSTANCE,
+ handler).build()));
+ }
+
@Test
public void printsHelpOnNoInput() throws Throwable {
ProcessResult result = ToolHelper.forkR8(temp.getRoot().toPath());
