From ba6a7d45755a2552ebcb4bf5915b4c44a4405275 Mon Sep 17 00:00:00 2001 From: kush Date: Fri, 4 May 2018 08:55:27 -0700 Subject: [PATCH] Kill Legacy Fileset implementation. RELNOTES: None PiperOrigin-RevId: 195422399 --- .../analysis/config/BuildConfiguration.java | 4 -- .../FilesetOutputConfiguredTarget.java | 23 +------- .../lib/analysis/fileset/FilesetProvider.java | 7 --- .../build/lib/buildtool/ExecutionTool.java | 3 +- .../lib/exec/FilesetActionContextImpl.java | 36 +----------- .../rules/fileset/FilesetActionContext.java | 7 --- .../lib/rules/fileset/SymlinkTraversal.java | 56 ------------------- 7 files changed, 7 insertions(+), 129 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/fileset/SymlinkTraversal.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index ade168c05d969e..1dc144022423e9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -1746,10 +1746,6 @@ public boolean legacyExternalRunfiles() { return options.legacyExternalRunfiles; } - public boolean getSkyframeNativeFileset() { - return true; - } - /** * Returns user-specified test environment variables and their values, as set by the --test_env * options. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FilesetOutputConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FilesetOutputConfiguredTarget.java index 39fabc3d31d22d..08dc1ac78ec0a6 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FilesetOutputConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FilesetOutputConfiguredTarget.java @@ -27,8 +27,6 @@ import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents; import com.google.devtools.build.lib.skyframe.BuildConfigurationValue; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.vfs.PathFragment; -import javax.annotation.Nullable; /** * A configured target for output files generated by {@code Fileset} rules. They are almost the same @@ -42,16 +40,14 @@ @AutoCodec public final class FilesetOutputConfiguredTarget extends OutputFileConfiguredTarget implements FilesetProvider { - private final Artifact filesetInputManifest; - private final PathFragment filesetLinkDir; - @Nullable private final ImmutableList filesetTraversals; + private final ImmutableList filesetTraversals; public FilesetOutputConfiguredTarget( TargetContext targetContext, OutputFile outputFile, TransitiveInfoCollection generatingRule, Artifact outputArtifact, - @Nullable ImmutableList traversals) { + ImmutableList traversals) { this( targetContext.getLabel(), targetContext.getConfigurationKey(), @@ -74,26 +70,13 @@ public FilesetOutputConfiguredTarget( NestedSet visibility, TransitiveInfoCollection generatingRule, Artifact artifact, - @Nullable ImmutableList traversals) { + ImmutableList traversals) { super(label, configurationKey, visibility, artifact, generatingRule); FilesetProvider provider = generatingRule.getProvider(FilesetProvider.class); Preconditions.checkArgument(provider != null); - filesetInputManifest = provider.getFilesetInputManifest(); - filesetLinkDir = provider.getFilesetLinkDir(); filesetTraversals = traversals; } - @Override - public Artifact getFilesetInputManifest() { - return filesetInputManifest; - } - - @Override - public PathFragment getFilesetLinkDir() { - return filesetLinkDir; - } - - @Nullable @Override public ImmutableList getTraversals() { return filesetTraversals; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/fileset/FilesetProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/fileset/FilesetProvider.java index fe326070d5df0d..225a40eb4256b1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/fileset/FilesetProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/fileset/FilesetProvider.java @@ -15,23 +15,16 @@ package com.google.devtools.build.lib.analysis.fileset; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.FilesetTraversalParams; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.vfs.PathFragment; -import javax.annotation.Nullable; /** * Information needed by a Fileset to do the right thing when it depends on another Fileset. */ public interface FilesetProvider extends TransitiveInfoProvider { - Artifact getFilesetInputManifest(); - PathFragment getFilesetLinkDir(); - /** * Returns a list of the traversals that went into this Fileset. Only used by Skyframe-native * filesets, so will be null if Skyframe-native filesets are not enabled. */ - @Nullable ImmutableList getTraversals(); } 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 cb058474a4f09f..55594de186eff6 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 @@ -136,8 +136,7 @@ public class ExecutionTool { for (BlazeModule module : runtime.getBlazeModules()) { module.executorInit(env, request, builder); } - builder.addActionContextProvider( - new FilesetActionContextImpl.Provider(env.getReporter(), env.getWorkspaceName())); + builder.addActionContextProvider(new FilesetActionContextImpl.Provider(env.getWorkspaceName())); builder.addActionContext(new SymlinkTreeStrategy( env.getOutputService(), env.getBlazeWorkspace().getBinTools())); // TODO(philwo) - the ExecutionTool should not add arbitrary dependencies on its own, instead diff --git a/src/main/java/com/google/devtools/build/lib/exec/FilesetActionContextImpl.java b/src/main/java/com/google/devtools/build/lib/exec/FilesetActionContextImpl.java index d21b27069e1a25..63713527883317 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/FilesetActionContextImpl.java +++ b/src/main/java/com/google/devtools/build/lib/exec/FilesetActionContextImpl.java @@ -14,14 +14,9 @@ package com.google.devtools.build.lib.exec; import com.google.common.collect.ImmutableList; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.devtools.build.lib.actions.ActionContext; import com.google.devtools.build.lib.actions.ExecutionStrategy; -import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; /** * Context for Fileset manifest actions. It currently only provides a ThreadPoolExecutor. @@ -38,48 +33,23 @@ public final class FilesetActionContextImpl implements FilesetActionContext { */ public static class Provider extends ActionContextProvider { private FilesetActionContextImpl impl; - private final Reporter reporter; - private final ThreadPoolExecutor filesetPool; - public Provider(Reporter reporter, String workspaceName) { - this.reporter = reporter; - this.filesetPool = newFilesetPool(100); - this.impl = new FilesetActionContextImpl(filesetPool, workspaceName); - } - - private static ThreadPoolExecutor newFilesetPool(int threads) { - ThreadPoolExecutor pool = new ThreadPoolExecutor(threads, threads, 3L, TimeUnit.SECONDS, - new LinkedBlockingQueue()); - // Do not consume threads when not in use. - pool.allowCoreThreadTimeOut(true); - pool.setThreadFactory(new ThreadFactoryBuilder().setNameFormat("Fileset worker %d").build()); - return pool; + public Provider(String workspaceName) { + this.impl = new FilesetActionContextImpl(workspaceName); } @Override public Iterable getActionContexts() { return ImmutableList.of(impl); } - - @Override - public void executionPhaseEnding() { - BlazeExecutor.shutdownHelperPool(reporter, filesetPool, "Fileset"); - } } - private final ThreadPoolExecutor filesetPool; private final String workspaceName; - private FilesetActionContextImpl(ThreadPoolExecutor filesetPool, String workspaceName) { - this.filesetPool = filesetPool; + private FilesetActionContextImpl(String workspaceName) { this.workspaceName = workspaceName; } - @Override - public ThreadPoolExecutor getFilesetPool() { - return filesetPool; - } - @Override public String getWorkspaceName() { return workspaceName; diff --git a/src/main/java/com/google/devtools/build/lib/rules/fileset/FilesetActionContext.java b/src/main/java/com/google/devtools/build/lib/rules/fileset/FilesetActionContext.java index 9cfc92083ac5d8..76eb4177aaf76b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/fileset/FilesetActionContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/fileset/FilesetActionContext.java @@ -14,18 +14,11 @@ package com.google.devtools.build.lib.rules.fileset; import com.google.devtools.build.lib.actions.ActionContext; -import java.util.concurrent.ThreadPoolExecutor; /** * Action context for fileset collection actions. */ public interface FilesetActionContext extends ActionContext { - - /** - * Returns a thread pool for fileset symlink tree creation. - */ - ThreadPoolExecutor getFilesetPool(); - /** * Returns the name of the workspace the build is run in. */ diff --git a/src/main/java/com/google/devtools/build/lib/rules/fileset/SymlinkTraversal.java b/src/main/java/com/google/devtools/build/lib/rules/fileset/SymlinkTraversal.java deleted file mode 100644 index a26d5b1a8c21dd..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/fileset/SymlinkTraversal.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2014 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.rules.fileset; - -import com.google.devtools.build.lib.actions.ActionExecutionContext; -import com.google.devtools.build.lib.util.Fingerprint; -import java.io.IOException; -import java.util.concurrent.ThreadPoolExecutor; - -/** - * An interface which contains a method to compute a symlink mapping. - */ -public interface SymlinkTraversal { - - /** - * Adds symlinks to the given FilesetLinks. - * - * @throws IOException if a filesystem operation fails. - * @throws InterruptedException if the traversal is interrupted. - */ - void addSymlinks( - ActionExecutionContext actionExecutionContext, - FilesetLinks links, - ThreadPoolExecutor filesetPool) - throws IOException, InterruptedException; - - /** - * Add the traversal's fingerprint to the given Fingerprint. - * @param fp the Fingerprint to combine. - */ - void fingerprint(Fingerprint fp); - - /** - * @return true iff this traversal must be executed unconditionally. - */ - boolean executeUnconditionally(); - - /** - * Returns true if it's ever possible that {@link #executeUnconditionally} - * could evaluate to true during the lifetime of this instance, false - * otherwise. - */ - boolean isVolatile(); -}