diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java index aae70a6dfece07..b8dce6a6a2851e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkBaseExternalContext.java +++ b/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 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); @@ -1351,22 +1351,14 @@ during the analysis phase and thus cannot depends on a target result (the \ "string, Label or path 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( @@ -1399,7 +1391,7 @@ file when it is legal to do so (see watch() 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 +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 @@ -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; } @@ -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()); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 4c5ed3d3a6f040..bac46b6f9314dd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java +++ b/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 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 watch() 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); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 9d9c8d12ec5f93..e50d5c5cc8d6ff 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -53,7 +53,8 @@ *
  • 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}. * */ public final class BuildLanguageOptions extends OptionsBase { @@ -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 repository_ctx that are passed a Label will no" + + " longer automatically watch the file under that label for changes even if" + + " watch = \"no\", and repository_ctx.path no longer" + + " causes the returned path to be watched. Use repository_ctx.watch" + + " instead.") + public boolean incompatibleNoImplicitWatchLabel; + @Option( name = "incompatible_stop_exporting_build_file_path", defaultValue = "false", @@ -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) @@ -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"; diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 2d452a8f230929..cc5f3a3fd0d10d 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/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", - ], ) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java index 05c30e32699610..8530a03f5d9f67 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java +++ b/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); @@ -161,6 +167,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, @@ -190,11 +203,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); } @@ -237,16 +255,16 @@ public void testWhich() throws Exception { @Test public void testFile() throws Exception { setUpContextForRule("test"); - context.createFile(context.path("foobar"), "", true, true, thread); - context.createFile(context.path("foo/bar"), "foobar", true, true, thread); - context.createFile(context.path("bar/foo/bar"), "", true, true, thread); + context.createFile(context.getPath("foobar"), "", true, true, thread); + context.createFile(context.getPath("foo/bar"), "foobar", true, true, thread); + context.createFile(context.getPath("bar/foo/bar"), "", true, true, thread); testOutputFile(outputDirectory.getChild("foobar"), ""); testOutputFile(outputDirectory.getRelative("foo/bar"), "foobar"); testOutputFile(outputDirectory.getRelative("bar/foo/bar"), ""); try { - context.createFile(context.path("/absolute"), "", true, true, thread); + context.createFile(context.getPath("/absolute"), "", true, true, thread); fail("Expected error on creating path outside of the repository directory"); } catch (RepositoryFunctionException ex) { assertThat(ex) @@ -255,7 +273,7 @@ public void testFile() throws Exception { .isEqualTo("Cannot write outside of the repository directory for path /absolute"); } try { - context.createFile(context.path("../somepath"), "", true, true, thread); + context.createFile(context.getPath("../somepath"), "", true, true, thread); fail("Expected error on creating path outside of the repository directory"); } catch (RepositoryFunctionException ex) { assertThat(ex) @@ -264,7 +282,7 @@ public void testFile() throws Exception { .isEqualTo("Cannot write outside of the repository directory for path /somepath"); } try { - context.createFile(context.path("foo/../../somepath"), "", true, true, thread); + context.createFile(context.getPath("foo/../../somepath"), "", true, true, thread); fail("Expected error on creating path outside of the repository directory"); } catch (RepositoryFunctionException ex) { assertThat(ex) @@ -278,14 +296,16 @@ public void testFile() throws Exception { public void testDelete() throws Exception { setUpContextForRule("testDelete"); Path bar = outputDirectory.getRelative("foo/bar"); - StarlarkPath barPath = context.path(bar.getPathString()); + Object path1 = bar.getPathString(); + StarlarkPath barPath = context.getPath(path1); context.createFile(barPath, "content", true, true, thread); assertThat(context.delete(barPath, thread)).isTrue(); assertThat(context.delete(barPath, thread)).isFalse(); Path tempFile = scratch.file("/abcde/b", "123"); - assertThat(context.delete(context.path(tempFile.getPathString()), thread)).isTrue(); + Object path = tempFile.getPathString(); + assertThat(context.delete(context.getPath(path), thread)).isTrue(); Path innerDir = scratch.dir("/some/inner"); scratch.dir("/some/inner/deeper"); @@ -315,20 +335,20 @@ public void testDelete() throws Exception { @Test public void testRead() throws Exception { setUpContextForRule("test"); - context.createFile(context.path("foo/bar"), "foobar", true, true, thread); + context.createFile(context.getPath("foo/bar"), "foobar", true, true, thread); - String content = context.readFile(context.path("foo/bar"), "auto", thread); + String content = context.readFile(context.getPath("foo/bar"), "auto", thread); assertThat(content).isEqualTo("foobar"); } @Test public void testPatch() throws Exception { setUpContextForRule("test"); - StarlarkPath foo = context.path("foo"); + StarlarkPath foo = context.getPath("foo"); context.createFile(foo, "line one\n", false, true, thread); - StarlarkPath patchFile = context.path("my.patch"); + StarlarkPath patchFile = context.getPath("my.patch"); context.createFile( - context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); + context.getPath("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); context.patch(patchFile, StarlarkInt.of(0), "auto", thread); testOutputFile(foo.getPath(), "line one\nline two\n"); } @@ -336,9 +356,9 @@ public void testPatch() throws Exception { @Test public void testCannotFindFileToPatch() throws Exception { setUpContextForRule("test"); - StarlarkPath patchFile = context.path("my.patch"); + StarlarkPath patchFile = context.getPath("my.patch"); context.createFile( - context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); + context.getPath("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); try { context.patch(patchFile, StarlarkInt.of(0), "auto", thread); fail("Expected RepositoryFunctionException"); @@ -355,9 +375,9 @@ public void testCannotFindFileToPatch() throws Exception { @Test public void testPatchOutsideOfExternalRepository() throws Exception { setUpContextForRule("test"); - StarlarkPath patchFile = context.path("my.patch"); + StarlarkPath patchFile = context.getPath("my.patch"); context.createFile( - context.path("my.patch"), + context.getPath("my.patch"), "--- ../other_root/foo\n" + "+++ ../other_root/foo\n" + ONE_LINE_PATCH, false, true, @@ -378,11 +398,11 @@ public void testPatchOutsideOfExternalRepository() throws Exception { @Test public void testPatchErrorWasThrown() throws Exception { setUpContextForRule("test"); - StarlarkPath foo = context.path("foo"); - StarlarkPath patchFile = context.path("my.patch"); + StarlarkPath foo = context.getPath("foo"); + StarlarkPath patchFile = context.getPath("my.patch"); context.createFile(foo, "line three\n", false, true, thread); context.createFile( - context.path("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); + context.getPath("my.patch"), "--- foo\n+++ foo\n" + ONE_LINE_PATCH, false, true, thread); try { context.patch(patchFile, StarlarkInt.of(0), "auto", thread); fail("Expected RepositoryFunctionException"); @@ -459,12 +479,12 @@ public void testRemoteExec() throws Exception { @Test public void testSymlink() throws Exception { setUpContextForRule("test"); - context.createFile(context.path("foo"), "foobar", true, true, thread); + context.createFile(context.getPath("foo"), "foobar", true, true, thread); - context.symlink(context.path("foo"), context.path("bar"), thread); + context.symlink(context.getPath("foo"), context.getPath("bar"), thread); testOutputFile(outputDirectory.getChild("bar"), "foobar"); - assertThat(context.path("bar").realpath()).isEqualTo(context.path("foo")); + assertThat(context.getPath("bar").realpath()).isEqualTo(context.getPath("foo")); } private static void testOutputFile(Path path, String content) throws IOException { @@ -481,11 +501,11 @@ public void testDirectoryListing() throws Exception { scratch.file("/my/folder/a"); scratch.file("/my/folder/b"); scratch.file("/my/folder/c"); - assertThat(context.path("/my/folder").readdir("no")) + assertThat(context.getPath("/my/folder").readdir("no")) .containsExactly( - context.path("/my/folder/a"), - context.path("/my/folder/b"), - context.path("/my/folder/c")); + context.getPath("/my/folder/a"), + context.getPath("/my/folder/b"), + context.getPath("/my/folder/c")); } @Test @@ -493,4 +513,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()); + StarlarkPath unusedPath = context.getPath(fakeFileLabel); + String unusedRead = 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()); + StarlarkPath unusedPath = context.getPath(fakeFileLabel); + String unusedRead = context.readFile(fakeFileLabel, "no", thread); + assertThat(context.getRecordedFileInputs()).isEmpty(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index e3b7a5e4db7af9..d2b8299d5cd777 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/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()) diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 27154ea188ec95..063a4a031c76a9 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2861,6 +2861,98 @@ EOF expect_log "I see: something" } +function test_incompatible_no_implicit_watch_label() { + # when reading a file through a label with watch="no", we should not watch it. + local outside_dir="${TEST_TMPDIR}/outside_dir" + mkdir -p "${outside_dir}" + echo nothing > ${outside_dir}/data.txt + + create_new_workspace + cat > $(setup_module_dot_bazel) < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "(path) I see: fire" + expect_log "(direct) I see: fire" + + # Running Bazel again shouldn't cause a refetch. + bazel build --incompatible_no_implicit_watch_label @foo >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # Even changing the file shouldn't cause a refetch. + cat > MODULE.bazel <& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" +} + +function test_no_incompatible_no_implicit_watch_label() { + # when reading a file through a label with watch="no", we do watch it. + local outside_dir="${TEST_TMPDIR}/outside_dir" + mkdir -p "${outside_dir}" + echo nothing > ${outside_dir}/data.txt + + create_new_workspace + cat > $(setup_module_dot_bazel) < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "(direct) I see: fire" + + # Running Bazel again shouldn't cause a refetch. + bazel build --noincompatible_no_implicit_watch_label @foo >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # Changing the file should cause a refetch. + cat > MODULE.bazel <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see:" +} + function test_bad_marker_file_ignored() { # when reading a file in another repo, we should watch it. local outside_dir="${TEST_TMPDIR}/outside_dir"