Update package lookup to check for files named BUILD.bazel before files named

BUILD.

Fixes #552.

RELNOTES[NEW]: Packages are defined in BUILD.bazel as well as BUILD files.

--
MOS_MIGRATED_REVID=138828981

1 files changed, 99 insertions, 52 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index 331a081b4e..f96ca84d57 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 
@@ -51,12 +52,15 @@ public class PackageLookupFunction implements SkyFunction {
 
   private final AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages;
   private final CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy;
+  private final List<BuildFileName> buildFilesByPriority;
 
   public PackageLookupFunction(
       AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages,
-      CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) {
+      CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy,
+      List<BuildFileName> buildFilesByPriority) {
     this.deletedPackages = deletedPackages;
     this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy;
+    this.buildFilesByPriority = buildFilesByPriority;
   }
 
   @Override
@@ -99,7 +103,7 @@ public class PackageLookupFunction implements SkyFunction {
       }
     }
 
-    return getPackageLookupValue(env, pkgLocator.getPathEntries(), packageKey, BuildFileName.BUILD);
+    return findPackageByBuildFile(env, pkgLocator, packageKey);
   }
 
   @Nullable
@@ -109,6 +113,32 @@ public class PackageLookupFunction implements SkyFunction {
   }
 
   @Nullable
