Open up visibility of BuildConfiguration$OutputDirectory for serialization and remove unused repositoryName field from BuildConfiguration.

PiperOrigin-RevId: 198723339

1 files changed, 65 insertions, 67 deletions

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 1aaa58c326..e0a42c8d5e 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
@@ -973,7 +973,6 @@ public class BuildConfiguration implements BuildConfigurationApi {
   private final FragmentClassSet fragmentClassSet;
 
   private final ImmutableMap<String, Class<? extends Fragment>> skylarkVisibleFragments;
-  private final String repositoryName;
   private final RepositoryName mainRepositoryName;
   private final ImmutableSet<String> reservedActionMnemonics;
   private CommandLineLimits commandLineLimits;
@@ -983,42 +982,38 @@ public class BuildConfiguration implements BuildConfigurationApi {
    *
    * <p>The computation of the output directory should be a non-injective mapping from
    * BuildConfiguration instances to strings. The result should identify the aspects of the
-   * configuration that should be reflected in the output file names.  Furthermore the
-   * returned string must not contain shell metacharacters.
+   * configuration that should be reflected in the output file names. Furthermore the returned
+   * string must not contain shell metacharacters.
    *
-   * <p>For configuration settings which are NOT part of the output directory name,
-   * rebuilding with a different value of such a setting will build in
-   * the same output directory.  This means that any actions whose
-   * keys (see Action.getKey()) have changed will be rerun.  That
-   * may result in a lot of recompilation.
+   * <p>For configuration settings which are NOT part of the output directory name, rebuilding with
+   * a different value of such a setting will build in the same output directory. This means that
+   * any actions whose keys (see Action.getKey()) have changed will be rerun. That may result in a
+   * lot of recompilation.
    *
-   * <p>For configuration settings which ARE part of the output directory name,
-   * rebuilding with a different value of such a setting will rebuild
-   * in a different output directory; this will result in higher disk
-   * usage and more work the <i>first</i> time you rebuild with a different
-   * setting, but will result in less work if you regularly switch
-   * back and forth between different settings.
+   * <p>For configuration settings which ARE part of the output directory name, rebuilding with a
+   * different value of such a setting will rebuild in a different output directory; this will
+   * result in higher disk usage and more work the <i>first</i> time you rebuild with a different
+   * setting, but will result in less work if you regularly switch back and forth between different
+   * settings.
    *
-   * <p>With one important exception, it's sound to choose any subset of the
-   * config's components for this string, it just alters the dimensionality
-   * of the cache.  In other words, it's a trade-off on the "injectiveness"
-   * scale: at one extreme (output directory name contains all data in the config, and is
-   * thus injective) you get extremely precise caching (no competition for the
-   * same output-file locations) but you have to rebuild for even the
-   * slightest change in configuration.  At the other extreme (the output
-   * (directory name is a constant) you have very high competition for
-   * output-file locations, but if a slight change in configuration doesn't
-   * affect a particular build step, you're guaranteed not to have to
-   * rebuild it. The important exception has to do with multiple configurations: every
-   * configuration in the build must have a different output directory name so that
-   * their artifacts do not conflict.
+   * <p>With one important exception, it's sound to choose any subset of the config's components for
+   * this string, it just alters the dimensionality of the cache. In other words, it's a trade-off
+   * on the "injectiveness" scale: at one extreme (output directory name contains all data in the
+   * config, and is thus injective) you get extremely precise caching (no competition for the same
+   * output-file locations) but you have to rebuild for even the slightest change in configuration.
+   * At the other extreme (the output (directory name is a constant) you have very high competition
+   * for output-file locations, but if a slight change in configuration doesn't affect a particular
+   * build step, you're guaranteed not to have to rebuild it. The important exception has to do with
+   * multiple configurations: every configuration in the build must have a different output
+   * directory name so that their artifacts do not conflict.
    *
-   * <p>The host configuration is special-cased: in order to guarantee that its output directory
-   * is always separate from that of the target configuration, we simply pin it to "host". We do
-   * this so that the build works even if the two configurations are too close (which is common)
-   * and so that the path of artifacts in the host configuration is a bit more readable.
+   * <p>The host configuration is special-cased: in order to guarantee that its output directory is
+   * always separate from that of the target configuration, we simply pin it to "host". We do this
+   * so that the build works even if the two configurations are too close (which is common) and so
+   * that the path of artifacts in the host configuration is a bit more readable.
    */
-  private enum OutputDirectory {
+  @AutoCodec.VisibleForSerialization
+  public enum OutputDirectory {
     BIN("bin"),
     GENFILES("genfiles"),
     MIDDLEMAN(true),
@@ -1045,11 +1040,9 @@ public class BuildConfiguration implements BuildConfigurationApi {
       this.middleman = false;
     }
 
-    ArtifactRoot getRoot(
-        RepositoryName repositoryName,
-        String outputDirName,
-        BlazeDirectories directories,
-        RepositoryName mainRepositoryName) {
+    @AutoCodec.VisibleForSerialization
+    public ArtifactRoot getRoot(
+        String outputDirName, BlazeDirectories directories, RepositoryName mainRepositoryName) {
       // e.g., execroot/repo1
       Path execRoot = directories.getExecRoot(mainRepositoryName.strippedName());
       // e.g., execroot/repo1/bazel-out/config/bin
@@ -1223,18 +1216,37 @@ public class BuildConfiguration implements BuildConfigurationApi {
       ImmutableSet<String> reservedActionMnemonics,
       ActionEnvironment actionEnvironment,
       String repositoryName) {
+    this(
+        directories,
+        fragmentsMap,
+        buildOptions,
+        buildOptionsDiff,
+        reservedActionMnemonics,
+        actionEnvironment,
+        RepositoryName.createFromValidStrippedName(repositoryName));
+  }
+
+  @AutoCodec.VisibleForSerialization
+  @AutoCodec.Instantiator
+  BuildConfiguration(
+      BlazeDirectories directories,
+      Map<Class<? extends Fragment>, Fragment> fragmentsMap,
+      BuildOptions buildOptions,
+      BuildOptions.OptionsDiffForReconstruction buildOptionsDiff,
+      ImmutableSet<String> reservedActionMnemonics,
+      ActionEnvironment actionEnvironment,
+      RepositoryName mainRepositoryName) {
     this.directories = directories;
     this.fragments = makeFragmentsMap(fragmentsMap);
     this.fragmentClassSet = FragmentClassSet.of(this.fragments.keySet());
 
     this.skylarkVisibleFragments = buildIndexOfSkylarkVisibleFragments();
-    this.repositoryName = repositoryName;
     this.buildOptions = buildOptions.clone();
     this.buildOptionsDiff = buildOptionsDiff;
     this.actionsEnabled = buildOptions.enableActions();
     this.options = buildOptions.get(Options.class);
     this.separateGenfilesDirectory = options.separateGenfilesDirectory;
-    this.mainRepositoryName = RepositoryName.createFromValidStrippedName(repositoryName);
+    this.mainRepositoryName = mainRepositoryName;
 
     // We can't use an ImmutableMap.Builder here; we need the ability to add entries with keys that
     // are already in the map so that the same define can be specified on the command line twice,
@@ -1250,26 +1262,19 @@ public class BuildConfiguration implements BuildConfigurationApi {
         ? options.outputDirectoryName : mnemonic;
 
     this.outputDirectoryForMainRepository =
-        OutputDirectory.OUTPUT.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.OUTPUT.getRoot(outputDirName, directories, mainRepositoryName);
     this.binDirectoryForMainRepository =
-        OutputDirectory.BIN.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.BIN.getRoot(outputDirName, directories, mainRepositoryName);
     this.includeDirectoryForMainRepository =
-        OutputDirectory.INCLUDE.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.INCLUDE.getRoot(outputDirName, directories, mainRepositoryName);
     this.genfilesDirectoryForMainRepository =
-        OutputDirectory.GENFILES.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.GENFILES.getRoot(outputDirName, directories, mainRepositoryName);
     this.coverageDirectoryForMainRepository =
-        OutputDirectory.COVERAGE.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.COVERAGE.getRoot(outputDirName, directories, mainRepositoryName);
     this.testlogsDirectoryForMainRepository =
-        OutputDirectory.TESTLOGS.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.TESTLOGS.getRoot(outputDirName, directories, mainRepositoryName);
     this.middlemanDirectoryForMainRepository =
-        OutputDirectory.MIDDLEMAN.getRoot(
-            RepositoryName.MAIN, outputDirName, directories, mainRepositoryName);
+        OutputDirectory.MIDDLEMAN.getRoot(outputDirName, directories, mainRepositoryName);
 
     this.actionEnv = actionEnvironment;
 
@@ -1405,8 +1410,7 @@ public class BuildConfiguration implements BuildConfigurationApi {
   public ArtifactRoot getOutputDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? outputDirectoryForMainRepository
-        : OutputDirectory.OUTPUT.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.OUTPUT.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   @Override
@@ -1428,8 +1432,7 @@ public class BuildConfiguration implements BuildConfigurationApi {
   public ArtifactRoot getBinDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? binDirectoryForMainRepository
-        : OutputDirectory.BIN.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.BIN.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   /**
@@ -1443,8 +1446,7 @@ public class BuildConfiguration implements BuildConfigurationApi {
   public ArtifactRoot getIncludeDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? includeDirectoryForMainRepository
-        : OutputDirectory.INCLUDE.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.INCLUDE.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   @Override
@@ -1464,8 +1466,7 @@ public class BuildConfiguration implements BuildConfigurationApi {
 
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? genfilesDirectoryForMainRepository
-        : OutputDirectory.GENFILES.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.GENFILES.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   /**
@@ -1476,16 +1477,14 @@ public class BuildConfiguration implements BuildConfigurationApi {
   public ArtifactRoot getCoverageMetadataDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? coverageDirectoryForMainRepository
-        : OutputDirectory.COVERAGE.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.COVERAGE.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   /** Returns the testlogs directory for this build configuration. */
   public ArtifactRoot getTestLogsDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? testlogsDirectoryForMainRepository
-        : OutputDirectory.TESTLOGS.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.TESTLOGS.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   /**
@@ -1511,8 +1510,7 @@ public class BuildConfiguration implements BuildConfigurationApi {
   public ArtifactRoot getMiddlemanDirectory(RepositoryName repositoryName) {
     return repositoryName.isMain() || repositoryName.equals(mainRepositoryName)
         ? middlemanDirectoryForMainRepository
-        : OutputDirectory.MIDDLEMAN.getRoot(
-            repositoryName, outputDirName, directories, mainRepositoryName);
+        : OutputDirectory.MIDDLEMAN.getRoot(outputDirName, directories, mainRepositoryName);
   }
 
   public boolean isStrictFilesets() {