Reduce the number of ways to create a SingleRunfilesSupplier.

Simplify from 3 constructors and 2 factory methods to just 1 of each. When I add caching support, it's going to get too messy.

Also remove unnecessary `AutoCodec`, though I may need to add it back later.

PiperOrigin-RevId: 355241474

src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java

@@ -14,22 +14,18 @@
 
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
-import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Map;
 import javax.annotation.Nullable;
 
 /** {@link RunfilesSupplier} implementation wrapping a single {@link Runfiles} directory mapping. */
-@AutoCodec
 public class SingleRunfilesSupplier implements RunfilesSupplier {
   private final PathFragment runfilesDir;
   private final Runfiles runfiles;
@@ -38,19 +34,15 @@ public class SingleRunfilesSupplier implements RunfilesSupplier {
   private final boolean runfileLinksEnabled;
 
   /**
-   * Create an instance for runfiles directory.
-   *
-   * @param runfilesDir the path of the runfiles directory relative to the exec root
-   * @param runfiles the associated runfiles
-   * @param buildRunfileLinks whether runfile symlinks are created during build
-   * @param runfileLinksEnabled whether it's allowed to create runfile symlinks
+   * Creates a no-manifest {@link SingleRunfilesSupplier} from the given {@link RunfilesSupport}.
    */
-  public SingleRunfilesSupplier(
-      PathFragment runfilesDir,
-      Runfiles runfiles,
-      boolean buildRunfileLinks,
-      boolean runfileLinksEnabled) {
-    this(runfilesDir, runfiles, /*manifest=*/ null, buildRunfileLinks, runfileLinksEnabled);
+  public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) {
+    return new SingleRunfilesSupplier(
+        runfilesSupport.getRunfilesDirectoryExecPath(),
+        runfilesSupport.getRunfiles(),
+        /*manifest=*/ null,
+        runfilesSupport.isBuildRunfileLinks(),
+        runfilesSupport.isRunfilesEnabled());
   }
 
   /**
@@ -64,7 +56,6 @@ public SingleRunfilesSupplier(
    * @param buildRunfileLinks whether runfile symlinks are created during build
    * @param runfileLinksEnabled whether it's allowed to create runfile symlinks
    */
-  @AutoCodec.Instantiator
   public SingleRunfilesSupplier(
       PathFragment runfilesDir,
       Runfiles runfiles,
@@ -79,40 +70,6 @@ public SingleRunfilesSupplier(
     this.runfileLinksEnabled = runfileLinksEnabled;
   }
 
-  /** Use this constructor in tests only. */
-  @VisibleForTesting
-  public SingleRunfilesSupplier(PathFragment runfilesDir, Runfiles runfiles) {
-    this(
-        runfilesDir,
-        runfiles,
-        /*manifest=*/ null,
-        /*buildRunfileLinks=*/ false,
-        /*runfileLinksEnabled=*/ false);
-  }
-
-  /** Creates a runfiles supplier */
-  public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) {
-    return new SingleRunfilesSupplier(
-        runfilesSupport.getRunfilesDirectoryExecPath(),
-        runfilesSupport.getRunfiles(),
-        runfilesSupport.isBuildRunfileLinks(),
-        runfilesSupport.isRunfilesEnabled());
-  }
-
-  /** creates a runfiles supplier */
-  public static RunfilesSupplier create(
-      PathFragment runfilesDir,
-      Runfiles runfiles,
-      @Nullable Artifact manifest,
-      BuildConfiguration configuration) {
-    return new SingleRunfilesSupplier(
-        runfilesDir,
-        runfiles,
-        manifest,
-        configuration.buildRunfileLinks(),
-        configuration.buildRunfilesManifests());
-  }
-
   @Override
   public NestedSet<Artifact> getArtifacts() {
     return runfiles.getAllArtifacts();

src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java

@@ -376,6 +376,7 @@ private TestParams createTestAction(int shards)
               new SingleRunfilesSupplier(
                   /*runfilesDir=*/ persistentTestRunnerRunfiles.getSuffix(),
                   /*runfiles=*/ persistentTestRunnerRunfiles,
+                  /*manifest=*/ null,
                   /*buildRunfileLinks=*/ false,
                   /*runfileLinksEnabled=*/ false);
         } else {

src/test/java/com/google/devtools/build/lib/actions/BUILD

@@ -63,6 +63,7 @@ java_library(
         "//src/main/protobuf:action_cache_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
         "//src/test/java/com/google/devtools/build/lib/actions/util",
+        "//src/test/java/com/google/devtools/build/lib/analysis/util",
         "//src/test/java/com/google/devtools/build/lib/events:testutil",
         "//src/test/java/com/google/devtools/build/lib/testutil",
         "//src/test/java/com/google/devtools/build/lib/testutil:TestThread",

src/test/java/com/google/devtools/build/lib/actions/BaseSpawnTest.java

@@ -19,7 +19,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.SingleRunfilesSupplier;
+import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.util.Map;
 import org.junit.Test;
@@ -45,7 +45,8 @@ public void testGetEnvironmentAddsRunfilesWhenOnlyOneSuppliedViaRunfilesSupplier
     BaseSpawn underTest =
         minimalBaseSpawn(
             baseEnviron,
-            new SingleRunfilesSupplier(PathFragment.create(runfilesDir), Runfiles.EMPTY));
+            AnalysisTestUtil.createRunfilesSupplier(
+                PathFragment.create(runfilesDir), Runfiles.EMPTY));
 
     Map<String, String> expected = ImmutableMap.<String, String>builder()
         .putAll(baseEnviron)
@@ -63,8 +64,10 @@ public void testGetEnvironmentDoesntAddRunfilesWhenMultipleManifestsSupplied() {
         minimalBaseSpawn(
             baseEnviron,
             CompositeRunfilesSupplier.of(
-                new SingleRunfilesSupplier(PathFragment.create("rfdir1"), Runfiles.EMPTY),
-                new SingleRunfilesSupplier(PathFragment.create("rfdir2"), Runfiles.EMPTY)));
+                AnalysisTestUtil.createRunfilesSupplier(
+                    PathFragment.create("rfdir1"), Runfiles.EMPTY),
+                AnalysisTestUtil.createRunfilesSupplier(
+                    PathFragment.create("rfdir2"), Runfiles.EMPTY)));
 
     assertThat(underTest.getEnvironment()).isEqualTo(baseEnviron);
   }

src/test/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplierTest.java

@@ -49,15 +49,25 @@ public void testGetArtifactsWithSingleMapping() {
     List<Artifact> artifacts = mkArtifacts(rootDir, "thing1", "thing2");
 
     SingleRunfilesSupplier underTest =
-        new SingleRunfilesSupplier(PathFragment.create("notimportant"), mkRunfiles(artifacts));
+        new SingleRunfilesSupplier(
+            PathFragment.create("notimportant"),
+            mkRunfiles(artifacts),
+            /*manifest=*/ null,
+            /*buildRunfileLinks=*/ false,
+            /*runfileLinksEnabled=*/ false);
 
     assertThat(underTest.getArtifacts().toList()).containsExactlyElementsIn(artifacts);
   }
 
   @Test
   public void testGetManifestsWhenNone() {
     RunfilesSupplier underTest =
-        new SingleRunfilesSupplier(PathFragment.create("ignored"), Runfiles.EMPTY);
+        new SingleRunfilesSupplier(
+            PathFragment.create("ignored"),
+            Runfiles.EMPTY,
+            /*manifest=*/ null,
+            /*buildRunfileLinks=*/ false,
+            /*runfileLinksEnabled=*/ false);
     assertThat(underTest.getManifests()).isEmpty();
   }
 

src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java

@@ -32,9 +32,12 @@
 import com.google.devtools.build.lib.actions.MiddlemanFactory;
 import com.google.devtools.build.lib.actions.MutableActionGraph;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
+import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.OutputGroupInfo;
+import com.google.devtools.build.lib.analysis.Runfiles;
+import com.google.devtools.build.lib.analysis.SingleRunfilesSupplier;
 import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Key;
@@ -526,4 +529,14 @@ public static Set<String> artifactsToStrings(
     return files;
   }
 
+  /** Creates a {@link RunfilesSupplier} for use in tests. */
+  public static RunfilesSupplier createRunfilesSupplier(
+      PathFragment runfilesDir, Runfiles runfiles) {
+    return new SingleRunfilesSupplier(
+        runfilesDir,
+        runfiles,
+        /*manifest=*/ null,
+        /*buildRunfileLinks=*/ false,
+        /*runfileLinksEnabled=*/ false);
+  }
 }

