Make include() work with remote repositories.

This in itself is not very interesting because include() is deprecated, but it paves the way for fetching the appropriate dependencies for the eventual label-based load() statements.

--
MOS_MIGRATED_REVID=97849076

4 files changed, 114 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalRepositoryFunction.java
index cd7d6ac489..e79ef39153 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalRepositoryFunction.java
@@ -71,6 +71,8 @@ public class LocalRepositoryFunction extends RepositoryFunction {
     }
     FileValue repositoryValue = getRepositoryDirectory(repositoryPath, env);
     if (repositoryValue == null) {
+      // TODO(bazel-team): If this returns null, we unnecessarily recreate the symlink above on the
+      // second execution.
       return null;
     }
 
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 7dc81e81f4..56fa8df043 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
@@ -232,7 +232,7 @@ public class PackageFunction implements SkyFunction {
     Set<SkyKey> subincludePackageLookupDepKeys = Sets.newHashSet();
     for (Label label : pkg.getSubincludeLabels()) {
       // Declare a dependency on the package lookup for the package giving access to the label.
-      subincludePackageLookupDepKeys.add(PackageLookupValue.key(label.getPackageFragment()));
+      subincludePackageLookupDepKeys.add(PackageLookupValue.key(label.getPackageIdentifier()));
     }
     Pair<? extends Map<PathFragment, PackageLookupValue>, Boolean> subincludePackageLookupResult =
         getPackageLookupDepsAndPropagateInconsistentFilesystemExceptions(
@@ -457,9 +457,27 @@ public class PackageFunction implements SkyFunction {
       throw new PackageFunctionException(new BuildFileContainsErrorsException(
           packageId, e.getMessage()), Transience.TRANSIENT);
     }
-    SkylarkImportResult importResult = fetchImportsFromBuildFile(buildFilePath, buildFileFragment,
-        packageId, preludeStatements, inputSource, env);
-    if (importResult == null) {
+
+    StoredEventHandler eventHandler = new StoredEventHandler();
+    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(
+          inputSource, preludeStatements, eventHandler, null, true);
+
+    SkylarkImportResult importResult;
+    boolean includeRepositoriesFetched;
+    if (eventHandler.hasErrors()) {
+      // In case of Python preprocessing, errors have already been reported (see checkSyntax).
+      // In other cases, errors will be reported later.
+      // TODO(bazel-team): maybe we could get rid of checkSyntax and always report errors here?
+      importResult = new SkylarkImportResult(
+          ImmutableMap.<PathFragment, SkylarkEnvironment>of(), ImmutableList.<Label>of());
+      includeRepositoriesFetched = true;
+    } else {
+      importResult = fetchImportsFromBuildFile(buildFilePath, buildFileFragment,
+          packageId, buildFileAST, env);
+      includeRepositoriesFetched = fetchIncludeRepositoryDeps(env, buildFileAST);
+    }
+
+    if (importResult == null || !includeRepositoriesFetched) {
       return null;
     }
 
@@ -509,23 +527,26 @@ public class PackageFunction implements SkyFunction {
     return new PackageValue(pkg);
   }
 
+  private boolean fetchIncludeRepositoryDeps(Environment env, BuildFileAST ast) {
+    boolean ok = true;
+    for (Label label : ast.getIncludes()) {
+      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;
+  }
+
   private SkylarkImportResult fetchImportsFromBuildFile(Path buildFilePath,
       PathFragment buildFileFragment, PackageIdentifier packageIdentifier,
-      List<Statement> preludeStatements, ParserInputSource inputSource, Environment env)
+      BuildFileAST buildFileAST, Environment env)
       throws PackageFunctionException {
-    StoredEventHandler eventHandler = new StoredEventHandler();
-    BuildFileAST buildFileAST = BuildFileAST.parseBuildFile(
-          inputSource, preludeStatements, eventHandler, null, true);
-
-    if (eventHandler.hasErrors()) {
-      // In case of Python preprocessing, errors have already been reported (see checkSyntax).
-      // In other cases, errors will be reported later.
-      // TODO(bazel-team): maybe we could get rid of checkSyntax and always report errors here?
-      return new SkylarkImportResult(
-          ImmutableMap.<PathFragment, SkylarkEnvironment>of(),
-          ImmutableList.<Label>of());
-    }
-
     ImmutableCollection<PathFragment> imports = buildFileAST.getImports();
     Map<PathFragment, SkylarkEnvironment> importMap = new HashMap<>();
     ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder();
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 dc0995ac1b..35a9af8477 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
@@ -41,6 +41,8 @@ public class BuildFileAST extends ASTNode {
 
   private ImmutableSet<String> subincludes;
 
+  private ImmutableSet<Label> includes;
+
   /**
    * Whether any errors were encountered during scanning or parsing.
    */
@@ -94,6 +96,40 @@ public class BuildFileAST extends ASTNode {
     return subincludes.build();
   }
 
+  private ImmutableSet<Label> fetchIncludes(List<Statement> stmts) {
+    ImmutableSet.Builder<Label> 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;
+      }
+
+      try {
+        Label label = Label.parseAbsolute(((StringLiteral) arg).getValue());
+        result.add(label);
+      } catch (Label.SyntaxException e) {
+        // Ignore. This will be reported when the BUILD file is actually evaluated.
+      }
+    }
+
+    return result.build();
+  }
+
   /** Collects paths from all load statements */
   private ImmutableSet<PathFragment> fetchLoads(List<Statement> stmts) {
     ImmutableSet.Builder<PathFragment> loads = new ImmutableSet.Builder<>();
@@ -149,6 +185,14 @@ public class BuildFileAST extends ASTNode {
     return subincludes;
   }
 
+  public synchronized ImmutableSet<Label> getIncludes() {
+    if (includes == null) {
+      includes = fetchIncludes(stmts);
+    }
+
+    return includes;
+  }
+
   /**
    * Executes this build file in a given Environment.
    *
diff --git a/src/test/shell/bazel/local_repository_test.sh b/src/test/shell/bazel/local_repository_test.sh
index 8a83a927b3..ccba557d60 100755
--- a/src/test/shell/bazel/local_repository_test.sh
+++ b/src/test/shell/bazel/local_repository_test.sh
@@ -543,4 +543,33 @@ 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"
+}
+
 run_suite "local repository tests"