Skip to content

Commit a5376aa

Browse files
Wyveraldcopybara-github
Wyverald
authored and
copybara-github
committed
Watch arbitrary file in repo rules
* Starlark API changes * New method `rctx.watch()` that watches a path. As of right now, said path must exist and be a regular file, but that will change in follow-ups. * Existing method `rctx.read()` gets a new optional string parameter `watch` that defaults to `auto`. This causes the file read to be watched as well by default, even if it's not addressed by a label. See `rctx.read()` docs for the semantics of `watch`. * Both changes apply to `mctx` as well, except that no files outside the current Bazel workspace may be watched for `mctx`. * Marker file format changes * `FILE:` marker file entries can now be `FILE:` followed by either an absolute path (starts with a singular '/'), or a label-like ... thing (which is a label but has a slash in place of a colon). This is because we might be watching something inside a repo that is _not_ under a package. * Same change will happen to the `accumulatedFileDigests` sections of the module lockfile. * Code changes * `RepoRecordedInput.File` subclasses are renamed to `FileOutsideWorkspace` (absolute paths) and `FileInsideWorkspace` (label-like things) instead to better reflect their meaning. * Unified the file watching/digest checking logic in SingleExtensionEvalFunction with the RepoRecordedInput stuff. Follow-ups will unify more. * Moved RepoRecordedInput to its own BUILD target as it's being used outside the repo fetching machinery (namely, module extensions) * Since `FileOutsideWorkspace` has to be an absolute path (never relative), we don't need a "base directory" for the RepoRecordedInput parser. What we really need is just the FileSystem object, which we can get from the PathPackageLocator available in Skyframe. Refactored code to reflect that. Work towards #20952. Closes #21230. RELNOTES: Added a new method, `repository_ctx.watch()`, which asks Bazel to watch for changes to an arbitrary file. When said file changes, Bazel will refetch the repo. PiperOrigin-RevId: 607097422 Change-Id: Ib96a49461deddd7f4c786bd1c227de18b2dd71d7
1 parent c3c21d9 commit a5376aa

29 files changed

Lines changed: 727 additions & 235 deletions

