diff options
Diffstat (limited to 'src/main')
16 files changed, 386 insertions, 80 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractFileSymlinkExceptionUniquenessFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractChainUniquenessFunction.java index 5cd698c7ae..f6ce018a37 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractFileSymlinkExceptionUniquenessFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractChainUniquenessFunction.java @@ -15,30 +15,43 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -abstract class AbstractFileSymlinkExceptionUniquenessFunction implements SkyFunction { +/** + * Given a "cycle" of objects of type {@param S}, emits an error message for this cycle. + * The keys for this SkyFunction are assumed to deduplicate cycles that differ only in which element + * of the cycle they start at, so multiple paths to the cycle will be reported by a single execution + * of this function. + * + * <p>The cycle need not actually be a cycle -- any iterable exhibiting an error that is independent + * of the iterable's starting point can be an argument to this function. + */ +abstract class AbstractChainUniquenessFunction<S> implements SkyFunction { protected abstract String getConciseDescription(); + protected abstract String getHeaderMessage(); + protected abstract String getFooterMessage(); + protected abstract SkyValue getDummyValue(); + protected abstract String elementToString(S elt); + @Override public SkyValue compute(SkyKey skyKey, Environment env) { StringBuilder errorMessage = new StringBuilder(); errorMessage.append(getConciseDescription() + " detected\n"); errorMessage.append(getHeaderMessage() + "\n"); @SuppressWarnings("unchecked") - ImmutableList<RootedPath> chain = (ImmutableList<RootedPath>) skyKey.argument(); - for (RootedPath rootedPath : chain) { - errorMessage.append(rootedPath.asPath() + "\n"); + ImmutableList<S> chain = (ImmutableList<S>) skyKey.argument(); + for (S elt : chain) { + errorMessage.append(elementToString(elt) + "\n"); } errorMessage.append(getFooterMessage() + "\n"); // The purpose of this SkyFunction is the side effect of emitting an error message exactly - // once per build per unique symlink error. + // once per build per unique error. env.getListener().handle(Event.error(errorMessage.toString())); return getDummyValue(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractFileSymlinkExceptionUniquenessValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractChainUniquenessValue.java index 39c20cc534..69d88cb38e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractFileSymlinkExceptionUniquenessValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractChainUniquenessValue.java @@ -15,23 +15,22 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; /** - * A value for ensuring that a file symlink error is reported exactly once. This is achieved by - * forcing the same value key for two logically equivalent errors and letting Skyframe do its + * A value for ensuring that an error for a cycle/chain is reported exactly once. This is achieved + * by forcing the same value key for two logically equivalent errors and letting Skyframe do its * magic. */ -class AbstractFileSymlinkExceptionUniquenessValue implements SkyValue { - protected static SkyKey key(SkyFunctionName functionName, ImmutableList<RootedPath> chain) { +class AbstractChainUniquenessValue implements SkyValue { + protected static SkyKey key(SkyFunctionName functionName, ImmutableList<? extends Object> chain) { Preconditions.checkState(!chain.isEmpty()); return new SkyKey(functionName, canonicalize(chain)); } - private static ImmutableList<RootedPath> canonicalize(ImmutableList<RootedPath> cycle) { + private static ImmutableList<Object> canonicalize(ImmutableList<? extends Object> cycle) { int minPos = 0; String minString = cycle.get(0).toString(); for (int i = 1; i < cycle.size(); i++) { @@ -41,7 +40,7 @@ class AbstractFileSymlinkExceptionUniquenessValue implements SkyValue { minString = candidateString; } } - ImmutableList.Builder<RootedPath> builder = ImmutableList.builder(); + ImmutableList.Builder<Object> builder = ImmutableList.builder(); for (int i = 0; i < cycle.size(); i++) { int pos = (minPos + i) % cycle.size(); builder.add(cycle.get(pos)); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CycleUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/CycleUtils.java new file mode 100644 index 0000000000..c765ddf767 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CycleUtils.java @@ -0,0 +1,41 @@ +// Copyright 2015 Google Inc. 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. +package com.google.devtools.build.lib.skyframe; + +import com.google.common.base.Predicate; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.util.Pair; + +/** Simple utility for separating path and cycle from a combined iterable. */ +class CycleUtils { + private CycleUtils() {} + + static <S> Pair<ImmutableList<S>, ImmutableList<S>> splitIntoPathAndChain( + Predicate<S> startOfCycle, Iterable<S> pathAndCycle) { + boolean inPathToCycle = true; + ImmutableList.Builder<S> pathToCycleBuilder = ImmutableList.builder(); + ImmutableList.Builder<S> cycleBuilder = ImmutableList.builder(); + for (S elt : pathAndCycle) { + if (startOfCycle.apply(elt)) { + inPathToCycle = false; + } + if (inPathToCycle) { + pathToCycleBuilder.add(elt); + } else { + cycleBuilder.add(elt); + } + } + return Pair.of(pathToCycleBuilder.build(), cycleBuilder.build()); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java index bec8ed358e..8cfb363b6f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileFunction.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Sets; @@ -220,16 +221,18 @@ public class FileFunction implements SkyFunction { FileSymlinkException fse; if (symlinkTargetPath.equals(existingFloorPath)) { Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain = - splitIntoPathAndChain(symlinkTargetRootedPath.asPath(), symlinkChain); + CycleUtils.splitIntoPathAndChain( + isPathPredicate(symlinkTargetRootedPath.asPath()), symlinkChain); FileSymlinkCycleException fsce = new FileSymlinkCycleException(pathAndChain.getFirst(), pathAndChain.getSecond()); uniquenessKey = FileSymlinkCycleUniquenessValue.key(fsce.getCycle()); fse = fsce; } else { Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> pathAndChain = - splitIntoPathAndChain(existingFloorPath, - ImmutableList.copyOf(Iterables.concat(symlinkChain, - ImmutableList.of(symlinkTargetRootedPath)))); + CycleUtils.splitIntoPathAndChain( + isPathPredicate(existingFloorPath), + ImmutableList.copyOf( + Iterables.concat(symlinkChain, ImmutableList.of(symlinkTargetRootedPath)))); uniquenessKey = FileSymlinkInfiniteExpansionUniquenessValue.key(pathAndChain.getSecond()); fse = new FileSymlinkInfiniteExpansionException( pathAndChain.getFirst(), pathAndChain.getSecond()); @@ -244,22 +247,13 @@ public class FileFunction implements SkyFunction { return resolveFromAncestors(symlinkTargetRootedPath, env); } - private Pair<ImmutableList<RootedPath>, ImmutableList<RootedPath>> splitIntoPathAndChain( - Path startOfCycle, Iterable<RootedPath> symlinkRootedPaths) { - boolean inPathToCycle = true; - ImmutableList.Builder<RootedPath> pathToCycleBuilder = ImmutableList.builder(); - ImmutableList.Builder<RootedPath> cycleBuilder = ImmutableList.builder(); - for (RootedPath rootedPath : symlinkRootedPaths) { - if (rootedPath.asPath().equals(startOfCycle)) { - inPathToCycle = false; + private static final Predicate<RootedPath> isPathPredicate(final Path path) { + return new Predicate<RootedPath>() { + @Override + public boolean apply(RootedPath rootedPath) { + return rootedPath.asPath().equals(path); } - if (inPathToCycle) { - pathToCycleBuilder.add(rootedPath); - } else { - cycleBuilder.add(rootedPath); - } - } - return Pair.of(pathToCycleBuilder.build(), cycleBuilder.build()); + }; } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessFunction.java index 3ce5532b17..4077c65600 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessFunction.java @@ -13,18 +13,24 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyValue; /** A {@link SkyFunction} that has the side effect of reporting a file symlink cycle. */ public class FileSymlinkCycleUniquenessFunction - extends AbstractFileSymlinkExceptionUniquenessFunction { + extends AbstractChainUniquenessFunction<RootedPath> { @Override protected SkyValue getDummyValue() { return FileSymlinkCycleUniquenessValue.INSTANCE; } @Override + protected String elementToString(RootedPath elt) { + return elt.asPath().toString(); + } + + @Override protected String getConciseDescription() { return "circular symlinks"; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessValue.java index e8e9f28352..3e3d8c75c5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkCycleUniquenessValue.java @@ -20,16 +20,15 @@ import com.google.devtools.build.skyframe.SkyKey; /** * A value for ensuring that a file symlink cycle is reported exactly once. This is achieved by * forcing the same value key for two logically equivalent cycles (e.g. ['a' -> 'b' -> 'c' -> 'a'] - * and ['b' -> 'c' -> 'a' -> 'b']), and letting Skyframe do its magic. + * and ['b' -> 'c' -> 'a' -> 'b']), and letting Skyframe do its magic. */ -class FileSymlinkCycleUniquenessValue extends AbstractFileSymlinkExceptionUniquenessValue { +class FileSymlinkCycleUniquenessValue extends AbstractChainUniquenessValue { static final FileSymlinkCycleUniquenessValue INSTANCE = new FileSymlinkCycleUniquenessValue(); private FileSymlinkCycleUniquenessValue() { } static SkyKey key(ImmutableList<RootedPath> cycle) { - return AbstractFileSymlinkExceptionUniquenessValue.key( - SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS, cycle); + return AbstractChainUniquenessValue.key(SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS, cycle); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessFunction.java index 06a94619a0..bc559425cb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessFunction.java @@ -13,18 +13,24 @@ // limitations under the License. package com.google.devtools.build.lib.skyframe; +import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyValue; /** A {@link SkyFunction} that has the side effect of reporting a file symlink expansion error. */ public class FileSymlinkInfiniteExpansionUniquenessFunction - extends AbstractFileSymlinkExceptionUniquenessFunction { + extends AbstractChainUniquenessFunction<RootedPath> { @Override protected SkyValue getDummyValue() { return FileSymlinkInfiniteExpansionUniquenessValue.INSTANCE; } @Override + protected String elementToString(RootedPath elt) { + return elt.asPath().toString(); + } + + @Override protected String getConciseDescription() { return "infinite symlink expansion"; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessValue.java index e866b51572..0c152b1905 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileSymlinkInfiniteExpansionUniquenessValue.java @@ -32,7 +32,7 @@ class FileSymlinkInfiniteExpansionUniquenessValue implements SkyValue { } static SkyKey key(ImmutableList<RootedPath> cycle) { - return AbstractFileSymlinkExceptionUniquenessValue.key( + return AbstractChainUniquenessValue.key( SkyFunctions.FILE_SYMLINK_INFINITE_EXPANSION_UNIQUENESS, cycle); } } 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 9676aa7c35..df9a1b8421 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 @@ -36,9 +36,11 @@ import com.google.devtools.build.lib.packages.CachingPackageLocator; import com.google.devtools.build.lib.packages.InvalidPackageNameException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.Package.LegacyBuilder; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.PackageFactory.Globber; import com.google.devtools.build.lib.packages.Preprocessor; +import com.google.devtools.build.lib.packages.Preprocessor.Result; import com.google.devtools.build.lib.packages.RuleVisibility; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.profiler.Profiler; @@ -62,10 +64,12 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.google.devtools.build.skyframe.ValueOrException3; import com.google.devtools.build.skyframe.ValueOrException4; +import com.google.devtools.build.skyframe.ValueOrExceptionUtils; import java.io.IOException; import java.util.Collection; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -91,15 +95,23 @@ public class PackageFunction implements SkyFunction { private final PathFragment preludePath; + // Not final only for testing. + @Nullable private SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining; + static final String DEFAULTS_PACKAGE_NAME = "tools/defaults"; public static final String EXTERNAL_PACKAGE_NAME = "external"; - public PackageFunction(Reporter reporter, PackageFactory packageFactory, - CachingPackageLocator pkgLocator, AtomicBoolean showLoadingProgress, - Cache<PackageIdentifier, Package.LegacyBuilder> packageFunctionCache, - Cache<PackageIdentifier, Preprocessor.Result> preprocessCache, - AtomicInteger numPackagesLoaded) { + public PackageFunction( + Reporter reporter, + PackageFactory packageFactory, + CachingPackageLocator pkgLocator, + AtomicBoolean showLoadingProgress, + Cache<PackageIdentifier, LegacyBuilder> packageFunctionCache, + Cache<PackageIdentifier, Result> preprocessCache, + AtomicInteger numPackagesLoaded, + @Nullable SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining) { this.reporter = reporter; + this.skylarkImportLookupFunctionForInlining = skylarkImportLookupFunctionForInlining; // Can be null in tests. this.preludePath = packageFactory == null @@ -113,6 +125,11 @@ public class PackageFunction implements SkyFunction { this.numPackagesLoaded = numPackagesLoaded; } + public void setSkylarkImportLookupFunctionForInliningForTesting( + SkylarkImportLookupFunction skylarkImportLookupFunctionForInlining) { + this.skylarkImportLookupFunctionForInlining = skylarkImportLookupFunctionForInlining; + } + private static void maybeThrowFilesystemInconsistency(PackageIdentifier packageIdentifier, Exception skyframeException, boolean packageWasInError) throws InternalInconsistentFilesystemException { @@ -533,7 +550,7 @@ public class PackageFunction implements SkyFunction { Environment env, ParserInputSource inputSource, List<Statement> preludeStatements) - throws PackageFunctionException { + throws PackageFunctionException, InterruptedException { StoredEventHandler eventHandler = new StoredEventHandler(); BuildFileAST buildFileAST = BuildFileAST.parseBuildFile( @@ -574,6 +591,21 @@ public class PackageFunction implements SkyFunction { return SkylarkImportLookupValue.key(repository, buildFileFragment, importFile); } + private static SkyKey getImportKeyAndMaybeThrowException( + Map.Entry<Location, PathFragment> entry, + PathFragment preludePath, + PathFragment buildFileFragment, + PackageIdentifier packageId) + throws PackageFunctionException { + try { + return getImportKey(entry, preludePath, buildFileFragment, packageId); + } catch (ASTLookupInputException e) { + // The load syntax is bad in the BUILD file so BuildFileContainsErrorsException is OK. + throw new PackageFunctionException( + new BuildFileContainsErrorsException(packageId, e.getMessage()), Transience.PERSISTENT); + } + } + /** * Fetch the skylark loads for this BUILD file. If any of them haven't been computed yet, * returns null. @@ -585,33 +617,72 @@ public class PackageFunction implements SkyFunction { PackageIdentifier packageId, BuildFileAST buildFileAST, Environment env) - throws PackageFunctionException { + throws PackageFunctionException, InterruptedException { ImmutableMap<Location, PathFragment> imports = buildFileAST.getImports(); Map<PathFragment, Extension> importMap = Maps.newHashMapWithExpectedSize(imports.size()); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); - Map<SkyKey, PathFragment> skylarkImports = Maps.newHashMapWithExpectedSize(imports.size()); - for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) { - try { - skylarkImports.put( - getImportKey(entry, preludePath, buildFileFragment, packageId), entry.getValue()); - } catch (ASTLookupInputException e) { - // The load syntax is bad in the BUILD file so BuildFileContainsErrorsException is OK. - throw new PackageFunctionException( - new BuildFileContainsErrorsException(packageId, e.getMessage()), Transience.PERSISTENT); - } - } Map< SkyKey, ValueOrException4< SkylarkImportFailedException, InconsistentFilesystemException, ASTLookupInputException, BuildFileNotFoundException>> - skylarkImportMap = - env.getValuesOrThrow( - skylarkImports.keySet(), - SkylarkImportFailedException.class, - InconsistentFilesystemException.class, - ASTLookupInputException.class, - BuildFileNotFoundException.class); + skylarkImportMap; + Map<SkyKey, PathFragment> skylarkImports = Maps.newHashMapWithExpectedSize(imports.size()); + if (skylarkImportLookupFunctionForInlining != null) { + skylarkImportMap = Maps.newHashMapWithExpectedSize(imports.size()); + for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) { + SkyKey importKey = + getImportKeyAndMaybeThrowException(entry, preludePath, buildFileFragment, packageId); + skylarkImports.put(importKey, entry.getValue()); + ValueOrException4< + SkylarkImportFailedException, InconsistentFilesystemException, + ASTLookupInputException, BuildFileNotFoundException> + lookupResult; + Set<SkyKey> visitedKeysForCycle = new LinkedHashSet<>(); + try { + SkyValue value = + skylarkImportLookupFunctionForInlining.computeWithInlineCalls(importKey, env, + visitedKeysForCycle); + if (value == null) { + Preconditions.checkState(env.valuesMissing(), importKey); + // Don't give up on computing. This is against the Skyframe contract, but we don't want + // to pay the price of serializing all these calls, since they are fundamentally + // independent. + lookupResult = ValueOrExceptionUtils.ofNullValue(); + } else { + lookupResult = ValueOrExceptionUtils.ofValue(value); + } + } catch (SkyFunctionException e) { + Exception cause = e.getCause(); + if (cause instanceof SkylarkImportFailedException) { + lookupResult = ValueOrExceptionUtils.ofExn1((SkylarkImportFailedException) cause); + } else if (cause instanceof InconsistentFilesystemException) { + lookupResult = ValueOrExceptionUtils.ofExn2((InconsistentFilesystemException) cause); + } else if (cause instanceof ASTLookupInputException) { + lookupResult = ValueOrExceptionUtils.ofExn3((ASTLookupInputException) cause); + } else if (cause instanceof BuildFileNotFoundException) { + lookupResult = ValueOrExceptionUtils.ofExn4((BuildFileNotFoundException) cause); + } else { + throw new IllegalStateException("Unexpected type for " + importKey, e); + } + } + skylarkImportMap.put(importKey, lookupResult); + } + } else { + for (Map.Entry<Location, PathFragment> entry : imports.entrySet()) { + skylarkImports.put( + getImportKeyAndMaybeThrowException(entry, preludePath, buildFileFragment, packageId), + entry.getValue()); + } + skylarkImportMap = + env.getValuesOrThrow( + skylarkImports.keySet(), + SkylarkImportFailedException.class, + InconsistentFilesystemException.class, + ASTLookupInputException.class, + BuildFileNotFoundException.class); + } + for (Map.Entry< SkyKey, ValueOrException4< diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 37d625c8bb..6812e15f64 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -38,6 +38,8 @@ public final class SkyFunctions { public static final SkyFunctionName AST_FILE_LOOKUP = SkyFunctionName.create("AST_FILE_LOOKUP"); public static final SkyFunctionName SKYLARK_IMPORTS_LOOKUP = SkyFunctionName.create("SKYLARK_IMPORTS_LOOKUP"); + static final SkyFunctionName SKYLARK_IMPORT_CYCLE = + SkyFunctionName.create("SKYLARK_IMPORT_CYCLE"); public static final SkyFunctionName GLOB = SkyFunctionName.create("GLOB"); public static final SkyFunctionName PACKAGE = SkyFunctionName.create("PACKAGE"); public static final SkyFunctionName PACKAGE_ERROR = SkyFunctionName.create("PACKAGE_ERROR"); 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 74f3bd4391..103932c3d2 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 @@ -79,8 +79,10 @@ import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Package; +import com.google.devtools.build.lib.packages.Package.LegacyBuilder; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Preprocessor; +import com.google.devtools.build.lib.packages.Preprocessor.Result; import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.RuleVisibility; import com.google.devtools.build.lib.packages.Target; @@ -325,8 +327,10 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction(deletedPackages)); map.put(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, new ContainingPackageLookupFunction()); map.put(SkyFunctions.AST_FILE_LOOKUP, new ASTFileLookupFunction(pkgLocator, ruleClassProvider)); - map.put(SkyFunctions.SKYLARK_IMPORTS_LOOKUP, new SkylarkImportLookupFunction( - ruleClassProvider, pkgFactory)); + map.put( + SkyFunctions.SKYLARK_IMPORTS_LOOKUP, + newSkylarkImportLookupFunction(ruleClassProvider, pkgFactory)); + map.put(SkyFunctions.SKYLARK_IMPORT_CYCLE, new SkylarkImportUniqueCycleFunction()); map.put(SkyFunctions.GLOB, newGlobFunction()); map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction(pkgLocator)); map.put(SkyFunctions.PREPARE_DEPS_OF_PATTERNS, new PrepareDepsOfPatternsFunction()); @@ -334,9 +338,17 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { map.put(SkyFunctions.PREPARE_DEPS_OF_TARGETS_UNDER_DIRECTORY, new PrepareDepsOfTargetsUnderDirectoryFunction()); map.put(SkyFunctions.RECURSIVE_PKG, new RecursivePkgFunction()); - map.put(SkyFunctions.PACKAGE, new PackageFunction( - reporter, pkgFactory, packageManager, showLoadingProgress, packageFunctionCache, - preprocessCache, numPackagesLoaded)); + map.put( + SkyFunctions.PACKAGE, + newPackageFunction( + reporter, + pkgFactory, + packageManager, + showLoadingProgress, + packageFunctionCache, + preprocessCache, + numPackagesLoaded, + ruleClassProvider)); map.put(SkyFunctions.PACKAGE_ERROR, new PackageErrorFunction()); map.put(SkyFunctions.TARGET_MARKER, new TargetMarkerFunction()); map.put(SkyFunctions.TRANSITIVE_TARGET, new TransitiveTargetFunction(ruleClassProvider)); @@ -378,6 +390,31 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { return new GlobFunction(/*alwaysUseDirListing=*/false); } + protected PackageFunction newPackageFunction( + Reporter reporter, + PackageFactory pkgFactory, + PackageManager packageManager, + AtomicBoolean showLoadingProgress, + Cache<PackageIdentifier, LegacyBuilder> packageFunctionCache, + Cache<PackageIdentifier, Result> preprocessCache, + AtomicInteger numPackagesLoaded, + RuleClassProvider ruleClassProvider) { + return new PackageFunction( + reporter, + pkgFactory, + packageManager, + showLoadingProgress, + packageFunctionCache, + preprocessCache, + numPackagesLoaded, + null); + } + + protected SkyFunction newSkylarkImportLookupFunction( + RuleClassProvider ruleClassProvider, PackageFactory pkgFactory) { + return new SkylarkImportLookupFunction(ruleClassProvider, this.pkgFactory); + } + protected PerBuildSyscallCache newPerBuildSyscallCache() { return PerBuildSyscallCache.newUnboundedCache(); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java index de09f75d0a..d02227b2e8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.common.base.Preconditions; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import com.google.devtools.build.lib.cmdline.Label; @@ -39,6 +40,9 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.util.Map; +import java.util.Set; + +import javax.annotation.Nullable; /** * A Skyframe function to look up and import a single Skylark extension. @@ -57,6 +61,17 @@ public class SkylarkImportLookupFunction implements SkyFunction { @Override public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException, InterruptedException { + return computeInternal(skyKey, env, null); + } + + SkyValue computeWithInlineCalls(SkyKey skyKey, Environment env, Set<SkyKey> visitedKeysForCycle) + throws SkyFunctionException, InterruptedException { + return computeInternal(skyKey, env, Preconditions.checkNotNull(visitedKeysForCycle, skyKey)); + } + + SkyValue computeInternal(SkyKey skyKey, Environment env, + @Nullable Set<SkyKey> visitedKeysForCycle) + throws SkyFunctionException, InterruptedException { PackageIdentifier arg = (PackageIdentifier) skyKey.argument(); PathFragment file = arg.getPackageFragment(); ASTFileLookupValue astLookupValue = null; @@ -84,6 +99,12 @@ public class SkylarkImportLookupFunction implements SkyFunction { SkylarkImportFailedException.skylarkErrors(file)); } + Label label = pathFragmentToLabel(arg.getRepository(), file, env); + if (label == null) { + Preconditions.checkState(env.valuesMissing(), "null label with no missing %s", file); + return null; + } + Map<Location, PathFragment> astImports = ast.getImports(); Map<PathFragment, Extension> importMap = Maps.newHashMapWithExpectedSize(astImports.size()); ImmutableList.Builder<SkylarkFileDependency> fileDependencies = ImmutableList.builder(); @@ -97,12 +118,47 @@ public class SkylarkImportLookupFunction implements SkyFunction { throw new SkylarkImportLookupFunctionException(e, Transience.PERSISTENT); } } - Map<SkyKey, SkyValue> skylarkImportMap = env.getValues(skylarkImports.keySet()); - if (env.valuesMissing()) { + Map<SkyKey, SkyValue> skylarkImportMap; + boolean valuesMissing = false; + if (visitedKeysForCycle == null) { + // Not inlining. + skylarkImportMap = env.getValues(skylarkImports.keySet()); + valuesMissing = env.valuesMissing(); + } else { + // inlining calls to SkylarkImportLookupFunction. + if (!visitedKeysForCycle.add(skyKey)) { + ImmutableList<SkyKey> cycle = + CycleUtils.splitIntoPathAndChain(Predicates.equalTo(skyKey), visitedKeysForCycle) + .second; + if (env.getValue(SkylarkImportUniqueCycleValue.key(cycle)) == null) { + return null; + } + throw new SkylarkImportLookupFunctionException( + new SkylarkImportFailedException("Skylark import cycle")); + } + skylarkImportMap = Maps.newHashMapWithExpectedSize(astImports.size()); + for (SkyKey skylarkImport : skylarkImports.keySet()) { + SkyValue skyValue = this.computeWithInlineCalls(skylarkImport, env, visitedKeysForCycle); + if (skyValue == null) { + Preconditions.checkState( + env.valuesMissing(), "no skylark import value for %s", skylarkImport); + // Don't give up on computing. This is against the Skyframe contract, but we don't want to + // pay the price of serializing all these calls, since they are fundamentally independent. + valuesMissing = true; + } else { + skylarkImportMap.put(skylarkImport, skyValue); + } + } + // All imports traversed, this key can no longer be part of a cycle. + visitedKeysForCycle.remove(skyKey); + } + + if (valuesMissing) { // This means some imports are unavailable. return null; } + for (Map.Entry<SkyKey, SkyValue> entry : skylarkImportMap.entrySet()) { SkylarkImportLookupValue importLookupValue = (SkylarkImportLookupValue) entry.getValue(); importMap.put( @@ -110,17 +166,9 @@ public class SkylarkImportLookupFunction implements SkyFunction { fileDependencies.add(importLookupValue.getDependency()); } - Label label = pathFragmentToLabel(arg.getRepository(), file, env); - - if (label == null) { - Preconditions.checkState(env.valuesMissing(), "label null but no missing for %s", file); - return null; - } - // Skylark UserDefinedFunction-s in that file will share this function definition Environment, // which will be frozen by the time it is returned by createExtension. - Extension extension = - createExtension(ast, file, importMap, env); + Extension extension = createExtension(ast, file, importMap, env); return new SkylarkImportLookupValue( extension, new SkylarkFileDependency(label, fileDependencies.build())); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java index 0ac9fd4402..390f958658 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java @@ -71,6 +71,12 @@ public class SkylarkImportLookupValue implements SkyValue { return key(pkgIdentifier.getRepository(), pkgIdentifier.getPackageFragment()); } + /** + * Returns a SkyKey to get a SkylarkImportLookupValue. Note that SkylarkImportLookupValue + * computations may be inlined to avoid having them in the graph. Callers should confirm whether + * inlining is desired and either do the computation directly themselves (if inlined) or request + * this key's value from the environment (if not). + */ static SkyKey key(RepositoryName repo, PathFragment fromFile, PathFragment fileToImport) throws ASTLookupInputException { PathFragment computedPath; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleFunction.java new file mode 100644 index 0000000000..d7ea4a3a8f --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleFunction.java @@ -0,0 +1,52 @@ +// Copyright 2015 Google Inc. 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. +package com.google.devtools.build.lib.skyframe; + +import com.google.devtools.build.lib.cmdline.PackageIdentifier; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +/** + * Emits an error message exactly once when a Skylark import cycle is found when running inlined + * {@link SkylarkImportLookupFunction}s. + */ +class SkylarkImportUniqueCycleFunction extends AbstractChainUniquenessFunction<SkyKey> { + private static final SkyValue INSTANCE = new SkyValue() {}; + + @Override + protected String getConciseDescription() { + return "cycle in referenced extension files"; + } + + @Override + protected String getHeaderMessage() { + return ""; + } + + @Override + protected String getFooterMessage() { + return ""; + } + + @Override + protected SkyValue getDummyValue() { + return INSTANCE; + } + + @Override + protected String elementToString(SkyKey elt) { + PackageIdentifier pkgId = (PackageIdentifier) elt.argument(); + return pkgId.getPathFragment().getPathString(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleValue.java new file mode 100644 index 0000000000..548310a200 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportUniqueCycleValue.java @@ -0,0 +1,28 @@ +// Copyright 2015 Google Inc. 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. +package com.google.devtools.build.lib.skyframe; + +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; + +/** + * If {@link SkylarkImportLookupFunction} is inlined, this is used to emit cycle errors exactly + * once. + */ +public class SkylarkImportUniqueCycleValue implements SkyValue { + static SkyKey key(ImmutableList<SkyKey> cycle) { + return AbstractChainUniquenessValue.key(SkyFunctions.SKYLARK_IMPORT_CYCLE, cycle); + } +} diff --git a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java index 9e346ebcb5..c1c94a48c2 100644 --- a/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/InMemoryMemoizingEvaluator.java @@ -310,6 +310,10 @@ public final class InMemoryMemoizingEvaluator implements MemoizingEvaluator { } } + public ImmutableMap<? extends SkyFunctionName, ? extends SkyFunction> + getSkyFunctionsForTesting() { + return skyFunctions; + } public static final Predicate<Event> DEFAULT_STORED_EVENT_FILTER = new Predicate<Event>() { @Override public boolean apply(Event event) { |