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

3 files changed, 218 insertions, 51 deletions

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;