Pass the source path -> Artifact mapping into FdoSupport from a provider instead of special-casing it.

This removes the need to deserialize artifacts in FdoSupport, which in turn removes the need to support artifact deserialization at all, which makes our lives simpler and Thoreauvian programming is good.

--
MOS_MIGRATED_REVID=115660698

3 files changed, 2 insertions, 68 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
index 1979df4e2f..2e491b17e0 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/ArtifactFactoryTest.java
@@ -251,37 +251,4 @@ public class ArtifactFactoryTest {
       return result;
     }
   }
-
-  @Test
-  public void testArtifactDeserializationWithoutReusedArtifacts() throws Exception {
-    PathFragment derivedPath = outRoot.getExecPath().getRelative("fruit/banana");
-    artifactFactory.clear();
-    artifactFactory.setDerivedArtifactRoots(ImmutableList.of(outRoot));
-    MockPackageRootResolver rootResolver = new MockPackageRootResolver();
-    rootResolver.setPackageRoots(
-        ImmutableMap.of(PackageIdentifier.createInDefaultRepo(""), clientRoot));
-    Artifact artifact1 = artifactFactory.deserializeArtifact(derivedPath, rootResolver);
-    Artifact artifact2 = artifactFactory.deserializeArtifact(derivedPath, rootResolver);
-    assertEquals(artifact1, artifact2);
-    assertNull(artifact1.getOwner());
-    assertNull(artifact2.getOwner());
-    assertEquals(derivedPath, artifact1.getExecPath());
-    assertEquals(derivedPath, artifact2.getExecPath());
-
-    // Source artifacts are always reused
-    PathFragment sourcePath = clientRoot.getExecPath().getRelative("fruit/mango");
-    artifact1 = artifactFactory.deserializeArtifact(sourcePath, rootResolver);
-    artifact2 = artifactFactory.deserializeArtifact(sourcePath, rootResolver);
-    assertSame(artifact1, artifact2);
-    assertEquals(sourcePath, artifact1.getExecPath());
-  }
-
-  @Test
-  public void testDeserializationWithInvalidPath() throws Exception {
-    artifactFactory.clear();
-    PathFragment randomPath = new PathFragment("maracuja/lemon/kiwi");
-    Artifact artifact = artifactFactory.deserializeArtifact(randomPath,
-        new MockPackageRootResolver());
-    assertNull(artifact);
-  }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 77b9ad81ec..5e9dcd41f8 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -26,7 +26,6 @@ import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.eventbus.EventBus;
@@ -116,12 +115,6 @@ public class LoadingPhaseRunnerTest {
     LoadingResult loadingResult = assertNoErrors(tester.load("//base:hello"));
     assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello"));
     assertNull(loadingResult.getTestsToRun());
-    // TODO(ulfjack): We don't collect package roots if we don't run the loading phase.
-    if (runsLoadingPhase()) {
-      assertEquals(
-          ImmutableMap.of(PackageIdentifier.createInDefaultRepo("base"), tester.getWorkspace()),
-          loadingResult.getPackageRoots());
-    }
   }
 
   @Test
@@ -150,7 +143,6 @@ public class LoadingPhaseRunnerTest {
     assertThat(loadingResult.hasLoadingError()).isFalse();
     assertThat(loadingResult.getTargets()).isEmpty();
     assertThat(loadingResult.getTestsToRun()).isNull();
-    assertThat(loadingResult.getPackageRoots()).isEmpty();
     tester.assertContainsError("Skipping '//base:missing': no such package 'base'");
     tester.assertContainsWarning("Target pattern parsing failed.");
   }
@@ -163,7 +155,6 @@ public class LoadingPhaseRunnerTest {
     assertThat(loadingResult.hasLoadingError()).isFalse();
     assertThat(loadingResult.getTargets()).isEmpty();
     assertThat(loadingResult.getTestsToRun()).isNull();
-    assertThat(loadingResult.getPackageRoots()).isEmpty();
     tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'");
     tester.assertContainsWarning("Target pattern parsing failed.");
   }
@@ -193,7 +184,6 @@ public class LoadingPhaseRunnerTest {
     assertTrue(loadingResult.hasLoadingError());
     assertThat(loadingResult.getTargets()).containsExactlyElementsIn(ImmutableList.<Target>of());
     assertNull(loadingResult.getTestsToRun());
-    assertTrue(loadingResult.getPackageRoots().size() <= 1);
     tester.assertContainsError("no such package 'nonexistent': BUILD file not found");
   }
 
@@ -321,10 +311,6 @@ public class LoadingPhaseRunnerTest {
     LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests"));
     assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_test"));
     assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//cc:my_test"));
-    if (runsLoadingPhase()) {
-      assertThat(loadingResult.getPackageRoots().entrySet())
-          .contains(entryFor(PackageIdentifier.createInDefaultRepo("cc"), tester.getWorkspace()));
-    }
     assertThat(tester.getOriginalTargets())
         .containsExactlyElementsIn(getTargets("//cc:tests", "//cc:my_test"));
     assertThat(tester.getTestSuiteTargets())
@@ -450,7 +436,6 @@ public class LoadingPhaseRunnerTest {
     LoadingResult secondResult = assertNoErrors(tester.load("//base:hello"));
     assertEquals(firstResult.getTargets(), secondResult.getTargets());
     assertEquals(firstResult.getTestsToRun(), secondResult.getTestsToRun());
-    assertEquals(firstResult.getPackageRoots(), secondResult.getPackageRoots());
   }
 
   /**
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java
index 5771f522d9..13951591c1 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkyframeLabelVisitorTestCase.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.events.EventCollector;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.packages.ConstantRuleVisibility;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
-import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.util.PackageLoadingTestCase;
@@ -121,8 +120,7 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas
    * Check that the expected targets were exactly those visited, and that the packages of these
    * expected targets were exactly those packages visited.
    */
-  protected void assertExpectedTargets(
-      Set<String> expectedLabels, boolean expectError, Set<Target> startingTargets)
+  protected void assertExpectedTargets(Set<String> expectedLabels, Set<Target> startingTargets)
       throws Exception {
     Set<Label> visitedLabels =
         getVisitedLabels(
@@ -143,13 +141,6 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas
     }
 
     assertEquals(expectedPkgs, getVisitedPackageNames(startingTargets));
-    if (!expectError) {
-      Set<PathFragment> visitedPkgs = new HashSet<>();
-      for (Package pkg : getErrorFreeVisitedPackages(startingTargets)) {
-        visitedPkgs.add(pkg.getNameFragment());
-      }
-      assertEquals(expectedPkgs, visitedPkgs);
-    }
   }
 
   /**
@@ -176,7 +167,7 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas
             reporter, startingTargets, ImmutableSet.<Label>of(), keepGoing, 200, Integer.MAX_VALUE);
 
     assertNotSame(expectError, result);
-    assertExpectedTargets(expectedLabels, expectError, startingTargets);
+    assertExpectedTargets(expectedLabels, startingTargets);
   }
 
   /**
@@ -286,15 +277,6 @@ abstract public class SkyframeLabelVisitorTestCase extends PackageLoadingTestCas
     return builder.build();
   }
 
-  protected Set<Package> getErrorFreeVisitedPackages(Set<Target> startingTargets) {
-    ImmutableSet.Builder<Package> builder = ImmutableSet.builder();
-    builder.addAll(visitor.getErrorFreeVisitedPackages(reporter));
-    for (Target target : startingTargets) {
-      builder.add(target.getPackage());
-    }
-    return builder.build();
-  }
-
   @Before
   public final void initializeVisitor() throws Exception {
     skyframeExecutor = super.createSkyframeExecutor(