Dirty Package node can still pool intern labels

This change fixes a remaining issue in `LabelPool#getOrWeakIntern`. By changing to get node from graph using `toValue()` method, we are promised to get `PackageValue` even if node is dirty or changed.

PiperOrigin-RevId: 524377579
Change-Id: I69859977751fe4c664828438c3645e21e67c6d9c

src/main/java/com/google/devtools/build/skyframe/InMemoryGraphImpl.java

@@ -312,13 +312,16 @@ public Label getOrWeakIntern(Label sample) {
       PackageValue.Key packageKey = PackageValue.key(packageIdentifier);
 
       InMemoryNodeEntry inMemoryNodeEntry = nodeMap.get(packageKey);
-      if (inMemoryNodeEntry == null || !inMemoryNodeEntry.isDone()) {
-        // If we cannot get the InMemoryNodeEntry, or it is not done, values are not available. So
-        // we can only weak intern sample.
+      if (inMemoryNodeEntry == null) {
+        // If we cannot get the InMemoryNodeEntry, just weak intern the sample.
         return interner.weakIntern(sample);
       }
 
       SkyValue value = inMemoryNodeEntry.toValue();
+      if (value == null) {
+        return interner.weakIntern(sample);
+      }
+
       checkState(value instanceof PackageValue, value);
       ImmutableSortedMap<String, Target> targets = ((PackageValue) value).getPackage().getTargets();
       Target target = targets.get(sample.getName());

src/test/java/com/google/devtools/build/lib/cmdline/LabelInternerIntegrationTest.java

@@ -119,13 +119,39 @@ public void labelInterner_noDuplicatesAndNoWeakInterned() throws Exception {
   }
 
   @Test
-  public void labelInterner_removeDirtyPackageStillWeakInternItsLabels() throws Exception {
+  public void labelInterner_dirtyPackageStillPoolInternLabel() throws Exception {
     write("hello/BUILD", "cc_binary(name = 'foo', srcs = ['foo.cc'])");
     write("hello/foo.cc", "int main() {", "  return 0;", "}");
     buildTarget("//hello:foo");
 
     InMemoryGraph graph = skyframeExecutor().getEvaluator().getInMemoryGraph();
+    PackageValue.Key packageKey =
+        PackageValue.key(PackageIdentifier.createInMainRepo(/* name= */ "hello"));
+    NodeEntry nodeEntry = graph.get(/* requestor= */ null, Reason.OTHER, packageKey);
+    assertThat(nodeEntry).isNotNull();
+
+    ImmutableSet<Label> targetLabels =
+        ((PackageValue) nodeEntry.toValue())
+            .getPackage().getTargets().values().stream()
+                .map(TargetApi::getLabel)
+                .collect(toImmutableSet());
+
+    nodeEntry.markDirty(DirtyType.DIRTY);
 
+    // Expect `intern` a duplicate instance to return the canonical one stored in the pool.
+    targetLabels.forEach(
+        l ->
+            assertThat(Label.createUnvalidated(l.getPackageIdentifier(), l.getName()))
+                .isSameInstanceAs(l));
+  }
+
+  @Test
+  public void labelInterner_removeDirtyPackageStillWeakInternItsLabels() throws Exception {
+    write("hello/BUILD", "cc_binary(name = 'foo', srcs = ['foo.cc'])");
+    write("hello/foo.cc", "int main() {", "  return 0;", "}");
+    buildTarget("//hello:foo");
+
+    InMemoryGraph graph = skyframeExecutor().getEvaluator().getInMemoryGraph();
     PackageValue.Key packageKey =
         PackageValue.key(PackageIdentifier.createInMainRepo(/* name= */ "hello"));
     NodeEntry nodeEntry = graph.get(/* requestor= */ null, Reason.OTHER, packageKey);