Support watching a path even if it's a directory or nonexistent

- `rctx.watch()` now supports watching a path even if it's a directory or nonexistent.
  - a path's status changing counts triggers a refetch.
- added `path.is_dir` so that users can tell whether a path points to a directory or a file.
- added `watch_X` parameters to the following methods, with behavior similar to the `watch` parameter in `rctx.read()`
  - `rctx.extract()`, `rctx.patch()`, `rctx.template()` (for the template file), `rctx.symlink()` (for the symlink target)

Work towards #20952.

Closes #21339.

PiperOrigin-RevId: 607415094
Change-Id: Iebb6bc28174d05277a034ba3cf5e0c9bf90ce027

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java

@@ -1185,6 +1185,9 @@ public String readFile(Object path, String watch, StarlarkThread thread)
             p.toString(), getIdentifyingStringForLogging(), thread.getCallerLocation());
     env.getListener().post(w);
     maybeWatch(p, ShouldWatch.fromString(watch));
+    if (p.isDir()) {
+      throw Starlark.errorf("attempting to read() a directory: %s", p);
+    }
     try {
       return FileSystemUtils.readContent(p.getPath(), ISO_8859_1);
     } catch (IOException e) {
@@ -1270,9 +1273,6 @@ protected void maybeWatch(StarlarkPath starlarkPath, ShouldWatch shouldWatch)
       if (fileValue == null) {
         throw new NeedsSkyframeRestartException();
       }
-      if (!fileValue.isFile() || fileValue.isSpecialFile()) {
-        throw Starlark.errorf("Not a regular file: %s", pair.second.asPath().getPathString());
-      }
 
       recordedFileInputs.put(pair.first, RepoRecordedInput.File.fileValueToMarkerValue(fileValue));
     } catch (IOException e) {
@@ -1283,12 +1283,19 @@ protected void maybeWatch(StarlarkPath starlarkPath, ShouldWatch shouldWatch)
   @StarlarkMethod(
       name = "watch",
       doc =
-          "Tells Bazel to watch for changes to the given file. Any changes to the file will "
+          "Tells Bazel to watch for changes to the given path, whether or not it exists, or "
+              + "whether it's a file or a directory. Any changes to the file or directory will "
               + "invalidate this repository or module extension, and cause it to be refetched or "
-              + "re-evaluated next time.<p>Note that attempting to watch files inside the repo "
-              + "currently being fetched, or inside the working directory of the current module "
-              + "extension, will result in an error. A module extension attempting to watch a file "
-              + "outside the current Bazel workspace will also result in an error.",
+              + "re-evaluated next time.<p>\"Changes\" include changes to the contents of the file "
+              + "(if the path is a file); if the path was a file but is now a directory, or vice "
+              + "versa; and if the path starts or stops existing. Notably, this does <em>not</em> "
+              + "include changes to any files under the directory if the path is a directory. For "
+              // TODO: add `watch_dir()`
+              + "that, use <a href=\"#watch_dir\"><code>watch_dir()</code></a> instead.<p>Note "
+              + "that attempting to watch paths inside the repo currently being fetched, or inside "
+              + "the working directory of the current module extension, will result in an error. A "
+              + "module extension attempting to watch a path outside the current Bazel workspace "
+              + "will also result in an error.",
       parameters = {
         @Param(
             name = "path",

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPath.java

@@ -77,7 +77,6 @@ public String getBasename() {
 
   @StarlarkMethod(
       name = "readdir",
-      structField = false,
       doc = "The list of entries in the directory denoted by this path.")
   public ImmutableList<StarlarkPath> readdir() throws IOException {
     ImmutableList.Builder<StarlarkPath> builder = ImmutableList.builder();
@@ -118,11 +117,27 @@ public StarlarkPath getChild(Tuple relativePaths) throws EvalException {
   @StarlarkMethod(
       name = "exists",
       structField = true,
-      doc = "Returns true if the file denoted by this path exists.")
+      doc =
+          "Returns true if the file or directory denoted by this path exists.<p>Note that "
+              + "accessing this field does <em>not</em> cause the path to be watched. If you'd "
+              + "like the repo rule or module extension to be sensitive to the path's existence, "
+              + "use the <code>watch()</code> method on the context object.")
   public boolean exists() {
     return path.exists();
   }
 
+  @StarlarkMethod(
+      name = "is_dir",
+      structField = true,
+      doc =
+          "Returns true if this path points to a directory.<p>Note that accessing this field does "
+              + "<em>not</em> cause the path to be watched. If you'd like the repo rule or module "
+              + "extension to be sensitive to whether the path is a directory or a file, use the "
+              + "<code>watch()</code> method on the context object.")
+  public boolean isDir() {
+    return path.isDirectory();
+  }
+
   @StarlarkMethod(
       name = "realpath",
       structField = true,

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java

@@ -198,9 +198,21 @@ private StarlarkPath externalPath(String method, Object pathObject)
               @ParamType(type = Label.class),
               @ParamType(type = StarlarkPath.class)
             },
-            doc = "The path of the symlink to create, relative to the repository directory."),
+            doc = "The path of the symlink to create."),
+        @Param(
+            name = "watch_target",
+            defaultValue = "'auto'",
+            positional = false,
+            named = true,
+            doc =
+                "whether to <a href=\"#watch\">watch</a> the symlink target. Can be the string "
+                    + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking "
+                    + "the <a href=\"#watch\"><code>watch()</code></a> method; passing 'no' does "
+                    + "not attempt to watch the path; passing 'auto' will only attempt to watch "
+                    + "the file when it is legal to do so (see <code>watch()</code> docs for more "
+                    + "information."),
       })
-  public void symlink(Object target, Object linkName, StarlarkThread thread)
+  public void symlink(Object target, Object linkName, String watchTarget, StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
     StarlarkPath targetPath = getPath("symlink()", target);
     StarlarkPath linkPath = getPath("symlink()", linkName);
@@ -211,6 +223,7 @@ public void symlink(Object target, Object linkName, StarlarkThread thread)
             getIdentifyingStringForLogging(),
             thread.getCallerLocation());
     env.getListener().post(w);
+    maybeWatch(targetPath, ShouldWatch.fromString(watchTarget));
     try {
       checkInOutputDirectory("write", linkPath);
       makeDirectories(linkPath.getPath());
@@ -269,12 +282,25 @@ public void symlink(Object target, Object linkName, StarlarkThread thread)
             defaultValue = "True",
             named = true,
             doc = "set the executable flag on the created file, true by default."),
+        @Param(
+            name = "watch_template",
+            defaultValue = "'auto'",
+            positional = false,
+            named = true,
+            doc =
+                "whether to <a href=\"#watch\">watch</a> the template file. Can be the string "
+                    + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking "
+                    + "the <a href=\"#watch\"><code>watch()</code></a> method; passing 'no' does "
+                    + "not attempt to watch the file; passing 'auto' will only attempt to watch "
+                    + "the file when it is legal to do so (see <code>watch()</code> docs for more "
+                    + "information."),
       })
   public void createFileFromTemplate(
       Object path,
       Object template,
       Dict<?, ?> substitutions, // <String, String> expected
       Boolean executable,
+      String watchTemplate,
       StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
     StarlarkPath p = getPath("template()", path);
@@ -290,6 +316,10 @@ public void createFileFromTemplate(
             getIdentifyingStringForLogging(),
             thread.getCallerLocation());
     env.getListener().post(w);
+    if (t.isDir()) {
+      throw Starlark.errorf("attempting to use a directory as template: %s", t);
+    }
+    maybeWatch(t, ShouldWatch.fromString(watchTemplate));
     try {
       checkInOutputDirectory("write", p);
       makeDirectories(p.getPath());
@@ -385,8 +415,20 @@ public boolean delete(Object pathObject, StarlarkThread thread)
             named = true,
             defaultValue = "0",
             doc = "strip the specified number of leading components from file names."),
+        @Param(
+            name = "watch_patch",
+            defaultValue = "'auto'",
+            positional = false,
+            named = true,
+            doc =
+                "whether to <a href=\"#watch\">watch</a> the patch file. Can be the string "
+                    + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking "
+                    + "the <a href=\"#watch\"><code>watch()</code></a> method; passing 'no' does "
+                    + "not attempt to watch the file; passing 'auto' will only attempt to watch "
+                    + "the file when it is legal to do so (see <code>watch()</code> docs for more "
+                    + "information."),
       })
