diff options
Diffstat (limited to 'src/main')
9 files changed, 173 insertions, 23 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java index 87105d549c..95c943bfa5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java @@ -98,8 +98,8 @@ public abstract class AbstractAction implements Action { } @Override - public void updateInputsFromCache( - ArtifactResolver artifactResolver, Collection<PathFragment> inputPaths) { + public boolean updateInputsFromCache(ArtifactResolver artifactResolver, + PackageRootResolver resolver, Collection<PathFragment> inputPaths) { throw new IllegalStateException( "Method must be overridden for actions that may have unknown inputs."); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java index 4970203ece..efaa0dc3c8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Action.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Action.java @@ -101,7 +101,7 @@ public interface Action extends ActionMetadata, Describable { * Method used to find inputs before execution for an action that * {@link ActionMetadata#discoversInputs}. */ - public void discoverInputs(ActionExecutionContext actionExecutionContext) + void discoverInputs(ActionExecutionContext actionExecutionContext) throws ActionExecutionException, InterruptedException; /** @@ -116,9 +116,13 @@ public interface Action extends ActionMetadata, Describable { * * @param artifactResolver the artifact factory that can be used to manufacture artifacts * @param inputPaths List of relative (to the execution root) input paths + * @param resolver object which helps to resolve some of the artifacts + * @return false if some dependencies are missing and we need to update again later, + * otherwise true. */ - public void updateInputsFromCache( - ArtifactResolver artifactResolver, Collection<PathFragment> inputPaths); + boolean updateInputsFromCache( + ArtifactResolver artifactResolver, PackageRootResolver resolver, + Collection<PathFragment> inputPaths); /** * Return a best-guess estimate of the operation's resource consumption on the diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java index 2525c8dea8..4b970e3540 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java @@ -134,7 +134,7 @@ public class ActionCacheChecker { // Note: the handler should only be used for DEPCHECKER events; there's no // guarantee it will be available for other events. public Token getTokenIfNeedToExecute(Action action, EventHandler handler, - MetadataHandler metadataHandler) { + MetadataHandler metadataHandler, PackageRootResolver resolver) { // TODO(bazel-team): (2010) For RunfilesAction/SymlinkAction and similar actions that // produce only symlinks we should not check whether inputs are valid at all - all that matters // that inputs and outputs are still exist (and new inputs have not appeared). All other checks @@ -157,7 +157,15 @@ public class ActionCacheChecker { if (!inputsKnown) { Preconditions.checkState(action.discoversInputs()); entry = getCacheEntry(action); - updateActionInputs(action, entry); + boolean ranSuccessfully = updateActionInputs(action, entry, resolver); + // If during update of inputs skyframe was missing some dependencies (for example, + // ContainingPackageLookupValue inside of ArtifactFactory.resolveSourceArtifact), we need to + // wait for those dependencies to be resolved. So next time when we will call corresponding + // ActionExecutionFunction all those dependencies will have been resolved and we can continue + // action execution process. + if (!ranSuccessfully) { + return Token.NEED_TO_RERUN; + } } if (mustExecute(action, entry, handler, metadataHandler)) { return new Token(getKeyString(action)); @@ -220,9 +228,10 @@ public class ActionCacheChecker { actionCache.put(key, entry); } - protected void updateActionInputs(Action action, ActionCache.Entry entry) { + protected boolean updateActionInputs(Action action, ActionCache.Entry entry, + PackageRootResolver resolver) { if (entry == null || entry.isCorrupted()) { - return; + return true; } List<PathFragment> outputs = new ArrayList<>(); @@ -238,7 +247,7 @@ public class ActionCacheChecker { inputs.add(execPath); } } - action.updateInputsFromCache(artifactResolver, inputs); + return action.updateInputsFromCache(artifactResolver, resolver, inputs); } /** @@ -332,6 +341,7 @@ public class ActionCacheChecker { /** Wrapper for all context needed by the ActionCacheChecker to handle a single action. */ public static final class Token { + public static final Token NEED_TO_RERUN = new Token("need to rerun"); private final String cacheKey; private Token(String cacheKey) { diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java index 99a53cbc62..72e29c9232 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactFactory.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.packages.PackageIdentifier; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -248,6 +249,44 @@ public class ArtifactFactory implements ArtifactResolver, ArtifactSerializer, Ar return null; // not a path that we can find... } + @Override + public synchronized Map<PathFragment, Artifact> resolveSourceArtifacts( + Iterable<PathFragment> execPaths, PackageRootResolver resolver) { + Map<PathFragment, Artifact> result = new HashMap<>(); + ArrayList<PathFragment> unresolvedPaths = new ArrayList<>(); + + for (PathFragment execPath : execPaths) { + PathFragment execPathNormalized = execPath.normalize(); + // First try a quick map lookup to see if the artifact already exists. + Artifact a = pathToSourceArtifact.get(execPathNormalized); + if (a != null) { + result.put(execPath, a); + } else if (findDerivedRoot(execRoot.getRelative(execPathNormalized)) != null) { + // Don't create an artifact if it's derived. + result.put(execPath, null); + } else { + // Remember this path, maybe we can resolve it with the help of PackageRootResolver. + unresolvedPaths.add(execPath); + } + } + Map<PathFragment, Root> sourceRoots = resolver.findPackageRoots(unresolvedPaths); + // We are missing some dependencies. We need to rerun this method later. + if (sourceRoots == null) { + return null; + } + for (PathFragment path : unresolvedPaths) { + Root sourceRoot = sourceRoots.get(path); + if (sourceRoot != null) { + // We have found corresponding source root, so we should create a new source artifact. + result.put(path, getSourceArtifact(path.normalize(), sourceRoot, ArtifactOwner.NULL_OWNER)); + } else { + // Not a path that we can find... + result.put(path, null); + } + } + return result; + } + /** * Finds the derived root for a full path by comparing against the known * derived artifact roots. diff --git a/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java index b74c17b0f9..baebfec0f5 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactResolver.java @@ -16,6 +16,10 @@ package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.vfs.PathFragment; +import java.util.Map; + +import javax.annotation.Nullable; + /** * An interface for resolving artifact names to {@link Artifact} objects. Should only be used * in the internal machinery of Blaze: rule implementations are not allowed to do this. @@ -53,4 +57,23 @@ public interface ArtifactResolver { * the root can not be determined and the artifact did not exist before. */ Artifact resolveSourceArtifact(PathFragment execPath); + + /** + * Resolves source Artifacts given execRoot-relative paths. + * + * <p>Never creates or returns derived artifacts, only source artifacts. + * + * <p>Note: this method should only be used when the roots are unknowable, such as from the + * post-compile .d or manifest scanning methods. + * + * @param execPaths list of exec paths of the artifacts to resolve + * @param resolver object that helps to resolve package root of given paths + * @return map which contains list of execPaths and corresponding Artifacts. Map can contain + * existing or new source Artifacts for the given execPaths. The artifact is null if the + * root cannot be determined and the artifact did not exist before. Return null if any + * dependencies are missing. + */ + @Nullable + Map<PathFragment, Artifact> resolveSourceArtifacts(Iterable<PathFragment> execPaths, + PackageRootResolver resolver); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 550eecb48d..4d145826f9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -32,6 +32,7 @@ import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander; import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.extra.CppCompileInfo; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; @@ -827,21 +828,34 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable } @Override - public void updateInputsFromCache( - ArtifactResolver artifactResolver, Collection<PathFragment> inputPaths) { + public boolean updateInputsFromCache( + ArtifactResolver artifactResolver, PackageRootResolver resolver, + Collection<PathFragment> inputPaths) { // Note that this method may trigger a violation of the desirable invariant that getInputs() // is a superset of getMandatoryInputs(). See bug about an "action not in canonical form" // error message and the integration test test_crosstool_change_and_failure(). - Map<PathFragment, Artifact> allowedDerivedInputsMap = getAllowedDerivedInputsMap(); List<Artifact> inputs = new ArrayList<>(); + List<PathFragment> unresolvedPaths = new ArrayList<>(); for (PathFragment execPath : inputPaths) { - // The artifact may be a derived artifact, and if it has been created already, then we still - // want to keep it to preserve incrementality. Artifact artifact = allowedDerivedInputsMap.get(execPath); - if (artifact == null) { - artifact = artifactResolver.resolveSourceArtifact(execPath); + if (artifact != null) { + inputs.add(artifact); + } else { + // Remember this execPath, we will try to resolve it as a source artifact. + unresolvedPaths.add(execPath); } + } + + Map<PathFragment, Artifact> resolvedArtifacts = + artifactResolver.resolveSourceArtifacts(unresolvedPaths, resolver); + if (resolvedArtifacts == null) { + // We are missing some dependencies. We need to rerun this update later. + return false; + } + + for (PathFragment execPath : unresolvedPaths) { + Artifact artifact = resolvedArtifacts.get(execPath); // If PathFragment cannot be resolved into the artifact - ignore it. This could happen if // rule definition has changed and action no longer depends on, e.g., additional source file // in the separate package and that package is no longer referenced anywhere else. @@ -855,6 +869,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable synchronized (this) { setInputs(inputs); } + return true; } private Map<PathFragment, Artifact> getAllowedDerivedInputsMap() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java index 250235d2c8..9be7dc3464 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.ArtifactResolver; import com.google.devtools.build.lib.actions.DelegateSpawn; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; @@ -152,15 +153,20 @@ public final class ExtraAction extends SpawnAction { } @Override - public void updateInputsFromCache(ArtifactResolver artifactResolver, - Collection<PathFragment> inputPaths) { + public boolean updateInputsFromCache(ArtifactResolver artifactResolver, + PackageRootResolver resolver, Collection<PathFragment> inputPaths) { // We update the inputs directly from the shadowed action. Set<PathFragment> extraActionPathFragments = ImmutableSet.copyOf(Artifact.asPathFragments(extraActionInputs)); - shadowedAction.updateInputsFromCache(artifactResolver, + boolean noMissingDependencies = shadowedAction.updateInputsFromCache(artifactResolver, resolver, Collections2.filter(inputPaths, Predicates.in(extraActionPathFragments))); + if (!noMissingDependencies) { + // This update needs to be rerun. + return false; + } Preconditions.checkState(shadowedAction.inputsKnown(), "%s %s", this, shadowedAction); updateInputs(shadowedAction.getInputs()); + return true; } /** diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 14208602d4..ba24480456 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -26,12 +26,16 @@ import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionExcep import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.MissingInputFileException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; +import com.google.devtools.build.lib.actions.PackageRootResolver; +import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.PackageIdentifier; import com.google.devtools.build.lib.syntax.Label; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyKey; @@ -115,6 +119,49 @@ public class ActionExecutionFunction implements SkyFunction { return result; } + + /** + * Skyframe implementation of {@link PackageRootResolver}. Should be used only from SkyFunctions, + * because it uses SkyFunction.Environment for evaluation of ContainingPackageLookupValue. + */ + private static class PackageRootResolverWithEnvironment implements PackageRootResolver { + private final Environment env; + + public PackageRootResolverWithEnvironment(Environment env) { + this.env = env; + } + + @Override + public Map<PathFragment, Root> findPackageRoots(Iterable<PathFragment> execPaths) { + Map<PathFragment, SkyKey> depKeys = new HashMap<>(); + // Create SkyKeys list based on execPaths. + for (PathFragment path : execPaths) { + depKeys.put(path, + ContainingPackageLookupValue.key(PackageIdentifier.createInDefaultRepo(path))); + } + Map<SkyKey, SkyValue> values = env.getValues(depKeys.values()); + if (env.valuesMissing()) { + // Some values are not computed yet. + return null; + } + Map<PathFragment, Root> result = new HashMap<>(); + for (PathFragment path : execPaths) { + // TODO(bazel-team): Add check for errors here, when loading phase will be removed. + // For now all possible errors that ContainingPackageLookupFunction can generate + // are caught in previous phases. + ContainingPackageLookupValue value = + (ContainingPackageLookupValue) values.get(depKeys.get(path)); + if (value.hasContainingPackage()) { + // We have found corresponding root for current execPath. + result.put(path, Root.asSourceRoot(value.getContainingPackageRoot())); + } else { + // We haven't found corresponding root for current execPath. + result.put(path, null); + } + } + return result; + } + } private ActionExecutionValue checkCacheAndExecuteIfNeeded( Action action, @@ -146,7 +193,11 @@ public class ActionExecutionFunction implements SkyFunction { tsgm); metadataHandler = skyframeActionExecutor.constructMetadataHandler(fileAndMetadataCache); - token = skyframeActionExecutor.checkActionCache(action, metadataHandler, actionStartTime); + token = skyframeActionExecutor.checkActionCache(action, metadataHandler, + new PackageRootResolverWithEnvironment(env), actionStartTime); + if (token == Token.NEED_TO_RERUN) { + return null; + } } if (token == null && inputArtifactData != null) { // We got a hit from the action cache -- no need to execute. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index bd591e1b11..e7f626a1ea 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -46,6 +46,7 @@ import com.google.devtools.build.lib.actions.MapBasedActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; +import com.google.devtools.build.lib.actions.PackageRootResolver; import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.actions.TargetOutOfDateException; @@ -550,10 +551,11 @@ public final class SkyframeActionExecutor { * if the action is up to date, and non-null if it needs to be executed, in which case that token * should be provided to the ActionCacheChecker after execution. */ - Token checkActionCache(Action action, MetadataHandler metadataHandler, long actionStartTime) { + Token checkActionCache(Action action, MetadataHandler metadataHandler, + PackageRootResolver resolver, long actionStartTime) { profiler.startTask(ProfilerTask.ACTION_CHECK, action); Token token = actionCacheChecker.getTokenIfNeedToExecute( - action, explain ? reporter : null, metadataHandler); + action, explain ? reporter : null, metadataHandler, resolver); profiler.completeTask(ProfilerTask.ACTION_CHECK); if (token == null) { boolean eventPosted = false; |