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

5 files changed, 156 insertions, 11 deletions

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