‎MODULE.bazel.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ java_library(
155155
"//src/main/java/com/google/devtools/build/lib/cmdline",
156156
"//src/main/java/com/google/devtools/build/lib/events",
157157
"//src/main/java/com/google/devtools/build/lib/packages",
158+
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
158159
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
159160
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
160161
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
@@ -217,6 +218,7 @@ java_library(
217218
"//src/main/java/com/google/devtools/build/lib/events",
218219
"//src/main/java/com/google/devtools/build/lib/packages",
219220
"//src/main/java/com/google/devtools/build/lib/profiler",
221+
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
220222
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
221223
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
222224
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_failed_exception",

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
@GenerateTypeAdapter
3838
public abstract class BazelLockFileValue implements SkyValue, Postable {
3939

40-
public static final int LOCK_FILE_VERSION = 4;
40+
public static final int LOCK_FILE_VERSION = 5;
4141

4242
@SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;
4343

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.devtools.build.lib.cmdline.Label;
3030
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
3131
import com.google.devtools.build.lib.cmdline.RepositoryName;
32+
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
3233
import com.google.devtools.build.lib.vfs.Path;
3334
import com.google.gson.Gson;
3435
import com.google.gson.GsonBuilder;
@@ -444,6 +445,20 @@ public Location read(JsonReader jsonReader) throws IOException {
444445
}
445446
}
446447

448+
private static final TypeAdapter<RepoRecordedInput.File> REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER =
449+
new TypeAdapter<>() {
450+
@Override
451+
public void write(JsonWriter jsonWriter, RepoRecordedInput.File value) throws IOException {
452+
jsonWriter.value(value.toStringInternal());
453+
}
454+
455+
@Override
456+
public RepoRecordedInput.File read(JsonReader jsonReader) throws IOException {
457+
return (RepoRecordedInput.File)
458+
RepoRecordedInput.File.PARSER.parse(jsonReader.nextString());
459+
}
460+
};
461+
447462
public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
448463
return new GsonBuilder()
449464
.setPrettyPrinting()
@@ -468,6 +483,7 @@ public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
468483
.registerTypeAdapter(ModuleExtensionId.IsolationKey.class, ISOLATION_KEY_TYPE_ADAPTER)
469484
.registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
470485
.registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
486+
.registerTypeAdapter(RepoRecordedInput.File.class, REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER)
471487
.create();
472488
}
473489

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
import com.google.auto.value.AutoValue;
1818
import com.google.common.collect.ImmutableMap;
1919
import com.google.common.collect.ImmutableTable;
20-
import com.google.devtools.build.lib.cmdline.Label;
2120
import com.google.devtools.build.lib.cmdline.RepositoryName;
2221
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
22+
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
2323
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
2424
import java.util.Optional;
2525

@@ -41,7 +41,7 @@ public static Builder builder() {
4141
@SuppressWarnings("mutable")
4242
public abstract byte[] getBzlTransitiveDigest();
4343

44-
public abstract ImmutableMap<Label, String> getAccumulatedFileDigests();
44+
public abstract ImmutableMap<RepoRecordedInput.File, String> getRecordedFileInputs();
4545

4646
public abstract ImmutableMap<String, String> getEnvVariables();
4747

@@ -65,7 +65,8 @@ public abstract static class Builder {
6565

6666
public abstract Builder setBzlTransitiveDigest(byte[] digest);
6767

68-
public abstract Builder setAccumulatedFileDigests(ImmutableMap<Label, String> value);
68+
public abstract Builder setRecordedFileInputs(
69+
ImmutableMap<RepoRecordedInput.File, String> value);
6970

7071
public abstract Builder setEnvVariables(ImmutableMap<String, String> value);
7172

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import com.google.common.collect.ImmutableMap;
1818
import com.google.devtools.build.docgen.annot.DocCategory;
19+
import com.google.devtools.build.lib.analysis.BlazeDirectories;
1920
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
2021
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkBaseExternalContext;
2122
import com.google.devtools.build.lib.runtime.ProcessWrapper;
@@ -50,6 +51,7 @@ public class ModuleExtensionContext extends StarlarkBaseExternalContext {
5051

5152
protected ModuleExtensionContext(
5253
Path workingDirectory,
54+
BlazeDirectories directories,
5355
Environment env,
5456
Map<String, String> envVariables,
5557
DownloadManager downloadManager,
@@ -62,13 +64,15 @@ protected ModuleExtensionContext(
6264
boolean rootModuleHasNonDevDependency) {
6365
super(
6466
workingDirectory,
67+
directories,
6568
env,
6669
envVariables,
6770
downloadManager,
6871
timeoutScaling,
6972
processWrapper,
7073
starlarkSemantics,
71-
remoteExecutor);
74+
remoteExecutor,
75+
/* allowWatchingFilesOutsideWorkspace= */ false);
7276
this.extensionId = extensionId;
7377
this.modules = modules;
7478
this.rootModuleHasNonDevDependency = rootModuleHasNonDevDependency;

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java‎

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import com.google.common.collect.Iterables;
3030
import com.google.common.collect.Maps;
3131
import com.google.common.collect.Table;
32-
import com.google.devtools.build.lib.actions.FileValue;
3332
import com.google.devtools.build.lib.analysis.BlazeDirectories;
3433
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
3534
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
@@ -48,6 +47,7 @@
4847
import com.google.devtools.build.lib.profiler.ProfilerTask;
4948
import com.google.devtools.build.lib.profiler.SilentCloseable;
5049
import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException;
50+
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
5151
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
5252
import com.google.devtools.build.lib.runtime.ProcessWrapper;
5353
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
@@ -61,7 +61,6 @@
6161
import com.google.devtools.build.lib.util.Fingerprint;
6262
import com.google.devtools.build.lib.util.OS;
6363
import com.google.devtools.build.lib.vfs.Path;
64-
import com.google.devtools.build.lib.vfs.RootedPath;
6564
import com.google.devtools.build.skyframe.SkyFunction;
6665
import com.google.devtools.build.skyframe.SkyFunctionException;
6766
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -71,7 +70,6 @@
7170
import java.io.IOException;
7271
import java.util.ArrayList;
7372
import java.util.Arrays;
74-
import java.util.HashMap;
7573
import java.util.HashSet;
7674
import java.util.Iterator;
7775
import java.util.Map;
@@ -219,7 +217,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
219217
extension.getEvalFactors(),
220218
LockFileModuleExtension.builder()
221219
.setBzlTransitiveDigest(extension.getBzlTransitiveDigest())
222-
.setAccumulatedFileDigests(moduleExtensionResult.getAccumulatedFileDigests())
220+
.setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs())
223221
.setEnvVariables(extension.getEnvVars())
224222
.setGeneratedRepoSpecs(generatedRepoSpecs)
225223
.setModuleExtensionMetadata(moduleExtensionMetadata)
@@ -291,7 +289,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile(
291289
+ extensionId
292290
+ "' have changed");
293291
}
294-
if (didFilesChange(env, lockedExtension.getAccumulatedFileDigests())) {
292+
if (didFilesChange(env, directories, lockedExtension.getRecordedFileInputs())) {
295293
diffRecorder.record(
296294
"One or more files the extension '" + extensionId + "' is using have changed");
297295
}
@@ -393,39 +391,15 @@ private static boolean didRepoMappingsChange(
393391
}
394392

395393
private static boolean didFilesChange(
396-
Environment env, ImmutableMap<Label, String> accumulatedFileDigests)
394+
Environment env,
395+
BlazeDirectories directories,
396+
ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs)
397397
throws InterruptedException, NeedsSkyframeRestartException {
398-
// Turn labels into FileValue keys & get those values
399-
Map<Label, FileValue.Key> fileKeys = new HashMap<>();
400-
for (Label label : accumulatedFileDigests.keySet()) {
401-
try {
402-
RootedPath rootedPath = RepositoryFunction.getRootedPathFromLabel(label, env);
403-
fileKeys.put(label, FileValue.key(rootedPath));
404-
} catch (NeedsSkyframeRestartException e) {
405-
throw e;
406-
} catch (EvalException e) {
407-
// Consider those exception to be a cause for invalidation
408-
return true;
409-
}
410-
}
411-
SkyframeLookupResult result = env.getValuesAndExceptions(fileKeys.values());
398+
boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedFileInputs);
412399
if (env.valuesMissing()) {
413400
throw new NeedsSkyframeRestartException();
414401
}
415-
416-
// Compare the collected file values with the hashes stored in the lockfile
417-
for (Entry<Label, String> entry : accumulatedFileDigests.entrySet()) {
418-
FileValue fileValue = (FileValue) result.get(fileKeys.get(entry.getKey()));
419-
try {
420-
if (!entry.getValue().equals(RepositoryFunction.fileValueToMarkerValue(fileValue))) {
421-
return true;
422-
}
423-
} catch (IOException e) {
424-
// Consider those exception to be a cause for invalidation
425-
return true;
426-
}
427-
}
428-
return false;
402+
return !upToDate;
429403
}
430404

431405
/**
@@ -956,7 +930,7 @@ public RunModuleExtensionResult run(
956930
}
957931
}
958932
return RunModuleExtensionResult.create(
959-
moduleContext.getAccumulatedFileDigests(),
933+
moduleContext.getRecordedFileInputs(),
960934
threadContext.getGeneratedRepoSpecs(),
961935
moduleExtensionMetadata,
962936
repoMappingRecorder.recordedEntries());
@@ -992,6 +966,7 @@ private ModuleExtensionContext createContext(
992966
rootUsage != null && rootUsage.getHasNonDevUseExtension();
993967
return new ModuleExtensionContext(
994968
workingDirectory,
969+
directories,
995970
env,
996971
clientEnvironmentSupplier.get(),
997972
downloadManager,
@@ -1016,7 +991,7 @@ static final class SingleExtensionEvalFunctionException extends SkyFunctionExcep
1016991
@AutoValue
1017992
abstract static class RunModuleExtensionResult {
1018993

1019-
abstract ImmutableMap<Label, String> getAccumulatedFileDigests();
994+
abstract ImmutableMap<RepoRecordedInput.File, String> getRecordedFileInputs();
1020995

1021996
abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();
1022997

@@ -1025,12 +1000,12 @@ abstract static class RunModuleExtensionResult {
10251000
abstract ImmutableTable<RepositoryName, String, RepositoryName> getRecordedRepoMappingEntries();
10261001

10271002
static RunModuleExtensionResult create(
1028-
ImmutableMap<Label, String> accumulatedFileDigests,
1003+
ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs,
10291004
ImmutableMap<String, RepoSpec> generatedRepoSpecs,
10301005
Optional<ModuleExtensionMetadata> moduleExtensionMetadata,
10311006
ImmutableTable<RepositoryName, String, RepositoryName> recordedRepoMappingEntries) {
10321007
return new AutoValue_SingleExtensionEvalFunction_RunModuleExtensionResult(
1033-
accumulatedFileDigests,
1008+
recordedFileInputs,
10341009
generatedRepoSpecs,
10351010
moduleExtensionMetadata,
10361011
recordedRepoMappingEntries);

‎src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ java_library(
3737
"//src/main/java/com/google/devtools/build/lib/events",
3838
"//src/main/java/com/google/devtools/build/lib/packages",
3939
"//src/main/java/com/google/devtools/build/lib/repository:repository_events",
40+
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
4041
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
4142
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
4243
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value",

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ java_library(
3535
"//src/main/java/com/google/devtools/build/lib/profiler",
3636
"//src/main/java/com/google/devtools/build/lib/repository:repository_events",
3737
"//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event",
38+
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
3839
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
3940
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
4041
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_attribute_mapper",
@@ -46,7 +47,6 @@ java_library(
4647
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
4748
"//src/main/java/com/google/devtools/build/lib/util",
4849
"//src/main/java/com/google/devtools/build/lib/util:string",
49-
"//src/main/java/com/google/devtools/build/lib/util/io",
5050
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
5151
"//src/main/java/com/google/devtools/build/lib/vfs",
5252
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",

0 commit comments

Comments
 (0)