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/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
