Contractually document BuildConfiguration.Fragment as immutable and strengthen

the immutability interfaces of existing implementations.

Eventually we want all implementations to comply, but right now CppConfiguration
is a glaring exception due to FDO/LIPO support.

We don't want more exceptions to arise.

This is prep work for pre-trimming ConfigurationFragment.key's BuildOptions input
to just the options needed by the fragment. That implies fragments can be shared across configurations, so that needs to be safe.

--
MOS_MIGRATED_REVID=113408041

12 files changed, 109 insertions, 79 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 85da06b3f7..61fac245e3 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -483,6 +483,7 @@ java_library(
         ":build-info",
         ":collect",
         ":common",
+        ":concurrent",
         ":events",
         ":ideinfo",
         ":java-rules",
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 9cd86248f7..985d5e476c 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
@@ -121,6 +121,10 @@ public final class BuildConfiguration {
 
   /**
    * An interface for language-specific configurations.
+   *
+   * <p>All implementations must be immutable and communicate this as clearly as possible
+   * (e.g. declare {@link ImmutableList} signatures on their interfaces vs. {@link List}).
+   * This is because fragment instances may be shared across configurations.
    */
   public abstract static class Fragment {
     /**
@@ -155,6 +159,8 @@ public final class BuildConfiguration {
      * analysis. During the analysis phase disk I/O operations are disallowed.
      *
      * <p>This hook is called for all configurations after the loading phase is complete.
+     *
+     * <p>Do not use this method to change your fragment's state.
      */
     @SuppressWarnings("unused")
     public void prepareHook(Path execPath, ArtifactFactory artifactFactory,
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
index f868ccc0bd..e176520bae 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelConfiguration.java
@@ -22,12 +22,14 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
  * Bazel-specific configuration fragment.
  */
+@Immutable
 public class BazelConfiguration extends Fragment {
   /**
    * Loader for Bazel-specific settings.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
index a10d9593c6..fa5e21942a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonConfiguration.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.common.options.Option;
 
 import javax.annotation.Nullable;
@@ -29,6 +30,7 @@ import javax.annotation.Nullable;
 /**
  * Bazel-specific Python configuration.
  */
+@Immutable
 public class BazelPythonConfiguration extends BuildConfiguration.Fragment {
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index 564a344325..57423dfbdb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactor
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.Attribute.SplitTransition;
 import com.google.devtools.common.options.Converters;
 import com.google.devtools.common.options.EnumConverter;
@@ -39,6 +40,7 @@ import java.util.List;
 /**
  * Configuration fragment for Android rules.
  */
+@Immutable
 public class AndroidConfiguration extends BuildConfiguration.Fragment {
 
   /** Converter for --android_crosstool_top. */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
index 01265a618f..c7e56ea4da 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/apple/AppleConfiguration.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.apple;
 
 import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -24,10 +25,10 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactor
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions.AppleBitcodeMode;
 import com.google.devtools.build.lib.util.Preconditions;
 
-import java.util.List;
 import java.util.Map;
 
 import javax.annotation.Nullable;
@@ -35,6 +36,7 @@ import javax.annotation.Nullable;
 /**
  * A configuration containing flags required for Apple platforms and tools.
  */
+@Immutable
 public class AppleConfiguration extends BuildConfiguration.Fragment {
   public static final String XCODE_VERSION_ENV_NAME = "XCODE_VERSION_OVERRIDE";
   /**
@@ -53,7 +55,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
   private final DottedVersion iosSdkVersion;
   private final String iosCpu;
   private final Optional<DottedVersion> xcodeVersion;
-  private final List<String> iosMultiCpus;
+  private final ImmutableList<String> iosMultiCpus;
   private final AppleBitcodeMode bitcodeMode;
   private final Label xcodeConfigLabel;
   @Nullable private final Label defaultProvisioningProfileLabel;
@@ -63,7 +65,8 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
     this.iosSdkVersion = Preconditions.checkNotNull(appleOptions.iosSdkVersion, "iosSdkVersion");
     this.xcodeVersion = Preconditions.checkNotNull(xcodeVersionOverride);
     this.iosCpu = Preconditions.checkNotNull(appleOptions.iosCpu, "iosCpu");
-    this.iosMultiCpus = Preconditions.checkNotNull(appleOptions.iosMultiCpus, "iosMultiCpus");
+    this.iosMultiCpus = ImmutableList.copyOf(
+        Preconditions.checkNotNull(appleOptions.iosMultiCpus, "iosMultiCpus"));
     this.bitcodeMode = appleOptions.appleBitcodeMode;
     this.xcodeConfigLabel =
         Preconditions.checkNotNull(appleOptions.xcodeVersionConfig, "xcodeConfigLabel");
@@ -175,7 +178,7 @@ public class AppleConfiguration extends BuildConfiguration.Fragment {
    * List of all CPUs that this invocation is being built for. Different from {@link #getIosCpu()}
    * which is the specific CPU <b>this target</b> is being built for.
    */
-  public List<String> getIosMultiCpus() {
+  public ImmutableList<String> getIosMultiCpus() {
     return iosMultiCpus;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 71045e4fd0..9a81fec0db 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
@@ -63,7 +63,6 @@ import com.google.protobuf.TextFormat.ParseException;
 import java.io.IOException;
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.List;
@@ -212,18 +211,19 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   /**
    * Represents an optional flag that can be toggled using the package features mechanism.
    */
+  @Immutable
   @VisibleForTesting
   static class OptionalFlag implements Serializable {
     private final String name;
-    private final List<String> flags;
+    private final ImmutableList<String> flags;
 
     @VisibleForTesting
-    OptionalFlag(String name, List<String> flags) {
+    OptionalFlag(String name, ImmutableList<String> flags) {
       this.name = name;
       this.flags = flags;
     }
 
-    private List<String> getFlags() {
+    private ImmutableList<String> getFlags() {
       return flags;
     }
 
@@ -232,23 +232,24 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     }
   }
 
+  @Immutable
   @VisibleForTesting
   static class FlagList implements Serializable {
-    private List<String> prefixFlags;
-    private List<OptionalFlag> optionalFlags;
-    private List<String> suffixFlags;
+    private final ImmutableList<String> prefixFlags;
+    private final ImmutableList<OptionalFlag> optionalFlags;
+    private final ImmutableList<String> suffixFlags;
 
     @VisibleForTesting
-    FlagList(List<String> prefixFlags,
-                      List<OptionalFlag> optionalFlags,
-                      List<String> suffixFlags) {
+    FlagList(ImmutableList<String> prefixFlags,
+        ImmutableList<OptionalFlag> optionalFlags,
+        ImmutableList<String> suffixFlags) {
       this.prefixFlags = prefixFlags;
       this.optionalFlags = optionalFlags;
       this.suffixFlags = suffixFlags;
     }
 
     @VisibleForTesting
-    List<String> evaluate(Iterable<String> features) {
+    ImmutableList<String> evaluate(Iterable<String> features) {
       ImmutableSet<String> featureSet = ImmutableSet.copyOf(features);
       ImmutableList.Builder<String> result = ImmutableList.builder();
       result.addAll(prefixFlags);
@@ -312,7 +313,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   private final PathFragment ldExecutable;
 
   // Only used during construction.
-  private final List<String> commonLinkOptions;
+  private final ImmutableList<String> commonLinkOptions;
   private final ListMultimap<CompilationMode, String> linkOptionsFromCompilationMode;
   private final ListMultimap<LipoMode, String> linkOptionsFromLipoMode;
   private final ListMultimap<LinkingMode, String> linkOptionsFromLinkingMode;
@@ -320,23 +321,23 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   private final FlagList compilerFlags;
   private final FlagList cxxFlags;
   private final FlagList unfilteredCompilerFlags;
-  private final List<String> cOptions;
+  private final ImmutableList<String> cOptions;
 
-  private FlagList fullyStaticLinkFlags;
-  private FlagList mostlyStaticLinkFlags;
-  private FlagList mostlyStaticSharedLinkFlags;
-  private FlagList dynamicLinkFlags;
-  private FlagList dynamicLibraryLinkFlags;
-  private final List<String> testOnlyLinkFlags;
+  private final FlagList fullyStaticLinkFlags;
+  private final FlagList mostlyStaticLinkFlags;
+  private final FlagList mostlyStaticSharedLinkFlags;
+  private final FlagList dynamicLinkFlags;
+  private final FlagList dynamicLibraryLinkFlags;
+  private final ImmutableList<String> testOnlyLinkFlags;
 
-  private final List<String> linkOptions;
+  private final ImmutableList<String> linkOptions;
 
-  private final List<String> objcopyOptions;
-  private final List<String> ldOptions;
-  private final List<String> arOptions;
-  private final List<String> arThinArchivesOptions;
+  private final ImmutableList<String> objcopyOptions;
+  private final ImmutableList<String> ldOptions;
+  private final ImmutableList<String> arOptions;
+  private final ImmutableList<String> arThinArchivesOptions;
 
-  private final Map<String, String> additionalMakeVariables;
+  private final ImmutableMap<String, String> additionalMakeVariables;
 
   private final CppOptions cppOptions;
 
@@ -515,7 +516,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     dynamicLibraryLinkFlags = new FlagList(
         ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()),
         convertOptionalOptions(toolchain.getOptionalDynamicLibraryLinkerFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
     this.objcopyOptions = ImmutableList.copyOf(toolchain.getObjcopyEmbedFlagList());
     this.ldOptions = ImmutableList.copyOf(toolchain.getLdEmbedFlagList());
     this.arOptions = copyOrDefaultIfEmpty(toolchain.getArFlagList(), "rcsD");
@@ -582,7 +583,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     unfilteredCompilerFlags = new FlagList(
         unfilteredCoptsBuilder.build(),
         convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
 
     ImmutableList.Builder<String> linkoptsBuilder = ImmutableList.builder();
     linkoptsBuilder.addAll(cppOptions.linkoptList);
@@ -606,7 +607,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     this.compilerFlags = new FlagList(
         coptsBuilder.build(),
         convertOptionalOptions(toolchain.getOptionalCompilerFlagList()),
-        cppOptions.coptList);
+        ImmutableList.copyOf(cppOptions.coptList));
 
     this.cOptions = ImmutableList.copyOf(cppOptions.conlyoptList);
 
@@ -618,7 +619,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     this.cxxFlags = new FlagList(
         cxxOptsBuilder.build(),
         convertOptionalOptions(toolchain.getOptionalCxxFlagList()),
-        cppOptions.cxxoptList);
+        ImmutableList.copyOf(cppOptions.cxxoptList));
 
     this.ldExecutable = getToolPathFragment(CppConfiguration.Tool.LD);
 
@@ -631,22 +632,22 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
         configureLinkerOptions(compilationMode, lipoMode, LinkingMode.FULLY_STATIC,
                                ldExecutable, stripBinaries),
         convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
     mostlyStaticLinkFlags = new FlagList(
         configureLinkerOptions(compilationMode, lipoMode, LinkingMode.MOSTLY_STATIC,
                                ldExecutable, stripBinaries),
         convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
     mostlyStaticSharedLinkFlags = new FlagList(
         configureLinkerOptions(compilationMode, lipoMode,
                                LinkingMode.MOSTLY_STATIC_LIBRARIES, ldExecutable, stripBinaries),
         convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
     dynamicLinkFlags = new FlagList(
         configureLinkerOptions(compilationMode, lipoMode, LinkingMode.DYNAMIC,
                                ldExecutable, stripBinaries),
         convertOptionalOptions(toolchain.getOptionalLinkerFlagList()),
-        Collections.<String>emptyList());
+        ImmutableList.<String>of());
     testOnlyLinkFlags = ImmutableList.copyOf(toolchain.getTestOnlyLinkerFlagList());
 
     Map<String, String> makeVariablesBuilder = new HashMap<>();
@@ -665,10 +666,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
     this.additionalMakeVariables = ImmutableMap.copyOf(makeVariablesBuilder);
   }
 
-  private List<OptionalFlag> convertOptionalOptions(
+  private ImmutableList<OptionalFlag> convertOptionalOptions(
           List<CrosstoolConfig.CToolchain.OptionalFlag> optionalFlagList)
       throws IllegalArgumentException {
-    List<OptionalFlag> result = new ArrayList<>();
+    ImmutableList.Builder<OptionalFlag> result = ImmutableList.builder();
 
     for (CrosstoolConfig.CToolchain.OptionalFlag crosstoolOptionalFlag : optionalFlagList) {
       String name = crosstoolOptionalFlag.getDefaultSettingName();
@@ -677,7 +678,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
           ImmutableList.copyOf(crosstoolOptionalFlag.getFlagList())));
     }
 
-    return result;
+    return result.build();
   }
 
   // TODO(bazel-team): Remove this once bazel supports all crosstool flags through
@@ -928,7 +929,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   }
 
   @VisibleForTesting
-  List<String> configureLinkerOptions(
+  ImmutableList<String> configureLinkerOptions(
       CompilationMode compilationMode, LipoMode lipoMode, LinkingMode linkingMode,
       PathFragment ldExecutable, boolean stripBinaries) {
     List<String> result = new ArrayList<>();
@@ -1155,7 +1156,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   /**
    * Returns the ar flags to be used.
    */
-  public List<String> getArFlags(boolean thinArchives) {
+  public ImmutableList<String> getArFlags(boolean thinArchives) {
     return thinArchives ? arThinArchivesOptions : arOptions;
   }
 
@@ -1214,7 +1215,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "There may be additional C-specific or C++-specific options that should be used, "
             + "in addition to the ones returned by this method"
   )
-  public List<String> getCompilerOptions(Iterable<String> features) {
+  public ImmutableList<String> getCompilerOptions(Iterable<String> features) {
     return compilerFlags.evaluate(features);
   }
 
@@ -1227,7 +1228,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
       doc = "Returns the list of additional C-specific options to use for compiling C. "
       + "These should be go on the command line after the common options returned by "
       + "<code>compiler_options</code>")
-  public List<String> getCOptions() {
+  public ImmutableList<String> getCOptions() {
     return cOptions;
   }
 
@@ -1243,7 +1244,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "These should be go on the command line after the common options returned by "
             + "<code>compiler_options</code>"
   )
-  public List<String> getCxxOptions(Iterable<String> features) {
+  public ImmutableList<String> getCxxOptions(Iterable<String> features) {
     return cxxFlags.evaluate(features);
   }
 
@@ -1257,7 +1258,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
         "Returns the default list of options which cannot be filtered by BUILD "
             + "rules. These should be appended to the command line after filtering."
   )
-  public List<String> getUnfilteredCompilerOptions(Iterable<String> features) {
+  public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) {
     return unfilteredCompilerFlags.evaluate(features);
   }
 
@@ -1271,7 +1272,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   @SkylarkCallable(name = "link_options", structField = true,
       doc = "Returns the set of command-line linker options, including any flags "
       + "inferred from the command-line options.")
-  public List<String> getLinkOptions() {
+  public ImmutableList<String> getLinkOptions() {
     return linkOptions;
   }
 
@@ -1290,7 +1291,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "outputs. Does not include command-line options passed via --linkopt or "
             + "--linkopts."
   )
-  public List<String> getFullyStaticLinkOptions(Iterable<String> features, Boolean sharedLib) {
+  public ImmutableList<String> getFullyStaticLinkOptions(Iterable<String> features,
+      Boolean sharedLib) {
     if (sharedLib) {
       return getSharedLibraryLinkOptions(mostlyStaticLinkFlags, features);
     } else {
@@ -1313,7 +1315,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "outputs. Does not include command-line options passed via --linkopt or "
             + "--linkopts."
   )
-  public List<String> getMostlyStaticLinkOptions(Iterable<String> features, Boolean sharedLib) {
+  public ImmutableList<String> getMostlyStaticLinkOptions(Iterable<String> features,
+      Boolean sharedLib) {
     if (sharedLib) {
       return getSharedLibraryLinkOptions(
           supportsEmbeddedRuntimes ? mostlyStaticSharedLinkFlags : dynamicLinkFlags,
@@ -1338,7 +1341,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
             + "fully or mostly statically linked. Does not include command-line options "
             + "passed via --linkopt or --linkopts."
   )
-  public List<String> getDynamicLinkOptions(Iterable<String> features, Boolean sharedLib) {
+  public ImmutableList<String> getDynamicLinkOptions(Iterable<String> features, Boolean sharedLib) {
     if (sharedLib) {
       return getSharedLibraryLinkOptions(dynamicLinkFlags, features);
     } else {
@@ -1350,7 +1353,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * Returns link options for the specified flag list, combined with universal options
    * for all shared libraries (regardless of link staticness).
    */
-  private List<String> getSharedLibraryLinkOptions(FlagList flags, Iterable<String> features) {
+  private ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags,
+      Iterable<String> features) {
     return ImmutableList.<String>builder()
         .addAll(flags.evaluate(features))
         .addAll(dynamicLibraryLinkFlags.evaluate(features))
@@ -1361,7 +1365,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * Returns test-only link options such that certain test-specific features can be configured
    * separately (e.g. lazy binding).
    */
-  public List<String> getTestOnlyLinkOptions() {
+  public ImmutableList<String> getTestOnlyLinkOptions() {
     return testOnlyLinkFlags;
   }
 
@@ -1371,7 +1375,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * binary files to object files, or {@code null} if this operation is not
    * supported.
    */
-  public List<String> getObjCopyOptionsForEmbedding() {
+  public ImmutableList<String> getObjCopyOptionsForEmbedding() {
     return objcopyOptions;
   }
 
@@ -1380,7 +1384,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * binary files to object files, or {@code null} if this operation is not
    * supported.
    */
-  public List<String> getLdOptionsForEmbedding() {
+  public ImmutableList<String> getLdOptionsForEmbedding() {
     return ldOptions;
   }
 
@@ -1393,7 +1397,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * though the entry may be an empty string.
    */
   @VisibleForTesting
-  public Map<String, String> getAdditionalMakeVariables() {
+  public ImmutableMap<String, String> getAdditionalMakeVariables() {
     return additionalMakeVariables;
   }
 
@@ -1483,8 +1487,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * Returns the additional options to pass to strip when generating a
    * {@code <name>.stripped} binary by this build.
    */
-  public List<String> getStripOpts() {
-    return cppOptions.stripoptList;
+  public ImmutableList<String> getStripOpts() {
+    return ImmutableList.copyOf(cppOptions.stripoptList);
   }
 
   /**
@@ -1498,8 +1502,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
    * Returns the {@link PerLabelOptions} to apply to the gcc command line, if
    * the label of the compiled file matches the regular expression.
    */
-  public List<PerLabelOptions> getPerFileCopts() {
-    return cppOptions.perFileCopts;
+  public ImmutableList<PerLabelOptions> getPerFileCopts() {
+    return ImmutableList.copyOf(cppOptions.perFileCopts);
   }
 
   public Label getLipoContextLabel() {
@@ -1516,8 +1520,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment {
   /**
    * Returns the extra warnings enabled for C compilation.
    */
-  public List<String> getCWarns() {
-    return cppOptions.cWarns;
+  public ImmutableList<String> getCWarns() {
+    return ImmutableList.copyOf(cppOptions.cWarns);
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/J2ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/J2ObjcConfiguration.java
index 51db8897d2..d71d9c0a62 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/J2ObjcConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/J2ObjcConfiguration.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 
@@ -32,6 +33,7 @@ import java.util.Set;
  * This configuration fragment is used by Java rules that can be transpiled
  * (specifically, J2ObjCAspects thereof).
  */
+@Immutable
 public class J2ObjcConfiguration extends Fragment {
   /**
    * Always-on flags for J2ObjC translation. These flags will always be used for invoking the J2ObjC
@@ -77,7 +79,7 @@ public class J2ObjcConfiguration extends Fragment {
     }
   }
 
-  private final Set<String> translationFlags;
+  private final ImmutableSet<String> translationFlags;
   private final boolean removeDeadCode;
 
   J2ObjcConfiguration(J2ObjcCommandLineOptions j2ObjcOptions) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
index 2983140fd0..8a3c6232bc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
@@ -195,7 +195,7 @@ public final class JavaConfiguration extends Fragment {
       doc = "The default flags for the Java compiler.")
   // TODO(bazel-team): this is the command-line passed options, we should remove from skylark
   // probably.
-  public List<String> getDefaultJavacFlags() {
+  public ImmutableList<String> getDefaultJavacFlags() {
     return commandLineJavacFlags;
   }
 
@@ -255,15 +255,15 @@ public final class JavaConfiguration extends Fragment {
   /**
    * Returns the extra warnings enabled for Java compilation.
    */
-  public List<String> getJavaWarns() {
+  public ImmutableList<String> getJavaWarns() {
     return javaWarns;
   }
 
-  public List<String> getDefaultJvmFlags() {
+  public ImmutableList<String> getDefaultJvmFlags() {
     return defaultJvmFlags;
   }
 
-  public List<String> getCheckedConstraints() {
+  public ImmutableList<String> getCheckedConstraints() {
     return checkedConstraints;
   }
 
@@ -297,7 +297,7 @@ public final class JavaConfiguration extends Fragment {
     return javacExtdir;
   }
 
-  public List<String> getJavacOpts() {
+  public ImmutableList<String> getJavacOpts() {
     return javacOpts;
   }
 
@@ -312,14 +312,14 @@ public final class JavaConfiguration extends Fragment {
   /**
    * Returns all labels provided with --extra_proguard_specs.
    */
-  public List<Label> getExtraProguardSpecs() {
+  public ImmutableList<Label> getExtraProguardSpecs() {
     return extraProguardSpecs;
   }
 
   /**
    * Returns the raw translation targets.
    */
-  public List<Label> getTranslationTargets() {
+  public ImmutableList<Label> getTranslationTargets() {
     return translationTargets;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
index 4e24f66870..20337639ef 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcConfiguration.java
@@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.rules.apple.DottedVersion;
 import com.google.devtools.build.lib.rules.objc.ReleaseBundlingSupport.SplitArchTransition.ConfigurationDistinguisher;
 import com.google.devtools.build.lib.util.Preconditions;
@@ -34,6 +35,7 @@ import javax.annotation.Nullable;
 /**
  * A compiler configuration containing flags required for Objective-C compilation.
  */
+@Immutable
 public class ObjcConfiguration extends BuildConfiguration.Fragment {
   @VisibleForTesting
   static final ImmutableList<String> DBG_COPTS = ImmutableList.of("-O0", "-DDEBUG=1",
@@ -50,10 +52,10 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
   private final String iosSimulatorDevice;
   private final boolean generateDebugSymbols;
   private final boolean runMemleaks;
-  private final List<String> copts;
+  private final ImmutableList<String> copts;
   private final CompilationMode compilationMode;
   private final String iosSplitCpu;
-  private final List<String> fastbuildOptions;
+  private final ImmutableList<String> fastbuildOptions;
   private final boolean enableBinaryStripping;
   private final boolean moduleMapsEnabled;
   private final ConfigurationDistinguisher configurationDistinguisher;
@@ -124,7 +126,7 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
   /**
    * Returns the default set of clang options for the current compilation mode.
    */
-  public List<String> getCoptsForCompilationMode() {
+  public ImmutableList<String> getCoptsForCompilationMode() {
     switch (compilationMode) {
       case DBG:
         return DBG_COPTS;
@@ -141,7 +143,7 @@ public class ObjcConfiguration extends BuildConfiguration.Fragment {
    * Returns options passed to (Apple) clang when compiling Objective C. These options should be
    * applied after any default options but before options specified in the attributes of the rule.
    */
-  public List<String> getCopts() {
+  public ImmutableList<String> getCopts() {
     return copts;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
index 86a16be99b..db2bb3d1b7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
@@ -14,6 +14,7 @@
 
 package com.google.devtools.build.lib.rules.proto;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -21,6 +22,7 @@ import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.common.options.Option;
 
 import java.util.List;
@@ -28,6 +30,7 @@ import java.util.List;
 /**
  * Configuration for Protocol Buffer Libraries.
  */
+@Immutable
 public class ProtoConfiguration extends Fragment {
 
   /**
@@ -71,23 +74,24 @@ public class ProtoConfiguration extends Fragment {
     }
   }
 
-  private final Options options;
+  private final boolean experimentalProtoExtraActions;
+  private final ImmutableList<String> protocOpts;
 
   public ProtoConfiguration(Options options) {
-    this.options = options;
+    this.experimentalProtoExtraActions = options.experimentalProtoExtraActions;
+    this.protocOpts = ImmutableList.copyOf(options.protocOpts);
   }
 
-  public List<String> protocOpts() {
-    return options.protocOpts;
+  public ImmutableList<String> protocOpts() {
+    return protocOpts;
   }
 
-
   /**
    * Returns true if we will run extra actions for actions that are not run by default. If this
    * is enabled, e.g. all extra_actions for alternative api-versions or language-flavours of a
    * proto_library target are run.
    */
   public boolean runExperimentalProtoExtraActions() {
-    return options.experimentalProtoExtraActions;
+    return experimentalProtoExtraActions;
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
index 9ffcefa4e7..3898906374 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
@@ -14,11 +14,13 @@
 package com.google.devtools.build.lib.rules.python;
 
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 
 /**
  * The configuration fragment containing information about the various pieces of infrastructure
  * needed to run Python compilations.
  */
+@Immutable
 public class PythonConfiguration extends BuildConfiguration.Fragment {
   private final boolean ignorePythonVersionAttribute;
   private final PythonVersion defaultPythonVersion;