diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
5 files changed, 133 insertions, 62 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; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java index 8ad5ae0c06..32d1e9007f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java @@ -39,6 +39,7 @@ public abstract class PackageLookupValue implements SkyValue { * The file (BUILD, WORKSPACE, etc.) that defines this package, referred to as the "build file". */ public enum BuildFileName { + WORKSPACE("WORKSPACE") { @Override public PathFragment getBuildFileFragment(PackageIdentifier packageIdentifier) { @@ -50,6 +51,12 @@ public abstract class PackageLookupValue implements SkyValue { public PathFragment getBuildFileFragment(PackageIdentifier packageIdentifier) { return packageIdentifier.getPackageFragment().getChild(getFilename()); } + }, + BUILD_DOT_BAZEL("BUILD.bazel") { + @Override + public PathFragment getBuildFileFragment(PackageIdentifier packageIdentifier) { + return packageIdentifier.getPackageFragment().getChild(getFilename()); + } }; private static final BuildFileName[] VALUES = BuildFileName.values(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 7a45eeded1..40e8a5d14f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAc import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFilesKnowledge; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; @@ -73,6 +74,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.EnumSet; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -112,7 +114,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { Iterable<SkyValueDirtinessChecker> customDirtinessCheckers, PathFragment blacklistedPackagePrefixesFile, String productName, - CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) { + CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, + List<BuildFileName> buildFilesByPriority) { super( evaluatorSupplier, pkgFactory, @@ -127,7 +130,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, blacklistedPackagePrefixesFile, productName, - crossRepositoryLabelViolationStrategy); + crossRepositoryLabelViolationStrategy, + buildFilesByPriority); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); this.customDirtinessCheckers = customDirtinessCheckers; } @@ -145,7 +149,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { ImmutableList<PrecomputedValue.Injected> extraPrecomputedValues, Iterable<SkyValueDirtinessChecker> customDirtinessCheckers, String productName, - CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) { + CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, + List<BuildFileName> buildFilesByPriority) { return create( pkgFactory, directories, @@ -160,7 +165,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { customDirtinessCheckers, /*blacklistedPackagePrefixesFile=*/ PathFragment.EMPTY_FRAGMENT, productName, - crossRepositoryLabelViolationStrategy); + crossRepositoryLabelViolationStrategy, + buildFilesByPriority); } private static SequencedSkyframeExecutor create( @@ -177,7 +183,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { Iterable<SkyValueDirtinessChecker> customDirtinessCheckers, PathFragment blacklistedPackagePrefixesFile, String productName, - CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) { + CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, + List<BuildFileName> buildFilesByPriority) { SequencedSkyframeExecutor skyframeExecutor = new SequencedSkyframeExecutor( InMemoryMemoizingEvaluator.SUPPLIER, @@ -194,7 +201,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { customDirtinessCheckers, blacklistedPackagePrefixesFile, productName, - crossRepositoryLabelViolationStrategy); + crossRepositoryLabelViolationStrategy, + buildFilesByPriority); skyframeExecutor.init(); return skyframeExecutor; } @@ -221,7 +229,8 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { ImmutableList.<SkyValueDirtinessChecker>of(), blacklistedPackagePrefixesFile, productName, - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index 1e3b046738..4c92f23612 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.config.BinTools; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Preprocessor; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionName; @@ -59,6 +60,7 @@ public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory extraPrecomputedValues, customDirtinessCheckers, productName, - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 39cf8c8e06..ec330247dc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -108,6 +108,7 @@ import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.FileDirtines import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageFunction.CacheEntryWithGlobDeps; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver; import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier; import com.google.devtools.build.lib.util.AbruptExitException; @@ -274,6 +275,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { private final RuleClassProvider ruleClassProvider; private final CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy; + + private final List<BuildFileName> buildFilesByPriority; private static final Logger LOG = Logger.getLogger(SkyframeExecutor.class.getName()); @@ -291,7 +294,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { ExternalFileAction externalFileAction, PathFragment blacklistedPackagePrefixesFile, String productName, - CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy) { + CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy, + List<BuildFileName> buildFilesByPriority) { // Strictly speaking, these arguments are not required for initialization, but all current // callsites have them at hand, so we might as well set them during construction. this.evaluatorSupplier = evaluatorSupplier; @@ -325,6 +329,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { pkgLocator, this.externalFileAction, directories); this.productName = productName; this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy; + this.buildFilesByPriority = buildFilesByPriority; } private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions( @@ -348,7 +353,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); map.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, crossRepositoryLabelViolationStrategy)); + new PackageLookupFunction( + deletedPackages, crossRepositoryLabelViolationStrategy, buildFilesByPriority)); map.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()); map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(ruleClassProvider)); map.put( |