From aaf1d0696ffe8a6abe66c3d40947ff4f88a11ca0 Mon Sep 17 00:00:00 2001
From: Googler <jhorvitz@google.com>
Date: Tue, 28 Mar 2023 12:20:23 -0700
Subject: [PATCH] Don't intern `RootedPath` on Windows.

It remains to be investigated why instances that compare equal can't be used interchangeably and what test coverage we're lacking. See issue #17904.

PiperOrigin-RevId: 520099860
Change-Id: Ifa64b662658add884e64bc4b8ae42a1261fec4c1
---
 .../com/google/devtools/build/lib/vfs/BUILD   |  1 +
 .../devtools/build/lib/vfs/RootedPath.java    | 20 +++++++++++++------
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD
index 9655e34157ed4d..f80997f0305041 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD
@@ -62,6 +62,7 @@ java_library(
         exclude = PATH_FRAGMENT_SOURCES + OUTPUT_SERVICE_SOURCES + OS_PATH_POLICY_SOURCES,
     ),
     deps = [
+        ":ospathpolicy",
         ":pathfragment",
         "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/RootedPath.java b/src/main/java/com/google/devtools/build/lib/vfs/RootedPath.java
index 3cf3bec280468e..c74062eacc0869 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/RootedPath.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/RootedPath.java
@@ -26,15 +26,21 @@
  * <p>Two {@link RootedPath}s are considered equal iff they have equal roots and equal relative
  * paths.
  *
- * <p>Instances are interned, which results in a large memory benefit (see cl/516855266). In
- * addition to being a {@link SkyKey} itself, {@link RootedPath} is used as a field in several other
- * common {@link SkyKey} types. Interning on the level of those keys does not deduplicate referenced
- * {@link RootedPath} instances which are also used as a {@link SkyKey} directly.
+ * <p>Instances are interned (except on Windows), which results in a large memory benefit (see
+ * cl/516855266). In addition to being a {@link SkyKey} itself, {@link RootedPath} is used as a
+ * field in several other common {@link SkyKey} types. Interning on the level of those keys does not
+ * deduplicate referenced {@link RootedPath} instances which are also used as a {@link SkyKey}
+ * directly.
  */
 @AutoCodec
 public final class RootedPath implements Comparable<RootedPath>, FileStateKey {
 
-  private static final SkyKeyInterner<RootedPath> interner = SkyKey.newInterner();
+  // Interning on Windows (case-insensitive) surfaces a bug where paths that only differ in casing
+  // use the same RootedPath instance.
+  // TODO(#17904): Investigate this bug and add test coverage.
+  @Nullable
+  private static final SkyKeyInterner<RootedPath> interner =
+      OsPathPolicy.getFilePathOs().isCaseSensitive() ? SkyKey.newInterner() : null;
 
   private final Root root;
   private final PathFragment rootRelativePath;
@@ -52,7 +58,8 @@ static RootedPath createInternal(Root root, PathFragment rootRelativePath) {
         "rootRelativePath: %s root: %s",
         rootRelativePath,
         root);
-    return interner.intern(new RootedPath(root, rootRelativePath));
+    var rootedPath = new RootedPath(root, rootRelativePath);
+    return interner != null ? interner.intern(rootedPath) : rootedPath;
   }
 
   private RootedPath(Root root, PathFragment rootRelativePath) {
@@ -153,6 +160,7 @@ public RootedPath argument() {
   }
 
   @Override
+  @Nullable
   public SkyKeyInterner<?> getSkyKeyInterner() {
     return interner;
   }