Skip to content

Commit

Permalink
Drop FileInvalidationData.type.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aoeui authored and copybara-github committed Oct 10, 2024
1 parent de66620 commit 0d7446c
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()));
Expand Down Expand Up @@ -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);
Expand Down
32 changes: 4 additions & 28 deletions src/main/protobuf/file_invalidation_data.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 0d7446c

Please sign in to comment.