Skip to content

Commit

Permalink
Watch arbitrary file in repo rules
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Wyverald authored and copybara-github committed Feb 14, 2024
1 parent c3c21d9 commit a5376aa
Show file tree
Hide file tree
Showing 29 changed files with 727 additions and 235 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ java_library(
"//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/rules:repository/repo_recorded_input",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
Expand Down Expand Up @@ -217,6 +218,7 @@ java_library(
"//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/profiler",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_failed_exception",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
@GenerateTypeAdapter
public abstract class BazelLockFileValue implements SkyValue, Postable {

public static final int LOCK_FILE_VERSION = 4;
public static final int LOCK_FILE_VERSION = 5;

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
import com.google.devtools.build.lib.vfs.Path;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
Expand Down Expand Up @@ -444,6 +445,20 @@ public Location read(JsonReader jsonReader) throws IOException {
}
}

private static final TypeAdapter<RepoRecordedInput.File> REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER =
new TypeAdapter<>() {
@Override
public void write(JsonWriter jsonWriter, RepoRecordedInput.File value) throws IOException {
jsonWriter.value(value.toStringInternal());
}

@Override
public RepoRecordedInput.File read(JsonReader jsonReader) throws IOException {
return (RepoRecordedInput.File)
RepoRecordedInput.File.PARSER.parse(jsonReader.nextString());
}
};

public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
return new GsonBuilder()
.setPrettyPrinting()
Expand All @@ -468,6 +483,7 @@ public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) {
.registerTypeAdapter(ModuleExtensionId.IsolationKey.class, ISOLATION_KEY_TYPE_ADAPTER)
.registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter())
.registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER)
.registerTypeAdapter(RepoRecordedInput.File.class, REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER)
.create();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableTable;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
import java.util.Optional;

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

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

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

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

public abstract Builder setBzlTransitiveDigest(byte[] digest);

public abstract Builder setAccumulatedFileDigests(ImmutableMap<Label, String> value);
public abstract Builder setRecordedFileInputs(
ImmutableMap<RepoRecordedInput.File, String> value);

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkBaseExternalContext;
import com.google.devtools.build.lib.runtime.ProcessWrapper;
Expand Down Expand Up @@ -50,6 +51,7 @@ public class ModuleExtensionContext extends StarlarkBaseExternalContext {

protected ModuleExtensionContext(
Path workingDirectory,
BlazeDirectories directories,
Environment env,
Map<String, String> envVariables,
DownloadManager downloadManager,
Expand All @@ -62,13 +64,15 @@ protected ModuleExtensionContext(
boolean rootModuleHasNonDevDependency) {
super(
workingDirectory,
directories,
env,
envVariables,
downloadManager,
timeoutScaling,
processWrapper,
starlarkSemantics,
remoteExecutor);
remoteExecutor,
/* allowWatchingFilesOutsideWorkspace= */ false);
this.extensionId = extensionId;
this.modules = modules;
this.rootModuleHasNonDevDependency = rootModuleHasNonDevDependency;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Table;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
Expand All @@ -48,6 +47,7 @@
import com.google.devtools.build.lib.profiler.ProfilerTask;
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException;
import com.google.devtools.build.lib.rules.repository.RepoRecordedInput;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.runtime.ProcessWrapper;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
Expand All @@ -61,7 +61,6 @@
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.util.OS;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
Expand All @@ -71,7 +70,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
Expand Down Expand Up @@ -219,7 +217,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
extension.getEvalFactors(),
LockFileModuleExtension.builder()
.setBzlTransitiveDigest(extension.getBzlTransitiveDigest())
.setAccumulatedFileDigests(moduleExtensionResult.getAccumulatedFileDigests())
.setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs())
.setEnvVariables(extension.getEnvVars())
.setGeneratedRepoSpecs(generatedRepoSpecs)
.setModuleExtensionMetadata(moduleExtensionMetadata)
Expand Down Expand Up @@ -291,7 +289,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile(
+ extensionId
+ "' have changed");
}
if (didFilesChange(env, lockedExtension.getAccumulatedFileDigests())) {
if (didFilesChange(env, directories, lockedExtension.getRecordedFileInputs())) {
diffRecorder.record(
"One or more files the extension '" + extensionId + "' is using have changed");
}
Expand Down Expand Up @@ -393,39 +391,15 @@ private static boolean didRepoMappingsChange(
}

private static boolean didFilesChange(
Environment env, ImmutableMap<Label, String> accumulatedFileDigests)
Environment env,
BlazeDirectories directories,
ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs)
throws InterruptedException, NeedsSkyframeRestartException {
// Turn labels into FileValue keys & get those values
Map<Label, FileValue.Key> fileKeys = new HashMap<>();
for (Label label : accumulatedFileDigests.keySet()) {
try {
RootedPath rootedPath = RepositoryFunction.getRootedPathFromLabel(label, env);
fileKeys.put(label, FileValue.key(rootedPath));
} catch (NeedsSkyframeRestartException e) {
throw e;
} catch (EvalException e) {
// Consider those exception to be a cause for invalidation
return true;
}
}
SkyframeLookupResult result = env.getValuesAndExceptions(fileKeys.values());
boolean upToDate = RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedFileInputs);
if (env.valuesMissing()) {
throw new NeedsSkyframeRestartException();
}

