Allow for Path and Root resolution when extracting action metadata. RELNOTES: None PiperOrigin-RevId: 199572597
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 64e9b29..fa22da4 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
@@ -45,6 +45,7 @@ import com.google.devtools.build.lib.skyframe.InputArtifactData.MutableInputArtifactData; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.SkyFunction; @@ -385,7 +386,7 @@ } // This may be recreated if we discover inputs. ActionMetadataHandler metadataHandler = new ActionMetadataHandler(state.inputArtifactData, - action.getOutputs(), tsgm.get()); + action.getOutputs(), tsgm.get(), pathResolver(state.actionFileSystem)); long actionStartTime = BlazeClock.nanoTime(); // We only need to check the action cache if we haven't done it on a previous run. if (!state.hasCheckedActionCache()) { @@ -462,7 +463,8 @@ new PerActionFileCache(state.inputArtifactData, /*missingArtifactsAllowed=*/ false); } metadataHandler = - new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get()); + new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get(), + pathResolver(state.actionFileSystem)); // Set the MetadataHandler to accept output information. metadataHandler.discardOutputMetadata(); } @@ -535,7 +537,8 @@ // markOmitted is only called for remote execution, and this code only gets executed for // local execution. metadataHandler = - new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get()); + new ActionMetadataHandler(state.inputArtifactData, action.getOutputs(), tsgm.get(), + pathResolver(state.actionFileSystem)); } } Preconditions.checkState(!env.valuesMissing(), action); @@ -544,6 +547,23 @@ return state.value; } + private ArtifactPathResolver pathResolver(ActionFileSystem actionFileSystem) { + if (actionFileSystem != null) { + return new ArtifactPathResolver() { + @Override + public Path toPath(Artifact artifact) { + return actionFileSystem.getPath(artifact.getPath().getPathString()); + } + + @Override + public Root transformRoot(Root root) { + return Root.toFileSystem(root, actionFileSystem); + } + }; + } + return ArtifactPathResolver.IDENTITY; + } + private static final Function<Artifact, SkyKey> TO_NONMANDATORY_SKYKEY = new Function<Artifact, SkyKey>() { @Nullable
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 345c4c8..cd99db9 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
@@ -202,8 +202,6 @@ @Override protected long getFileSize(Path path, boolean followSymlinks) throws IOException { - Preconditions.checkArgument( - followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path); return getMetadataOrThrowFileNotFound(path).getSize(); } @@ -214,8 +212,6 @@ @Override protected long getLastModifiedTime(Path path, boolean followSymlinks) throws IOException { - Preconditions.checkArgument( - followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path); return getMetadataOrThrowFileNotFound(path).getModifiedTime(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index f3f7e09..b68930e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java
@@ -120,6 +120,7 @@ * Use {@link #getTimestampGranularityMonitor(Artifact)} to fetch this member. */ private final TimestampGranularityMonitor tsgm; + private final ArtifactPathResolver artifactPathResolver; /** * Whether the action is being executed or not; this flag is set to true in @@ -131,10 +132,12 @@ public ActionMetadataHandler( InputArtifactData inputArtifactData, Iterable<Artifact> outputs, - TimestampGranularityMonitor tsgm) { + TimestampGranularityMonitor tsgm, + ArtifactPathResolver artifactPathResolver) { this.inputArtifactData = Preconditions.checkNotNull(inputArtifactData); this.outputs = ImmutableSet.copyOf(outputs); this.tsgm = tsgm; + this.artifactPathResolver = artifactPathResolver; } /** @@ -390,7 +393,8 @@ throws IOException { Preconditions.checkState(artifact.isTreeArtifact(), artifact); - if (!artifact.getPath().isDirectory() || artifact.getPath().isSymbolicLink()) { + if (!artifactPathResolver.toPath(artifact).isDirectory() + || artifactPathResolver.toPath(artifact).isSymbolicLink()) { return TreeArtifactValue.MISSING_TREE_ARTIFACT; } @@ -580,7 +584,7 @@ setPathReadOnlyAndExecutable(artifact); } - FileValue value = fileValueFromArtifact(artifact, statNoFollow, + FileValue value = fileValueFromArtifact(artifact, artifactPathResolver, statNoFollow, getTimestampGranularityMonitor(artifact)); FileValue oldFsValue = outputArtifactData.putIfAbsent(artifact, value); checkInconsistentData(artifact, oldFsValue, value); @@ -591,9 +595,18 @@ static FileValue fileValueFromArtifact(Artifact artifact, @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm) throws IOException { - Path path = artifact.getPath(); + return fileValueFromArtifact(artifact, ArtifactPathResolver.IDENTITY, statNoFollow, tsgm); + } + + private static FileValue fileValueFromArtifact(Artifact artifact, + ArtifactPathResolver artifactPathResolver, + @Nullable FileStatusWithDigest statNoFollow, @Nullable TimestampGranularityMonitor tsgm) + throws IOException { + Path path = artifactPathResolver.toPath(artifact); RootedPath rootedPath = - RootedPath.toRootedPath(artifact.getRoot().getRoot(), artifact.getRootRelativePath()); + RootedPath.toRootedPath( + artifactPathResolver.transformRoot(artifact.getRoot().getRoot()), + artifact.getRootRelativePath()); if (statNoFollow == null) { statNoFollow = FileStatusWithDigestAdapter.adapt(path.statIfFound(Symlinks.NOFOLLOW)); if (statNoFollow == null) { @@ -614,7 +627,8 @@ } RootedPath realRootedPath = RootedPath.toRootedPathMaybeUnderRoot( - realPath, ImmutableList.of(artifact.getRoot().getRoot())); + realPath, + ImmutableList.of(artifactPathResolver.transformRoot(artifact.getRoot().getRoot()))); FileStateValue fileStateValue = FileStateValue.createWithStatNoFollow(rootedPath, statNoFollow, tsgm); // TODO(bazel-team): consider avoiding a 'stat' here when the symlink target hasn't changed @@ -631,7 +645,7 @@ if (injectedFiles.contains(artifact)) { return; } - Path path = artifact.getPath(); + Path path = artifactPathResolver.toPath(artifact); if (path.isFile(Symlinks.NOFOLLOW)) { // i.e. regular files only. // We trust the files created by the execution engine to be non symlinks with expected // chmod() settings already applied. @@ -641,7 +655,7 @@ private void setTreeReadOnlyAndExecutable(SpecialArtifact parent, PathFragment subpath) throws IOException { - Path path = parent.getPath().getRelative(subpath); + Path path = artifactPathResolver.toPath(parent).getRelative(subpath); if (path.isDirectory()) { path.chmod(0555); for (Path child : path.getDirectoryEntries()) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java new file mode 100644 index 0000000..9d0bf76 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactPathResolver.java
@@ -0,0 +1,38 @@ +// Copyright 2018 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. +package com.google.devtools.build.lib.skyframe; + +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.Root; + +/** + * An indirection layer on Path resolution of {@link Artifact} and {@link Root}. + */ +interface ArtifactPathResolver { + Path toPath(Artifact artifact); + Root transformRoot(Root root); + + ArtifactPathResolver IDENTITY = new ArtifactPathResolver() { + @Override + public Path toPath(Artifact artifact) { + return artifact.getPath(); + } + + @Override + public Root transformRoot(Root root) { + return root; + } + }; +}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Root.java b/src/main/java/com/google/devtools/build/lib/vfs/Root.java index c7c7890..454e9b2 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/Root.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/Root.java
@@ -40,6 +40,12 @@ return fileSystem.getAbsoluteRoot(); } + public static Root toFileSystem(Root root, FileSystem fileSystem) { + return root.isAbsolute() + ? new AbsoluteRoot(fileSystem) + : new PathRoot(fileSystem.getPath(root.asPath().asFragment())); + } + /** Returns a path by concatenating the root and the root-relative path. */ public abstract Path getRelative(PathFragment rootRelativePath);
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java b/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java index dd0f611..640f8c4 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java +++ b/src/test/java/com/google/devtools/build/lib/vfs/RootTest.java
@@ -67,6 +67,16 @@ } @Test + public void testFilesystemTransform() throws Exception { + FileSystem fs2 = new InMemoryFileSystem(BlazeClock.instance()); + Root root = Root.fromPath(fs.getPath("/foo")); + Root root2 = Root.toFileSystem(root, fs2); + assertThat(root2.asPath().getFileSystem()).isSameAs(fs2); + assertThat(root2.asPath().asFragment()).isEqualTo(PathFragment.create("/foo")); + assertThat(root.isAbsolute()).isFalse(); + } + + @Test public void testFileSystemAbsoluteRoot() throws Exception { Root root = Root.absoluteRoot(fs); assertThat(root.asPath()).isNull();