src/test/java/com/google/devtools/build/lib/exec/SpawnInputExpanderTest.java

@@ -39,7 +39,7 @@
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.analysis.Runfiles;
-import com.google.devtools.build.lib.analysis.SingleRunfilesSupplier;
+import com.google.devtools.build.lib.analysis.util.AnalysisTestUtil;
 import com.google.devtools.build.lib.exec.util.FakeActionInputFileCache;
 import com.google.devtools.build.lib.exec.util.SpawnBuilder;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -89,7 +89,7 @@ public void testRunfilesSingleFile() throws Exception {
             fs.getPath("/root/dir/file"));
     Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(
         artifact,
@@ -107,7 +107,7 @@ public void testRunfilesWithFileset() throws Exception {
     Artifact artifact = createFilesetArtifact("foo/biz/fs_out");
     Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(
         artifact,
@@ -147,7 +147,7 @@ public void testRunfilesDirectoryStrict() {
             fs.getPath("/root/dir/file"));
     Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1));
 
@@ -168,7 +168,7 @@ public void testRunfilesDirectoryNonStrict() throws Exception {
             fs.getPath("/root/dir/file"));
     Runfiles runfiles = new Runfiles.Builder("workspace").addArtifact(artifact).build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(artifact, FileArtifactValue.createForDirectoryWithMtime(-1));
 
@@ -192,7 +192,7 @@ public void testRunfilesTwoFiles() throws Exception {
     Runfiles runfiles =
         new Runfiles.Builder("workspace").addArtifact(artifact1).addArtifact(artifact2).build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(
         artifact1,
@@ -222,7 +222,7 @@ public void testRunfilesSymlink() throws Exception {
             .addSymlink(PathFragment.create("symlink"), artifact)
             .build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(
         artifact,
@@ -246,7 +246,7 @@ public void testRunfilesRootSymlink() throws Exception {
             .addRootSymlink(PathFragment.create("symlink"), artifact)
             .build();
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache mockCache = new FakeActionInputFileCache();
     mockCache.put(
         artifact,
@@ -280,7 +280,7 @@ public void testRunfilesWithTreeArtifacts() throws Exception {
           }
         };
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache fakeCache = new FakeActionInputFileCache();
     fakeCache.put(file1, FileArtifactValue.createForTesting(file1));
     fakeCache.put(file2, FileArtifactValue.createForTesting(file2));
@@ -313,7 +313,7 @@ public void testRunfilesWithTreeArtifactsInSymlinks() throws Exception {
           }
         };
     RunfilesSupplier supplier =
-        new SingleRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
+        AnalysisTestUtil.createRunfilesSupplier(PathFragment.create("runfiles"), runfiles);
     FakeActionInputFileCache fakeCache = new FakeActionInputFileCache();
     fakeCache.put(file1, FileArtifactValue.createForTesting(file1));
     fakeCache.put(file2, FileArtifactValue.createForTesting(file2));