diff options
6 files changed, 145 insertions, 3 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java index f78e0afb75..c36de6fd0c 100644 --- a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java +++ b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingResult.java @@ -13,9 +13,9 @@ // limitations under the License. package com.google.devtools.build.lib.pkgcache; +import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.packages.Target; - import java.util.Collection; /** @@ -63,4 +63,15 @@ public final class LoadingResult { public String getWorkspaceName() { return workspaceName; } + + @Override + public String toString() { + return MoreObjects.toStringHelper(LoadingResult.class) + .add("hasTargetPatternError", hasTargetPatternError) + .add("hasLoadingError", hasLoadingError) + .add("targetsToAnalyze", targetsToAnalyze) + .add("testsToRun", testsToRun) + .add("workspaceName", workspaceName) + .toString(); + } }
\ No newline at end of file diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index b8b1e292b7..04ff12caf5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -47,6 +47,8 @@ public final class SkyFunctions { public static final SkyFunctionName PACKAGE_ERROR = SkyFunctionName.create("PACKAGE_ERROR"); public static final SkyFunctionName TARGET_MARKER = SkyFunctionName.create("TARGET_MARKER"); public static final SkyFunctionName TARGET_PATTERN = SkyFunctionName.create("TARGET_PATTERN"); + public static final SkyFunctionName TARGET_PATTERN_ERROR = + SkyFunctionName.create("TARGET_PATTERN_ERROR"); public static final SkyFunctionName PREPARE_DEPS_OF_PATTERNS = SkyFunctionName.create("PREPARE_DEPS_OF_PATTERNS"); public static final SkyFunctionName PREPARE_DEPS_OF_PATTERN = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 7ae5ac59e7..93775243af 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -410,6 +410,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { packageProgress)); map.put(SkyFunctions.PACKAGE_ERROR, new PackageErrorFunction()); map.put(SkyFunctions.TARGET_MARKER, new TargetMarkerFunction()); + map.put(SkyFunctions.TARGET_PATTERN_ERROR, new TargetPatternErrorFunction()); map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider)); map.put(Label.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction()); map.put( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternErrorFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternErrorFunction.java new file mode 100644 index 0000000000..cf21520c85 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternErrorFunction.java @@ -0,0 +1,61 @@ +// Copyright 2015 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import com.google.devtools.build.lib.cmdline.TargetParsingException; +import com.google.devtools.build.skyframe.LegacySkyKey; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionException; +import com.google.devtools.build.skyframe.SkyFunctionException.Transience; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import javax.annotation.Nullable; + +/** + * SkyFunction that throws a {@link TargetParsingException} for target pattern that could not be + * parsed. Must only be requested when a SkyFunction wishes to ignore the errors + * in a target pattern in keep_going mode, but to shut down the build in nokeep_going mode. + * + * <p>This SkyFunction never returns a value, only throws a {@link TargetParsingException}, and + * should never return null, since all of its dependencies should already be present. + */ +public class TargetPatternErrorFunction implements SkyFunction { + // We pass in the error message, which isn't ideal. We could consider reparsing the original + // pattern instead, but that requires more information. + public static SkyKey key(String errorMessage) { + return LegacySkyKey.create(SkyFunctions.TARGET_PATTERN_ERROR, errorMessage); + } + + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) + throws TargetErrorFunctionException, InterruptedException { + String errorMessage = (String) skyKey.argument(); + throw new TargetErrorFunctionException( + new TargetParsingException(errorMessage), Transience.PERSISTENT); + } + + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + + private static class TargetErrorFunctionException extends SkyFunctionException { + public TargetErrorFunctionException( + TargetParsingException cause, Transience transience) { + super(cause, transience); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java index 75d38b81d3..28803ff1bb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java @@ -206,8 +206,9 @@ final class TargetPatternPhaseFunction implements SkyFunction { */ private static ResolvedTargets<Target> getTargetsToBuild( Environment env, TargetPatternPhaseKey options, PathPackageLocator pkgPath) - throws InterruptedException { + throws InterruptedException { List<TargetPatternKey> patternSkyKeys = new ArrayList<>(); + ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder(); for (TargetPatternSkyKeyOrException keyOrException : TargetPatternValue.keys( options.getTargetPatterns(), @@ -224,6 +225,16 @@ final class TargetPatternPhaseFunction implements SkyFunction { // pattern could be parsed successfully). env.getListener().post( new ParsingFailedEvent(keyOrException.getOriginalPattern(), e.getMessage())); + try { + env.getValueOrThrow( + TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class); + } catch (TargetParsingException ignore) { + // We ignore this. Keep going is active. + } + env.getListener().handle( + Event.error( + "Skipping '" + keyOrException.getOriginalPattern() + "': " + e.getMessage())); + builder.setError(); } } Map<SkyKey, ValueOrException<TargetParsingException>> resolvedPatterns = @@ -232,7 +243,6 @@ final class TargetPatternPhaseFunction implements SkyFunction { return null; } - ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder(); for (TargetPatternKey pattern : patternSkyKeys) { TargetPatternValue value; try { @@ -265,6 +275,12 @@ final class TargetPatternPhaseFunction implements SkyFunction { } catch (MissingDepException e) { return null; } catch (TargetParsingException e) { + try { + env.getValueOrThrow( + TargetPatternErrorFunction.key(e.getMessage()), TargetParsingException.class); + } catch (TargetParsingException ignore) { + // We ignore this. Keep going is active. + } env.getListener().handle(Event.error(e.getMessage())); return ResolvedTargets.failed(); } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index c2180ef14f..6814b89f4a 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -148,6 +148,43 @@ public class LoadingPhaseRunnerTest { } @Test + public void testMistypedTarget() throws Exception { + try { + tester.load("foo//bar:missing"); + fail(); + } catch (TargetParsingException e) { + assertThat(e).hasMessageThat().contains( + "invalid target format 'foo//bar:missing': " + + "invalid package name 'foo//bar': " + + "package names may not contain '//' path separators"); + } + } + + @Test + public void testEmptyTarget() throws Exception { + try { + tester.load(""); + fail(); + } catch (TargetParsingException e) { + assertThat(e).hasMessageThat().contains("the empty string is not a valid target"); + } + } + + @Test + public void testMistypedTargetKeepGoing() throws Exception { + LoadingResult result = tester.loadKeepGoing("foo//bar:missing"); + // Legacy loading phase does _not_ report a target pattern error, and it's work to fix, so we + // skip this check for now. + if (useSkyframeTargetPatternEval()) { + assertThat(result.hasTargetPatternError()).isTrue(); + } + tester.assertContainsError( + "invalid target format 'foo//bar:missing': " + + "invalid package name 'foo//bar': " + + "package names may not contain '//' path separators"); + } + + @Test public void testBadTargetPatternWithTest() throws Exception { tester.addFile("base/BUILD"); LoadingResult loadingResult = tester.loadTestsKeepGoing("//base:missing"); @@ -553,6 +590,20 @@ public class LoadingPhaseRunnerTest { } @Test + public void testCompileOneDependencyReferencesFile() throws Exception { + tester.addFile("base/BUILD", + "cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])"); + tester.useLoadingOptions("--compile_one_dependency"); + try { + tester.load("//base:hello"); + fail(); + } catch (TargetParsingException e) { + assertThat(e).hasMessageThat() + .contains("--compile_one_dependency target '//base:hello' must be a file"); + } + } + + @Test public void testParsingFailureReported() throws Exception { LoadingResult loadingResult = tester.loadKeepGoing("//does_not_exist"); assertThat(loadingResult.hasTargetPatternError()).isTrue(); |