diff options
Diffstat (limited to 'src')
14 files changed, 448 insertions, 67 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 c192738a22..2b09e7b803 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 @@ -989,7 +989,7 @@ public final class PackageFactory { // Logged messages are used as a testability hook tracing the parsing progress LOG.fine("Starting to parse " + packageId); BuildFileAST buildFileAST = BuildFileAST.parseBuildFile( - preprocessingResult.result, preludeStatements, localReporter, false); + preprocessingResult.result, preludeStatements, localReporter, locator, false); LOG.fine("Finished parsing of " + packageId); MakeEnvironment.Builder makeEnv = new MakeEnvironment.Builder(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 22df6d780e..a19b36bd18 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -75,7 +75,7 @@ public class WorkspaceFactory { throws InterruptedException { StoredEventHandler localReporter = new StoredEventHandler(); BuildFileAST buildFileAST; - buildFileAST = BuildFileAST.parseBuildFile(source, localReporter, false); + buildFileAST = BuildFileAST.parseBuildFile(source, localReporter, null, false); if (buildFileAST.containsErrors()) { localReporter.handle(Event.error("WORKSPACE file could not be parsed")); } else { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java index a7cd395b33..9bc6bf226c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ASTFileLookupFunction.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.syntax.BuildFileAST; @@ -111,10 +112,13 @@ public class ASTFileLookupFunction implements SkyFunction { private final AtomicReference<PathPackageLocator> pkgLocator; private final RuleClassProvider ruleClassProvider; + private final CachingPackageLocator packageManager; public ASTFileLookupFunction(AtomicReference<PathPackageLocator> pkgLocator, + CachingPackageLocator packageManager, RuleClassProvider ruleClassProvider) { this.pkgLocator = pkgLocator; + this.packageManager = packageManager; this.ruleClassProvider = ruleClassProvider; } @@ -139,7 +143,7 @@ public class ASTFileLookupFunction implements SkyFunction { if (parseAsSkylark) { try (Mutability mutability = Mutability.create("validate")) { ast = BuildFileAST.parseSkylarkFile(path, fileSize, env.getListener(), - new ValidationEnvironment( + packageManager, new ValidationEnvironment( ruleClassProvider.createSkylarkRuleClassEnvironment( mutability, env.getListener(), @@ -149,7 +153,7 @@ public class ASTFileLookupFunction implements SkyFunction { .setupDynamic(Runtime.PKG_NAME, Runtime.NONE))); } } else { - ast = BuildFileAST.parseBuildFile(path, fileSize, env.getListener(), false); + ast = BuildFileAST.parseBuildFile(path, fileSize, env.getListener(), packageManager, false); } } catch (IOException e) { throw new ASTLookupFunctionException(new ErrorReadingSkylarkExtensionException( 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 427e758b29..eb580be9ba 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 @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Sets; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.PackageIdentifier.RepositoryName; import com.google.devtools.build.lib.events.Event; @@ -524,6 +525,32 @@ public class PackageFunction implements SkyFunction { return new PackageValue(pkg); } + /** + * Returns true if includes referencing a different repository have already been computed. + */ + private boolean fetchIncludeRepositoryDeps(Environment env, BuildFileAST ast) { + boolean ok = true; + for (String include : ast.getIncludes()) { + Label label; + try { + label = Label.parseAbsolute(include); + } catch (LabelSyntaxException e) { + // Ignore. This will be reported when the BUILD file is actually evaluated. + continue; + } + if (!label.getPackageIdentifier().getRepository().isDefault()) { + // If this is the default repository, the include refers to the same repository, whose + // RepositoryValue is already a dependency of this PackageValue. + if (env.getValue(RepositoryValue.key( + label.getPackageIdentifier().getRepository())) == null) { + ok = false; + } + } + } + + return ok; + } + // TODO(bazel-team): this should take the AST so we don't parse the file twice. @Nullable private SkylarkImportResult discoverSkylarkImports( @@ -540,16 +567,24 @@ public class PackageFunction implements SkyFunction { inputSource, preludeStatements, eventHandler, + /* package locator */ null, /* parse python */ false); SkylarkImportResult importResult; + boolean includeRepositoriesFetched; if (eventHandler.hasErrors()) { importResult = new SkylarkImportResult( ImmutableMap.<PathFragment, Extension>of(), ImmutableList.<Label>of()); + includeRepositoriesFetched = true; } else { importResult = fetchImportsFromBuildFile(buildFilePath, buildFileFragment, packageId, buildFileAST, env); + includeRepositoriesFetched = fetchIncludeRepositoryDeps(env, buildFileAST); + } + + if (!includeRepositoriesFetched) { + return null; } return importResult; 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 c16ac7c8ef..252df16a3b 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 @@ -312,7 +312,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); map.put(SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction(deletedPackages)); map.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()); - map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgLocator, ruleClassProvider)); + map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction( + pkgLocator, packageManager, ruleClassProvider)); map.put(SkyFunctions.SKYLARK_IMPORTS_LOOKUP, new SkylarkImportLookupFunction( ruleClassProvider, pkgFactory)); map.put(SkyFunctions.GLOB, newGlobFunction()); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java index e6d00ef0bc..60e84f94a5 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java @@ -15,10 +15,12 @@ package com.google.devtools.build.lib.syntax; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.hash.HashCode; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -38,6 +40,8 @@ public class BuildFileAST extends ASTNode { private ImmutableMap<Location, PathFragment> loads; + private ImmutableSet<String> includes; + /** * Whether any errors were encountered during scanning or parsing. */ @@ -45,11 +49,11 @@ public class BuildFileAST extends ASTNode { private final String contentHashCode; - private BuildFileAST(List<Statement> preludeStatements, Parser.ParseResult result) { - this(preludeStatements, result, null); + private BuildFileAST(Lexer lexer, List<Statement> preludeStatements, Parser.ParseResult result) { + this(lexer, preludeStatements, result, null); } - private BuildFileAST(List<Statement> preludeStatements, + private BuildFileAST(Lexer lexer, List<Statement> preludeStatements, Parser.ParseResult result, String contentHashCode) { this.stmts = ImmutableList.<Statement>builder() .addAll(preludeStatements) @@ -58,7 +62,42 @@ public class BuildFileAST extends ASTNode { this.comments = ImmutableList.copyOf(result.comments); this.containsErrors = result.containsErrors; this.contentHashCode = contentHashCode; - setLocation(result.location); + if (!result.statements.isEmpty()) { + setLocation(lexer.createLocation( + result.statements.get(0).getLocation().getStartOffset(), + result.statements.get(result.statements.size() - 1).getLocation().getEndOffset())); + } else { + setLocation(Location.fromPathFragment(lexer.getFilename())); + } + } + + private ImmutableSet<String> fetchIncludes(List<Statement> stmts) { + ImmutableSet.Builder<String> result = new ImmutableSet.Builder<>(); + for (Statement stmt : stmts) { + if (!(stmt instanceof ExpressionStatement)) { + continue; + } + + ExpressionStatement expr = (ExpressionStatement) stmt; + if (!(expr.getExpression() instanceof FuncallExpression)) { + continue; + } + + FuncallExpression funcall = (FuncallExpression) expr.getExpression(); + if (!funcall.getFunction().getName().equals("include") + || funcall.getArguments().size() != 1) { + continue; + } + + Expression arg = funcall.getArguments().get(0).value; + if (!(arg instanceof StringLiteral)) { + continue; + } + + result.add(((StringLiteral) arg).getValue()); + } + + return result.build(); } /** Collects paths from all load statements */ @@ -106,6 +145,14 @@ public class BuildFileAST extends ASTNode { return loads; } + public synchronized ImmutableSet<String> getIncludes() { + if (includes == null) { + includes = fetchIncludes(stmts); + } + + return includes; + } + /** * Executes this build file in a given Environment. * @@ -164,17 +211,17 @@ public class BuildFileAST extends ASTNode { * @throws IOException if the file cannot not be read. */ public static BuildFileAST parseBuildFile(Path buildFile, EventHandler eventHandler, - boolean parsePython) + CachingPackageLocator locator, boolean parsePython) throws IOException { - return parseBuildFile(buildFile, buildFile.getFileSize(), eventHandler, parsePython); + return parseBuildFile(buildFile, buildFile.getFileSize(), eventHandler, locator, parsePython); } public static BuildFileAST parseBuildFile(Path buildFile, long fileSize, EventHandler eventHandler, - boolean parsePython) + CachingPackageLocator locator, boolean parsePython) throws IOException { ParserInputSource inputSource = ParserInputSource.create(buildFile, fileSize); - return parseBuildFile(inputSource, eventHandler, parsePython); + return parseBuildFile(inputSource, eventHandler, locator, parsePython); } /** @@ -184,15 +231,27 @@ public class BuildFileAST extends ASTNode { public static BuildFileAST parseBuildFile(ParserInputSource input, List<Statement> preludeStatements, EventHandler eventHandler, + CachingPackageLocator locator, boolean parsePython) { - Parser.ParseResult result = Parser.parseFile(input, eventHandler, parsePython); - return new BuildFileAST(preludeStatements, result); + Lexer lexer = new Lexer(input, eventHandler, parsePython); + Parser.ParseResult result = Parser.parseFile(lexer, eventHandler, locator, parsePython); + return new BuildFileAST(lexer, preludeStatements, result); } public static BuildFileAST parseBuildFile(ParserInputSource input, EventHandler eventHandler, - boolean parsePython) { - Parser.ParseResult result = Parser.parseFile(input, eventHandler, parsePython); - return new BuildFileAST(ImmutableList.<Statement>of(), result); + CachingPackageLocator locator, boolean parsePython) { + Lexer lexer = new Lexer(input, eventHandler, parsePython); + Parser.ParseResult result = Parser.parseFile(lexer, eventHandler, locator, parsePython); + return new BuildFileAST(lexer, ImmutableList.<Statement>of(), result); + } + + /** + * Parse the specified build file, returning its AST. All errors during + * scanning or parsing will be reported to the reporter. + */ + public static BuildFileAST parseBuildFile(Lexer lexer, EventHandler eventHandler) { + Parser.ParseResult result = Parser.parseFile(lexer, eventHandler, null, false); + return new BuildFileAST(lexer, ImmutableList.<Statement>of(), result); } /** @@ -202,17 +261,20 @@ public class BuildFileAST extends ASTNode { * @throws IOException if the file cannot not be read. */ public static BuildFileAST parseSkylarkFile(Path file, EventHandler eventHandler, - ValidationEnvironment validationEnvironment) throws IOException { - return parseSkylarkFile(file, file.getFileSize(), eventHandler, + CachingPackageLocator locator, ValidationEnvironment validationEnvironment) + throws IOException { + return parseSkylarkFile(file, file.getFileSize(), eventHandler, locator, validationEnvironment); } public static BuildFileAST parseSkylarkFile(Path file, long fileSize, EventHandler eventHandler, - ValidationEnvironment validationEnvironment) throws IOException { + CachingPackageLocator locator, ValidationEnvironment validationEnvironment) + throws IOException { ParserInputSource input = ParserInputSource.create(file, fileSize); + Lexer lexer = new Lexer(input, eventHandler, false); Parser.ParseResult result = - Parser.parseFileForSkylark(input, eventHandler, validationEnvironment); - return new BuildFileAST(ImmutableList.<Statement>of(), result, + Parser.parseFileForSkylark(lexer, eventHandler, locator, validationEnvironment); + return new BuildFileAST(lexer, ImmutableList.<Statement>of(), result, HashCode.fromBytes(file.getMD5Digest()).toString()); } @@ -223,7 +285,7 @@ public class BuildFileAST extends ASTNode { */ public static boolean checkSyntax(ParserInputSource input, EventHandler eventHandler, boolean parsePython) { - return !parseBuildFile(input, eventHandler, parsePython).containsErrors(); + return !parseBuildFile(input, eventHandler, null, parsePython).containsErrors(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java index 266c2c79aa..89360a765a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java @@ -19,14 +19,17 @@ import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.syntax.Mutability.Freezable; import com.google.devtools.build.lib.syntax.Mutability.MutabilityException; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Pair; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.Serializable; @@ -908,16 +911,45 @@ public final class Environment implements Freezable { } }; + /** Mock package locator class */ + private static final class EmptyPackageLocator implements CachingPackageLocator { + @Override + public Path getBuildFileForPackage(PackageIdentifier packageName) { + return null; + } + } + + /** A mock package locator */ + @VisibleForTesting + static final CachingPackageLocator EMPTY_PACKAGE_LOCATOR = new EmptyPackageLocator(); + + /** + * Creates a Lexer without a supporting file. + * @param input a list of lines of code + */ + @VisibleForTesting + Lexer createLexer(String... input) { + return new Lexer(ParserInputSource.create(Joiner.on("\n").join(input), null), + eventHandler); + } + /** * Parses some String input without a supporting file, returning statements and comments. * @param input a list of lines of code */ @VisibleForTesting - Parser.ParseResult parseFileWithComments(String... inputLines) { - ParserInputSource input = ParserInputSource.create(Joiner.on("\n").join(inputLines), null); + Parser.ParseResult parseFileWithComments(String... input) { return isSkylark - ? Parser.parseFileForSkylark(input, eventHandler, new ValidationEnvironment(this)) - : Parser.parseFile(input, eventHandler, /*parsePython=*/false); + ? Parser.parseFileForSkylark( + createLexer(input), + eventHandler, + EMPTY_PACKAGE_LOCATOR, + new ValidationEnvironment(this)) + : Parser.parseFile( + createLexer(input), + eventHandler, + EMPTY_PACKAGE_LOCATOR, + /*parsePython=*/false); } /** diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java index d685dcf5fe..2734e4ff6f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java @@ -23,14 +23,19 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral; import com.google.devtools.build.lib.syntax.IfStatement.ConditionalStatements; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.EnumSet; @@ -59,19 +64,14 @@ class Parser { /** The comments from the parsed file. */ public final List<Comment> comments; - /** Represents every statement in the file. */ - public final Location location; - /** Whether the file contained any errors. */ public final boolean containsErrors; - public ParseResult(List<Statement> statements, List<Comment> comments, Location location, - boolean containsErrors) { + public ParseResult(List<Statement> statements, List<Comment> comments, boolean containsErrors) { // No need to copy here; when the object is created, the parser instance is just about to go // out of scope and be garbage collected. this.statements = Preconditions.checkNotNull(statements); this.comments = Preconditions.checkNotNull(comments); - this.location = location; this.containsErrors = containsErrors; } } @@ -180,23 +180,28 @@ class Parser { private int errorsCount; private boolean recoveryMode; // stop reporting errors until next statement - private Parser(Lexer lexer, EventHandler eventHandler, ParsingMode parsingMode) { + private CachingPackageLocator locator; + + private List<PathFragment> includedFiles; + + private Parser( + Lexer lexer, + EventHandler eventHandler, + CachingPackageLocator locator, + ParsingMode parsingMode) { this.lexer = lexer; this.eventHandler = eventHandler; this.parsingMode = parsingMode; this.tokens = lexer.getTokens().iterator(); this.comments = new ArrayList<>(); + this.locator = locator; + this.includedFiles = new ArrayList<>(); + this.includedFiles.add(lexer.getFilename()); nextToken(); } - private static Location locationFromStatements(Lexer lexer, List<Statement> statements) { - if (!statements.isEmpty()) { - return lexer.createLocation( - statements.get(0).getLocation().getStartOffset(), - statements.get(statements.size() - 1).getLocation().getEndOffset()); - } else { - return Location.fromPathFragment(lexer.getFilename()); - } + private Parser(Lexer lexer, EventHandler eventHandler, CachingPackageLocator locator) { + this(lexer, eventHandler, locator, BUILD); } /** @@ -204,13 +209,12 @@ class Parser { * encountered during parsing are reported via "reporter". */ public static ParseResult parseFile( - ParserInputSource input, EventHandler eventHandler, boolean parsePython) { - Lexer lexer = new Lexer(input, eventHandler, parsePython); + Lexer lexer, EventHandler eventHandler, CachingPackageLocator locator, boolean parsePython) { ParsingMode parsingMode = parsePython ? PYTHON : BUILD; - Parser parser = new Parser(lexer, eventHandler, parsingMode); + Parser parser = new Parser(lexer, eventHandler, locator, parsingMode); List<Statement> statements = parser.parseFileInput(); - return new ParseResult(statements, parser.comments, locationFromStatements(lexer, statements), - parser.errorsCount > 0 || lexer.containsErrors()); + return new ParseResult( + statements, parser.comments, parser.errorsCount > 0 || lexer.containsErrors()); } /** @@ -219,11 +223,11 @@ class Parser { * that are not part of the core BUILD language. */ public static ParseResult parseFileForSkylark( - ParserInputSource input, + Lexer lexer, EventHandler eventHandler, + CachingPackageLocator locator, @Nullable ValidationEnvironment validationEnvironment) { - Lexer lexer = new Lexer(input, eventHandler, false); - Parser parser = new Parser(lexer, eventHandler, SKYLARK); + Parser parser = new Parser(lexer, eventHandler, locator, SKYLARK); List<Statement> statements = parser.parseFileInput(); boolean hasSemanticalErrors = false; try { @@ -237,7 +241,7 @@ class Parser { } hasSemanticalErrors = true; } - return new ParseResult(statements, parser.comments, locationFromStatements(lexer, statements), + return new ParseResult(statements, parser.comments, parser.errorsCount > 0 || lexer.containsErrors() || hasSemanticalErrors); } @@ -247,8 +251,7 @@ class Parser { * by newline tokens. */ @VisibleForTesting - public static Expression parseExpression(ParserInputSource input, EventHandler eventHandler) { - Lexer lexer = new Lexer(input, eventHandler, false); + public static Expression parseExpression(Lexer lexer, EventHandler eventHandler) { Parser parser = new Parser(lexer, eventHandler, null); Expression result = parser.parseExpression(); while (parser.token.kind == TokenKind.NEWLINE) { @@ -258,6 +261,10 @@ class Parser { return result; } + private void addIncludedFiles(List<PathFragment> files) { + this.includedFiles.addAll(files); + } + private void reportError(Location location, String message) { errorsCount++; // Limit the number of reported errors to avoid spamming output. @@ -594,6 +601,60 @@ class Parser { return setLocation(new DictionaryEntryLiteral(key, value), start, value); } + private ExpressionStatement mocksubincludeExpression( + String labelName, String file, Location location) { + List<Argument.Passed> args = new ArrayList<>(); + args.add(setLocation(new Argument.Positional( + new StringLiteral(labelName, '"')), location)); + args.add(setLocation(new Argument.Positional( + new StringLiteral(file, '"')), location)); + Identifier mockIdent = setLocation(new Identifier("mocksubinclude"), location); + Expression funCall = new FuncallExpression(null, mockIdent, args); + return setLocation(new ExpressionStatement(funCall), location); + } + + // parse a file from an include call + private void include(String labelName, List<Statement> list, Location location) { + if (locator == null) { + return; + } + + try { + Label label = Label.parseAbsolute(labelName); + // Note that this doesn't really work if the label belongs to a different repository, because + // there is no guarantee that its RepositoryValue has been evaluated. In an ideal world, we + // could put a Skyframe dependency the appropriate PackageLookupValue, but we can't do that + // because package loading is not completely Skyframized. + Path packagePath = locator.getBuildFileForPackage(label.getPackageIdentifier()); + if (packagePath == null) { + reportError(location, "Package '" + label.getPackageIdentifier() + "' not found"); + list.add(mocksubincludeExpression(labelName, "", location)); + return; + } + Path path = packagePath.getParentDirectory(); + Path file = path.getRelative(label.getName()); + + if (this.includedFiles.contains(file.asFragment())) { + reportError(location, "Recursive inclusion of file '" + path + "'"); + return; + } + ParserInputSource inputSource = ParserInputSource.create(file); + + // Insert call to the mocksubinclude function to get the dependencies right. + list.add(mocksubincludeExpression(labelName, file.toString(), location)); + + Lexer lexer = new Lexer(inputSource, eventHandler, parsingMode == PYTHON); + Parser parser = new Parser(lexer, eventHandler, locator, parsingMode); + parser.addIncludedFiles(this.includedFiles); + list.addAll(parser.parseFileInput()); + } catch (LabelSyntaxException e) { + reportError(location, "Invalid label '" + labelName + "'"); + } catch (IOException e) { + reportError(location, "Include of '" + labelName + "' failed: " + e.getMessage()); + list.add(mocksubincludeExpression(labelName, "", location)); + } + } + /** * Parse a String literal value, e.g. "str". */ @@ -1023,7 +1084,7 @@ class Parser { parseTopLevelStatement(list); } } - Profiler.instance().logSimpleTask(startTime, ProfilerTask.SKYLARK_PARSER, ""); + Profiler.instance().logSimpleTask(startTime, ProfilerTask.SKYLARK_PARSER, includedFiles); return list; } @@ -1113,12 +1174,24 @@ class Parser { private void parseTopLevelStatement(List<Statement> list) { // In Python grammar, there is no "top-level statement" and imports are // considered as "small statements". We are a bit stricter than Python here. + int start = token.left; + // Check if there is an include if (token.kind == TokenKind.IDENTIFIER) { Token identToken = token; Identifier ident = parseIdent(); - if (ident.getName().equals("load") && token.kind == TokenKind.LPAREN) { + if (ident.getName().equals("include") + && token.kind == TokenKind.LPAREN + && parsingMode == BUILD) { + expect(TokenKind.LPAREN); + if (token.kind == TokenKind.STRING) { + include((String) token.value, list, lexer.createLocation(start, token.right)); + } + expect(TokenKind.STRING); + expect(TokenKind.RPAREN); + return; + } else if (ident.getName().equals("load") && token.kind == TokenKind.LPAREN) { expect(TokenKind.LPAREN); parseLoad(list); return; 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 82c6aa515f..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 @@ -107,7 +107,8 @@ public class PackageFactoryApparatus { */ public BuildFileAST ast(Path buildFile) throws IOException { ParserInputSource inputSource = ParserInputSource.create(buildFile); - return BuildFileAST.parseBuildFile(inputSource, events.reporter(), /*parsePython=*/false); + return BuildFileAST.parseBuildFile(inputSource, events.reporter(), locator, + /*parsePython=*/false); } /** diff --git a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java index ecbb85bc34..00e9062d63 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/BuildFileASTTest.java @@ -48,6 +48,8 @@ public class BuildFileASTTest extends EvaluationTestCase { } } + private CachingPackageLocator locator = new ScratchPathPackageLocator(); + @Override public Environment newEnvironment() throws Exception { return newBuildEnvironment(); @@ -59,7 +61,7 @@ public class BuildFileASTTest extends EvaluationTestCase { */ private BuildFileAST parseBuildFile(String... lines) throws IOException { Path file = scratch.file("/a/build/file/BUILD", lines); - return BuildFileAST.parseBuildFile(file, getEventHandler(), false); + return BuildFileAST.parseBuildFile(file, getEventHandler(), locator, false); } @Test @@ -69,7 +71,7 @@ public class BuildFileASTTest extends EvaluationTestCase { "", "x = [1,2,'foo',4] + [1,2, \"%s%d\" % ('foo', 1)]"); - BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), false); + BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false); assertTrue(buildfile.exec(env, getEventHandler())); @@ -90,7 +92,7 @@ public class BuildFileASTTest extends EvaluationTestCase { "z = x + y"); setFailFast(false); - BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), false); + BuildFileAST buildfile = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), null, false); assertFalse(buildfile.exec(env, getEventHandler())); Event e = assertContainsEvent("unsupported operand type(s) for +: 'int' and 'List'"); @@ -179,4 +181,123 @@ public class BuildFileASTTest extends EvaluationTestCase { // This message should not be printed anymore. assertNull(findEvent(getEventCollector(), "contains syntax error(s)")); } + + @Test + public void testInclude() throws Exception { + scratch.file("/foo/bar/BUILD", + "c = 4\n" + + "d = 5\n"); + Path buildFile = scratch.file("/BUILD", + "a = 2\n" + + "include(\"//foo/bar:BUILD\")\n" + + "b = 4\n"); + + BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), + locator, false); + + assertFalse(buildFileAST.containsErrors()); + assertThat(buildFileAST.getStatements()).hasSize(5); + } + + @Test + public void testInclude2() throws Exception { + scratch.file("/foo/bar/defs", + "a = 1\n"); + Path buildFile = scratch.file("/BUILD", + "include(\"//foo/bar:defs\")\n" + + "b = a + 1\n"); + + BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(buildFile, getEventHandler(), + locator, false); + + assertFalse(buildFileAST.containsErrors()); + assertThat(buildFileAST.getStatements()).hasSize(3); + + setFailFast(false); + assertFalse(buildFileAST.exec(env, getEventHandler())); + assertEquals(2, env.lookup("b")); + } + + @Test + public void testMultipleIncludes() throws Exception { + String fileA = + "include(\"//foo:fileB\")\n" + + "include(\"//foo:fileC\")\n"; + scratch.file("/foo/fileB", + "b = 3\n" + + "include(\"//foo:fileD\")\n"); + scratch.file("/foo/fileC", + "include(\"//foo:fileD\")\n" + + "c = b + 2\n"); + scratch.file("/foo/fileD", + "b = b + 1\n"); // this code is included twice + + BuildFileAST buildFileAST = parseBuildFile(fileA); + assertFalse(buildFileAST.containsErrors()); + assertThat(buildFileAST.getStatements()).hasSize(8); + + setFailFast(false); + assertFalse(buildFileAST.exec(env, getEventHandler())); + assertEquals(5, env.lookup("b")); + assertEquals(7, env.lookup("c")); + } + + @Test + public void testFailInclude() throws Exception { + setFailFast(false); + BuildFileAST buildFileAST = parseBuildFile("include(\"//nonexistent\")"); + assertThat(buildFileAST.getStatements()).hasSize(1); + assertContainsEvent("Include of '//nonexistent' failed"); + } + + + @Test + public void testFailInclude2() throws Exception { + setFailFast(false); + Path buildFile = scratch.file("/foo/bar/BUILD", + "include(\"//nonexistent:foo\")\n"); + BuildFileAST buildFileAST = BuildFileAST.parseBuildFile( + buildFile, getEventHandler(), Environment.EMPTY_PACKAGE_LOCATOR, false); + assertThat(buildFileAST.getStatements()).hasSize(1); + assertContainsEvent("Package 'nonexistent' not found"); + } + + @Test + public void testInvalidInclude() throws Exception { + setFailFast(false); + BuildFileAST buildFileAST = parseBuildFile("include(2)"); + assertThat(buildFileAST.getStatements()).isEmpty(); + assertContainsEvent("syntax error at '2'"); + } + + @Test + public void testRecursiveInclude() throws Exception { + setFailFast(false); + Path buildFile = scratch.file("/foo/bar/BUILD", + "include(\"//foo/bar:BUILD\")\n"); + + BuildFileAST.parseBuildFile(buildFile, getEventHandler(), locator, false); + assertContainsEvent("Recursive inclusion"); + } + + @Test + public void testParseErrorInclude() throws Exception { + setFailFast(false); + + scratch.file("/foo/bar/file", + "a = 2 + % 3\n"); // parse error + + parseBuildFile("include(\"//foo/bar:file\")"); + + // Check the location is properly reported + Event event = assertContainsEvent("syntax error at '%': expected expression"); + assertEquals("/foo/bar/file:1:9", event.getLocation().print()); + } + + @Test + public void testNonExistentIncludeReported() throws Exception { + setFailFast(false); + BuildFileAST buildFileAST = parseBuildFile("include('//foo:bar')"); + assertThat(buildFileAST.getStatements()).hasSize(1); + } } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java index 553385d58b..e26234af0c 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTestCase.java @@ -16,7 +16,6 @@ package com.google.devtools.build.lib.syntax; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; -import com.google.common.base.Joiner; import com.google.common.truth.Ordered; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventCollector; @@ -138,9 +137,7 @@ public class EvaluationTestCase { /** Parses an Expression from string without a supporting file */ Expression parseExpression(String... input) { - return Parser.parseExpression( - ParserInputSource.create(Joiner.on("\n").join(input), null), - getEventHandler()); + return Parser.parseExpression(env.createLexer(input), getEventHandler()); } public EvaluationTestCase update(String varname, Object value) throws Exception { diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java index 9dc2a15db0..75913b42d5 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java @@ -20,7 +20,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.syntax.DictionaryLiteral.DictionaryEntryLiteral; @@ -61,8 +60,9 @@ public class ParserTest extends EvaluationTestCase { /** Parses a build code (not Skylark) with PythonProcessing enabled */ private List<Statement> parseFileWithPython(String... input) { return Parser.parseFile( - ParserInputSource.create(Joiner.on("\n").join(input), null), + buildEnvironment.createLexer(input), getEventHandler(), + Environment.EMPTY_PACKAGE_LOCATOR, /*parsePython=*/true).statements; } @@ -1252,4 +1252,18 @@ public class ParserTest extends EvaluationTestCase { " else: return a"); assertContainsEvent("syntax error at 'else'"); } + + @Test + public void testIncludeFailureSkylark() throws Exception { + setFailFast(false); + parseFileForSkylark("include('//foo:bar')"); + assertContainsEvent("function 'include' does not exist"); + } + + @Test + public void testIncludeFailure() throws Exception { + setFailFast(false); + parseFile("include('nonexistent')\n"); + assertContainsEvent("Invalid label 'nonexistent'"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitorTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitorTest.java index 76f70ad4ea..72edba4765 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitorTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitorTest.java @@ -15,6 +15,8 @@ package com.google.devtools.build.lib.syntax; import static com.google.common.truth.Truth.assertThat; +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.vfs.Path; @@ -33,10 +35,20 @@ public class SyntaxTreeVisitorTest { private Scratch scratch = new Scratch(); + private class ScratchPathPackageLocator implements CachingPackageLocator { + @Override + public Path getBuildFileForPackage(PackageIdentifier packageName) { + return scratch.resolve(packageName.getPackageFragment()).getRelative("BUILD"); + } + } + + private CachingPackageLocator locator = new ScratchPathPackageLocator(); + /** Parses the contents of the specified string and returns the AST. */ private BuildFileAST parse(String... lines) throws IOException { Path file = scratch.file("/a/build/file/BUILD", lines); - return BuildFileAST.parseSkylarkFile(file, null /* reporter */, null /*validationEnvironment*/); + return BuildFileAST.parseSkylarkFile( + file, null /* reporter */, locator, null /*validationEnvironment*/); } @Test diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh index 9fc7a76e7a..56987dbc79 100755 --- a/src/test/shell/bazel/local_repository_test.sh +++ b/src/test/shell/bazel/local_repository_test.sh @@ -605,6 +605,35 @@ EOF bazel build @r//:fg || fail "build failed" } +function test_include_from_local_repository() { + local r=$TEST_TMPDIR/r + rm -fr $r + mkdir $r + touch $r/WORKSPACE + mkdir -p $r/b + cat > $r/b/BUILD <<EOF +exports_files(["include"]) +EOF + + cat > $r/b/include <<EOF +filegroup(name = "foo") +EOF + + cat > WORKSPACE <<EOF +local_repository( + name = "r", + path = "$r", +) +EOF + + mkdir -p a + cat > a/BUILD <<EOF +include("@r//b:include") +EOF + + bazel query '//a:foo' || fail "query failed" +} + function test_cc_binary_in_local_repository() { local r=$TEST_TMPDIR/r rm -fr $r |