-  public void patch(Object patchFile, StarlarkInt stripI, StarlarkThread thread)
+  public void patch(Object patchFile, StarlarkInt stripI, String watchPatch, StarlarkThread thread)
       throws EvalException, RepositoryFunctionException, InterruptedException {
     int strip = Starlark.toInt(stripI, "strip");
     StarlarkPath starlarkPath = getPath("patch()", patchFile);
@@ -397,6 +439,10 @@ public void patch(Object patchFile, StarlarkInt stripI, StarlarkThread thread)
             getIdentifyingStringForLogging(),
             thread.getCallerLocation());
     env.getListener().post(w);
+    if (starlarkPath.isDir()) {
+      throw Starlark.errorf("attempting to use a directory as patch file: %s", starlarkPath);
+    }
+    maybeWatch(starlarkPath, ShouldWatch.fromString(watchPatch));
     try {
       PatchUtil.apply(starlarkPath.getPath(), strip, workingDirectory);
     } catch (PatchFailedException e) {
@@ -457,12 +503,25 @@ public void patch(Object patchFile, StarlarkInt stripI, StarlarkThread thread)
                     + " any directory prefix adjustment. This can be used to extract archives that"
                     + " contain non-Unicode filenames, or which have files that would extract to"
                     + " the same path on case-insensitive filesystems."),
