Make path.readdir() watch the dirents by default

- Added a new parameter `watch` to `path.readdir()` that allows one to watch for changes in entries under a given directory. - only names are watched; 'entry types' are not. This somewhat matches the return value of `path.readdir()`, which only contains entry names - same restrictions as `rctx.watch()` in terms of which paths can be watched and which cannot; similarly for `mctx`. - Made a big-ish refactor that eliminated the two `RepoRecordedInput.File` subclasses, and pulled the difference into a separate `RepoRecordedInput.RepoCacheFriendlyPath` instead. This new path class is used by the new `RepoRecordedInput.Dirents` as well. Work towards #20952.
bazelbuild · Feb 15, 2024 · 551f437 · 551f437
1 parent 3f77be0
commit 551f437
Show file tree

Hide file tree

Showing 20 changed files with 552 additions and 170 deletions.
diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java
@@ -37,7 +37,7 @@
 @GenerateTypeAdapter
 public abstract class BazelLockFileValue implements SkyValue, Postable {
 
-  public static final int LOCK_FILE_VERSION = 5;
+  public static final int LOCK_FILE_VERSION = 6;
 
   @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;
 

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java
@@ -459,6 +459,22 @@ public RepoRecordedInput.File read(JsonReader jsonReader) throws IOException {
         }
       };
 
+  private static final TypeAdapter<RepoRecordedInput.Dirents>
+      REPO_RECORDED_INPUT_DIRENTS_TYPE_ADAPTER =
+          new TypeAdapter<>() {
+            @Override
+            public void write(JsonWriter jsonWriter, RepoRecordedInput.Dirents value)
+                throws IOException {
+              jsonWriter.value(value.toStringInternal());
+            }
+
+            @Override
+            public RepoRecordedInput.Dirents read(JsonReader jsonReader) throws IOException {
+              return (RepoRecordedInput.Dirents)
+                  RepoRecordedInput.Dirents.PARSER.parse(jsonReader.nextString());
+            }
+          };
+
   public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
     return new GsonBuilder()
         .setPrettyPrinting()
@@ -484,6 +500,8 @@ public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
         .registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
         .registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
         .registerTypeAdapter(RepoRecordedInput.File.class, REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER)
+        .registerTypeAdapter(
+            RepoRecordedInput.Dirents.class, REPO_RECORDED_INPUT_DIRENTS_TYPE_ADAPTER)
         .create();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java
@@ -43,6 +43,8 @@ public static Builder builder() {
 
   public abstract ImmutableMap<RepoRecordedInput.File, String> getRecordedFileInputs();
 
+  public abstract ImmutableMap<RepoRecordedInput.Dirents, String> getRecordedDirentsInputs();
+
   public abstract ImmutableMap<String, String> getEnvVariables();
 
   public abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();
@@ -68,6 +70,9 @@ public abstract static class Builder {
     public abstract Builder setRecordedFileInputs(
         ImmutableMap<RepoRecordedInput.File, String> value);
 
+    public abstract Builder setRecordedDirentsInputs(
+        ImmutableMap<RepoRecordedInput.Dirents, String> value);
+
     public abstract Builder setEnvVariables(ImmutableMap<String, String> value);
 
     public abstract Builder setGeneratedRepoSpecs(ImmutableMap<String, RepoSpec> value);

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java
@@ -72,7 +72,7 @@ protected ModuleExtensionContext(
         processWrapper,
         starlarkSemantics,
         remoteExecutor,
-        /* allowWatchingFilesOutsideWorkspace= */ false);
+        /* allowWatchingPathsOutsideWorkspace= */ false);
     this.extensionId = extensionId;
     this.modules = modules;
     this.rootModuleHasNonDevDependency = rootModuleHasNonDevDependency;

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
@@ -218,6 +218,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                   LockFileModuleExtension.builder()
                       .setBzlTransitiveDigest(extension.getBzlTransitiveDigest())
                       .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs())
+                      .setRecordedDirentsInputs(moduleExtensionResult.getRecordedDirentsInputs())
                       .setEnvVariables(extension.getEnvVars())
                       .setGeneratedRepoSpecs(generatedRepoSpecs)
                       .setModuleExtensionMetadata(moduleExtensionMetadata)
@@ -289,10 +290,16 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile(
                 + extensionId
                 + "' have changed");
       }
-      if (didFilesChange(env, directories, lockedExtension.getRecordedFileInputs())) {
+      if (didRecordedInputsChange(env, directories, lockedExtension.getRecordedFileInputs())) {
         diffRecorder.record(
             "One or more files the extension '" + extensionId + "' is using have changed");
       }
+      if (didRecordedInputsChange(env, directories, lockedExtension.getRecordedDirentsInputs())) {
+        diffRecorder.record(
+            "One or more directory listings watched by the extension '"
+                + extensionId
+                + "' have changed");
+      }
     } catch (DiffFoundEarlyExitException ignored) {
       // ignored
     }
@@ -390,12 +397,12 @@ private static boolean didRepoMappingsChange(
     return false;
   }
 
-  private static boolean didFilesChange(
+  private static boolean didRecordedInputsChange(
       Environment env,
       BlazeDirectories directories,
-      ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs)
+      ImmutableMap<? extends RepoRecordedInput, String> recordedInputs)
       throws InterruptedException, NeedsSkyframeRestartException {
-    boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedFileInputs);
+    boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedInputs);
     if (env.valuesMissing()) {
       throw new NeedsSkyframeRestartException();
     }
@@ -762,6 +769,7 @@ public RunModuleExtensionResult run(
         generatedRepoSpecs.put(name, repoSpec);
       }
       return RunModuleExtensionResult.create(
+          ImmutableMap.of(),
           ImmutableMap.of(),
           generatedRepoSpecs.buildOrThrow(),
           Optional.of(ModuleExtensionMetadata.REPRODUCIBLE),
@@ -931,6 +939,7 @@ public RunModuleExtensionResult run(
       }
       return RunModuleExtensionResult.create(
           moduleContext.getRecordedFileInputs(),
+          moduleContext.getRecordedDirentsInputs(),
           threadContext.getGeneratedRepoSpecs(),
           moduleExtensionMetadata,
           repoMappingRecorder.recordedEntries());
@@ -993,6 +1002,8 @@ abstract static class RunModuleExtensionResult {
 
     abstract ImmutableMap<RepoRecordedInput.File, String> getRecordedFileInputs();
 
+    abstract ImmutableMap<RepoRecordedInput.Dirents, String> getRecordedDirentsInputs();
+
     abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();
 
     abstract Optional<ModuleExtensionMetadata> getModuleExtensionMetadata();
@@ -1001,11 +1012,13 @@ abstract static class RunModuleExtensionResult {
 
     static RunModuleExtensionResult create(
         ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs,
+        ImmutableMap<RepoRecordedInput.Dirents, String> recordedDirentsInputs,
         ImmutableMap<String, RepoSpec> generatedRepoSpecs,
         Optional<ModuleExtensionMetadata> moduleExtensionMetadata,
         ImmutableTable<RepositoryName, String, RepositoryName> recordedRepoMappingEntries) {
       return new AutoValue_SingleExtensionEvalFunction_RunModuleExtensionResult(
           recordedFileInputs,
+          recordedDirentsInputs,
           generatedRepoSpecs,
           moduleExtensionMetadata,
           recordedRepoMappingEntries);

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD
@@ -42,6 +42,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_file_helper",
         "//src/main/java/com/google/devtools/build/lib/shell",
         "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",