// Compare the collected file values with the hashes stored in the lockfile
for (Entry<Label, String> entry : accumulatedFileDigests.entrySet()) {
FileValue fileValue = (FileValue) result.get(fileKeys.get(entry.getKey()));
try {
if (!entry.getValue().equals(RepositoryFunction.fileValueToMarkerValue(fileValue))) {
return true;
}
} catch (IOException e) {
// Consider those exception to be a cause for invalidation
return true;
}
}
return false;
return !upToDate;
}

/**
Expand Down Expand Up @@ -956,7 +930,7 @@ public RunModuleExtensionResult run(
}
}
return RunModuleExtensionResult.create(
moduleContext.getAccumulatedFileDigests(),
moduleContext.getRecordedFileInputs(),
threadContext.getGeneratedRepoSpecs(),
moduleExtensionMetadata,
repoMappingRecorder.recordedEntries());
Expand Down Expand Up @@ -992,6 +966,7 @@ private ModuleExtensionContext createContext(
rootUsage != null && rootUsage.getHasNonDevUseExtension();
return new ModuleExtensionContext(
workingDirectory,
directories,
env,
clientEnvironmentSupplier.get(),
downloadManager,
Expand All @@ -1016,7 +991,7 @@ static final class SingleExtensionEvalFunctionException extends SkyFunctionExcep
@AutoValue
abstract static class RunModuleExtensionResult {

abstract ImmutableMap<Label, String> getAccumulatedFileDigests();
abstract ImmutableMap<RepoRecordedInput.File, String> getRecordedFileInputs();

abstract ImmutableMap<String, RepoSpec> getGeneratedRepoSpecs();

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

static RunModuleExtensionResult create(
ImmutableMap<Label, String> accumulatedFileDigests,
ImmutableMap<RepoRecordedInput.File, String> recordedFileInputs,
ImmutableMap<String, RepoSpec> generatedRepoSpecs,
Optional<ModuleExtensionMetadata> moduleExtensionMetadata,
ImmutableTable<RepositoryName, String, RepositoryName> recordedRepoMappingEntries) {
return new AutoValue_SingleExtensionEvalFunction_RunModuleExtensionResult(
accumulatedFileDigests,
recordedFileInputs,
generatedRepoSpecs,
moduleExtensionMetadata,
recordedRepoMappingEntries);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ java_library(
"//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/repository:repository_events",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/repository:repository_events",
"//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_attribute_mapper",
Expand All @@ -46,7 +47,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/util/io:out-err",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
Expand Down
Loading

0 comments on commit a5376aa

Please sign in to comment.