diff options
author | Ulf Adams <ulfjack@google.com> | 2016-04-19 12:55:12 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-04-19 13:52:07 +0000 |
commit | 10993fe27a62d5a4e683a206291c1bd44a492daf (patch) | |
tree | 7d0926a88d1f40524a6d597f0685c7b9cca3dbc2 /src/main/java/com/google | |
parent | c31f4c544cf26424375dbd579338ef9680b97012 (diff) |
Review a number of action subclasses and update them according to the spec.
Consists of adding @Immutable annotations, adding final modifiers, and changing
the types of fields to immutable types.
--
MOS_MIGRATED_REVID=120221067
Diffstat (limited to 'src/main/java/com/google')
28 files changed, 116 insertions, 68 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java index b0b163e396..82e67978e0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.util.Preconditions; @@ -26,6 +27,7 @@ import javax.annotation.Nullable; * but to avoid storing heavyweight analysis objects in actions, and to avoid coupling between the * analysis and actions packages, the RuleConfiguredTarget provides an instance of this class. */ +@Immutable public final class ActionOwner { /** An action owner for special cases. Usage is strongly discouraged. */ public static final ActionOwner SYSTEM_ACTION_OWNER = diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 57e680824e..0b1248c538 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -131,7 +131,7 @@ public final class RuleContext extends TargetContext * Targets from FilesetEntry.files, or null if the user omitted it. */ @Nullable - public List<TransitiveInfoCollection> getFiles() { + public ImmutableList<TransitiveInfoCollection> getFiles() { return files; } } @@ -782,7 +782,7 @@ public final class RuleContext extends TargetContext * @param attributeName the name of the attribute to process * @return a list of strings containing the expanded and tokenized values for the attribute */ - public List<String> getTokenizedStringListAttr(String attributeName) { + public ImmutableList<String> getTokenizedStringListAttr(String attributeName) { return getExpandedStringListAttr(attributeName, Tokenize.YES); } @@ -793,7 +793,7 @@ public final class RuleContext extends TargetContext * @param attributeName the name of the attribute to process * @return a list of strings containing the processed values for the attribute */ - public List<String> getExpandedStringListAttr(String attributeName, Tokenize tokenize) { + public ImmutableList<String> getExpandedStringListAttr(String attributeName, Tokenize tokenize) { if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) { // TODO(bazel-team): This should be an error. return ImmutableList.of(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java index 04792e8db4..f1e633967f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.PathFragment; @@ -49,12 +50,14 @@ import javax.annotation.Nullable; * <p>Note that this action carefully avoids building the manifest content in * memory. */ -public class SourceManifestAction extends AbstractFileWriteAction { +@Immutable // if all ManifestWriter implementations are immutable +public final class SourceManifestAction extends AbstractFileWriteAction { private static final String GUID = "07459553-a3d0-4d37-9d78-18ed942470f4"; /** - * Interface for defining manifest formatting and reporting specifics. + * Interface for defining manifest formatting and reporting specifics. Implementations must be + * immutable. */ @VisibleForTesting interface ManifestWriter { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java index 3e235bb0e1..d87772d61f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java @@ -20,6 +20,7 @@ import com.google.common.io.ByteStreams; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +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; @@ -30,6 +31,7 @@ import java.io.OutputStream; /** * Action to write a binary file. */ +@Immutable // if source is immutable public final class BinaryFileWriteAction extends AbstractFileWriteAction { private static final String GUID = "eeee07fe-4b40-11e4-82d6-eba0b4f713e2"; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java index 1907f7c223..23c1484ba8 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import java.io.IOException; @@ -31,7 +32,8 @@ import java.util.Collection; * <p>TODO(bazel-team): Choose a better name to distinguish this class from * {@link BinaryFileWriteAction}. */ -public class FileWriteAction extends AbstractFileWriteAction { +@Immutable // if fileContents is immutable +public final class FileWriteAction extends AbstractFileWriteAction { private static final String GUID = "332877c7-ca9f-4731-b387-54f620408522"; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java index 939d9fc41e..2a1c9b8e21 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.ShellEscaper; @@ -31,6 +32,7 @@ import java.nio.charset.Charset; /** * Action to write a parameter file for a {@link CommandLine}. */ +@Immutable // if commandLine and charset are immutable public final class ParameterFileWriteAction extends AbstractFileWriteAction { private static final String GUID = "45f678d8-e395-401e-8446-e795ccc6361f"; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java index 10d4d7285b..fe404e1168 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java @@ -18,10 +18,12 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Objects; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.ResourceFileLoader; import com.google.devtools.build.lib.util.StringUtilities; @@ -34,12 +36,12 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.List; -import java.util.Map; /** * Action to expand a template and write the expanded content to a file. */ -public class TemplateExpansionAction extends AbstractFileWriteAction { +@Immutable // if all substitutions are immutable +public final class TemplateExpansionAction extends AbstractFileWriteAction { private static final String GUID = "786c1fe0-dca8-407a-b108-e1ecd6d1bc7f"; @@ -52,6 +54,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction { * <p>It should be assumed that the {@link #getKey} invocation is cheap, and * that the {@link #getValue} invocation is expensive. */ + @Immutable // if the keys and values in the passed in lists and maps are all immutable public abstract static class Substitution { private Substitution() { } @@ -80,7 +83,8 @@ public class TemplateExpansionAction extends AbstractFileWriteAction { * Returns an immutable Substitution instance for the key and list of values. The * values will be joined by spaces before substitution. */ - public static Substitution ofSpaceSeparatedList(final String key, final List<?> value) { + public static Substitution ofSpaceSeparatedList( + final String key, final ImmutableList<?> value) { return new Substitution() { @Override public String getKey() { @@ -101,7 +105,8 @@ public class TemplateExpansionAction extends AbstractFileWriteAction { * * <p>For example, the map <(a,1), (b,2), (c,3)> will become "a=1 b=2 c=3". */ - public static Substitution ofSpaceSeparatedMap(final String key, final Map<?, ?> value) { + public static Substitution ofSpaceSeparatedMap( + final String key, final ImmutableMap<?, ?> value) { return new Substitution() { @Override public String getKey() { @@ -164,6 +169,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction { * A template that contains text content, or alternatively throws an {@link * IOException}. */ + @Immutable // all subclasses are immutable public abstract static class Template { private static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8; @@ -258,7 +264,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction { } private final Template template; - private final List<Substitution> substitutions; + private final ImmutableList<Substitution> substitutions; /** * Creates a new TemplateExpansionAction instance. 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 349d6d451d..fd7671d06e 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 @@ -2200,7 +2200,7 @@ public final class BuildConfiguration { * Returns user-specified test environment variables and their values, as * set by the --test_env options. */ - public Map<String, String> getTestEnv() { + public ImmutableMap<String, String> getTestEnv() { return testEnvironment; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index b140349dbd..fae681b68e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -181,7 +181,7 @@ public class BazelJavaSemantics implements JavaSemantics { arguments.add(Substitution.of("%java_start_class%", ShellEscaper.escapeString(javaStartClass))); - arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", jvmFlags)); + arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", ImmutableList.copyOf(jvmFlags))); ruleContext.registerAction(new TemplateExpansionAction( ruleContext.getActionOwner(), executable, STUB_SCRIPT, arguments, true)); diff --git a/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java b/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java index 7900cab618..69abf331cb 100644 --- a/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java +++ b/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java @@ -34,6 +34,9 @@ import java.util.List; * @see CollectionUtils#checkImmutable(Iterable) */ public final class IterablesChain<T> implements Iterable<T> { + public static <T> Iterable<T> concat(Iterable<? extends T> a, Iterable<? extends T> b) { + return IterablesChain.<T>builder().add(a).add(b).build(); + } private final Iterable<T> chain; @@ -61,7 +64,7 @@ public final class IterablesChain<T> implements Iterable<T> { * */ public static class Builder<T> { - private List<Iterable<T>> iterables = new ArrayList<>(); + private List<Iterable<? extends T>> iterables = new ArrayList<>(); private boolean deduplicate; private Builder() { @@ -72,7 +75,7 @@ public final class IterablesChain<T> implements Iterable<T> { * * <p>If the iterable can not be confirmed to be immutable, a runtime error is thrown. */ - public Builder<T> add(Iterable<T> iterable) { + public Builder<T> add(Iterable<? extends T> iterable) { CollectionUtils.checkImmutable(iterable); if (!Iterables.isEmpty(iterable)) { iterables.add(iterable); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index 48a8a931cd..8c9fc5f2e0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction.Builder; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.collect.IterablesChain; 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; @@ -769,7 +770,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory { ResourceApk resourceApk, NativeLibs nativeLibs) { - Iterable<Artifact> jars = Iterables.concat( + Iterable<Artifact> jars = IterablesChain.concat( resourceClasses.getArchiveInputs(true), androidCommon.getRuntimeJars()); AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext); diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java index fcd16afc46..9adf9eb8af 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java @@ -103,7 +103,7 @@ public final class AndroidResourcesProvider implements TransitiveInfoProvider { private final boolean manifestExported; private final Artifact javaSourceJar; private final Artifact rTxt; - private Artifact symbolsTxt; + private final Artifact symbolsTxt; public ResourceContainer(Label label, String javaPackage, diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java index 8218156465..d78f976ccb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java @@ -22,6 +22,8 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.collect.CollectionUtils; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.android.apkmanifest.ApkManifestOuterClass; import com.google.devtools.build.lib.rules.android.apkmanifest.ApkManifestOuterClass.ApkManifest; import com.google.devtools.build.lib.util.Fingerprint; @@ -33,7 +35,8 @@ import java.io.OutputStream; import java.io.PrintStream; import java.util.Map; -public class ApkManifestAction extends AbstractFileWriteAction { +@Immutable +public final class ApkManifestAction extends AbstractFileWriteAction { private static Iterable<Artifact> makeInputs( AndroidSdkProvider sdk, @@ -92,8 +95,8 @@ public class ApkManifestAction extends AbstractFileWriteAction { Iterable<Artifact> jars, ResourceApk resourceApk, NativeLibs nativeLibs) { - super(owner, makeInputs(sdk, jars, resourceApk, nativeLibs), outputFile, false); + CollectionUtils.checkImmutable(jars); this.textOutput = textOutput; this.sdk = sdk; this.jars = jars; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java index 78c8db2cde..dbcf0bfa98 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java @@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.CcLinkParams; import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.cpp.CppFileTypes; @@ -43,6 +44,7 @@ import java.util.Set; import javax.annotation.Nullable; /** Represents the collection of native libraries (.so) to be installed in the APK. */ +@Immutable public final class NativeLibs { public static final NativeLibs EMPTY = new NativeLibs(ImmutableMap.<String, Iterable<Artifact>>of(), null); @@ -105,10 +107,10 @@ public final class NativeLibs { } // Map from architecture (CPU folder to place the library in) to libraries for that CPU - private final Map<String, Iterable<Artifact>> nativeLibs; + private final ImmutableMap<String, Iterable<Artifact>> nativeLibs; private final Artifact nativeLibsName; - private NativeLibs(Map<String, Iterable<Artifact>> nativeLibs, Artifact nativeLibsName) { + private NativeLibs(ImmutableMap<String, Iterable<Artifact>> nativeLibs, Artifact nativeLibsName) { this.nativeLibs = nativeLibs; this.nativeLibsName = nativeLibsName; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java index 8a14507c95..0b7bbfc4fc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java @@ -25,7 +25,7 @@ import javax.annotation.Nullable; * unsigned APKs. */ @Immutable -public class ResourceApk { +public final class ResourceApk { // TODO(bazel-team): The only field that is legitimately nullable is javaSrcJar. The rest is // marked as such due to .fromTransitiveResources(). It seems like there should be a better way // to do this. diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java index dfd4df3f79..f8309de42e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.cmdline.Label; 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.rules.android.AndroidResourcesProvider.ResourceContainer; @@ -32,7 +33,8 @@ import com.google.devtools.build.lib.rules.android.AndroidResourcesProvider.Reso * library is included in both the transitive and direct dependencies, it will appear twice. This * requires consumers to manage duplicated resources gracefully. */ -public class ResourceDependencies { +@Immutable +public final class ResourceDependencies { /** * Contains all the transitive resources that are not generated by the direct ancestors of the * current rule. diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java b/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java index 0adf00840d..6ddf13da19 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.common.options.EnumConverter; import com.google.devtools.common.options.Option; @@ -33,7 +34,8 @@ import java.util.List; * An action that writes the a parameter file to {@code incremental_install.py} based on the command * line arguments to {@code blaze mobile-install}. */ -public class WriteAdbArgsAction extends AbstractFileWriteAction { +@Immutable // note that it accesses data non-hermetically during the execution phase +public final class WriteAdbArgsAction extends AbstractFileWriteAction { private static final String GUID = "16720416-3c01-4b0a-a543-ead7e563a1ca"; /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java index 651eed48ef..aa365d4b1b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java @@ -20,7 +20,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; * Structure for C++ module maps. Stores the name of the module and a .cppmap artifact. */ @Immutable -public class CppModuleMap { +public final class CppModuleMap { // NOTE: If you add a field here, you'll likely need to update CppModuleMapAction.computeKey(). private final Artifact artifact; private final String name; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java index 71e491d3e1..3fea2c4c0a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.PathFragment; @@ -36,7 +37,8 @@ import java.util.List; * Creates C++ module map artifact genfiles. These are then passed to Clang to * do dependency checking. */ -public class CppModuleMapAction extends AbstractFileWriteAction { +@Immutable +public final class CppModuleMapAction extends AbstractFileWriteAction { private static final String GUID = "4f407081-1951-40c1-befc-d6b4daff5de3"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java index b0151eb8b4..1edef180e3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.WorkspaceStatusAction; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +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; @@ -38,6 +39,7 @@ import java.util.Map; * An action that creates a C++ header containing the build information in the * form of #define directives. */ +@Immutable public final class WriteBuildInfoHeaderAction extends AbstractFileWriteAction { private static final String GUID = "b0798174-1352-4a54-854a-9785aaea491b"; diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java index bc3f79d61c..817234c1a4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +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; @@ -32,11 +33,12 @@ import java.io.OutputStream; * .xa file for use by an extra action. This can only be done at execution time because actions may * store information only known at execution time into the protocol buffer. */ -public class ExtraActionInfoFileWriteAction extends AbstractFileWriteAction { - private final Action shadowedAction; - +@Immutable // if shadowedAction is immutable +public final class ExtraActionInfoFileWriteAction extends AbstractFileWriteAction { private static final String UUID = "1759f81d-e72e-477d-b182-c4532bdbaeeb"; + private final Action shadowedAction; + ExtraActionInfoFileWriteAction(ActionOwner owner, Artifact extraActionInfoFile, Action shadowedAction) { super(owner, ImmutableList.<Artifact>of(), extraActionInfoFile, false); diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD b/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD index b6c9ddeb2c..46edcfae09 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD @@ -12,6 +12,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:build-base", "//src/main/java/com/google/devtools/build/lib:collect", "//src/main/java/com/google/devtools/build/lib:common", + "//src/main/java/com/google/devtools/build/lib:concurrent", "//src/main/java/com/google/devtools/build/lib:events", "//src/main/java/com/google/devtools/build/lib:packages-internal", "//src/main/java/com/google/devtools/build/lib:util", diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java index b61d6ac59b..3f2cd64703 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java @@ -22,6 +22,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; @@ -37,6 +38,7 @@ import com.google.devtools.build.lib.cmdline.TargetPattern; 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.events.EventHandler; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -82,7 +84,6 @@ import java.io.OutputStream; import java.io.PrintStream; import java.nio.channels.ClosedByInterruptException; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -150,25 +151,7 @@ public class GenQuery implements RuleConfiguredTargetFactory { } ruleContext.registerAction( - new AbstractFileWriteAction( - ruleContext.getActionOwner(), Collections.<Artifact>emptySet(), outputArtifact, false) { - @Override - public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { - return new DeterministicWriter() { - @Override - public void writeOutputFile(OutputStream out) throws IOException { - out.write(result); - } - }; - } - - @Override - protected String computeKey() { - Fingerprint f = new Fingerprint(); - f.addBytes(result); - return f.hexDigestAndReset(); - } - }); + new QueryResultAction(ruleContext.getActionOwner(), outputArtifact, result)); NestedSet<Artifact> filesToBuild = NestedSetBuilder.create(Order.STABLE_ORDER, outputArtifact); return new RuleConfiguredTargetBuilder(ruleContext) @@ -329,6 +312,33 @@ public class GenQuery implements RuleConfiguredTargetFactory { return outputStream.toByteArray(); } + @Immutable // assuming no other reference to result + private static final class QueryResultAction extends AbstractFileWriteAction { + private final byte[] result; + + private QueryResultAction(ActionOwner owner, Artifact output, byte[] result) { + super(owner, ImmutableList.<Artifact>of(), output, /*makeExecutable=*/false); + this.result = result; + } + + @Override + public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) { + return new DeterministicWriter() { + @Override + public void writeOutputFile(OutputStream out) throws IOException { + out.write(result); + } + }; + } + + @Override + protected String computeKey() { + Fingerprint f = new Fingerprint(); + f.addBytes(result); + return f.hexDigestAndReset(); + } + } + /** * Provide target pattern evaluation to the query operations using Skyframe dep lookup. For thread * safety, we must synchronize access to the SkyFunction.Environment. diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java index fd94eab868..00e54eae1b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java @@ -79,7 +79,7 @@ public class TestSupport { String runMemleaks = ruleContext.getFragment(ObjcConfiguration.class).runMemleaks() ? "true" : "false"; - Map<String, String> testEnv = ruleContext.getConfiguration().getTestEnv(); + ImmutableMap<String, String> testEnv = ruleContext.getConfiguration().getTestEnv(); // The substitutions below are common for simulator and lab device. ImmutableList.Builder<Substitution> substitutions = diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java index 20ae43eb27..99626d2118 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.cmdline.Label; 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.util.Fingerprint; import com.google.devtools.build.lib.vfs.PathFragment; @@ -42,12 +43,13 @@ import java.util.List; * Generates baseline (empty) coverage for the given non-test target. */ @VisibleForTesting +@Immutable public final class BaselineCoverageAction extends AbstractFileWriteAction implements NotifyOnActionCacheHit { - private final Iterable<Artifact> instrumentedFiles; + private final NestedSet<Artifact> instrumentedFiles; private BaselineCoverageAction( - ActionOwner owner, Iterable<Artifact> instrumentedFiles, Artifact output) { + ActionOwner owner, NestedSet<Artifact> instrumentedFiles, Artifact output) { super(owner, ImmutableList.<Artifact>of(), output, false); this.instrumentedFiles = instrumentedFiles; } @@ -110,7 +112,8 @@ public final class BaselineCoverageAction extends AbstractFileWriteAction * Returns collection of baseline coverage artifacts associated with the given target. * Will always return 0 or 1 elements. */ - static NestedSet<Artifact> create(RuleContext ruleContext, Iterable<Artifact> instrumentedFiles) { + static NestedSet<Artifact> create( + RuleContext ruleContext, NestedSet<Artifact> instrumentedFiles) { // Baseline coverage artifacts will still go into "testlogs" directory. Artifact coverageData = ruleContext.getPackageRelativeArtifact( new PathFragment(ruleContext.getTarget().getName()).getChild("baseline_coverage.dat"), diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java index 96224f0fb7..4f1884b25f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction; +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.util.RegexFilter; @@ -33,21 +34,20 @@ import java.io.OutputStream; import java.io.OutputStreamWriter; import java.io.Writer; import java.util.Arrays; -import java.util.Collection; /** * Creates instrumented file manifest to list instrumented source files. */ -class InstrumentedFileManifestAction extends AbstractFileWriteAction { - +@Immutable +final class InstrumentedFileManifestAction extends AbstractFileWriteAction { private static final String GUID = "d9ddb800-f9a1-01Da-238d-988311a8475b"; - private final Collection<Artifact> collectedSourceFiles; - private final Collection<Artifact> metadataFiles; + private final ImmutableList<Artifact> collectedSourceFiles; + private final ImmutableList<Artifact> metadataFiles; private final RegexFilter instrumentationFilter; - private InstrumentedFileManifestAction(ActionOwner owner, Collection<Artifact> inputs, - Collection<Artifact> additionalSourceFiles, Collection<Artifact> gcnoFiles, + private InstrumentedFileManifestAction(ActionOwner owner, ImmutableList<Artifact> inputs, + ImmutableList<Artifact> additionalSourceFiles, ImmutableList<Artifact> gcnoFiles, Artifact output, RegexFilter instrumentationFilter) { super(owner, inputs, output, false); this.collectedSourceFiles = additionalSourceFiles; @@ -102,17 +102,14 @@ class InstrumentedFileManifestAction extends AbstractFileWriteAction { * @param metadataFiles *.gcno/*.em files collected by the {@link InstrumentedFilesCollector} * @return instrumented file manifest artifact */ - public static Artifact getInstrumentedFileManifest(final RuleContext ruleContext, - final Collection<Artifact> additionalSourceFiles, final Collection<Artifact> metadataFiles) { + public static Artifact getInstrumentedFileManifest(RuleContext ruleContext, + ImmutableList<Artifact> additionalSourceFiles, ImmutableList<Artifact> metadataFiles) { // Instrumented manifest makes sense only for rules with binary output. Preconditions.checkState(ruleContext.getRule().hasBinaryOutput()); Artifact instrumentedFileManifest = ruleContext.getPackageRelativeArtifact( ruleContext.getTarget().getName() + ".instrumented_files", ruleContext.getConfiguration().getBinDirectory()); - // Instrumented manifest artifact might already exist in case when multiple test - // actions that use slightly different subsets of runfiles set are generated for the same rule. - // So check whether we need to create a new action instance. ImmutableList<Artifact> inputs = ImmutableList.<Artifact>builder() .addAll(additionalSourceFiles) .addAll(metadataFiles) diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java index 856d41d49d..73235cdac2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.common.options.EnumConverter; -import java.util.Collection; import java.util.List; import java.util.Map; @@ -204,7 +203,7 @@ public final class TestActionBuilder { if (collectCodeCoverage) { // Add instrumented file manifest artifact to the list of inputs. This file will contain // exec paths of all source files that should be included into the code coverage output. - Collection<Artifact> metadataFiles = + ImmutableList<Artifact> metadataFiles = ImmutableList.copyOf(instrumentedFiles.getInstrumentationMetadataFiles()); inputsBuilder.addTransitive(NestedSetBuilder.wrap(Order.STABLE_ORDER, metadataFiles)); for (TransitiveInfoCollection dep : diff --git a/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java b/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java index 8484f440ed..950e276de2 100644 --- a/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java +++ b/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java @@ -15,6 +15,7 @@ package com.google.devtools.build.lib.util; import com.google.common.base.Joiner; +import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; @@ -33,7 +34,8 @@ import java.util.regex.PatternSyntaxException; * any of the excluded regex expressions and if it matches at least one * included regex expression. */ -public class RegexFilter implements Serializable { +@Immutable +public final class RegexFilter implements Serializable { private final Pattern inclusionPattern; private final Pattern exclusionPattern; private final int hashCode; |