Skip to content

Commit

Permalink
Add --incompatible_no_implicit_watch_label
Browse files Browse the repository at this point in the history
When the flag is flipped, `ctx.path` no longer watches paths resolved from `Label` parameters and other methods on `ctx` no longer watch such paths even with `watch = "no"`.

Closes #23750.

PiperOrigin-RevId: 681978073
Change-Id: I9bef5c735db54c971beb806aa1f1a0eb287e3ff2
  • Loading branch information
fmeum authored and copybara-github committed Oct 3, 2024
1 parent a555dfb commit 5019697
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ public Object download(
checksumValidation = e;
}

StarlarkPath outputPath = getPath("download()", output);
StarlarkPath outputPath = getPath(output);
WorkspaceRuleEvent w =
WorkspaceRuleEvent.newDownloadEvent(
urls,
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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(
Expand All @@ -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 =
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1351,22 +1351,14 @@ during the analysis phase and thus cannot depends on a target result (the \
"<code>string</code>, <code>Label</code> or <code>path</code> from which to create"
+ " a path from.")
})
public StarlarkPath path(Object path) throws EvalException, InterruptedException {
return getPath("path()", 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 {
public StarlarkPath getPath(Object path) throws EvalException, InterruptedException {
return switch (path) {
case String s -> new StarlarkPath(this, workingDirectory.getRelative(s));
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(
Expand Down Expand Up @@ -1399,7 +1391,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());
Expand Down Expand Up @@ -1552,7 +1544,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
Expand Down Expand Up @@ -1835,7 +1827,7 @@ public StarlarkExecutionResult execute(

Path workingDirectoryPath;
if (overrideWorkingDirectory != null && !overrideWorkingDirectory.isEmpty()) {
workingDirectoryPath = getPath("execute()", overrideWorkingDirectory).getPath();
workingDirectoryPath = getPath(overrideWorkingDirectory).getPath();
} else {
workingDirectoryPath = workingDirectory;
}
Expand Down Expand Up @@ -1922,7 +1914,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());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -282,6 +283,20 @@ 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",
Expand Down Expand Up @@ -831,6 +846,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)
Expand Down Expand Up @@ -944,6 +960,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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -54,7 +56,6 @@ java_library(
java_test(
name = "StarlarkTests",
testonly = 1,
srcs = glob(["*.java"]),
tags = [
"rules",
],
Expand All @@ -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",
],
)
Loading

0 comments on commit 5019697

Please sign in to comment.