Allow globs to be used outside of the skyframe package.
--
MOS_MIGRATED_REVID=93647914
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
index d6d08fa..48714f2 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java
@@ -14,9 +14,14 @@
package com.google.devtools.build.lib.actions;
+import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.Artifact.MiddlemanExpander;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
import com.google.devtools.build.lib.util.io.FileOutErr;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunction.Environment;
+
+import javax.annotation.Nullable;
/**
* A class that groups services in the scope of the action. Like the FileOutErr object.
@@ -28,14 +33,38 @@
private final MetadataHandler metadataHandler;
private final FileOutErr fileOutErr;
private final MiddlemanExpander middlemanExpander;
+ @Nullable
+ private final Environment env;
- public ActionExecutionContext(Executor executor, ActionInputFileCache actionInputFileCache,
- MetadataHandler metadataHandler, FileOutErr fileOutErr, MiddlemanExpander middlemanExpander) {
+ private ActionExecutionContext(Executor executor, ActionInputFileCache actionInputFileCache,
+ MetadataHandler metadataHandler, FileOutErr fileOutErr,
+ @Nullable MiddlemanExpander middlemanExpander,
+ @Nullable SkyFunction.Environment env) {
this.actionInputFileCache = actionInputFileCache;
this.metadataHandler = metadataHandler;
this.fileOutErr = fileOutErr;
this.executor = executor;
this.middlemanExpander = middlemanExpander;
+ this.env = env;
+ }
+
+ public ActionExecutionContext(Executor executor, ActionInputFileCache actionInputFileCache,
+ MetadataHandler metadataHandler, FileOutErr fileOutErr, MiddlemanExpander middlemanExpander) {
+ this(executor, actionInputFileCache, metadataHandler, fileOutErr, middlemanExpander, null);
+ }
+
+ public static ActionExecutionContext normal(Executor executor,
+ ActionInputFileCache actionInputFileCache, MetadataHandler metadataHandler,
+ FileOutErr fileOutErr, MiddlemanExpander middlemanExpander) {
+ return new ActionExecutionContext(executor, actionInputFileCache, metadataHandler, fileOutErr,
+ middlemanExpander, null);
+ }
+
+ public static ActionExecutionContext forInputDiscovery(Executor executor,
+ ActionInputFileCache actionInputFileCache, MetadataHandler metadataHandler,
+ FileOutErr fileOutErr, Environment env) {
+ return new ActionExecutionContext(executor, actionInputFileCache, metadataHandler, fileOutErr,
+ null, env);
}
public ActionInputFileCache getActionInputFileCache() {
@@ -63,11 +92,18 @@
}
/**
+ * Provides a mechanism for the action to request values from Skyframe while it discovers inputs.
+ */
+ public Environment getEnvironmentForDiscoveringInputs() {
+ return Preconditions.checkNotNull(env);
+ }
+
+ /**
* Allows us to create a new context that overrides the FileOutErr with another one. This is
* useful for muting the output for example.
*/
public ActionExecutionContext withFileOutErr(FileOutErr fileOutErr) {
return new ActionExecutionContext(executor, actionInputFileCache, metadataHandler, fileOutErr,
- middlemanExpander);
+ middlemanExpander, env);
}
}
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 d0ac469..770899e 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
@@ -296,16 +296,19 @@
// This may be recreated if we discover inputs.
PerActionFileCache perActionFileCache = new PerActionFileCache(state.inputArtifactData);
- ActionExecutionContext actionExecutionContext =
- skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
- metadataHandler, state.expandedMiddlemen);
+ ActionExecutionContext actionExecutionContext = null;
boolean inputsDiscoveredDuringActionExecution = false;
Map<Artifact, FileArtifactValue> metadataFoundDuringActionExecution = null;
try {
if (action.discoversInputs()) {
if (!state.hasDiscoveredInputs()) {
- state.discoveredInputs =
- skyframeActionExecutor.discoverInputs(action, actionExecutionContext);
+ try {
+ state.discoveredInputs = skyframeActionExecutor.discoverInputs(action,
+ perActionFileCache, metadataHandler, env);
+ } catch (MissingDepException e) {
+ Preconditions.checkState(env.valuesMissing(), action);
+ return null;
+ }
if (state.discoveredInputs == null) {
// Action had nothing to tell us about discovered inputs before execution. We'll have to
// add them afterwards.
@@ -325,20 +328,22 @@
perActionFileCache = new PerActionFileCache(state.inputArtifactData);
metadataHandler =
new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm);
- actionExecutionContext =
- skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
- metadataHandler, state.expandedMiddlemen);
}
}
+ actionExecutionContext =
+ skyframeActionExecutor.constructActionExecutionContext(perActionFileCache,
+ metadataHandler, state.expandedMiddlemen);
if (!state.hasExecutedAction()) {
state.value = skyframeActionExecutor.executeAction(action,
metadataHandler, actionStartTime, actionExecutionContext);
}
} finally {
- try {
- actionExecutionContext.getFileOutErr().close();
- } catch (IOException e) {
- // Nothing we can do here.
+ if (actionExecutionContext != null) {
+ try {
+ actionExecutionContext.getFileOutErr().close();
+ } catch (IOException e) {
+ // Nothing we can do here.
+ }
}
if (inputsDiscoveredDuringActionExecution) {
metadataFoundDuringActionExecution =
@@ -524,6 +529,12 @@
}
/**
+ * Exception to be thrown if an action is missing Skyframe dependencies that it finds are missing
+ * during execution/input discovery.
+ */
+ public static class MissingDepException extends RuntimeException {}
+
+ /**
* Should be called once execution is over, and the intra-build cache of in-progress computations
* should be discarded. If the cache is non-empty (due to an interrupted/failed build), failure to
* call complete() can both cause a memory leak and incorrect results on the subsequent build.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
index 16516b5..0383534 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
@@ -29,7 +29,7 @@
public abstract boolean hasContainingPackage();
/** If there is a containing package, returns its name. */
- abstract PackageIdentifier getContainingPackageName();
+ public abstract PackageIdentifier getContainingPackageName();
/** If there is a containing package, returns its package root */
public abstract Path getContainingPackageRoot();
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
index 030b50b..3d06d7c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
@@ -49,7 +50,8 @@
*
* <p>All subclasses must implement {@link #equals} and {@link #hashCode} properly.
*/
-abstract class FileStateValue implements SkyValue {
+@VisibleForTesting
+public abstract class FileStateValue implements SkyValue {
static final FileStateValue DIRECTORY_FILE_STATE_NODE = DirectoryFileStateValue.INSTANCE;
static final FileStateValue NONEXISTENT_FILE_STATE_NODE = NonexistentFileStateValue.INSTANCE;
@@ -92,8 +94,9 @@
+ "neither a file nor directory nor symlink.");
}
+ @VisibleForTesting
@ThreadSafe
- static SkyKey key(RootedPath rootedPath) {
+ public static SkyKey key(RootedPath rootedPath) {
return new SkyKey(SkyFunctions.FILE_STATE, rootedPath);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java
index 6de0fbd..c93370c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobValue.java
@@ -25,12 +25,10 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
-/**
- * A value corresponding to a glob.
- */
+/** A value corresponding to a glob. */
@Immutable
@ThreadSafe
-final class GlobValue implements SkyValue {
+public final class GlobValue implements SkyValue {
static final GlobValue EMPTY = new GlobValue(
NestedSetBuilder.<PathFragment>emptySet(Order.STABLE_ORDER));
@@ -44,7 +42,7 @@
/**
* Returns glob matches.
*/
- NestedSet<PathFragment> getMatches() {
+ public NestedSet<PathFragment> getMatches() {
return matches;
}
@@ -75,7 +73,8 @@
* @throws InvalidGlobPatternException if the pattern is not valid.
*/
@ThreadSafe
- static SkyKey key(PackageIdentifier packageId, String pattern, boolean excludeDirs)
+ public static SkyKey key(PackageIdentifier packageId, String pattern, boolean excludeDirs,
+ PathFragment subdir)
throws InvalidGlobPatternException {
if (pattern.indexOf('?') != -1) {
throw new InvalidGlobPatternException(pattern, "wildcard ? forbidden");
@@ -86,7 +85,7 @@
throw new InvalidGlobPatternException(pattern, error);
}
- return internalKey(packageId, PathFragment.EMPTY_FRAGMENT, pattern, excludeDirs);
+ return internalKey(packageId, subdir, pattern, excludeDirs);
}
/**
@@ -116,7 +115,7 @@
* An exception that indicates that a glob pattern is syntactically invalid.
*/
@ThreadSafe
- static final class InvalidGlobPatternException extends Exception {
+ public static final class InvalidGlobPatternException extends Exception {
private final String pattern;
InvalidGlobPatternException(String pattern, String error) {
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 7f2f224..8b354cd 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
@@ -293,7 +293,7 @@
boolean excludeDirs = globPattern.getSecond();
SkyKey globSkyKey;
try {
- globSkyKey = GlobValue.key(packageId, pattern, excludeDirs);
+ globSkyKey = GlobValue.key(packageId, pattern, excludeDirs, PathFragment.EMPTY_FRAGMENT);
} catch (InvalidGlobPatternException e) {
// Globs that make it to pkg.getGlobPatterns() should already be filtered for errors.
throw new IllegalStateException(e);
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 4000d0b..9b986c1 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
@@ -65,6 +65,7 @@
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Symlinks;
+import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.protobuf.ByteString;
import java.io.IOException;
@@ -517,8 +518,16 @@
* <p>This method is just a wrapper around {@link Action#discoverInputs} that properly processes
* any ActionExecutionException thrown before rethrowing it to the caller.
*/
- Collection<Artifact> discoverInputs(Action action, ActionExecutionContext actionExecutionContext)
+ Collection<Artifact> discoverInputs(Action action, PerActionFileCache graphFileCache,
+ MetadataHandler metadataHandler, Environment env)
throws ActionExecutionException, InterruptedException {
+ ActionExecutionContext actionExecutionContext =
+ ActionExecutionContext.forInputDiscovery(
+ executorEngine,
+ new DelegatingPairFileCache(graphFileCache, perBuildFileCache),
+ metadataHandler,
+ actionLogBufferPathGenerator.generate(),
+ env);
try {
return action.discoverInputs(actionExecutionContext);
} catch (ActionExecutionException e) {
diff --git a/src/main/java/com/google/devtools/build/skyframe/ValueOrExceptionUtils.java b/src/main/java/com/google/devtools/build/skyframe/ValueOrExceptionUtils.java
index e66f4fa..32bc254 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ValueOrExceptionUtils.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ValueOrExceptionUtils.java
@@ -13,10 +13,13 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
+import com.google.common.annotations.VisibleForTesting;
+
import javax.annotation.Nullable;
/** Utilities for producing and consuming ValueOrException(2|3|4)? instances. */
-class ValueOrExceptionUtils {
+@VisibleForTesting
+public class ValueOrExceptionUtils {
/** The bottom exception type. */
class BottomException extends Exception {
diff --git a/src/main/java/com/google/devtools/build/skyframe/ValueOrUntypedException.java b/src/main/java/com/google/devtools/build/skyframe/ValueOrUntypedException.java
index c7ea7d4..0b2e680 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ValueOrUntypedException.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ValueOrUntypedException.java
@@ -13,6 +13,8 @@
// limitations under the License.
package com.google.devtools.build.skyframe;
+import com.google.common.annotations.VisibleForTesting;
+
import javax.annotation.Nullable;
/**
@@ -22,7 +24,8 @@
* {@link ValueOrExceptionUtils}. It's an abstract class (as opposed to an interface) to avoid
* exposing the methods outside the package.
*/
-abstract class ValueOrUntypedException {
+@VisibleForTesting
+public abstract class ValueOrUntypedException {
/** Returns the stored value, if there was one. */
@Nullable
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
index 2f601d4..565601b 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java
@@ -37,20 +37,33 @@
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
+import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.exec.SingleBuildFileCache;
import com.google.devtools.build.lib.syntax.Label;
import com.google.devtools.build.lib.util.FileType;
+import com.google.devtools.build.lib.util.ResourceUsage;
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.skyframe.AbstractSkyFunctionEnvironment;
+import com.google.devtools.build.skyframe.BuildDriver;
+import com.google.devtools.build.skyframe.ErrorInfo;
+import com.google.devtools.build.skyframe.EvaluationResult;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyValue;
+import com.google.devtools.build.skyframe.ValueOrExceptionUtils;
+import com.google.devtools.build.skyframe.ValueOrUntypedException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
@@ -80,6 +93,74 @@
: ActionInputHelper.actionGraphMiddlemanExpander(actionGraph));
}
+ public static ActionExecutionContext createContextForInputDiscovery(Executor executor,
+ FileOutErr fileOutErr, Path execRoot, MetadataHandler metadataHandler,
+ BuildDriver buildDriver) {
+ return ActionExecutionContext.forInputDiscovery(
+ executor,
+ new SingleBuildFileCache(execRoot.getPathString(), execRoot.getFileSystem()),
+ metadataHandler, fileOutErr,
+ new BlockingSkyFunctionEnvironment(buildDriver,
+ executor == null ? null : executor.getEventHandler()));
+
+ }
+
+
+ /**
+ * {@link SkyFunction.Environment} that internally makes a full Skyframe evaluate call for the
+ * requested keys, blocking until the values are ready.
+ */
+ private static class BlockingSkyFunctionEnvironment extends AbstractSkyFunctionEnvironment {
+ private final BuildDriver driver;
+ private final EventHandler eventHandler;
+
+ private BlockingSkyFunctionEnvironment(BuildDriver driver, EventHandler eventHandler) {
+ this.driver = driver;
+ this.eventHandler = eventHandler;
+ }
+
+ @Override
+ protected Map<SkyKey, ValueOrUntypedException> getValueOrUntypedExceptions(
+ Iterable<SkyKey> depKeys) {
+ EvaluationResult<SkyValue> evaluationResult;
+ Map<SkyKey, ValueOrUntypedException> result = new HashMap<>();
+ try {
+ evaluationResult = driver.evaluate(depKeys, /*keepGoing=*/false,
+ ResourceUsage.getAvailableProcessors(), eventHandler);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ for (SkyKey key : depKeys) {
+ result.put(key, ValueOrExceptionUtils.ofNull());
+ }
+ return result;
+ }
+ for (SkyKey key : depKeys) {
+ SkyValue value = evaluationResult.get(key);
+ if (value != null) {
+ result.put(key, ValueOrExceptionUtils.ofValue(value));
+ continue;
+ }
+ ErrorInfo errorInfo = evaluationResult.getError(key);
+ if (errorInfo == null || errorInfo.getException() == null) {
+ result.put(key, ValueOrExceptionUtils.ofNull());
+ continue;
+ }
+ result.put(key, ValueOrExceptionUtils.ofExn(errorInfo.getException()));
+ }
+ return result;
+ }
+
+ @Override
+ public EventHandler getListener() {
+ return null;
+ }
+
+ @Override
+ public boolean inErrorBubblingForTesting() {
+ return false;
+ }
+ }
+
/**
* A dummy ActionOwner implementation for use in tests.
*/
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java b/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
index 08d3158..eff9ffb 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/util/FsApparatus.java
@@ -24,6 +24,7 @@
import junit.framework.AssertionFailedError;
import java.io.File;
+import java.io.FileNotFoundException;
import java.io.IOException;
/**
@@ -89,10 +90,8 @@
}
/**
- * Initializes this apparatus (if it hasn't been initialized yet), and creates
- * a scratch file in the scratch filesystem with the given {@code pathName}
- * with {@code lines} being its content. The method returns a Path instance
- * for the scratch file.
+ * Creates a scratch file in the scratch filesystem with the given {@code pathName} with
+ * {@code lines} being its content. The method returns a Path instance for the scratch file.
*/
public Path file(String pathName, String... lines) throws IOException {
Path file = path(pathName);
@@ -110,6 +109,18 @@
}
/**
+ * Creates or recreates a scratch file just like {@link #file} but tolerating an existing file.
+ */
+ public Path overwriteFile(String pathName, String... lines) throws IOException {
+ try {
+ path(pathName).delete();
+ } catch (FileNotFoundException e) {
+ // Ignored.
+ }
+ return file(pathName, lines);
+ }
+
+ /**
* Initializes this apparatus (if it hasn't been initialized yet), and creates
* a directory in the scratch filesystem, with the given {@code pathName}.
* Creates parent directories as necessary.
