diff options
24 files changed, 331 insertions, 81 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( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index 406b360316..25322fde31 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -53,6 +53,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -179,7 +180,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase { getPrecomputedValues(), ImmutableList.<SkyValueDirtinessChecker>of(), analysisMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class); packageCacheOptions.showLoadingProgress = true; packageCacheOptions.globbingThreads = 3; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index a25b4989cd..4a4b03dbc4 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -123,6 +123,7 @@ import com.google.devtools.build.lib.skyframe.AspectValue; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -226,7 +227,8 @@ public abstract class BuildViewTestCase extends FoundationTestCase { getPrecomputedValues(), ImmutableList.<SkyValueDirtinessChecker>of(), analysisMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC; packageCacheOptions.showLoadingProgress = true; packageCacheOptions.globbingThreads = 7; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java index 66a071ecf2..a3ea604fab 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.pkgcache.PackageCacheOptions; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -115,7 +116,8 @@ public abstract class ConfigurationTestCase extends FoundationTestCase { ImmutableList.<PrecomputedValue.Injected>of(), ImmutableList.<SkyValueDirtinessChecker>of(), analysisMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class); packageCacheOptions.showLoadingProgress = true; diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java index 03d8fdf30d..3925329b4e 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.pkgcache.PackageManager; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -116,7 +117,8 @@ public abstract class PackageLoadingTestCase extends FoundationTestCase { ImmutableList.<PrecomputedValue.Injected>of(), ImmutableList.<SkyValueDirtinessChecker>of(), loadingMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); return skyframeExecutor; } diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java index 32e8b4b30a..bf49ba0539 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.LoadingMock; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -485,7 +486,8 @@ public class IncrementalLoadingTest { ImmutableList.<PrecomputedValue.Injected>of(), ImmutableList.<SkyValueDirtinessChecker>of(), loadingMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class); packageCacheOptions.defaultVisibility = ConstantRuleVisibility.PUBLIC; packageCacheOptions.showLoadingProgress = true; diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index c1279acab7..d917969a67 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.MockToolsConfig; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -613,7 +614,8 @@ public class LoadingPhaseRunnerTest { ImmutableList.<PrecomputedValue.Injected>of(), ImmutableList.<SkyValueDirtinessChecker>of(), analysisMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); PathPackageLocator pkgLocator = PathPackageLocator.create( null, options.packagePath, storedErrors, workspace, workspace); PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class); diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java index 77e34d95fa..fa22c66ea7 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.util.MockToolsConfig; import com.google.devtools.build.lib.skyframe.DiffAwareness; 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.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; @@ -99,7 +100,8 @@ public class PackageCacheTest extends FoundationTestCase { ImmutableList.<PrecomputedValue.Injected>of(), ImmutableList.<SkyValueDirtinessChecker>of(), analysisMock.getProductName(), - CrossRepositoryLabelViolationStrategy.ERROR); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD)); setUpSkyframe(parsePackageCacheOptions()); } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java index cd017bec2d..e3c6a97a43 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTestCase.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.testutil.TestUtils; @@ -93,7 +94,10 @@ abstract class ArtifactFunctionTestCase { new PackageFunction(null, null, null, null, null, null, null)) .put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(null, CrossRepositoryLabelViolationStrategy.ERROR)) + new PackageLookupFunction( + null, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))) .put( SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(TestRuleClassProvider.getRuleClassProvider())) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index fa5f891cff..1260439a7d 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -70,7 +71,10 @@ public class ContainingPackageLookupFunctionTest extends FoundationTestCase { Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR)); + new PackageLookupFunction( + deletedPackages, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()); skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, new BlacklistedPackagePrefixesFunction()); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java index 6de849a0cd..91dbc6c4ae 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java @@ -42,6 +42,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; @@ -145,7 +146,8 @@ public class FileFunctionTest { SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()), - CrossRepositoryLabelViolationStrategy.ERROR)) + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))) .put( SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(TestRuleClassProvider.getRuleClassProvider())) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java index e030cefd3d..40d6d4e1fe 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.packages.FilesetEntry.SymlinkBehavior; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.Fingerprint; @@ -101,7 +102,10 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase { SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, new RecursiveFilesystemTraversalFunction()); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR)); + new PackageLookupFunction( + deletedPackages, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, new BlacklistedPackagePrefixesFunction()); skyFunctions.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction()); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java index e4bf7f1dec..d6d0bf79a4 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesystemValueCheckerTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -119,7 +120,8 @@ public class FilesystemValueCheckerTest { SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( new AtomicReference<>(ImmutableSet.<PackageIdentifier>of()), - CrossRepositoryLabelViolationStrategy.ERROR)); + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put(SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(TestRuleClassProvider.getRuleClassProvider())); skyFunctions.put(SkyFunctions.WORKSPACE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java index 4ee3a8efda..27abfaa775 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.ManualClock; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -136,7 +137,10 @@ public abstract class GlobFunctionTest { skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR)); + new PackageLookupFunction( + deletedPackages, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, new BlacklistedPackagePrefixesFunction()); skyFunctions.put( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java index e51aa42842..564e4665f5 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -73,7 +74,10 @@ public class LocalRepositoryLookupFunctionTest extends FoundationTestCase { Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR)); + new PackageLookupFunction( + deletedPackages, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put( SkyFunctions.FILE_STATE, new FileStateFunction( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index c69cddfca4..0c4c1d4728 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -71,12 +71,13 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase { private MemoizingEvaluator evaluator; private SequentialBuildDriver driver; private RecordingDifferencer differencer; + private Path emptyPackagePath; protected abstract CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy(); @Before public final void setUp() throws Exception { - Path emptyPackagePath = rootDirectory.getRelative("somewhere/else"); + emptyPackagePath = rootDirectory.getRelative("somewhere/else"); scratch.file("parentpackage/BUILD"); AnalysisMock analysisMock = AnalysisMock.get(); @@ -92,7 +93,10 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase { Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>(); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, crossRepositoryLabelViolationStrategy())); + new PackageLookupFunction( + deletedPackages, + crossRepositoryLabelViolationStrategy(), + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put( SkyFunctions.PACKAGE, new PackageFunction(null, null, null, null, null, null, null)); @@ -238,7 +242,7 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase { } @Test - public void testEverythingIsGood() throws Exception { + public void testEverythingIsGood_BUILD() throws Exception { scratch.file("parentpackage/everythinggood/BUILD"); PackageLookupValue packageLookupValue = lookupPackage("parentpackage/everythinggood"); assertTrue(packageLookupValue.packageExists()); @@ -247,6 +251,37 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase { } @Test + public void testEverythingIsGood_BUILD_bazel() throws Exception { + scratch.file("parentpackage/everythinggood/BUILD.bazel"); + PackageLookupValue packageLookupValue = lookupPackage("parentpackage/everythinggood"); + assertTrue(packageLookupValue.packageExists()); + assertEquals(rootDirectory, packageLookupValue.getRoot()); + assertEquals(BuildFileName.BUILD_DOT_BAZEL, packageLookupValue.getBuildFileName()); + } + + @Test + public void testEverythingIsGood_both() throws Exception { + scratch.file("parentpackage/everythinggood/BUILD"); + scratch.file("parentpackage/everythinggood/BUILD.bazel"); + PackageLookupValue packageLookupValue = lookupPackage("parentpackage/everythinggood"); + assertTrue(packageLookupValue.packageExists()); + assertEquals(rootDirectory, packageLookupValue.getRoot()); + assertEquals(BuildFileName.BUILD_DOT_BAZEL, packageLookupValue.getBuildFileName()); + } + + @Test + public void testBuildFilesInMultiplePackagePaths() throws Exception { + scratch.file(emptyPackagePath.getPathString() + "/foo/BUILD"); + scratch.file("foo/BUILD.bazel"); + + // BUILD file in the first package path should be preferred to BUILD.bazel in the second. + PackageLookupValue packageLookupValue = lookupPackage("foo"); + assertTrue(packageLookupValue.packageExists()); + assertEquals(emptyPackagePath, packageLookupValue.getRoot()); + assertEquals(BuildFileName.BUILD, packageLookupValue.getBuildFileName()); + } + + @Test public void testEmptyPackageName() throws Exception { scratch.file("BUILD"); PackageLookupValue packageLookupValue = lookupPackage(""); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index 58b31e7d84..a45d7b1e77 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; 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.RecursiveFilesystemTraversalValue.ResolvedFile; import com.google.devtools.build.lib.skyframe.RecursiveFilesystemTraversalValue.TraversalRequest; import com.google.devtools.build.lib.testutil.FoundationTestCase; @@ -105,7 +106,10 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, new RecursiveFilesystemTraversalFunction()); skyFunctions.put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR)); + new PackageLookupFunction( + deletedPackages, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))); skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES, new BlacklistedPackagePrefixesFunction()); skyFunctions.put(SkyFunctions.PACKAGE, diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java index 18f029391d..0bd57e2f95 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.exec.SingleBuildFileCache; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; +import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName; import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; @@ -187,7 +188,10 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { new PackageFunction(null, null, null, null, null, null, null)) .put( SkyFunctions.PACKAGE_LOOKUP, - new PackageLookupFunction(null, CrossRepositoryLabelViolationStrategy.ERROR)) + new PackageLookupFunction( + null, + CrossRepositoryLabelViolationStrategy.ERROR, + ImmutableList.of(BuildFileName.BUILD_DOT_BAZEL, BuildFileName.BUILD))) .put( SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(TestRuleClassProvider.getRuleClassProvider())) diff --git a/src/test/shell/bazel/BUILD b/src/test/shell/bazel/BUILD index 3b268e2121..e24b174083 100644 --- a/src/test/shell/bazel/BUILD +++ b/src/test/shell/bazel/BUILD @@ -397,6 +397,13 @@ sh_test( data = [":test-deps"], ) +sh_test( + name = "build_files_test", + size = "medium", + srcs = ["build_files_test.sh"], + data = [":test-deps"], +) + test_suite( name = "all_tests", visibility = ["//visibility:public"], diff --git a/src/test/shell/bazel/build_files_test.sh b/src/test/shell/bazel/build_files_test.sh new file mode 100755 index 0000000000..c6756f5003 --- /dev/null +++ b/src/test/shell/bazel/build_files_test.sh @@ -0,0 +1,91 @@ +#!/bin/bash +# +# Copyright 2016 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the proper checking of BUILD and BUILD.bazel files. + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +function write_rule { + name=$1 + shift + srcs="" + for src in "$@"; do + srcs="\"$src\", $srcs" + done + + cat <<EOF +genrule( + name = "$name", + srcs = [ + $srcs + ], + outs = ["$name.out"], + cmd = "echo $name > \$@", + visibility = ["//visibility:public"], +) +EOF +} + +# Only a BUILD file is present: sees rules in BUILD. +function test_build_only { + create_new_workspace + write_rule build_only >> BUILD + + bazel build //:build_only >& $TEST_log || fail "build should succeed" +} + +# Only a BUILD.bazel file is present: sees rules in BUILD.bazel. +function test_build_bazel_only { + create_new_workspace + write_rule build_bazel_only >> BUILD.bazel + + bazel build //:build_bazel_only >& $TEST_log || fail "build should succeed" +} + +# BUILD and BUILD.bazel file is present: sees rules in BUILD.bazel. +function test_build_and_build_bazel { + create_new_workspace + write_rule build_only >> BUILD + write_rule build_bazel_only >> BUILD.bazel + + bazel build //:build_bazel_only >& $TEST_log || fail "build should succeed" + # This rule doesn't actually exist. + bazel build //:build_only >& $TEST_log && fail "build shouldn't succeed" + expect_log "no such target '//:build_only'" +} + +function test_multiple_package_roots { + # Create a main workspace with a BUILD.bazel file. + create_new_workspace + write_rule build_bazel_only > BUILD.bazel + + # Create an alternate package path with a BUILD file. + local other_root=$TEST_TMPDIR/other_root/${WORKSPACE_NAME} + mkdir -p $other_root + write_rule build_only > $other_root/BUILD + + add_to_bazelrc "build --package_path $other_root:." + bazel build //:build_only >& $TEST_log || fail "build should succeed" + # This rule doesn't actually exist. + bazel build //:build_bazel_only >& $TEST_log && fail "build shouldn't succeed" + expect_log "no such target '//:build_bazel_only'" +} + +run_suite "build files tests" + |