Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,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) {
Expand Down Expand Up @@ -1274,9 +1277,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) {
Expand All @@ -1287,12 +1287,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,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");
}

Expand All @@ -338,7 +338,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)
Expand All @@ -361,7 +361,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)
Expand All @@ -382,7 +382,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)
Expand Down Expand Up @@ -459,7 +459,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"));
Expand Down
1 change: 1 addition & 0 deletions src/test/py/bazel/bzlmod/bazel_fetch_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import os
import tempfile
from absl.testing import absltest

from src.test.py.bazel import test_base
from src.test.py.bazel.bzlmod.test_utils import BazelRegistry

Expand Down
Loading
Loading