+  private PackageLookupValue findPackageByBuildFile(
+      Environment env, PathPackageLocator pkgLocator, PackageIdentifier packageKey)
+      throws PackageLookupFunctionException, InterruptedException {
+    // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
+    // to having restart the SkyFunction after every new dependency. However, if we try to batch
+    // the missing value keys, more dependencies than necessary will be declared. This wart can be
+    // fixed once we have nicer continuation support [skyframe-loading]
+    for (Path packagePathEntry : pkgLocator.getPathEntries()) {
+
+      // This checks for the build file names in the correct precedence order.
+      for (BuildFileName buildFileName : buildFilesByPriority) {
+        PackageLookupValue result =
+            getPackageLookupValue(env, packagePathEntry, packageKey, buildFileName);
+        if (result == null) {
+          return null;
+        }
+        if (result != PackageLookupValue.NO_BUILD_FILE_VALUE) {
+          return result;
+        }
+      }
+    }
+
+    return PackageLookupValue.NO_BUILD_FILE_VALUE;
+  }
+
+  @Nullable
   private static FileValue getFileValue(
       RootedPath fileRootedPath, Environment env, PackageIdentifier packageIdentifier)
       throws PackageLookupFunctionException, InterruptedException {
@@ -143,67 +173,84 @@ public class PackageLookupFunction implements SkyFunction {
       PackageIdentifier packageIdentifier,
       BuildFileName buildFileName)
       throws PackageLookupFunctionException, InterruptedException {
+
     // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
     // to having restart the SkyFunction after every new dependency. However, if we try to batch
     // the missing value keys, more dependencies than necessary will be declared. This wart can be
     // fixed once we have nicer continuation support [skyframe-loading]
     for (Path packagePathEntry : packagePathEntries) {
-      PathFragment buildFileFragment = buildFileName.getBuildFileFragment(packageIdentifier);
-      RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry,
-          buildFileFragment);
-
-      if (crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.ERROR) {
-        // Is this path part of a local repository?
-        RootedPath currentPath =
-            RootedPath.toRootedPath(packagePathEntry, buildFileFragment.getParentDirectory());
-        SkyKey repositoryLookupKey = LocalRepositoryLookupValue.key(currentPath);
-
-        // TODO(jcater): Consider parallelizing these lookups.
-        LocalRepositoryLookupValue localRepository;
-        try {
-          localRepository =
-              (LocalRepositoryLookupValue)
-                  env.getValueOrThrow(
-                      repositoryLookupKey, ErrorDeterminingRepositoryException.class);
-          if (localRepository == null) {
-            return null;
-          }
-        } catch (ErrorDeterminingRepositoryException e) {
-          // If the directory selected isn't part of a repository, that's an error.
-          // TODO(katre): Improve the error message given here.
-          throw new PackageLookupFunctionException(
-              new BuildFileNotFoundException(
-                  packageIdentifier,
-                  "Unable to determine the local repository for directory "
-                      + currentPath.asPath().getPathString()),
-              Transience.PERSISTENT);
-        }
+      PackageLookupValue result =
+          getPackageLookupValue(env, packagePathEntry, packageIdentifier, buildFileName);
+      if (result == null) {
+        return null;
+      }
+      if (result != PackageLookupValue.NO_BUILD_FILE_VALUE) {
+        return result;
+      }
+    }
+    return PackageLookupValue.NO_BUILD_FILE_VALUE;
+  }
 
-        if (localRepository.exists()
-            && !localRepository.getRepository().equals(packageIdentifier.getRepository())) {
-          // There is a repository mismatch, this is an error.
-          // TODO(jcater): Work out the correct package name for this error message.
-          return PackageLookupValue.invalidPackageName(
-              "Package crosses into repository " + localRepository.getRepository().getName());
-        }
+  private PackageLookupValue getPackageLookupValue(
+      Environment env,
+      Path packagePathEntry,
+      PackageIdentifier packageIdentifier,
+      BuildFileName buildFileName)
+      throws InterruptedException, PackageLookupFunctionException {
+    PathFragment buildFileFragment = buildFileName.getBuildFileFragment(packageIdentifier);
+    RootedPath buildFileRootedPath = RootedPath.toRootedPath(packagePathEntry, buildFileFragment);
 
-        // There's no local repository, keep going.
-      } else {
-        // Future-proof against adding future values to CrossRepositoryLabelViolationStrategy.
-        Preconditions.checkState(
-            crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.IGNORE,
-            crossRepositoryLabelViolationStrategy);
-      }
+    if (crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.ERROR) {
+      // Is this path part of a local repository?
+      RootedPath currentPath =
+          RootedPath.toRootedPath(packagePathEntry, buildFileFragment.getParentDirectory());
+      SkyKey repositoryLookupKey = LocalRepositoryLookupValue.key(currentPath);
 
-      // Check for the existence of the build file.
-      FileValue fileValue = getFileValue(buildFileRootedPath, env, packageIdentifier);
-      if (fileValue == null) {
-        return null;
+      // TODO(jcater): Consider parallelizing these lookups.
+      LocalRepositoryLookupValue localRepository;
+      try {
+        localRepository =
+            (LocalRepositoryLookupValue)
+                env.getValueOrThrow(repositoryLookupKey, ErrorDeterminingRepositoryException.class);
+        if (localRepository == null) {
+          return null;
+        }
+      } catch (ErrorDeterminingRepositoryException e) {
+        // If the directory selected isn't part of a repository, that's an error.
+        // TODO(katre): Improve the error message given here.
+        throw new PackageLookupFunctionException(
+            new BuildFileNotFoundException(
+                packageIdentifier,
+                "Unable to determine the local repository for directory "
+                    + currentPath.asPath().getPathString()),
+            Transience.PERSISTENT);
       }
-      if (fileValue.isFile()) {
-        return PackageLookupValue.success(buildFileRootedPath.getRoot(), buildFileName);
+
+      if (localRepository.exists()
+          && !localRepository.getRepository().equals(packageIdentifier.getRepository())) {
+        // There is a repository mismatch, this is an error.
+        // TODO(jcater): Work out the correct package name for this error message.
+        return PackageLookupValue.invalidPackageName(
+            "Package crosses into repository " + localRepository.getRepository().getName());
       }
+
+      // There's no local repository, keep going.
+    } else {
+      // Future-proof against adding future values to CrossRepositoryLabelViolationStrategy.
+      Preconditions.checkState(
+          crossRepositoryLabelViolationStrategy == CrossRepositoryLabelViolationStrategy.IGNORE,
+          crossRepositoryLabelViolationStrategy);
+    }
+
+    // Check for the existence of the build file.
+    FileValue fileValue = getFileValue(buildFileRootedPath, env, packageIdentifier);
+    if (fileValue == null) {
+      return null;
     }
+    if (fileValue.isFile()) {
+      return PackageLookupValue.success(buildFileRootedPath.getRoot(), buildFileName);
+    }
+
     return PackageLookupValue.NO_BUILD_FILE_VALUE;
   }