diff options
4 files changed, 85 insertions, 59 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 1a7c376731..82361f7bbb 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java @@ -1002,10 +1002,17 @@ public final class PackageFactory { prefetchGlobs(packageId, buildFileAST, preprocessingResult.preprocessed, buildFile, globber, defaultVisibility, makeEnv); return evaluateBuildFile( - externalPkg, packageId, buildFileAST, buildFile, globber, + externalPkg, + packageId, + buildFileAST, + buildFile, + globber, Iterables.concat(preprocessingEvents, localReporter.getEvents()), - defaultVisibility, preprocessingResult.containsErrors, - preprocessingResult.containsTransientErrors, makeEnv, imports, skylarkFileDependencies); + defaultVisibility, + preprocessingResult.containsErrors, + makeEnv, + imports, + skylarkFileDependencies); } catch (InterruptedException e) { globber.onInterrupt(); throw e; @@ -1034,8 +1041,14 @@ public final class PackageFactory { } Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator); - Preprocessor.Result preprocessingResult = - preprocess(packageId, buildFile, inputSource, globber); + Preprocessor.Result preprocessingResult; + try { + preprocessingResult = preprocess(packageId, inputSource, globber); + } catch (IOException e) { + eventHandler.handle( + Event.error(Location.fromFile(buildFile), "preprocessing failed: " + e.getMessage())); + throw new BuildFileContainsErrorsException(packageId, "preprocessing failed", e); + } ExternalPackage externalPkg = new ExternalPackage.Builder( buildFile.getRelative("WORKSPACE"), ruleClassProvider.getRunfilesPrefix()).build(); @@ -1060,20 +1073,13 @@ public final class PackageFactory { /** Preprocesses the given BUILD file. */ public Preprocessor.Result preprocess( - PackageIdentifier packageId, - Path buildFile, - CachingPackageLocator locator) throws InterruptedException { + PackageIdentifier packageId, Path buildFile, CachingPackageLocator locator) + throws InterruptedException, IOException { ParserInputSource inputSource; - try { - inputSource = ParserInputSource.create(buildFile); - } catch (IOException e) { - List<Event> events = ImmutableList.of( - Event.error(Location.fromFile(buildFile), e.getMessage())); - return Preprocessor.Result.transientError(buildFile.asFragment(), events); - } + inputSource = ParserInputSource.create(buildFile); Globber globber = createLegacyGlobber(buildFile.getParentDirectory(), packageId, locator); try { - return preprocess(packageId, buildFile, inputSource, globber); + return preprocess(packageId, inputSource, globber); } finally { globber.onCompletion(); } @@ -1084,10 +1090,8 @@ public final class PackageFactory { * {@link InterruptedException}. */ public Preprocessor.Result preprocess( - PackageIdentifier packageId, - Path buildFile, - ParserInputSource inputSource, - Globber globber) throws InterruptedException { + PackageIdentifier packageId, ParserInputSource inputSource, Globber globber) + throws InterruptedException, IOException { Preprocessor preprocessor = preprocessorFactory.getPreprocessor(); if (preprocessor == null) { return Preprocessor.Result.noPreprocessing(inputSource); @@ -1099,10 +1103,6 @@ public final class PackageFactory { globber, Environment.BUILD, ruleFactory.getRuleClassNames()); - } catch (IOException e) { - List<Event> events = ImmutableList.of(Event.error(Location.fromFile(buildFile), - "preprocessing failed: " + e.getMessage())); - return Preprocessor.Result.transientError(buildFile.asFragment(), events); } catch (InterruptedException e) { globber.onInterrupt(); throw e; @@ -1238,12 +1238,19 @@ public final class PackageFactory { * @see PackageFactory#PackageFactory */ @VisibleForTesting // used by PackageFactoryApparatus - public Package.LegacyBuilder evaluateBuildFile(Package externalPkg, - PackageIdentifier packageId, BuildFileAST buildFileAST, Path buildFilePath, Globber globber, - Iterable<Event> pastEvents, RuleVisibility defaultVisibility, boolean containsError, - boolean containsTransientError, MakeEnvironment.Builder pkgMakeEnv, + public Package.LegacyBuilder evaluateBuildFile( + Package externalPkg, + PackageIdentifier packageId, + BuildFileAST buildFileAST, + Path buildFilePath, + Globber globber, + Iterable<Event> pastEvents, + RuleVisibility defaultVisibility, + boolean containsError, + MakeEnvironment.Builder pkgMakeEnv, Map<PathFragment, Extension> imports, - ImmutableList<Label> skylarkFileDependencies) throws InterruptedException { + ImmutableList<Label> skylarkFileDependencies) + throws InterruptedException { Package.LegacyBuilder pkgBuilder = new Package.LegacyBuilder( packageId, ruleClassProvider.getRunfilesPrefix()); StoredEventHandler eventHandler = new StoredEventHandler(); @@ -1278,10 +1285,6 @@ public final class PackageFactory { pkgBuilder.setContainsErrors(); } - if (containsTransientError) { - pkgBuilder.setContainsTemporaryErrors(); - } - if (!validatePackageIdentifier(packageId, buildFileAST.getLocation(), eventHandler)) { pkgBuilder.setContainsErrors(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java index acf06f9460..f3c792fbb9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Preprocessor.java @@ -97,23 +97,26 @@ public interface Preprocessor { public final ParserInputSource result; public final boolean preprocessed; public final boolean containsErrors; - public final boolean containsTransientErrors; public final List<Event> events; - private Result(ParserInputSource result, - boolean preprocessed, boolean containsPersistentErrors, boolean containsTransientErrors, + private Result( + ParserInputSource result, + boolean preprocessed, + boolean containsErrors, List<Event> events) { this.result = result; this.preprocessed = preprocessed; - this.containsErrors = containsPersistentErrors || containsTransientErrors; - this.containsTransientErrors = containsTransientErrors; + this.containsErrors = containsErrors; this.events = ImmutableList.copyOf(events); } /** Convenience factory for a {@link Result} wrapping non-preprocessed BUILD file contents. */ public static Result noPreprocessing(ParserInputSource buildFileSource) { - return new Result(buildFileSource, /*preprocessed=*/false, /*containsErrors=*/false, - /*containsTransientErrors=*/false, ImmutableList.<Event>of()); + return new Result( + buildFileSource, + /*preprocessed=*/false, + /*containsErrors=*/false, + ImmutableList.<Event>of()); } /** @@ -123,32 +126,31 @@ public interface Preprocessor { */ public static Result success(ParserInputSource result, boolean containsErrors, List<Event> events) { - return new Result(result, /*preprocessed=*/true, /*containsPersistentErrors=*/containsErrors, - /*containsTransientErrors=*/false, events); + return new Result(result, /*preprocessed=*/true, containsErrors, events); } public static Result invalidSyntax(PathFragment buildFile, List<Event> events) { - return new Result(ParserInputSource.create(EMPTY_CHARS, buildFile), /*preprocessed=*/true, - /*containsPersistentErrors=*/true, /*containsTransientErrors=*/false, events); - } - - public static Result transientError(PathFragment buildFile, List<Event> events) { - return new Result(ParserInputSource.create(EMPTY_CHARS, buildFile), /*preprocessed=*/false, - /*containsPersistentErrors=*/false, /*containsTransientErrors=*/true, events); + return new Result( + ParserInputSource.create(EMPTY_CHARS, buildFile), + /*preprocessed=*/true, + /*containsErrors=*/true, + events); } } /** * Returns a Result resulting from applying Python preprocessing to the contents of "in". If * errors happen, they must be reported both as an event on eventHandler and in the function - * return value. + * return value. An IOException is only thrown when preparing for preprocessing. Once + * preprocessing actually begins, any I/O problems encountered will be reflected in the return + * value, not manifested as exceptions. * * @param in the BUILD file to be preprocessed. * @param packageName the BUILD file's package. * @param globber a globber for evaluating globs. * @param globals the global bindings for the Python environment. * @param ruleNames the set of names of all rules in the build language. - * @throws IOException if there was an I/O problem during preprocessing. + * @throws IOException if there was an I/O problem preparing for preprocessing. * @return a pair of the ParserInputSource and a map of subincludes seen during the evaluation */ Result preprocess( diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index 95fef27c7e..2de3167e86 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -800,9 +800,22 @@ public class PackageFunction implements SkyFunction { packageId, packageLocator); Preprocessor.Result preprocessingResult = preprocessCache.getIfPresent(packageId); if (preprocessingResult == null) { - preprocessingResult = replacementSource == null - ? packageFactory.preprocess(packageId, buildFilePath, inputSource, globber) - : Preprocessor.Result.noPreprocessing(replacementSource); + try { + preprocessingResult = + replacementSource == null + ? packageFactory.preprocess(packageId, inputSource, globber) + : Preprocessor.Result.noPreprocessing(replacementSource); + } catch (IOException e) { + env + .getListener() + .handle( + Event.error( + Location.fromFile(buildFilePath), + "preprocessing failed: " + e.getMessage())); + throw new PackageFunctionException( + new BuildFileContainsErrorsException(packageId, "preprocessing failed", e), + Transience.TRANSIENT); + } preprocessCache.put(packageId, preprocessingResult); } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java index 6306e468fb..e1f7271f3e 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryApparatus.java @@ -131,11 +131,19 @@ public class PackageFactoryApparatus { LegacyGlobber globber = new LegacyGlobber(globCache); ExternalPackage externalPkg = new ExternalPackage.Builder( buildFile.getParentDirectory().getRelative("WORKSPACE"), "TESTING").build(); - LegacyBuilder resultBuilder = factory.evaluateBuildFile( - externalPkg, packageId, buildFileAST, buildFile, - globber, ImmutableList.<Event>of(), ConstantRuleVisibility.PUBLIC, false, false, - new MakeEnvironment.Builder(), ImmutableMap.<PathFragment, Extension>of(), - ImmutableList.<Label>of()); + LegacyBuilder resultBuilder = + factory.evaluateBuildFile( + externalPkg, + packageId, + buildFileAST, + buildFile, + globber, + ImmutableList.<Event>of(), + ConstantRuleVisibility.PUBLIC, + false, + new MakeEnvironment.Builder(), + ImmutableMap.<PathFragment, Extension>of(), + ImmutableList.<Label>of()); Package result = resultBuilder.build(); Event.replayEventsOn(events.reporter(), result.getEvents()); return Pair.of(result, globCache); |