diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java index 8f3f4e8a8417cc..d42057382fe0e6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java @@ -39,6 +39,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; /** * A {@link RecursivePackageProvider} backed by an {@link Environment}. Its methods may throw {@link @@ -52,11 +53,18 @@ public final class EnvironmentBackedRecursivePackageProvider extends AbstractRecursivePackageProvider { private final Environment env; + private final AtomicBoolean encounteredPackageErrors = new AtomicBoolean(false); EnvironmentBackedRecursivePackageProvider(Environment env) { this.env = env; } + // TODO(nharmata): Audit the rest of the codebase to determine if we should be calling this method + // in more places. + boolean encounteredPackageErrors() { + return encounteredPackageErrors.get(); + } + @Override public Package getPackage(ExtendedEventHandler eventHandler, PackageIdentifier packageName) throws NoSuchPackageException, MissingDepException, InterruptedException { @@ -77,7 +85,11 @@ public Package getPackage(ExtendedEventHandler eventHandler, PackageIdentifier p Preconditions.checkState(env.valuesMissing(), "Should have thrown for %s", packageName); throw new MissingDepException(); } catch (BuildFileContainsErrorsException e) { - // Expected. + // If this is a keep_going build, then the user of this RecursivePackageProvider has two + // options for handling the "package in error" case. The user must either inspect the + // package returned by this method, or else determine whether any errors have been seen via + // the "encounteredPackageErrors" method. + encounteredPackageErrors.set(true); } } return pkgValue.getPackage(); @@ -107,6 +119,7 @@ public boolean isPackage(ExtendedEventHandler eventHandler, PackageIdentifier pa return packageLookupValue.packageExists(); } catch (NoSuchPackageException | InconsistentFilesystemException e) { env.getListener().handle(Event.error(e.getMessage())); + encounteredPackageErrors.set(true); return false; } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorFunction.java index acef91d2dfad14..3ede380c49588d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageErrorFunction.java @@ -33,10 +33,11 @@ /** * SkyFunction that throws a {@link BuildFileContainsErrorsException} for {@link Package} that - * loaded, but was in error. Must only be requested when a SkyFunction wishes to ignore the errors - * in a {@link Package} in keep_going mode, but to shut down the build in nokeep_going mode. Thus, - * this SkyFunction should only be requested when the corresponding {@link PackageFunction} has - * already been successfully called and the resulting Package contains an error. + * loaded, but was in error. Must only be requested when a SkyFunction wishes to ignore the Skyframe + * error from a {@link PackageValue} in keep_going mode, but to shut down the build in nokeep_going + * mode. Thus, this SkyFunction should only be requested when the corresponding {@link + * PackageFunction} has already been successfully called and the resulting Package contains an + * error. * *

This SkyFunction never returns a value, only throws a {@link BuildFileNotFoundException}, and * should never return null, since all of its dependencies should already be present. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java index 52308fd69850b9..b195308dcb1de3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternFunction.java @@ -82,7 +82,12 @@ public void process(Iterable partialResult) { // The exception type here has to match the one on the BatchCallback. Since the callback // defined above never throws, the exact type here is not really relevant. RuntimeException.class); - resolvedTargets = ResolvedTargets.builder().addAll(results).build(); + ResolvedTargets.Builder resolvedTargetsBuilder = + ResolvedTargets.builder().addAll(results); + if (provider.encounteredPackageErrors()) { + resolvedTargetsBuilder.setError(); + } + resolvedTargets = resolvedTargetsBuilder.build(); } catch (TargetParsingException e) { env.getListener().post(new ParsingFailedEvent(patternKey.getPattern(), e.getMessage())); throw new TargetPatternFunctionException(e); @@ -96,6 +101,9 @@ public void process(Iterable partialResult) { } Preconditions.checkNotNull(resolvedTargets, key); ResolvedTargets.Builder