diff options
27 files changed, 180 insertions, 926 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index b1dd9fdb81..210db2e347 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -583,6 +583,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/platform", "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/test", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:output_service", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/com/google/devtools/common/options", @@ -1315,6 +1316,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skylarkdebug/module:options", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:output_service", "//src/main/java/com/google/devtools/build/lib/windows", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java b/src/main/java/com/google/devtools/build/lib/actions/ArtifactSkyKey.java index e63b73c5b0..701aacaec4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactSkyKey.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ArtifactSkyKey.java @@ -11,11 +11,10 @@ // 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; +package com.google.devtools.build.lib.actions; import com.google.common.base.Preconditions; import com.google.common.collect.Interner; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; @@ -84,7 +83,7 @@ public class ArtifactSkyKey implements SkyKey { return artifact.equals(thatArtifactSkyKey.artifact); } - Artifact getArtifact() { + public Artifact getArtifact() { return artifact; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/InjectionListener.java b/src/main/java/com/google/devtools/build/lib/actions/InjectionListener.java index bf7350ba39..28d773f8d0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/InjectionListener.java +++ b/src/main/java/com/google/devtools/build/lib/actions/InjectionListener.java @@ -12,9 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.skyframe; +package com.google.devtools.build.lib.actions; -import com.google.devtools.build.lib.actions.ActionInput; import java.io.IOException; /** diff --git a/src/main/java/com/google/devtools/build/lib/actions/MetadataConsumer.java b/src/main/java/com/google/devtools/build/lib/actions/MetadataConsumer.java new file mode 100644 index 0000000000..d737d54f41 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/MetadataConsumer.java @@ -0,0 +1,26 @@ +// 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.actions; + +import java.io.IOException; + +/** + * Consumes metadata for artifacts. + * + * <p>May be called when metadata isn't otherwise propagated at the spawn level. + */ +@FunctionalInterface +public interface MetadataConsumer { + void accept(Artifact artifact, FileArtifactValue value) throws IOException; +} diff --git a/src/main/java/com/google/devtools/build/lib/actions/MissingDepException.java b/src/main/java/com/google/devtools/build/lib/actions/MissingDepException.java new file mode 100644 index 0000000000..17de74f180 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/MissingDepException.java @@ -0,0 +1,20 @@ +// 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.actions; + +/** + * Exception to be thrown if an action is missing Skyframe dependencies that it finds are missing + * during execution/input discovery. + */ +public final class MissingDepException extends RuntimeException {} diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index 78b350b9d7..be502c6024 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -74,12 +74,12 @@ import com.google.devtools.build.lib.skyframe.AspectValue; import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; import com.google.devtools.build.lib.skyframe.Builder; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; -import com.google.devtools.build.lib.skyframe.OutputService; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.ModifiedFileSet; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java index 82b24eca1a..4789333cdf 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java @@ -21,7 +21,7 @@ import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction; import com.google.devtools.build.lib.analysis.actions.SymlinkTreeActionContext; import com.google.devtools.build.lib.profiler.AutoProfiler; -import com.google.devtools.build.lib.skyframe.OutputService; +import com.google.devtools.build.lib.vfs.OutputService; import java.util.logging.Logger; /** diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java index c0097497e2..0aa813874a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java @@ -29,11 +29,11 @@ import com.google.devtools.build.lib.packages.NoSuchThingException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.RuleClassProvider; -import com.google.devtools.build.lib.skyframe.OutputService; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsClassProvider; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java index 34593b6662..cee7cab3e9 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java @@ -37,13 +37,13 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; -import com.google.devtools.build.lib.skyframe.OutputService; import com.google.devtools.build.lib.skyframe.SkyframeBuildView; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.FileSystemUtils; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.OptionsClassProvider; @@ -97,9 +97,7 @@ public final class CommandEnvironment { public Path getFileFromWorkspace(Label label) throws NoSuchThingException, InterruptedException, IOException { Target target = getPackageManager().getTarget(reporter, label); - return (outputService != null) - ? outputService.stageTool(target) - : target.getPackage().getPackageDirectory().getRelative(target.getName()); + return target.getPackage().getPackageDirectory().getRelative(target.getName()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java index 81120765cb..76c73041ec 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionArtifactCycleReporter.java @@ -19,6 +19,7 @@ import com.google.common.base.Predicates; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionLookupData; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.pkgcache.PackageProvider; import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey; 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 6f5044fbbf..b740a490c3 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 @@ -32,8 +32,10 @@ import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.MissingDepException; import com.google.devtools.build.lib.actions.MissingInputFileException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.actions.PackageRootResolver; @@ -48,6 +50,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.rules.cpp.IncludeScannable; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.skyframe.SkyFunction; @@ -210,11 +213,8 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver optionalInputs = ImmutableList.of(); } state.actionFileSystem = - new ActionFileSystem( - skyframeActionExecutor.getExecutorFileSystem(), - skyframeActionExecutor.getExecRoot().asFragment(), + skyframeActionExecutor.createActionFileSystem( directories.getRelativeOutputPath(), - skyframeActionExecutor.getSourceRoots(), checkedInputs.first, optionalInputs, action.getOutputs()); @@ -429,7 +429,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver if (action.discoversInputs()) { if (state.discoveredInputs == null) { try { - state.updateFileSystemContext(env, metadataHandler); + state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler); state.discoveredInputs = skyframeActionExecutor.discoverInputs( action, perActionFileCache, metadataHandler, env, state.actionFileSystem); @@ -495,7 +495,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver filesetMappings.put(actionInput.getExecPath(), filesetValue.getOutputSymlinks()); } - state.updateFileSystemContext(env, metadataHandler); + state.updateFileSystemContext(skyframeActionExecutor, env, metadataHandler); try (ActionExecutionContext actionExecutionContext = skyframeActionExecutor.getContext( perActionFileCache, @@ -550,12 +550,9 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver return state.value; } - private ArtifactPathResolver pathResolver(ActionFileSystem actionFileSystem) { - if (actionFileSystem != null) { - return ArtifactPathResolver.withTransformedFileSystem( - actionFileSystem.getPath(skyframeActionExecutor.getExecRoot().asFragment())); - } - return ArtifactPathResolver.forExecRoot(skyframeActionExecutor.getExecRoot()); + private ArtifactPathResolver pathResolver(@Nullable FileSystem actionFileSystem) { + return ArtifactPathResolver.createPathResolver( + actionFileSystem, skyframeActionExecutor.getExecRoot()); } private static final Function<Artifact, SkyKey> TO_NONMANDATORY_SKYKEY = @@ -774,12 +771,6 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver } /** - * 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. @@ -824,7 +815,7 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver Iterable<Artifact> discoveredInputs = null; Iterable<Artifact> discoveredInputsStage2 = null; ActionExecutionValue value = null; - ActionFileSystem actionFileSystem = null; + FileSystem actionFileSystem = null; boolean hasCollectedInputs() { return allInputs != null; @@ -848,9 +839,12 @@ public class ActionExecutionFunction implements SkyFunction, CompletionReceiver /** Must be called to assign values to the given variables as they change. */ void updateFileSystemContext( - SkyFunction.Environment env, ActionMetadataHandler metadataHandler) { + SkyframeActionExecutor executor, + SkyFunction.Environment env, + ActionMetadataHandler metadataHandler) { if (actionFileSystem != null) { - actionFileSystem.updateContext(env, metadataHandler::injectOutputData); + executor.updateActionFileSystemContext( + actionFileSystem, env, metadataHandler::injectOutputData); } } 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 deleted file mode 100644 index e5430355a6..0000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionFileSystem.java +++ /dev/null @@ -1,687 +0,0 @@ -// 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 static java.nio.charset.StandardCharsets.US_ASCII; - -import com.google.common.base.Preconditions; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Streams; -import com.google.common.hash.Hashing; -import com.google.common.io.BaseEncoding; -import com.google.common.io.ByteStreams; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputMap; -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.FileArtifactValue; -import com.google.devtools.build.lib.actions.FileArtifactValue.InlineFileArtifactValue; -import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; -import com.google.devtools.build.lib.actions.FileArtifactValue.SourceFileArtifactValue; -import com.google.devtools.build.lib.actions.FileStateType; -import com.google.devtools.build.lib.actions.MetadataProvider; -import com.google.devtools.build.lib.profiler.Profiler; -import com.google.devtools.build.lib.profiler.ProfilerTask; -import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.vfs.AbstractFileSystemWithCustomStat; -import com.google.devtools.build.lib.vfs.FileStatus; -import com.google.devtools.build.lib.vfs.FileSystem; -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; -import com.google.protobuf.ByteString; -import java.io.ByteArrayOutputStream; -import java.io.FileNotFoundException; -import java.io.IOException; -import java.io.InputStream; -import java.io.InterruptedIOException; -import java.io.OutputStream; -import java.util.Collection; -import java.util.HashMap; -import javax.annotation.Nullable; - -/** - * File system for actions. - * - * <p>This class is thread-safe except that - * - * <ul> - * <li>{@link updateContext} must be called exclusively of any other methods. - * <li>This class relies on synchronized access to {@link env}. If there are other threads, that - * access {@link env}, they must also used synchronized access. - * </ul> - */ -final class ActionFileSystem extends AbstractFileSystemWithCustomStat - implements MetadataProvider, InjectionListener { - private static final BaseEncoding LOWER_CASE_HEX = BaseEncoding.base16().lowerCase(); - - /** Actual underlying filesystem. */ - private final FileSystem delegate; - - private final PathFragment execRootFragment; - private final PathFragment outputPathFragment; - private final ImmutableList<PathFragment> sourceRoots; - - private final ActionInputMap inputArtifactData; - - /** exec path → artifact and metadata */ - private final HashMap<PathFragment, OptionalInputMetadata> optionalInputs; - - /** exec path → artifact and metadata */ - private final LoadingCache<PathFragment, OutputMetadata> outputs; - - /** Used to lookup metadata for optional inputs. */ - private SkyFunction.Environment env = null; - - /** - * Called whenever there is new metadata for an output. - * - * <p>This is backed by injection into an {@link ActionMetadataHandler} instance so should only be - * called once per artifact. - */ - private MetadataConsumer metadataConsumer = null; - - ActionFileSystem( - FileSystem delegate, - PathFragment execRoot, - String relativeOutputPath, - ImmutableList<Root> sourceRoots, - ActionInputMap inputArtifactData, - Iterable<Artifact> allowedInputs, - Iterable<Artifact> outputArtifacts) { - try (SilentCloseable c = - Profiler.instance().profile(ProfilerTask.ACTION_FS_STAGING, "staging")) { - this.delegate = delegate; - - this.execRootFragment = execRoot; - this.outputPathFragment = execRootFragment.getRelative(relativeOutputPath); - this.sourceRoots = - sourceRoots - .stream() - .map(root -> root.asPath().asFragment()) - .collect(ImmutableList.toImmutableList()); - - validateRoots(); - - this.inputArtifactData = inputArtifactData; - - this.optionalInputs = new HashMap<>(); - for (Artifact input : allowedInputs) { - // Skips staging source artifacts as a performance optimization. We may want to stage them - // if we want stricter enforcement of source sandboxing. - // - // TODO(shahan): there are no currently known cases where metadata is requested for an - // optional source input. If there are any, we may want to stage those. - if (input.isSourceArtifact() || inputArtifactData.getMetadata(input) != null) { - continue; - } - optionalInputs.computeIfAbsent( - input.getExecPath(), unused -> new OptionalInputMetadata(input)); - } - - ImmutableMap<PathFragment, Artifact> outputsMapping = Streams.stream(outputArtifacts) - .collect(ImmutableMap.toImmutableMap(Artifact::getExecPath, a -> a)); - this.outputs = CacheBuilder.newBuilder().build( - CacheLoader.from(path -> new OutputMetadata(outputsMapping.get(path)))); - } - } - - /** - * Must be called prior to access and updated as needed. - * - * <p>These cannot be passed into the constructor because while {@link ActionFileSystem} is - * action-scoped, the environment and metadata consumer change multiple times, at well defined - * points, during the lifetime of an action. - */ - public void updateContext(SkyFunction.Environment env, MetadataConsumer metadataConsumer) { - this.env = env; - this.metadataConsumer = metadataConsumer; - } - - // -------------------- MetadataProvider implementation -------------------- - - @Override - @Nullable - public FileArtifactValue getMetadata(ActionInput actionInput) throws IOException { - return getMetadataChecked(actionInput.getExecPath()); - } - - @Override - @Nullable - public ActionInput getInput(String execPath) { - ActionInput input = inputArtifactData.getInput(execPath); - if (input != null) { - return input; - } - OptionalInputMetadata metadata = - optionalInputs.get(PathFragment.createAlreadyNormalized(execPath)); - return metadata == null ? null : metadata.getArtifact(); - } - - // -------------------- InjectionListener Implementation -------------------- - - @Override - public void onInsert(ActionInput dest, byte[] digest, long size, int backendIndex) - throws IOException { - outputs.getUnchecked(dest.getExecPath()).set( - new RemoteFileArtifactValue(digest, size, backendIndex), - /*notifyConsumer=*/ false); - } - - // -------------------- FileSystem implementation -------------------- - - @Override - public boolean supportsModifications(Path path) { - return isOutput(path); - } - - @Override - public boolean supportsSymbolicLinksNatively(Path path) { - return isOutput(path); - } - - @Override - protected boolean supportsHardLinksNatively(Path path) { - return isOutput(path); - } - - @Override - public boolean isFilePathCaseSensitive() { - return true; - } - - @Override - protected FileStatus stat(Path path, boolean followSymlinks) throws IOException { - FileArtifactValue metadata = getMetadataOrThrowFileNotFound(path); - return new FileStatus() { - @Override - public boolean isFile() { - return metadata.getType() == FileStateType.REGULAR_FILE; - } - - @Override - public boolean isDirectory() { - // TODO(felly): Support directory awareness, and consider disallowing directory artifacts - // eagerly at ActionFS construction. - return false; - } - - @Override - public boolean isSymbolicLink() { - // TODO(felly): We should have minimal support for symlink awareness when looking at - // output --> src and src --> src symlinks. - return false; - } - - @Override - public boolean isSpecialFile() { - return metadata.getType() == FileStateType.SPECIAL_FILE; - } - - @Override - public long getSize() { - return metadata.getSize(); - } - - @Override - public long getLastModifiedTime() { - return metadata.getModifiedTime(); - } - - @Override - public long getLastChangeTime() { - return metadata.getModifiedTime(); - } - - @Override - public long getNodeId() { - throw new UnsupportedOperationException(); - } - }; - } - - /** ActionFileSystem currently doesn't track directories. */ - @Override - public boolean createDirectory(Path path) throws IOException { - return true; - } - - @Override - public void createDirectoryAndParents(Path path) throws IOException {} - - @Override - protected long getFileSize(Path path, boolean followSymlinks) throws IOException { - return stat(path, followSymlinks).getSize(); - } - - @Override - public boolean delete(Path path) throws IOException { - PathFragment execPath = asExecPath(path); - OutputMetadata output = outputs.getIfPresent(execPath); - return output != null && outputs.asMap().remove(execPath, output); - } - - @Override - protected long getLastModifiedTime(Path path, boolean followSymlinks) throws IOException { - return getMetadataOrThrowFileNotFound(path).getModifiedTime(); - } - - @Override - public void setLastModifiedTime(Path path, long newTime) throws IOException { - throw new UnsupportedOperationException(path.getPathString()); - } - - @Override - public byte[] getxattr(Path path, String name) throws IOException { - FileArtifactValue metadata = getMetadataChecked(asExecPath(path)); - if (metadata instanceof RemoteFileArtifactValue) { - RemoteFileArtifactValue remote = (RemoteFileArtifactValue) metadata; - // TODO(b/80244718): inject ActionFileSystem from elsewhere and replace with correct metadata - return ("/CENSORED_BY_LEAKR/" - + remote.getLocationIndex() - + "/" - + LOWER_CASE_HEX.encode(remote.getDigest())) - .getBytes(US_ASCII); - } - if (metadata instanceof SourceFileArtifactValue) { - return resolveSourcePath((SourceFileArtifactValue) metadata).getxattr(name); - } - return getSourcePath(path.asFragment()).getxattr(name); - } - - @Override - protected byte[] getFastDigest(Path path, HashFunction hash) throws IOException { - if (hash != HashFunction.MD5) { - return null; - } - return getMetadataOrThrowFileNotFound(path).getDigest(); - } - - @Override - protected byte[] getDigest(Path path, HashFunction hash) throws IOException { - return getFastDigest(path, hash); - } - - @Override - protected Collection<String> getDirectoryEntries(Path path) throws IOException { - // TODO(felly): Support directory traversal. - return ImmutableList.of(); - } - - private static String createSymbolicLinkErrorMessage( - Path linkPath, PathFragment targetFragment, String message) { - return "createSymbolicLink(" + linkPath + ", " + targetFragment + "): " + message; - } - - @Override - protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException { - // TODO(shahan): this might need to be loosened, but will require more information - Preconditions.checkArgument( - targetFragment.isAbsolute(), - "ActionFileSystem requires symlink targets to be absolute: %s -> %s", - linkPath, - targetFragment); - - // When creating symbolic links, it matters whether target is a source path or not because - // the metadata needs to be handled differently in that case. - PathFragment targetExecPath = null; - int sourceRootIndex = -1; // index into sourceRoots or -1 if not a source - if (targetFragment.startsWith(execRootFragment)) { - targetExecPath = targetFragment.relativeTo(execRootFragment); - } else { - for (int i = 0; i < sourceRoots.size(); ++i) { - if (targetFragment.startsWith(sourceRoots.get(i))) { - targetExecPath = targetFragment.relativeTo(sourceRoots.get(i)); - sourceRootIndex = i; - break; - } - } - if (sourceRootIndex == -1) { - throw new IllegalArgumentException( - linkPath - + " was not found under any known root: " - + execRootFragment - + ", " - + sourceRoots); - } - } - - FileArtifactValue inputMetadata = inputArtifactData.getMetadata(targetExecPath.getPathString()); - if (inputMetadata == null) { - OptionalInputMetadata metadataHolder = optionalInputs.get(targetExecPath); - if (metadataHolder != null) { - inputMetadata = metadataHolder.get(); - } - } - if (inputMetadata == null) { - throw new FileNotFoundException( - createSymbolicLinkErrorMessage( - linkPath, targetFragment, targetFragment + " is not an input.")); - } - OutputMetadata outputHolder = Preconditions.checkNotNull( - outputs.getUnchecked(asExecPath(linkPath)), - "Unexpected null output path: %s", linkPath); - 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()), - true); - } else { - outputHolder.set(inputMetadata, /*notifyConsumer=*/ true); - } - } - - @Override - protected PathFragment readSymbolicLink(Path path) throws IOException { - throw new UnsupportedOperationException(path.getPathString()); - } - - @Override - protected boolean exists(Path path, boolean followSymlinks) { - Preconditions.checkArgument( - followSymlinks, "ActionFileSystem doesn't support no-follow: %s", path); - return getMetadataUnchecked(path) != null; - } - - - @Override - protected boolean isReadable(Path path) throws IOException { - return exists(path, true); - } - - @Override - protected void setReadable(Path path, boolean readable) throws IOException {} - - @Override - protected boolean isWritable(Path path) throws IOException { - return isOutput(path); - } - - @Override - public void setWritable(Path path, boolean writable) throws IOException {} - - @Override - protected boolean isExecutable(Path path) throws IOException { - return true; - } - - @Override - protected void setExecutable(Path path, boolean executable) throws IOException {} - - @Override - protected InputStream getInputStream(Path path) throws IOException { - FileArtifactValue metadata = getMetadataChecked(asExecPath(path)); - InputStream inputStream = getInputStreamFromMetadata(metadata, path); - if (inputStream != null) { - return inputStream; - } - return getSourcePath(path.asFragment()).getInputStream(); - } - - @Nullable - private InputStream getInputStreamFromMetadata(FileArtifactValue metadata, Path path) - throws IOException { - if (metadata instanceof InlineFileArtifactValue) { - return ((InlineFileArtifactValue) metadata).getInputStream(); - } - if (metadata instanceof SourceFileArtifactValue) { - return resolveSourcePath((SourceFileArtifactValue) metadata).getInputStream(); - } - if (metadata instanceof RemoteFileArtifactValue) { - throw new IOException("ActionFileSystem cannot read remote file: " + path); - } - return null; - } - - @Override - protected OutputStream getOutputStream(Path path, boolean append) { - return outputs.getUnchecked(asExecPath(path)).getOutputStream(append, path); - } - - @Override - public void renameTo(Path sourcePath, Path targetPath) throws IOException { - PathFragment sourceExecPath = asExecPath(sourcePath); - OutputMetadata sourceMetadata = outputs.getIfPresent(sourceExecPath); - if (sourceMetadata == null) { - throw new IOException("No output file at " + sourcePath + " to move to " + targetPath); - } - outputs.put(asExecPath(targetPath), sourceMetadata); - outputs.invalidate(sourceExecPath); - } - - @Override - protected void createFSDependentHardLink(Path linkPath, Path originalPath) throws IOException { - throw new UnsupportedOperationException( - "createFSDependendHardLink(" + linkPath + ", " + originalPath + ")"); - } - - // -------------------- Implementation Helpers -------------------- - - private PathFragment asExecPath(Path path) { - return asExecPath(path.asFragment()); - } - - private PathFragment asExecPath(PathFragment fragment) { - if (fragment.startsWith(execRootFragment)) { - return fragment.relativeTo(execRootFragment); - } - for (PathFragment root : sourceRoots) { - if (fragment.startsWith(root)) { - return fragment.relativeTo(root); - } - } - throw new IllegalArgumentException( - fragment + " was not found under any known root: " + execRootFragment + ", " + sourceRoots); - } - - @Nullable - private FileArtifactValue getMetadataChecked(PathFragment execPath) throws IOException { - { - FileArtifactValue metadata = inputArtifactData.getMetadata(execPath.getPathString()); - if (metadata != null) { - return metadata; - } - } - { - OptionalInputMetadata metadataHolder = optionalInputs.get(execPath); - if (metadataHolder != null) { - return metadataHolder.get(); - } - } - { - OutputMetadata metadataHolder = outputs.getIfPresent(execPath); - if (metadataHolder != null) { - FileArtifactValue metadata = metadataHolder.get(); - if (metadata != null) { - return metadata; - } - } - } - return null; - } - - private FileArtifactValue getMetadataOrThrowFileNotFound(Path path) throws IOException { - FileArtifactValue metadata = getMetadataChecked(asExecPath(path)); - if (metadata == null) { - throw new FileNotFoundException(path.getPathString() + " was not found"); - } - return metadata; - } - - @Nullable - private FileArtifactValue getMetadataUnchecked(Path path) { - try { - return getMetadataChecked(asExecPath(path)); - } catch (IOException e) { - throw new IllegalStateException( - "Error getting metadata for " + path.getPathString() + ": " + e.getMessage(), e); - } - } - - private boolean isOutput(Path path) { - 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 delegate.getPath(path); - } - - /** - * Verifies that no root is the prefix of any other root. - * - * <p>TODO(shahan): if this is insufficiently general, we can topologically order on the prefix - * relation between roots. - */ - private void validateRoots() { - for (PathFragment root1 : sourceRoots) { - Preconditions.checkState( - !root1.startsWith(execRootFragment), "%s starts with %s", root1, execRootFragment); - Preconditions.checkState( - !execRootFragment.startsWith(root1), "%s starts with %s", execRootFragment, root1); - for (PathFragment root2 : sourceRoots) { - if (root1 == root2) { - continue; - } - Preconditions.checkState(!root1.startsWith(root2), "%s starts with %s", root1, root2); - } - } - } - - private static ByteString toByteString(byte[] digest) { - return ByteString.copyFrom(BaseEncoding.base16().lowerCase().encode(digest).getBytes(US_ASCII)); - } - - /** NB: resolves to the underlying filesytem instead of this one. */ - private Path resolveSourcePath(SourceFileArtifactValue metadata) throws IOException { - return getSourcePath(sourceRoots.get(metadata.getSourceRootIndex())) - .getRelative(metadata.getExecPath()); - } - - @FunctionalInterface - public interface MetadataConsumer { - void accept(Artifact artifact, FileArtifactValue value) throws IOException; - } - - private class OptionalInputMetadata { - private final Artifact artifact; - private volatile FileArtifactValue metadata = null; - - private OptionalInputMetadata(Artifact artifact) { - this.artifact = artifact; - } - - public Artifact getArtifact() { - return artifact; - } - - public FileArtifactValue get() throws IOException { - if (metadata == null) { - synchronized (this) { - if (metadata == null) { - try { - // TODO(shahan): {@link SkyFunction.Environment} requires single-threaded access so - // we enforce that here by making these (multithreaded) calls synchronized. It might - // be better to make the underlying methods synchronized to avoid having another - // caller unintentionally calling into the environment without locking. - // - // This is currently known to be reached from the distributor during remote include - // scanning which we expect to propagate exceptions up for skyframe restarts. - synchronized (env) { - metadata = (FileArtifactValue) env.getValue(ArtifactSkyKey.key(artifact, false)); - } - } catch (InterruptedException e) { - throw new InterruptedIOException(e.getMessage()); - } - if (metadata == null) { - throw new ActionExecutionFunction.MissingDepException(); - } - } - } - } - return metadata; - } - } - - private class OutputMetadata { - private final @Nullable Artifact artifact; - @Nullable private volatile FileArtifactValue metadata = null; - - private OutputMetadata(Artifact artifact) { - this.artifact = artifact; - } - - @Nullable - public FileArtifactValue get() { - return metadata; - } - - /** - * Sets the output metadata, and maybe notify the metadataConsumer. - * - * @param metadata the metadata to write - * @param notifyConsumer whether to notify metadataConsumer. Callers should not notify the - * metadataConsumer if it will be notified separately at the Spawn level. - */ - public void set(FileArtifactValue metadata, boolean notifyConsumer) throws IOException { - if (notifyConsumer && artifact != null) { - metadataConsumer.accept(artifact, metadata); - } - this.metadata = metadata; - } - - /** Callers are expected to close the returned stream. */ - public ByteArrayOutputStream getOutputStream(boolean append, Path path) { - ByteArrayOutputStream baos = new ByteArrayOutputStream() { - @Override - public void close() throws IOException { - flushInternal(true); - super.close(); - } - - @Override - public void flush() throws IOException { - flushInternal(false); - } - - private void flushInternal(boolean notify) throws IOException { - super.flush(); - byte[] data = toByteArray(); - set(new InlineFileArtifactValue(data, Hashing.md5().hashBytes(data).asBytes()), - /*notifyConsumer=*/ notify); - } - }; - - if (append && metadata != null) { - try (InputStream in = getInputStreamFromMetadata(metadata, path)) { - if (in == null) { - throw new IOException("Unable to read file " + path + " with metadata " + metadata); - } - ByteStreams.copy(in, baos); - } catch (IOException e) { - throw new IllegalStateException(e); - } - } - return baos; - } - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java index d49d533350..2a97fcf25a 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionTemplateExpansionFunction.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Actions.GeneratingActions; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactPrefixConflictException; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.skyframe.ActionTemplateExpansionValue.ActionTemplateExpansionKey; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java index e35be19f00..ac2f4cdab4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.actions.MissingInputFileException; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java index 3916b7b4a6..0f07d7ee79 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.skyframe; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.MissingInputFileException; import com.google.devtools.build.lib.analysis.AspectCompleteEvent; import com.google.devtools.build.lib.analysis.ConfiguredTarget; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java index a03f87977f..d91e46e805 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunction.java @@ -18,6 +18,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.collect.Collections2; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index b1cd6b022a..97f06fed14 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -103,7 +103,6 @@ import java.util.Map; import java.util.Set; import java.util.UUID; import java.util.concurrent.Callable; -import java.util.function.BooleanSupplier; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -152,8 +151,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { List<BuildFileName> buildFilesByPriority, ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, BuildOptions defaultBuildOptions, - MutableArtifactFactorySupplier mutableArtifactFactorySupplier, - BooleanSupplier usesActionFileSystem) { + MutableArtifactFactorySupplier mutableArtifactFactorySupplier) { super( evaluatorSupplier, pkgFactory, @@ -173,8 +171,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { GraphInconsistencyReceiver.THROWING, defaultBuildOptions, new PackageProgressReceiver(), - mutableArtifactFactorySupplier, - usesActionFileSystem); + mutableArtifactFactorySupplier); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); this.customDirtinessCheckers = customDirtinessCheckers; } @@ -211,8 +208,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { buildFilesByPriority, actionOnIOExceptionReadingBuildFile, defaultBuildOptions, - new MutableArtifactFactorySupplier(), - /*usesActionFileSystem=*/ () -> false); + new MutableArtifactFactorySupplier()); } public static SequencedSkyframeExecutor create( @@ -231,8 +227,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { List<BuildFileName> buildFilesByPriority, ActionOnIOExceptionReadingBuildFile actionOnIOExceptionReadingBuildFile, BuildOptions defaultBuildOptions, - MutableArtifactFactorySupplier mutableArtifactFactorySupplier, - BooleanSupplier usesActionFileSystem) { + MutableArtifactFactorySupplier mutableArtifactFactorySupplier) { SequencedSkyframeExecutor skyframeExecutor = new SequencedSkyframeExecutor( InMemoryMemoizingEvaluator.SUPPLIER, @@ -251,8 +246,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor { buildFilesByPriority, actionOnIOExceptionReadingBuildFile, defaultBuildOptions, - mutableArtifactFactorySupplier, - usesActionFileSystem); + mutableArtifactFactorySupplier); skyframeExecutor.init(); return skyframeExecutor; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java index 49aea6da6a..27f83013d3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java @@ -64,7 +64,6 @@ public class SequencedSkyframeExecutorFactory implements SkyframeExecutorFactory BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY, BazelSkyframeExecutorConstants.ACTION_ON_IO_EXCEPTION_READING_BUILD_FILE, defaultBuildOptions, - new MutableArtifactFactorySupplier(), - /*usesActionFileSystem=*/ () -> false); + new MutableArtifactFactorySupplier()); } } 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 c515163f27..0e009cd78b 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 @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionExecutionStatusReporter; import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.ActionInputPrefetcher; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.ActionLogBufferPathGenerator; @@ -59,6 +60,7 @@ import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.MapBasedActionGraph; +import com.google.devtools.build.lib.actions.MetadataConsumer; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.MutableActionGraph; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; @@ -82,6 +84,7 @@ import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; @@ -106,7 +109,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.FutureTask; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.Lock; -import java.util.function.BooleanSupplier; import java.util.function.Supplier; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -163,17 +165,14 @@ public final class SkyframeActionExecutor { private final AtomicReference<ActionExecutionStatusReporter> statusReporterRef; private OutputService outputService; private final Supplier<ImmutableList<Root>> sourceRootSupplier; - private final BooleanSupplier usesActionFileSystem; SkyframeActionExecutor( ActionKeyContext actionKeyContext, AtomicReference<ActionExecutionStatusReporter> statusReporterRef, - Supplier<ImmutableList<Root>> sourceRootSupplier, - BooleanSupplier usesActionFileSystem) { + Supplier<ImmutableList<Root>> sourceRootSupplier) { this.actionKeyContext = actionKeyContext; this.statusReporterRef = statusReporterRef; this.sourceRootSupplier = sourceRootSupplier; - this.usesActionFileSystem = usesActionFileSystem; } /** @@ -367,20 +366,33 @@ public final class SkyframeActionExecutor { this.clientEnv = clientEnv; } - public FileSystem getExecutorFileSystem() { - return executorEngine.getFileSystem(); + boolean usesActionFileSystem() { + return outputService != null && outputService.supportsActionFileSystem(); } - public Path getExecRoot() { + Path getExecRoot() { return executorEngine.getExecRoot(); } - public ImmutableList<Root> getSourceRoots() { - return sourceRootSupplier.get(); + /** REQUIRES: {@link usesActionFileSystem} is true */ + FileSystem createActionFileSystem( + String relativeOutputPath, + ActionInputMap inputArtifactData, + Iterable<Artifact> allowedInputs, + Iterable<Artifact> outputArtifacts) { + return outputService.createActionFileSystem( + executorEngine.getFileSystem(), + executorEngine.getExecRoot().asFragment(), + relativeOutputPath, + sourceRootSupplier.get(), + inputArtifactData, + allowedInputs, + outputArtifacts); } - public boolean usesActionFileSystem() { - return usesActionFileSystem.getAsBoolean(); + void updateActionFileSystemContext( + FileSystem actionFileSystem, Environment env, MetadataConsumer consumer) { + outputService.updateActionFileSystemContext(actionFileSystem, env, consumer); } void executionOver() { @@ -520,7 +532,7 @@ public final class SkyframeActionExecutor { MetadataHandler metadataHandler, Map<Artifact, Collection<Artifact>> expandedInputs, ImmutableMap<PathFragment, ImmutableList<FilesetOutputSymlink>> inputFilesetMappings, - @Nullable ActionFileSystem actionFileSystem) { + @Nullable FileSystem actionFileSystem) { FileOutErr fileOutErr = actionLogBufferPathGenerator.generate( ArtifactPathResolver.createPathResolver(actionFileSystem, executorEngine.getExecRoot())); return new ActionExecutionContext( @@ -648,7 +660,7 @@ public final class SkyframeActionExecutor { PerActionFileCache graphFileCache, MetadataHandler metadataHandler, Environment env, - @Nullable ActionFileSystem actionFileSystem) + @Nullable FileSystem actionFileSystem) throws ActionExecutionException, InterruptedException { ActionExecutionContext actionExecutionContext = ActionExecutionContext.forInputDiscovery( @@ -676,9 +688,9 @@ public final class SkyframeActionExecutor { } private MetadataProvider createFileCache( - MetadataProvider graphFileCache, @Nullable ActionFileSystem actionFileSystem) { - if (actionFileSystem != null) { - return actionFileSystem; + MetadataProvider graphFileCache, @Nullable FileSystem actionFileSystem) { + if (actionFileSystem instanceof MetadataProvider) { + return (MetadataProvider) actionFileSystem; } return new DelegatingPairFileCache(graphFileCache, perBuildFileCache); } 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 f024250102..b498a6e140 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 @@ -138,6 +138,7 @@ import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.Dirent; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.ModifiedFileSet; +import com.google.devtools.build.lib.vfs.OutputService; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; @@ -179,7 +180,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BooleanSupplier; import java.util.function.Supplier; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -345,8 +345,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { GraphInconsistencyReceiver graphInconsistencyReceiver, BuildOptions defaultBuildOptions, @Nullable PackageProgressReceiver packageProgress, - MutableArtifactFactorySupplier artifactResolverSupplier, - BooleanSupplier usesActionFileSystem) { + MutableArtifactFactorySupplier artifactResolverSupplier) { // Strictly speaking, these arguments are not required for initialization, but all current // callsites have them at hand, so we might as well set them during construction. this.evaluatorSupplier = evaluatorSupplier; @@ -360,8 +359,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory { syscalls, cyclesReporter, pkgLocator, numPackagesLoaded, this); this.resourceManager = ResourceManager.instance(); this.skyframeActionExecutor = - new SkyframeActionExecutor( - actionKeyContext, statusReporterRef, this::getPathEntries, usesActionFileSystem); + new SkyframeActionExecutor(actionKeyContext, statusReporterRef, this::getPathEntries); this.fileSystem = fileSystem; this.directories = Preconditions.checkNotNull(directories); this.actionKeyContext = Preconditions.checkNotNull(actionKeyContext); diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD index 8b11e78f08..e763c5f21e 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD +++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD @@ -14,6 +14,10 @@ PATH_FRAGMENT_SOURCES = [ "WindowsOsPathPolicy.java", ] +OUTPUT_SERVICE_SOURCES = [ + "OutputService.java", +] + java_library( name = "pathfragment", srcs = PATH_FRAGMENT_SOURCES, @@ -38,7 +42,7 @@ java_library( [ "*.java", ], - exclude = PATH_FRAGMENT_SOURCES, + exclude = PATH_FRAGMENT_SOURCES + OUTPUT_SERVICE_SOURCES, ), visibility = ["//visibility:public"], exports = [ @@ -58,3 +62,18 @@ java_library( "//third_party:jsr305", ], ) + +java_library( + name = "output_service", + srcs = OUTPUT_SERVICE_SOURCES, + deps = [ + ":pathfragment", + ":vfs", + "//src/main/java/com/google/devtools/build/lib:events", + "//src/main/java/com/google/devtools/build/lib:util", + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/skyframe", + "//third_party:guava", + "//third_party:jsr305", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index b09e68162b..55cadc77c0 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -12,22 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.skyframe; +package com.google.devtools.build.lib.vfs; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionInputMap; +import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.MetadataConsumer; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.events.EventHandler; -import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.vfs.BatchStat; -import com.google.devtools.build.lib.vfs.ModifiedFileSet; -import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.skyframe.SkyFunction; import java.io.IOException; import java.util.UUID; +import javax.annotation.Nullable; /** * An OutputService retains control over the Blaze output tree, and provides a higher level of @@ -44,11 +45,6 @@ public interface OutputService { String getFilesSystemName(); /** - * @return a human-readable, one word name for the service - */ - String getName(); - - /** * Start the build. * * @param buildId the UUID build identifier @@ -75,19 +71,6 @@ public interface OutputService { throws IOException, EnvironmentalExecException; /** - * Stages the given tool from the package path, possibly copying it to local disk. - * - * @param tool target representing the tool to stage - * @return a Path pointing to the staged target - */ - Path stageTool(Target tool) throws IOException; - - /** - * @return the name of the workspace this output service controls. - */ - String getWorkspace(); - - /** * @return the BatchStat instance or null. */ BatchStat getBatchStatter(); @@ -118,15 +101,43 @@ public interface OutputService { */ void clean() throws ExecException, InterruptedException; + /** @return true iff the file actually lives on a remote server */ + boolean isRemoteFile(Artifact file); + + default boolean supportsActionFileSystem() { + return false; + } + /** - * @param file the File - * @return true iff the file actually lives on a remote server + * @param sourceDelegate filesystem for reading source files (excludes output files) + * @param execRootFragment absolute path fragment pointing to the execution root + * @param relativeOutputPath execution root relative path to output + * @param sourceRoots list of directories on the package path (from {@link + * com.google.devtools.build.lib.pkgcache.PathPackageLocator}) + * @param inputArtifactData information about required inputs to the action + * @param allowedInputs optional inputs that might be added during input discovery + * @param outputArtifacts required outputs of the action + * @return an action-scoped filesystem if {@link supportsActionFileSystem} is true */ - boolean isRemoteFile(Path file); + @Nullable + default FileSystem createActionFileSystem( + FileSystem sourceDelegate, + PathFragment execRootFragment, + String relativeOutputPath, + ImmutableList<Root> sourceRoots, + ActionInputMap inputArtifactData, + Iterable<Artifact> allowedInputs, + Iterable<Artifact> outputArtifacts) { + return null; + } /** - * @param path a fully-resolved path - * @return true iff path is under this output service's control + * Updates the context used by the filesystem returned by {@link createActionFileSystem}. + * + * <p>Should be called as context changes throughout action execution. + * + * @param actionFileSystem must be a filesystem returned by {@link createActionFileSystem}. */ - boolean resolvedPathUnderTree(Path path); + default void updateActionFileSystemContext( + FileSystem actionFileSystem, SkyFunction.Environment env, MetadataConsumer consumer) {} } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ActionFileSystemTest.java deleted file mode 100644 index f155883ad1..0000000000 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ActionFileSystemTest.java +++ /dev/null @@ -1,138 +0,0 @@ -// 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 static com.google.common.truth.Truth.assertThat; - -import com.google.common.collect.ImmutableList; -import com.google.common.hash.HashCode; -import com.google.common.hash.Hashing; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputHelper; -import com.google.devtools.build.lib.actions.ActionInputMap; -import com.google.devtools.build.lib.vfs.FileSystem; -import com.google.devtools.build.lib.vfs.FileSystemUtils; -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 java.io.OutputStream; -import java.io.PrintStream; -import java.nio.charset.StandardCharsets; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** - * Tests for {@link ActionFileSystem}. - * - * It would be nice to derive from - * {@link com.google.devtools.build.lib.vfs.FileSystemTest;FileSystemTest} if/when ActionFileSystem - * becomes sufficiently general. - */ -@RunWith(JUnit4.class) -public class ActionFileSystemTest { - private ActionFileSystem actionFS; - private PathFragment execRootFragment; - private Path outputPath; - - @Before - public void freshFS() { - FileSystem delegateFS = new InMemoryFileSystem(); - execRootFragment = PathFragment.create("/path/to/execroot"); - String relativeOutputPath = "goog-out"; - actionFS = new ActionFileSystem(delegateFS, execRootFragment, relativeOutputPath, - ImmutableList.of(), new ActionInputMap(0), ImmutableList.of(), ImmutableList.of()); - outputPath = actionFS.getPath(execRootFragment.getRelative(relativeOutputPath)); - } - - @Test - public void testFileWrite() throws Exception { - String testData = "abc19"; - - Path file = outputPath.getRelative("foo/bar"); - FileSystemUtils.writeContentAsLatin1(file, testData); - assertThat(file.getFileSize()).isEqualTo(testData.length()); - assertThat(file.exists()).isTrue(); - assertThat(file.stat().isFile()).isTrue(); - assertThat(file.stat().isDirectory()).isFalse(); - - assertThat(file.delete()).isTrue(); - assertThat(file.exists()).isFalse(); - assertThat(file.delete()).isFalse(); - } - - @Test - public void testInjectUndeclaredOutput() throws Exception { - String testData = "abc19"; - byte[] fileContent = testData.getBytes(); - HashCode digest = Hashing.md5().hashBytes(fileContent); - Path file = outputPath.getRelative("foo/bar"); - assertThat(file.exists()).isFalse(); - actionFS.onInsert(asActionInput(file), digest.asBytes(), fileContent.length, 83); - - assertThat(file.exists()).isTrue(); - assertThat(file.stat().getSize()).isEqualTo(fileContent.length); - assertThat(file.getDigest()).isEqualTo(digest.asBytes()); - assertThat(file.getFastDigest()).isEqualTo(digest.asBytes()); - - assertThat(file.delete()).isTrue(); - assertThat(file.exists()).isFalse(); - assertThat(file.delete()).isFalse(); - } - - private ActionInput asActionInput(Path path) { - return ActionInputHelper.fromPath(path.asFragment().relativeTo(execRootFragment)); - } - - @Test - public void testAppendLocalFile() throws Exception { - String testData = "abc"; - - Path file = outputPath.getRelative("foo/bar"); - FileSystemUtils.writeContentAsLatin1(file, testData); - assertThat(new String(FileSystemUtils.readContentAsLatin1(file))).isEqualTo(testData); - - try (OutputStream out = file.getOutputStream(true)) { - PrintStream printStream = new PrintStream(out); - printStream.append("defg"); - printStream.flush(); - } - assertThat(new String(FileSystemUtils.readContentAsLatin1(file))).isEqualTo("abcdefg"); - - // Now make sure we can still overwrite the file in non-append mode. - FileSystemUtils.writeContentAsLatin1(file, "cheesy"); - assertThat(new String(FileSystemUtils.readContentAsLatin1(file))).isEqualTo("cheesy"); - } - - @Test - public void testFlushedButNotClosedFileWrite() throws Exception { - String testData = "abc19"; - - Path file = outputPath.getRelative("foo/bar"); - try (OutputStream out = file.getOutputStream()) { - assertThat(file.exists()).isFalse(); - - out.write(testData.getBytes(StandardCharsets.ISO_8859_1)); - assertThat(file.exists()).isFalse(); - - out.flush(); - assertThat(file.getFileSize()).isEqualTo(testData.length()); - assertThat(file.exists()).isTrue(); - assertThat(file.stat().isFile()).isTrue(); - assertThat(file.stat().isDirectory()).isFalse(); - } - assertThat(file.getFileSize()).isEqualTo(testData.length()); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java index da66c930e5..925056e593 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ArtifactFunctionTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.BasicActionLookupValue; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileValue; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java index ecce7ea5a0..fbb64fee80 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java @@ -28,6 +28,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; 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 2b87fb9232..b4075b35f1 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 @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.BasicActionLookupValue; import com.google.devtools.build.lib.actions.BuildFailedException; import com.google.devtools.build.lib.actions.Executor; @@ -192,8 +193,7 @@ public abstract class TimestampBuilderTestCase extends FoundationTestCase { new SkyframeActionExecutor( actionKeyContext, new AtomicReference<>(statusReporter), - /*sourceRootSupplier=*/ () -> ImmutableList.of(), - /*usesActionFileSystem=*/ () -> false); + /*sourceRootSupplier=*/ () -> ImmutableList.of()); Path actionOutputBase = scratch.dir("/usr/local/google/_blaze_jrluser/FAKEMD5/action_out/"); skyframeActionExecutor.setActionLogBufferPathGenerator( diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java index b27a520002..97816e33bd 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/TreeArtifactMetadataTest.java @@ -33,6 +33,7 @@ import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifactType; import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.actions.ArtifactSkyKey; import com.google.devtools.build.lib.actions.BasicActionLookupValue; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileValue; |