From 0d7446cdb288d665832f3ac68d83d2d97d415a28 Mon Sep 17 00:00:00 2001
From: Googler <shahan@google.com>
Date: Thu, 10 Oct 2024 08:05:09 -0700
Subject: [PATCH] Drop FileInvalidationData.type.

It's not the case that if we have DirectoryListingInvalidationData that
we will have all the corresponding FileInvalidationData for the contents
of that directory.

We'll use an overly-conservative approach ond invalidate the
DirectoryListingValue if any of its contents change. It's suboptimal
but still correct. We could at a later point persist the dirent types
from the DirectoryListingValue if this approximation is problematic.

PiperOrigin-RevId: 684446572
Change-Id: I97eeaf73f0c2f2fd2519bfde696bf06fa7161ba7
---
 .../analysis/FileDependencySerializer.java    | 16 +---------
 .../protobuf/file_invalidation_data.proto     | 32 +++----------------
 2 files changed, 5 insertions(+), 43 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java
index 39a51a613fcdc3..ad0b618d6a58aa 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java
@@ -38,7 +38,6 @@
 import com.google.devtools.build.lib.skyframe.serialization.analysis.InvalidationDataReference.FileInvalidationDataReference;
 import com.google.devtools.build.lib.skyframe.serialization.proto.DirectoryListingInvalidationData;
 import com.google.devtools.build.lib.skyframe.serialization.proto.FileInvalidationData;
-import com.google.devtools.build.lib.skyframe.serialization.proto.FileInvalidationData.DirentType;
 import com.google.devtools.build.lib.skyframe.serialization.proto.Symlink;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
@@ -133,7 +132,7 @@ FileInvalidationDataReference registerDependency(FileValue.Key key) {
     // If this is reached, this thread owns `reference` and must complete its future.
     boolean writeStatusSet = false;
     try {
-      var data = FileInvalidationData.newBuilder().setType(getDirentType(value));
+      var data = FileInvalidationData.newBuilder();
       var writeStatuses = new ArrayList<ListenableFuture<Void>>();
       FileInvalidationDataReference parentReference =
           registerDependency(FileValue.key(rootedPath.getParentDirectory()));
@@ -289,19 +288,6 @@ private void processSymlinks(
     }
   }
 
-  private static DirentType getDirentType(FileValue value) {
-    if (!value.exists()) {
-      return DirentType.NON_EXISTENT;
-    }
-    if (value.isFile()) {
-      return DirentType.FILE;
-    }
-    if (value.isDirectory()) {
-      return DirentType.DIRECTORY;
-    }
-    throw new IllegalArgumentException("not a recognized type " + value);
-  }
-
   private KeyBytesProvider getKeyBytes(String cacheKey, Consumer<String> overflowConsumer) {
     if (cacheKey.length() > MAX_KEY_LENGTH) {
       overflowConsumer.accept(cacheKey);
diff --git a/src/main/protobuf/file_invalidation_data.proto b/src/main/protobuf/file_invalidation_data.proto
index ef0a50bb20c695..41e848da61fd0f 100644
--- a/src/main/protobuf/file_invalidation_data.proto
+++ b/src/main/protobuf/file_invalidation_data.proto
@@ -52,10 +52,10 @@ option java_package = "com.google.devtools.build.lib.skyframe.serialization.prot
 // `FileInvalidationData` of its parent is the parent's MTSV, which is included
 // as `parent_mtsv`.
 //
-// In the common case, `FileInvalidationData` only contains `parent_mtsv` and
-// `type' and that's sufficient information to derive all the paths, assuming
-// that the parent lookups are transitively performed. The
-// `FileStateKey.argument` matches the path present in the key.
+// In the common case, `FileInvalidationData` is empty and that's sufficient
+// information to derive all the paths, assuming that the parent lookups are
+// transitively performed. The `FileStateKey.argument` matches the path present
+// in the key.
 //
 // In the presence of symlinks, special logic applies to produce the resolved
 // paths. A `symlinks` entry will be present. To unpack the symlink entry, we
@@ -88,30 +88,6 @@ message FileInvalidationData {
   // This field is repeated, with one entry for each symbolic link encountered
   // while resolving the path.
   repeated Symlink symlinks = 3;
-
-  // Type of the fully resolved entry.
-  //
-  // Symbolic links are not included in the DirentType enum below. They are
-  // exclusively represented using the symlinks field.
-  enum DirentType {
-    // Required by internal best practices.
-    // Not expected to occur.
-    UNDEFINED = 0;
-    // A regular file.
-    FILE = 1;
-    // A directory.
-    DIRECTORY = 2;
-    // Explicit marker for a non-existing file.
-    //
-    // Nonexistence of specific files is meaningful to Bazel, (e.g., BUILD
-    // files).
-    NON_EXISTENT = 3;
-  }
-
-  // Type of the final resolved file.
-  //
-  // Required.
-  DirentType type = 4;
 }
 
 // Invalidation information for a