From 83fce40e69ef35c90e69a57387b1f95b387ad453 Mon Sep 17 00:00:00 2001 From: Ulf Adams Date: Tue, 19 Apr 2016 11:32:24 +0000 Subject: [PATCH] Review a number of action subclasses and update them according to the spec. Second pass. Consists of adding @Immutable annotations, adding final modifiers, and changing the types of fields to immutable types. -- MOS_MIGRATED_REVID=120216592 --- .../google/devtools/build/lib/actions/FailAction.java | 4 ++-- .../devtools/build/lib/actions/MiddlemanAction.java | 4 +++- .../google/devtools/build/lib/analysis/PseudoAction.java | 1 - .../devtools/build/lib/analysis/SymlinkTreeAction.java | 4 +++- .../lib/analysis/actions/CreateIncSymlinkAction.java | 5 +++-- .../build/lib/rules/cpp/ExtractInclusionAction.java | 2 ++ .../devtools/build/lib/rules/cpp/FdoStubAction.java | 4 +++- .../devtools/build/lib/rules/cpp/SolibSymlinkAction.java | 3 ++- .../devtools/build/lib/rules/java/JavaCompileAction.java | 9 +++++---- .../build/lib/rules/objc/CompilationSupport.java | 5 +++-- .../google/devtools/build/lib/rules/python/PyCommon.java | 9 +++------ .../build/lib/analysis/util/AnalysisTestUtil.java | 4 +++- 12 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/FailAction.java b/src/main/java/com/google/devtools/build/lib/actions/FailAction.java index da6f599d37df24..15d01bd7cd6668 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/FailAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/FailAction.java @@ -15,13 +15,13 @@ package com.google.devtools.build.lib.actions; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; /** * FailAction is an Action that always fails to execute. (Used as scaffolding * for rules we haven't yet implemented. Also useful for testing.) */ -@ThreadSafe +@Immutable public final class FailAction extends AbstractAction { private static final String GUID = "626cb78a-810f-4af3-979c-ee194955f04c"; diff --git a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java index 8c25e4d22306e6..a8a91f6803ce1b 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java +++ b/src/main/java/com/google/devtools/build/lib/actions/MiddlemanAction.java @@ -15,6 +15,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Preconditions; /** @@ -22,7 +23,8 @@ * runs. This is useful for bundling up a bunch of dependencies that are shared * between individual targets in the action graph; for example generated header files. */ -public class MiddlemanAction extends AbstractAction { +@Immutable +public final class MiddlemanAction extends AbstractAction { public static final String MIDDLEMAN_MNEMONIC = "Middleman"; private final String description; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java index c482f2e61567e3..e0eb7125970c07 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PseudoAction.java @@ -35,7 +35,6 @@ * about rules to extra_actions. */ public class PseudoAction extends AbstractAction { - private final UUID uuid; private final String mnemonic; private final GeneratedExtension infoExtension; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SymlinkTreeAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SymlinkTreeAction.java index 0e98408a15f81a..5ebe26f0eb7e92 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SymlinkTreeAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SymlinkTreeAction.java @@ -22,6 +22,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; @@ -32,7 +33,8 @@ * Action responsible for the symlink tree creation. * Used to generate runfiles and fileset symlink farms. */ -public class SymlinkTreeAction extends AbstractAction { +@Immutable +public final class SymlinkTreeAction extends AbstractAction { private static final String GUID = "63412bda-4026-4c8e-a3ad-7deb397728d4"; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CreateIncSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CreateIncSymlinkAction.java index f374dfff9dab92..06d353dfc1bca3 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CreateIncSymlinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CreateIncSymlinkAction.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.Path; @@ -35,9 +36,9 @@ /** * This action creates a set of symbolic links. */ +@Immutable public final class CreateIncSymlinkAction extends AbstractAction { - - private final SortedMap symlinks; + private final ImmutableSortedMap symlinks; /** * Creates a new instance. The symlinks map maps symlinks to their targets, i.e. the symlink paths diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/ExtractInclusionAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/ExtractInclusionAction.java index 82a3a16f69a046..0a3d428bb83f9f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/ExtractInclusionAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/ExtractInclusionAction.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import java.io.IOException; @@ -38,6 +39,7 @@ * library depends on it, and only references one of the headers, the other * grep-includes will have been wasted. */ +@Immutable final class ExtractInclusionAction extends AbstractAction { private static final String GUID = "45b43e5a-4734-43bb-a05e-012313808142"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoStubAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoStubAction.java index bee74f7092d88b..6897aab0848ea4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoStubAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FdoStubAction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Executor; import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.vfs.Path; /** @@ -30,7 +31,8 @@ * would complain that these files have no generating action if we did not set it to an instance of * this class. */ -public class FdoStubAction extends AbstractAction { +@Immutable +public final class FdoStubAction extends AbstractAction { public FdoStubAction(ActionOwner owner, Artifact output) { // TODO(bazel-team): Make extracting the zip file a honest-to-God action so that we can do away // with this ugliness. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java index c5f766b4271648..e3250cabc2ca0e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SolibSymlinkAction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.LinkerInputs.LibraryToLink; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; @@ -44,8 +45,8 @@ * Such symlinks are used by the linker to ensure that all rpath entries can be * specified relative to the $ORIGIN. */ +@Immutable public final class SolibSymlinkAction extends AbstractAction { - private final Artifact library; private final Path target; private final Artifact symlink; diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 65a300ce66bbc6..8d5324ff791df4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; import com.google.devtools.build.lib.util.Fingerprint; @@ -71,8 +72,8 @@ /** * Action that represents a Java compilation. */ -@ThreadCompatible -public class JavaCompileAction extends AbstractAction { +@ThreadCompatible @Immutable +public final class JavaCompileAction extends AbstractAction { private static final String GUID = "786e174d-ed97-4e79-9f61-ae74430714cf"; private static final ResourceSet LOCAL_RESOURCES = @@ -100,7 +101,7 @@ public class JavaCompileAction extends AbstractAction { /** * The path to the extdir to specify to javac. */ - private final Collection extdirInputs; + private final ImmutableList extdirInputs; /** * The list of classpath entries to search for annotation processors. @@ -181,7 +182,7 @@ private JavaCompileAction( Artifact outputJar, NestedSet classpathEntries, ImmutableList bootclasspathEntries, - Collection extdirInputs, + ImmutableList extdirInputs, List processorPath, List processorNames, Collection messages, diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index d974deb37eece2..766197015756d6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -71,6 +71,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.TargetUtils; @@ -1069,8 +1070,8 @@ private CommandLine linkCommandLine( *

Required as a hack to the link command line because that may contain two commands, which are * then passed to {@code /bin/bash -c}, and accordingly need to be a single argument. */ - private static class SingleArgCommandLine extends CommandLine { - + @Immutable // if original is immutable + private static final class SingleArgCommandLine extends CommandLine { private final CommandLine original; private SingleArgCommandLine(CommandLine original) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index 2b01aec41adf8d..c375701f14c3c0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; @@ -560,7 +561,8 @@ private static String buildNoMainMatchesErrorText(boolean explicit, String propo } // Used purely to set the legacy ActionType of the ExtraActionInfo. - private static class PyPseudoAction extends PseudoAction { + @Immutable + private static final class PyPseudoAction extends PseudoAction { private static final UUID ACTION_UUID = UUID.fromString("8d720129-bc1a-481f-8c4c-dbe11dcef319"); public PyPseudoAction(ActionOwner owner, @@ -569,10 +571,5 @@ public PyPseudoAction(ActionOwner owner, PythonInfo info) { super(ACTION_UUID, owner, inputs, outputs, mnemonic, infoExtension, info); } - - @Override - public ExtraActionInfo.Builder getExtraActionInfo() { - return super.getExtraActionInfo(); - } } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java index 3d4e0b2ad43966..bdc79f892fc165 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java @@ -44,6 +44,7 @@ import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; @@ -185,7 +186,8 @@ public ImmutableSet getOrphanArtifacts() { } } - public static class DummyWorkspaceStatusAction extends WorkspaceStatusAction { + @Immutable + public static final class DummyWorkspaceStatusAction extends WorkspaceStatusAction { private final String key; private final Artifact stableStatus; private final Artifact volatileStatus;