diff options
4 files changed, 23 insertions, 13 deletions
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 63ddb692d6..46d81b475c 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 @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.actions.FilesetOutputSymlink; 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.analysis.BlazeDirectories; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LabelCause; import com.google.devtools.build.lib.clock.BlazeClock; @@ -82,12 +83,15 @@ import javax.annotation.Nullable; */ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver { private final SkyframeActionExecutor skyframeActionExecutor; + private final BlazeDirectories directories; private final AtomicReference<TimestampGranularityMonitor> tsgm; private ConcurrentMap<Action, ContinuationState> stateMap; public ActionExecutionFunction(SkyframeActionExecutor skyframeActionExecutor, + BlazeDirectories directories, AtomicReference<TimestampGranularityMonitor> tsgm) { this.skyframeActionExecutor = skyframeActionExecutor; + this.directories = directories; this.tsgm = tsgm; stateMap = Maps.newConcurrentMap(); } @@ -213,6 +217,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver new ActionFileSystem( skyframeActionExecutor.getExecutorFileSystem(), skyframeActionExecutor.getExecRoot(), + directories.getRelativeOutputPath(), skyframeActionExecutor.getSourceRoots(), checkedInputs.first, optionalInputs, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java index 757c310697..726042588b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java @@ -73,6 +73,7 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat private final FileSystem delegate; private final PathFragment execRootFragment; + private final PathFragment outputPathFragment; private final ImmutableList<PathFragment> sourceRoots; private final ActionInputMap inputArtifactData; @@ -97,6 +98,7 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat ActionFileSystem( FileSystem delegate, Path execRoot, + String relativeOutputPath, ImmutableList<Root> sourceRoots, ActionInputMap inputArtifactData, Iterable<Artifact> allowedInputs, @@ -106,6 +108,7 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat this.delegate = delegate; this.execRootFragment = execRoot.asFragment(); + this.outputPathFragment = execRootFragment.getRelative(relativeOutputPath); this.sourceRoots = sourceRoots .stream() @@ -297,7 +300,7 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat if (metadata instanceof SourceFileArtifactValue) { return resolveSourcePath((SourceFileArtifactValue) metadata).getxattr(name); } - return delegate.getPath(path.asFragment()).getxattr(name); + return getSourcePath(path.asFragment()).getxattr(name); } @Override @@ -371,6 +374,8 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat linkPath, targetFragment, linkPath + " is not an output.")); } if (sourceRootIndex >= 0) { + Preconditions.checkState(!targetExecPath.startsWith(outputPathFragment), "Target exec path " + + "%s does not start with output path fragment %s", targetExecPath, outputPathFragment); outputHolder.set( new SourceFileArtifactValue( targetExecPath, sourceRootIndex, inputMetadata.getDigest(), inputMetadata.getSize()), @@ -430,7 +435,7 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat !(metadata instanceof RemoteFileArtifactValue), "getInputStream called for remote file: %s", path); - return delegate.getPath(path.asFragment()).getInputStream(); + return getSourcePath(path.asFragment()).getInputStream(); } @Override @@ -520,13 +525,14 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat } private boolean isOutput(Path path) { - // TODO(felly): This method should instead just refer to potential output paths, which are - // anything under the output tree. - PathFragment fragment = path.asFragment(); - if (!fragment.startsWith(execRootFragment)) { - return false; + return path.asFragment().startsWith(outputPathFragment); + } + + private Path getSourcePath(PathFragment path) throws IOException { + if (path.startsWith(outputPathFragment)) { + throw new IOException("ActionFS cannot delegate to underlying output path for " + path); } - return outputs.getIfPresent(fragment.relativeTo(execRootFragment)) != null; + return delegate.getPath(path); } /** @@ -555,9 +561,8 @@ final class ActionFileSystem extends AbstractFileSystemWithCustomStat } /** NB: resolves to the underlying filesytem instead of this one. */ - private Path resolveSourcePath(SourceFileArtifactValue metadata) { - return delegate - .getPath(sourceRoots.get(metadata.getSourceRootIndex())) + private Path resolveSourcePath(SourceFileArtifactValue metadata) throws IOException { + return getSourcePath(sourceRoots.get(metadata.getSourceRootIndex())) .getRelative(metadata.getExecPath()); } 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 4d413a96fb..633b8b873b 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 @@ -518,7 +518,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { SkyFunctions.COVERAGE_REPORT, new CoverageReportFunction(actionKeyContext, removeActionsAfterEvaluation)); ActionExecutionFunction actionExecutionFunction = - new ActionExecutionFunction(skyframeActionExecutor, tsgm); + new ActionExecutionFunction(skyframeActionExecutor, directories, tsgm); map.put(SkyFunctions.ACTION_EXECUTION, actionExecutionFunction); this.actionExecutionFunction = actionExecutionFunction; map.put(SkyFunctions.RECURSIVE_FILESYSTEM_TRAVERSAL, 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 f93c53ae5f..2b87fb9232 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 @@ -211,7 +211,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { .put(Artifact.ARTIFACT, new ArtifactFunction()) .put( SkyFunctions.ACTION_EXECUTION, - new ActionExecutionFunction(skyframeActionExecutor, tsgmRef)) + new ActionExecutionFunction(skyframeActionExecutor, directories, tsgmRef)) .put( SkyFunctions.PACKAGE, new PackageFunction(null, null, null, null, null, null, null)) |