From 3fb6ac385df80a5f583072c0b84247d503e330b7 Mon Sep 17 00:00:00 2001 From: tomlu Date: Thu, 24 Aug 2017 00:44:45 +0200 Subject: Allow CommandLine expansion to throw an exception. This paves the way for Skylark-side compact command lines that can fail during expansion. In general, expansion happens in these scenarios: * Action execution expands the command line to execute it. This is fine since we are well equipped already to handle failing actions. * In the analysis phase we expand command lines to investigate whether we need a params file. This could be moved to the execution phase, which would have the benefit of getting params files out of the action graph and saving memory. * Key computation expands the command line. This could be fixed by allowing command lines to compute their own keys (which wouldn't try to expand the command line). This could have the benefit of being more efficient. * Extra actions expand the command line to construct the extra action proto. This could maybe be deferred to the execution phase (writing the extra action), which would also be more memory efficient. For failed key computations, we return a singleton "error" key. This means multiple actions that will fail will map to the same key. These actions will necessarily fail if executed, so we should not need to worry about these ending up in the action cache. If we do manage to make the command line compute its own keys then this will become moot in the future. RELNOTES: None PiperOrigin-RevId: 166266385 --- .../java/com/google/devtools/build/lib/exec/TestStrategy.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java') diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java index 04affbbd02..78a878657f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java @@ -23,7 +23,9 @@ import com.google.common.io.ByteStreams; import com.google.common.io.Closeables; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.CommandLineExpansionException; import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.analysis.test.TestActionContext; import com.google.devtools.build.lib.analysis.test.TestResult; @@ -183,7 +185,11 @@ public abstract class TestStrategy implements TestActionContext { // Execute the test using the alias in the runfiles tree, as mandated by the Test Encyclopedia. args.add(execSettings.getExecutable().getRootRelativePath().getCallablePathString()); - Iterables.addAll(args, execSettings.getArgs().arguments()); + try { + Iterables.addAll(args, execSettings.getArgs().arguments()); + } catch (CommandLineExpansionException e) { + throw new UserExecException(e); + } return ImmutableList.copyOf(args); } -- cgit v1.2.3