Add --incompatible_no_implicit_watch_label

When the flag is flipped, `ctx.path` no longer watches paths. It used to do so for `Label` arguments only, which is inconsistent with how the other `ctx` methods operate now that they also watch files.

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

@@ -742,7 +742,7 @@ public Object download(
       checksumValidation = e;
     }
 
-    StarlarkPath outputPath = getPath("download()", output);
+    StarlarkPath outputPath = getPath(output);
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newDownloadEvent(
             urls,
@@ -983,7 +983,7 @@ public StructImpl downloadAndExtract(
             identifyingStringForLogging,
             thread.getCallerLocation());
 
-    StarlarkPath outputPath = getPath("download_and_extract()", output);
+    StarlarkPath outputPath = getPath(output);
     checkInOutputDirectory("write", outputPath);
     createDirectory(outputPath.getPath());
 
@@ -1140,7 +1140,7 @@ public void extract(
       String watchArchive,
       StarlarkThread thread)
       throws RepositoryFunctionException, InterruptedException, EvalException {
-    StarlarkPath archivePath = getPath("extract()", archive);
+    StarlarkPath archivePath = getPath(archive);
 
     if (!archivePath.exists()) {
       throw new RepositoryFunctionException(
@@ -1151,7 +1151,7 @@ public void extract(
     }
     maybeWatch(archivePath, ShouldWatch.fromString(watchArchive));
 
-    StarlarkPath outputPath = getPath("extract()", output);
+    StarlarkPath outputPath = getPath(output);
     checkInOutputDirectory("write", outputPath);
 
     Map<String, String> renameFilesMap =
@@ -1252,7 +1252,7 @@ public boolean isFinished() {
   public void createFile(
       Object path, String content, Boolean executable, Boolean legacyUtf8, StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
-    StarlarkPath p = getPath("file()", path);
+    StarlarkPath p = getPath(path);
     byte[] contentBytes;
     if (legacyUtf8) {
       contentBytes = content.getBytes(UTF_8);
@@ -1352,21 +1352,17 @@ during the analysis phase and thus cannot depends on a target result (the \
                     + " a path from.")
       })
   public StarlarkPath path(Object path) throws EvalException, InterruptedException {
-    return getPath("path()", path);
+    return getPath(path);
   }
 
-  protected StarlarkPath getPath(String method, Object path)
-      throws EvalException, InterruptedException {
-    if (path instanceof String) {
-      return new StarlarkPath(this, workingDirectory.getRelative(path.toString()));
-    } else if (path instanceof Label label) {
-      return getPathFromLabel(label);
-    } else if (path instanceof StarlarkPath starlarkPath) {
-      return starlarkPath;
-    } else {
+  protected StarlarkPath getPath(Object path) throws EvalException, InterruptedException {
+    return switch (path) {
+      case String ignored -> new StarlarkPath(this, workingDirectory.getRelative(path.toString()));
+      case Label label -> getPathFromLabel(label);
+      case StarlarkPath starlarkPath -> starlarkPath;
       // This can never happen because we check it in the Starlark interpreter.
-      throw new IllegalArgumentException("expected string or label for path");
-    }
+      default -> throw new IllegalArgumentException("expected string or label for path");
+    };
   }
 
   @StarlarkMethod(
@@ -1399,7 +1395,7 @@ file when it is legal to do so (see <code>watch()</code> docs for more \
       })
   public String readFile(Object path, String watch, StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
-    StarlarkPath p = getPath("read()", path);
+    StarlarkPath p = getPath(path);
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newReadEvent(
             p.toString(), identifyingStringForLogging, thread.getCallerLocation());
@@ -1552,7 +1548,7 @@ protected void maybeWatchDirents(Path path, ShouldWatch shouldWatch)
       })
   public void watchForStarlark(Object path)
       throws RepositoryFunctionException, EvalException, InterruptedException {
-    maybeWatch(getPath("watch()", path), ShouldWatch.YES);
+    maybeWatch(getPath(path), ShouldWatch.YES);
   }
 
   // Create parent directories for the given path
@@ -1835,7 +1831,7 @@ public StarlarkExecutionResult execute(
 
     Path workingDirectoryPath;
     if (overrideWorkingDirectory != null && !overrideWorkingDirectory.isEmpty()) {
-      workingDirectoryPath = getPath("execute()", overrideWorkingDirectory).getPath();
+      workingDirectoryPath = getPath(overrideWorkingDirectory).getPath();
     } else {
       workingDirectoryPath = workingDirectory;
     }
@@ -1922,7 +1918,11 @@ protected StarlarkPath getPathFromLabel(Label label) throws EvalException, Inter
     RootedPath rootedPath = RepositoryFunction.getRootedPathFromLabel(label, env);
     StarlarkPath starlarkPath = new StarlarkPath(this, rootedPath.asPath());
     try {
-      maybeWatch(starlarkPath, ShouldWatch.AUTO);
+      maybeWatch(
+          starlarkPath,
+          starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL)
+              ? ShouldWatch.NO
+              : ShouldWatch.AUTO);
     } catch (RepositoryFunctionException e) {
       throw Starlark.errorf("%s", e.getCause().getMessage());
     }

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

@@ -169,7 +169,7 @@ public StructImpl getAttr() {
 
   private StarlarkPath externalPath(String method, Object pathObject)
       throws EvalException, InterruptedException {
-    StarlarkPath starlarkPath = getPath(method, pathObject);
+    StarlarkPath starlarkPath = getPath(pathObject);
     Path path = starlarkPath.getPath();
     if (packageLocator.getPathEntries().stream().noneMatch(root -> path.startsWith(root.asPath()))
         || path.startsWith(workingDirectory)) {
@@ -212,8 +212,8 @@ private StarlarkPath externalPath(String method, Object pathObject)
       })
   public void symlink(Object target, Object linkName, StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
-    StarlarkPath targetPath = getPath("symlink()", target);
-    StarlarkPath linkPath = getPath("symlink()", linkName);
+    StarlarkPath targetPath = getPath(target);
+    StarlarkPath linkPath = getPath(linkName);
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newSymlinkEvent(
             targetPath.toString(),
@@ -304,8 +304,8 @@ public void createFileFromTemplate(
       String watchTemplate,
       StarlarkThread thread)
       throws RepositoryFunctionException, EvalException, InterruptedException {
-    StarlarkPath p = getPath("template()", path);
-    StarlarkPath t = getPath("template()", template);
+    StarlarkPath p = getPath(path);
+    StarlarkPath t = getPath(template);
     Map<String, String> substitutionMap =
         Dict.cast(substitutions, String.class, String.class, "substitutions");
     WorkspaceRuleEvent w =
@@ -442,7 +442,7 @@ the file when it is legal to do so (see <code>watch()</code> docs for more \
   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);
+    StarlarkPath starlarkPath = getPath(patchFile);
     WorkspaceRuleEvent w =
         WorkspaceRuleEvent.newPatchEvent(
             starlarkPath.toString(),
@@ -487,7 +487,7 @@ public void patch(Object patchFile, StarlarkInt stripI, String watchPatch, Starl
       })
   public void watchTree(Object path)
       throws EvalException, InterruptedException, RepositoryFunctionException {
-    StarlarkPath p = getPath("watch_tree()", path);
+    StarlarkPath p = getPath(path);
     if (!p.isDir()) {
       throw Starlark.errorf("can't call watch_tree() on non-directory %s", p);
     }

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -53,7 +53,8 @@
  *   <li>Boolean semantic flags can toggle StarlarkMethod-annotated Java methods (or their
  *       parameters) on or off, making them selectively invisible to Starlark. To do this, add a new
  *       entry to {@link BuildLanguageOptions}, then specify the identifier in {@link
- *       StarlarkMethod#enableOnlyWithFlag} or {@link StarlarkMethod#disableWithFlag}.
+ *       net.starlark.java.annot.StarlarkMethod#enableOnlyWithFlag} or {@link
+ *       net.starlark.java.annot.StarlarkMethod#disableWithFlag}.
  * </ul>
  */
 public final class BuildLanguageOptions extends OptionsBase {
@@ -282,6 +283,19 @@ public final class BuildLanguageOptions extends OptionsBase {
               + " function.")
   public boolean experimentalIsolatedExtensionUsages;
 
+  @Option(
+      name = "incompatible_no_implicit_watch_label",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      metadataTags = OptionMetadataTag.INCOMPATIBLE_CHANGE,
+      effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
+      help =
+          "If true, then methods on <code>repository_ctx</code> that are passed a Label will no"
+              + " longer automatically watch the file under that label for changes even if"
+              + " <code>watch = \"no\"</code> and <code>repository_ctx.path</code> no longer causes"
+              + " the returned path to be watched. Use <code>repository_ctx.watch</code> instead.")
+  public boolean incompatibleNoImplicitWatchLabel;
+
   @Option(
       name = "incompatible_stop_exporting_build_file_path",
       defaultValue = "false",
@@ -820,6 +834,7 @@ public StarlarkSemantics toStarlarkSemantics() {
             .setBool(ENABLE_BZLMOD, enableBzlmod)
             .setBool(ENABLE_WORKSPACE, enableWorkspace)
             .setBool(EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, experimentalIsolatedExtensionUsages)
+            .setBool(INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL, incompatibleNoImplicitWatchLabel)
             .setBool(EXPERIMENTAL_ACTION_RESOURCE_SET, experimentalActionResourceSet)
             .setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi)
             .setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi)
@@ -930,6 +945,8 @@ public StarlarkSemantics toStarlarkSemantics() {
   public static final String ENABLE_WORKSPACE = "-enable_workspace";
   public static final String EXPERIMENTAL_ISOLATED_EXTENSION_USAGES =
       "-experimental_isolated_extension_usages";
+  public static final String INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL =
+      "-incompatible_no_implicit_watch_label";
   public static final String EXPERIMENTAL_GOOGLE_LEGACY_API = "-experimental_google_legacy_api";
   public static final String EXPERIMENTAL_PLATFORMS_API = "-experimental_platforms_api";
   public static final String EXPERIMENTAL_REPO_REMOTE_EXEC = "-experimental_repo_remote_exec";

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

@@ -19,6 +19,7 @@ java_library(
     srcs = glob(["*.java"]),
     deps = [
         "//src/main/java/com/google/devtools/build/lib:runtime",
+        "//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
         "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
@@ -31,6 +32,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
         "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
@@ -54,7 +56,6 @@ java_library(
 java_test(
     name = "StarlarkTests",
     testonly = 1,
-    srcs = glob(["*.java"]),
     tags = [
         "rules",
     ],
@@ -63,36 +64,4 @@ java_test(
         ":StarlarkTests_lib",
         "//src/test/java/com/google/devtools/build/lib:test_runner",
     ],
-    deps = [
-        "//src/main/java/com/google/devtools/build/lib:runtime",
-        "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
-        "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
-        "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
-        "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
-        "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/main/java/com/google/devtools/build/lib/events",
-        "//src/main/java/com/google/devtools/build/lib/packages",
-        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
-        "//src/main/java/com/google/devtools/build/lib/pkgcache",
-        "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
-        "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
-        "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
-        "//src/main/java/com/google/devtools/build/lib/vfs",
-        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
-        "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
-        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
-        "//src/main/java/net/starlark/java/eval",
-        "//src/main/java/net/starlark/java/syntax",
-        "//src/test/java/com/google/devtools/build/lib/analysis/util",
-        "//src/test/java/com/google/devtools/build/lib/starlark/util",
-        "//src/test/java/com/google/devtools/build/lib/testutil",
-        "//third_party:guava",
-        "//third_party:jsr305",
-        "//third_party:junit4",
-        "//third_party:mockito",
-        "//third_party:truth",
-    ],
 )

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

@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -25,13 +26,16 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.io.CharStreams;
+import com.google.devtools.build.lib.actions.FileValue;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.ServerDirectories;
 import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.packages.Attribute;
+import com.google.devtools.build.lib.packages.BuildFileName;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Package.Builder.PackageSettings;
 import com.google.devtools.build.lib.packages.PackageOverheadEstimator;
@@ -48,6 +52,7 @@
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult;
 import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants;
+import com.google.devtools.build.lib.skyframe.PackageLookupValue;
 import com.google.devtools.build.lib.testutil.Scratch;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -91,6 +96,7 @@ public final class StarlarkRepositoryContextTest {
   private Root root;
   private Path workspaceFile;
   private StarlarkRepositoryContext context;
+  private Label fakeFileLabel;
   private static final StarlarkThread thread =
       StarlarkThread.createTransient(Mutability.create("test"), StarlarkSemantics.DEFAULT);
 
@@ -159,6 +165,13 @@ private void setUpContextForRule(
     DownloadManager downloader = Mockito.mock(DownloadManager.class);
     SkyFunction.Environment environment = Mockito.mock(SkyFunction.Environment.class);
     when(environment.getListener()).thenReturn(listener);
+    fakeFileLabel = Label.parseCanonical("//:foo");
+    when(environment.getValue(PackageLookupValue.key(fakeFileLabel.getPackageIdentifier())))
+        .thenReturn(
+            PackageLookupValue.success(
+                Root.fromPath(workspaceFile.getParentDirectory()), BuildFileName.BUILD));
+    when(environment.getValueOrThrow(any(), eq(IOException.class)))
+        .thenReturn(Mockito.mock(FileValue.class));
     PathPackageLocator packageLocator =
         new PathPackageLocator(
             outputDirectory,
@@ -188,11 +201,16 @@ private void setUpContextForRule(
   }
 
   private void setUpContextForRule(String name) throws Exception {
+    setUpContextForRule(name, StarlarkSemantics.DEFAULT);
+  }
+
+  private void setUpContextForRule(String name, StarlarkSemantics starlarkSemantics)
+      throws Exception {
     setUpContextForRule(
         ImmutableMap.of("name", name),
         ImmutableSet.of(),
         ImmutableMap.of("FOO", "BAR"),
-        StarlarkSemantics.DEFAULT,
+        starlarkSemantics,
         /* repoRemoteExecutor= */ null);
   }
 
@@ -491,4 +509,26 @@ public void testWorkspaceRoot() throws Exception {
     setUpContextForRule("test");
     assertThat(context.getWorkspaceRoot().getPath()).isEqualTo(root.asPath());
   }
+
+  @Test
+  public void testNoIncompatibleNoImplicitWatchLabel() throws Exception {
+    setUpContextForRule("test");
+    scratch.file(root.getRelative("foo").getPathString());
+    context.path(fakeFileLabel);
+    context.readFile(fakeFileLabel, "no", thread);
+    assertThat(context.getRecordedFileInputs()).isNotEmpty();
+  }
+
+  @Test
+  public void testIncompatibleNoImplicitWatchLabel() throws Exception {
+    setUpContextForRule(
+        "test",
+        StarlarkSemantics.DEFAULT.toBuilder()
+            .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL, true)
+            .build());
+    scratch.file(root.getRelative("foo").getPathString());
+    context.path(fakeFileLabel);
+    context.readFile(fakeFileLabel, "no", thread);
+    assertThat(context.getRecordedFileInputs()).isEmpty();
+  }
 }

src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java

@@ -127,6 +127,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep
         "--enable_bzlmod=" + rand.nextBoolean(),
         "--enable_workspace=" + rand.nextBoolean(),
         "--experimental_isolated_extension_usages=" + rand.nextBoolean(),
+        "--incompatible_no_implicit_watch_label=" + rand.nextBoolean(),
         "--experimental_google_legacy_api=" + rand.nextBoolean(),
         "--experimental_platforms_api=" + rand.nextBoolean(),
         "--incompatible_allow_tags_propagation=" + rand.nextBoolean(), // flag, Java names differ
@@ -174,6 +175,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .setBool(BuildLanguageOptions.ENABLE_BZLMOD, rand.nextBoolean())
         .setBool(BuildLanguageOptions.ENABLE_WORKSPACE, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, rand.nextBoolean())
+        .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_IMPLICIT_WATCH_LABEL, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_GOOGLE_LEGACY_API, rand.nextBoolean())
         .setBool(BuildLanguageOptions.EXPERIMENTAL_PLATFORMS_API, rand.nextBoolean())
         .setBool(BuildLanguageOptions.INCOMPATIBLE_ALLOW_TAGS_PROPAGATION, rand.nextBoolean())