Rollback of commit 4e5037520e3067f9d8784e1c59f9545b96111cd4.
*** Reason for rollback ***
Broke tests, I mistakenly assumed it was flakiness.
*** Original change description ***
Remove support for the deprecated include() statement.
This is part of the crusade to eliminate as every dependency in Skylark on the rest of the code so that it can be moved deeper in the dependency graph.
RELNOTES: The include() statement in BUILD files is not supported anymore.
--
MOS_MIGRATED_REVID=103284257
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 c192738..2b09e7b 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 @@
// 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 22df6d7..a19b36b 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 @@
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 a7cd395..9bc6bf2 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 @@
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 @@
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 @@
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 @@
.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 427e758..eb580be 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.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 @@
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 @@
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 c16ac7c..252df16 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 @@
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 e6d00ef..60e84f9 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 @@
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 @@
private ImmutableMap<Location, PathFragment> loads;
+ private ImmutableSet<String> includes;
+
/**
* Whether any errors were encountered during scanning or parsing.
*/
@@ -45,11 +49,11 @@
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 @@
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 @@
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 @@
* @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 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 @@
* @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 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 266c2c7..89360a7 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.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 @@
}
};
+ /** 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 d685dcf..2734e4f 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.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 @@
/** 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 @@
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 @@
* 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 @@
* 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 @@
}
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 @@
* 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 @@
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 @@
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 @@
parseTopLevelStatement(list);
}
}
- Profiler.instance().logSimpleTask(startTime, ProfilerTask.SKYLARK_PARSER, "");
+ Profiler.instance().logSimpleTask(startTime, ProfilerTask.SKYLARK_PARSER, includedFiles);
return list;
}
@@ -1113,12 +1174,24 @@
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 82c6aa5..e1f7271 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 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 ecbb85b..00e9062 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 @@
}
}
+ private CachingPackageLocator locator = new ScratchPathPackageLocator();
+
@Override
public Environment newEnvironment() throws Exception {
return newBuildEnvironment();
@@ -59,7 +61,7 @@
*/
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 @@
"",
"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 @@
"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 @@
// 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 553385d..e26234a 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 @@
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 @@
/** 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 9dc2a15..75913b4 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.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 @@
/** 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 @@
" 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 76f70ad..72edba4 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 @@
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 @@
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 9fc7a76..56987db 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -605,6 +605,35 @@
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