+        @Param(
+            name = "watch_archive",
+            defaultValue = "'auto'",
+            positional = false,
+            named = true,
+            doc =
+                "whether to <a href=\"#watch\">watch</a> the archive file. Can be the string "
+                    + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking "
+                    + "the <a href=\"#watch\"><code>watch()</code></a> method; passing 'no' does "
+                    + "not attempt to watch the file; passing 'auto' will only attempt to watch "
+                    + "the file when it is legal to do so (see <code>watch()</code> docs for more "
+                    + "information."),
       })
   public void extract(
       Object archive,
       Object output,
       String stripPrefix,
       Dict<?, ?> renameFiles, // <String, String> expected
+      String watchArchive,
       StarlarkThread thread)
       throws RepositoryFunctionException, InterruptedException, EvalException {
     StarlarkPath archivePath = getPath("extract()", archive);
@@ -471,6 +530,10 @@ public void extract(
       throw new RepositoryFunctionException(
           Starlark.errorf("Archive path '%s' does not exist.", archivePath), Transience.TRANSIENT);
     }
+    if (archivePath.isDir()) {
+      throw Starlark.errorf("attempting to extract a directory: %s", archivePath);
+    }
+    maybeWatch(archivePath, ShouldWatch.fromString(watchArchive));
 
     StarlarkPath outputPath = getPath("extract()", output);
     checkInOutputDirectory("write", outputPath);

src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java

@@ -194,12 +194,16 @@ public Parser getParser() {
 
     /**
      * Convert to a {@link com.google.devtools.build.lib.actions.FileValue} to a String appropriate
-     * for placing in a repository marker file.
-     *
-     * @param fileValue The value to convert. It must correspond to a regular file.
+     * for placing in a repository marker file. The file need not exist, and can be a file or a
+     * directory.
      */
     public static String fileValueToMarkerValue(FileValue fileValue) throws IOException {
-      Preconditions.checkArgument(fileValue.isFile() && !fileValue.isSpecialFile());
+      if (fileValue.isDirectory()) {
+        return "DIR";
+      }
+      if (!fileValue.exists()) {
+        return "ENOENT";
+      }
       // Return the file content digest in hex. fileValue may or may not have the digest available.
       byte[] digest = fileValue.realFileStateValue().getDigest();
       if (digest == null) {

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java

@@ -93,7 +93,7 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
 
   // The marker file version is inject in the rule key digest so the rule key is always different
   // when we decide to update the format.
-  private static final int MARKER_FILE_VERSION = 5;
+  private static final int MARKER_FILE_VERSION = 6;
 
   // Mapping of rule class name to RepositoryFunction.
   private final ImmutableMap<String, RepositoryFunction> handlers;

src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java

@@ -324,7 +324,7 @@ public void testPatch() throws Exception {
     StarlarkPath patchFile = context.path("my.patch");
     context.createFile(
         context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread);
-    context.patch(patchFile, StarlarkInt.of(0), thread);
+    context.patch(patchFile, StarlarkInt.of(0), "auto", thread);
     testOutputFile(foo.getPath(), "line one\nline two\n");
   }
 
@@ -335,7 +335,7 @@ public void testCannotFindFileToPatch() throws Exception {
     context.createFile(
         context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread);
     try {
-      context.patch(patchFile, StarlarkInt.of(0), thread);
+      context.patch(patchFile, StarlarkInt.of(0), "auto", thread);
       fail("Expected RepositoryFunctionException");
     } catch (RepositoryFunctionException ex) {
       assertThat(ex)
@@ -358,7 +358,7 @@ public void testPatchOutsideOfExternalRepository() throws Exception {
         true,
         thread);
     try {
-      context.patch(patchFile, StarlarkInt.of(0), thread);
+      context.patch(patchFile, StarlarkInt.of(0), "auto", thread);
       fail("Expected RepositoryFunctionException");
     } catch (RepositoryFunctionException ex) {
       assertThat(ex)
@@ -379,7 +379,7 @@ public void testPatchErrorWasThrown() throws Exception {
     context.createFile(
         context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread);
     try {
-      context.patch(patchFile, StarlarkInt.of(0), thread);
+      context.patch(patchFile, StarlarkInt.of(0), "auto", thread);
       fail("Expected RepositoryFunctionException");
     } catch (RepositoryFunctionException ex) {
       assertThat(ex)
@@ -456,7 +456,7 @@ public void testSymlink() throws Exception {
     setUpContextForRule("test");
     context.createFile(context.path("foo"), "foobar", true, true, thread);
 
-    context.symlink(context.path("foo"), context.path("bar"), thread);
+    context.symlink(context.path("foo"), context.path("bar"), "auto", thread);
     testOutputFile(outputDirectory.getChild("bar"), "foobar");
 
     assertThat(context.path("bar").realpath()).isEqualTo(context.path("foo"));