From 18ba4492c752530b168daa563987400f7f9df4bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 20 Feb 2024 22:54:06 +0100 Subject: [PATCH] [7.1.0] Repo file/dir watching API (#21435) RELNOTES: Added new APIs to watch arbitrary files or directory trees `repository_ctx.watch`, `repository_ctx.watch_tree`. --- MODULE.bazel.lock | 2 +- .../devtools/build/lib/bazel/bzlmod/BUILD | 2 + .../lib/bazel/bzlmod/BazelLockFileValue.java | 2 +- .../lib/bazel/bzlmod/GsonTypeAdapterUtil.java | 34 + .../bazel/bzlmod/LockFileModuleExtension.java | 12 +- .../bazel/bzlmod/ModuleExtensionContext.java | 6 +- .../bzlmod/SingleExtensionEvalFunction.java | 66 +- .../devtools/build/lib/bazel/repository/BUILD | 1 + .../LocalConfigPlatformFunction.java | 3 +- .../build/lib/bazel/repository/starlark/BUILD | 3 + .../RepoFetchingSkyKeyComputeState.java | 5 +- .../starlark/StarlarkBaseExternalContext.java | 244 +++++-- .../repository/starlark/StarlarkPath.java | 78 ++- .../starlark/StarlarkRepositoryContext.java | 138 +++- .../starlark/StarlarkRepositoryFunction.java | 120 +--- .../android/AndroidNdkRepositoryFunction.java | 20 +- .../android/AndroidSdkRepositoryFunction.java | 13 +- .../build/lib/bazel/rules/android/BUILD | 1 + .../com/google/devtools/build/lib/rules/BUILD | 31 +- .../repository/LocalRepositoryFunction.java | 2 +- .../NewLocalRepositoryFunction.java | 4 +- .../repository/NewRepositoryFileHandler.java | 18 +- .../rules/repository/RepoRecordedInput.java | 658 ++++++++++++++++++ .../RepositoryDelegatorFunction.java | 71 +- .../rules/repository/RepositoryFunction.java | 200 +----- .../google/devtools/build/lib/skyframe/BUILD | 28 + .../skyframe/DirectoryTreeDigestFunction.java | 148 ++++ .../skyframe/DirectoryTreeDigestValue.java | 51 ++ .../build/lib/skyframe/SkyFunctions.java | 2 + .../build/lib/skyframe/SkyframeExecutor.java | 1 + .../build/lib/bazel/repository/starlark/BUILD | 8 +- .../repository/starlark/StarlarkPathTest.java | 12 +- .../StarlarkRepositoryContextTest.java | 34 +- .../devtools/build/lib/rules/repository/BUILD | 1 + .../repository/RepositoryFunctionTest.java | 6 +- .../google/devtools/build/lib/skyframe/BUILD | 3 + .../DirectoryTreeDigestFunctionTest.java | 226 ++++++ src/test/py/bazel/bzlmod/bazel_fetch_test.py | 2 +- .../py/bazel/bzlmod/bazel_lockfile_test.py | 172 +++++ .../shell/bazel/starlark_repository_test.sh | 406 ++++++++++- src/test/tools/bzlmod/MODULE.bazel.lock | 40 +- 41 files changed, 2400 insertions(+), 474 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestValue.java create mode 100644 src/test/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunctionTest.java diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 141508a08fd4b9..389445a275f790 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2293,7 +2293,7 @@ "general": { "bzlTransitiveDigest": "ViQGEDr/pPfdaylbQ9kIMC61dAyi2clQRXxliJle+HM=", "accumulatedFileDigests": { - "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "0493425c35cde8310abe79cfbe790ea6b209c7cf68ec80d48ba7dd8cc8170ca5", + "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "2752af68cac5408918d138220bd3b8de1048117eabec83aa75c670d841cc1e34", "@@//:MODULE.bazel": "623f08c8c353f74d1ec41fc159d1e53d1f707b7dd5e77e97a6136bc6ed93fc85" }, "envVariables": {}, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index dd396f11872f50..9d4441232f68aa 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -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/autocodec", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", @@ -216,6 +217,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_value", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index 3ae71c57c66135..3c85644a59a996 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -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 = 6; @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java index 808d83821d6291..a2b62f2789ccc4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java @@ -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; @@ -444,6 +445,36 @@ public Location read(JsonReader jsonReader) throws IOException { } } + private static final TypeAdapter 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()); + } + }; + + private static final TypeAdapter + REPO_RECORDED_INPUT_DIRENTS_TYPE_ADAPTER = + new TypeAdapter<>() { + @Override + public void write(JsonWriter jsonWriter, RepoRecordedInput.Dirents value) + throws IOException { + jsonWriter.value(value.toStringInternal()); + } + + @Override + public RepoRecordedInput.Dirents read(JsonReader jsonReader) throws IOException { + return (RepoRecordedInput.Dirents) + RepoRecordedInput.Dirents.PARSER.parse(jsonReader.nextString()); + } + }; + public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { return new GsonBuilder() .setPrettyPrinting() @@ -468,6 +499,9 @@ 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) + .registerTypeAdapter( + RepoRecordedInput.Dirents.class, REPO_RECORDED_INPUT_DIRENTS_TYPE_ADAPTER) .create(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java index 1443189e389a42..64e401ed6f224b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java @@ -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; @@ -41,7 +41,9 @@ public static Builder builder() { @SuppressWarnings("mutable") public abstract byte[] getBzlTransitiveDigest(); - public abstract ImmutableMap getAccumulatedFileDigests(); + public abstract ImmutableMap getRecordedFileInputs(); + + public abstract ImmutableMap getRecordedDirentsInputs(); public abstract ImmutableMap getEnvVariables(); @@ -65,7 +67,11 @@ public abstract static class Builder { public abstract Builder setBzlTransitiveDigest(byte[] digest); - public abstract Builder setAccumulatedFileDigests(ImmutableMap value); + public abstract Builder setRecordedFileInputs( + ImmutableMap value); + + public abstract Builder setRecordedDirentsInputs( + ImmutableMap value); public abstract Builder setEnvVariables(ImmutableMap value); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java index cdddef1c3a1479..1314114a770d86 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java @@ -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; @@ -50,6 +51,7 @@ public class ModuleExtensionContext extends StarlarkBaseExternalContext { protected ModuleExtensionContext( Path workingDirectory, + BlazeDirectories directories, Environment env, Map envVariables, DownloadManager downloadManager, @@ -62,13 +64,15 @@ protected ModuleExtensionContext( boolean rootModuleHasNonDevDependency) { super( workingDirectory, + directories, env, envVariables, downloadManager, timeoutScaling, processWrapper, starlarkSemantics, - remoteExecutor); + remoteExecutor, + /* allowWatchingPathsOutsideWorkspace= */ false); this.extensionId = extensionId; this.modules = modules; this.rootModuleHasNonDevDependency = rootModuleHasNonDevDependency; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 73e033d3592de9..8e02004ea1ce51 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -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; @@ -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; @@ -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; @@ -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; @@ -219,7 +217,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) extension.getEvalFactors(), LockFileModuleExtension.builder() .setBzlTransitiveDigest(extension.getBzlTransitiveDigest()) - .setAccumulatedFileDigests(moduleExtensionResult.getAccumulatedFileDigests()) + .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs()) + .setRecordedDirentsInputs(moduleExtensionResult.getRecordedDirentsInputs()) .setEnvVariables(extension.getEnvVars()) .setGeneratedRepoSpecs(generatedRepoSpecs) .setModuleExtensionMetadata(moduleExtensionMetadata) @@ -291,10 +290,16 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( + extensionId + "' have changed"); } - if (didFilesChange(env, lockedExtension.getAccumulatedFileDigests())) { + if (didRecordedInputsChange(env, directories, lockedExtension.getRecordedFileInputs())) { diffRecorder.record( "One or more files the extension '" + extensionId + "' is using have changed"); } + if (didRecordedInputsChange(env, directories, lockedExtension.getRecordedDirentsInputs())) { + diffRecorder.record( + "One or more directory listings watched by the extension '" + + extensionId + + "' have changed"); + } } catch (DiffFoundEarlyExitException ignored) { // ignored } @@ -392,40 +397,16 @@ private static boolean didRepoMappingsChange( return false; } - private static boolean didFilesChange( - Environment env, ImmutableMap accumulatedFileDigests) + private static boolean didRecordedInputsChange( + Environment env, + BlazeDirectories directories, + ImmutableMap recordedInputs) throws InterruptedException, NeedsSkyframeRestartException { - // Turn labels into FileValue keys & get those values - Map 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, recordedInputs); if (env.valuesMissing()) { throw new NeedsSkyframeRestartException(); } - - // Compare the collected file values with the hashes stored in the lockfile - for (Entry 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; } /** @@ -788,6 +769,7 @@ public RunModuleExtensionResult run( generatedRepoSpecs.put(name, repoSpec); } return RunModuleExtensionResult.create( + ImmutableMap.of(), ImmutableMap.of(), generatedRepoSpecs.buildOrThrow(), Optional.of(ModuleExtensionMetadata.REPRODUCIBLE), @@ -956,7 +938,8 @@ public RunModuleExtensionResult run( } } return RunModuleExtensionResult.create( - moduleContext.getAccumulatedFileDigests(), + moduleContext.getRecordedFileInputs(), + moduleContext.getRecordedDirentsInputs(), threadContext.getGeneratedRepoSpecs(), moduleExtensionMetadata, repoMappingRecorder.recordedEntries()); @@ -992,6 +975,7 @@ private ModuleExtensionContext createContext( rootUsage != null && rootUsage.getHasNonDevUseExtension(); return new ModuleExtensionContext( workingDirectory, + directories, env, clientEnvironmentSupplier.get(), downloadManager, @@ -1016,7 +1000,9 @@ static final class SingleExtensionEvalFunctionException extends SkyFunctionExcep @AutoValue abstract static class RunModuleExtensionResult { - abstract ImmutableMap getAccumulatedFileDigests(); + abstract ImmutableMap getRecordedFileInputs(); + + abstract ImmutableMap getRecordedDirentsInputs(); abstract ImmutableMap getGeneratedRepoSpecs(); @@ -1025,12 +1011,14 @@ abstract static class RunModuleExtensionResult { abstract ImmutableTable getRecordedRepoMappingEntries(); static RunModuleExtensionResult create( - ImmutableMap accumulatedFileDigests, + ImmutableMap recordedFileInputs, + ImmutableMap recordedDirentsInputs, ImmutableMap generatedRepoSpecs, Optional moduleExtensionMetadata, ImmutableTable recordedRepoMappingEntries) { return new AutoValue_SingleExtensionEvalFunction_RunModuleExtensionResult( - accumulatedFileDigests, + recordedFileInputs, + recordedDirentsInputs, generatedRepoSpecs, moduleExtensionMetadata, recordedRepoMappingEntries); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD index b624864b06e4b6..2f6a957d8f9cd6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/BUILD @@ -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", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunction.java index 9d2750c6fb2846..ef55022af8ceea 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/LocalConfigPlatformFunction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.bazel.ResolvedEvent; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.ResolvedFileValue; @@ -56,7 +57,7 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws RepositoryFunctionException { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 4ce0fabc14a265..880fb396010ae8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD @@ -35,12 +35,15 @@ 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", "//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_file_helper", "//src/main/java/com/google/devtools/build/lib/shell", "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value", "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java index a709603c830f05..19ce5cb66d849f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/RepoFetchingSkyKeyComputeState.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.bazel.repository.starlark; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunction.Environment.SkyKeyComputeState; @@ -71,12 +72,12 @@ enum Signal { @Nullable volatile Future workerFuture = null; /** - * This is where the {@code markerData} for the whole invocation is collected. + * This is where the recorded inputs & values for the whole invocation is collected. * *

{@link com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction} creates a * new map on each restart, so we can't simply plumb that in. */ - final Map markerData = new TreeMap<>(); + final Map recordedInputValues = new TreeMap<>(); SkyFunction.Environment signalForFreshEnv() throws InterruptedException { signalQueue.put(Signal.RESTART); 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 30e9f4f34d411c..263f093549200e 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 @@ -27,6 +27,7 @@ import com.google.common.collect.Maps; import com.google.common.util.concurrent.Futures; import com.google.devtools.build.lib.actions.FileValue; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.bazel.debug.WorkspaceRuleEvent; import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor; import com.google.devtools.build.lib.bazel.repository.DecompressorValue; @@ -36,6 +37,8 @@ import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.bazel.repository.downloader.HttpUtils; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelConstants; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress; @@ -47,12 +50,16 @@ 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.RepoRecordedInput.Dirents; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput.RepoCacheFriendlyPath; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult; import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; +import com.google.devtools.build.lib.skyframe.DirectoryListingValue; import com.google.devtools.build.lib.util.OsUtils; import com.google.devtools.build.lib.util.io.OutErr; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -62,7 +69,6 @@ import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; -import com.google.devtools.build.skyframe.SkyKey; import java.io.File; import java.io.IOException; import java.io.OutputStream; @@ -133,6 +139,7 @@ private interface AsyncTask { private static final int MAX_PROFILE_ARGS_LEN = 512; protected final Path workingDirectory; + protected final BlazeDirectories directories; protected final Environment env; protected final ImmutableMap envVariables; private final StarlarkOS osObject; @@ -140,21 +147,26 @@ private interface AsyncTask { protected final double timeoutScaling; @Nullable private final ProcessWrapper processWrapper; protected final StarlarkSemantics starlarkSemantics; - private final HashMap accumulatedFileDigests = new HashMap<>(); + private final HashMap recordedFileInputs = new HashMap<>(); + private final HashMap recordedDirentsInputs = new HashMap<>(); private final HashSet accumulatedEnvKeys = new HashSet<>(); private final RepositoryRemoteExecutor remoteExecutor; private final List asyncTasks; + private final boolean allowWatchingPathsOutsideWorkspace; protected StarlarkBaseExternalContext( Path workingDirectory, + BlazeDirectories directories, Environment env, Map envVariables, DownloadManager downloadManager, double timeoutScaling, @Nullable ProcessWrapper processWrapper, StarlarkSemantics starlarkSemantics, - @Nullable RepositoryRemoteExecutor remoteExecutor) { + @Nullable RepositoryRemoteExecutor remoteExecutor, + boolean allowWatchingPathsOutsideWorkspace) { this.workingDirectory = workingDirectory; + this.directories = directories; this.env = env; this.envVariables = ImmutableMap.copyOf(envVariables); this.osObject = new StarlarkOS(this.envVariables); @@ -164,6 +176,7 @@ protected StarlarkBaseExternalContext( this.starlarkSemantics = starlarkSemantics; this.remoteExecutor = remoteExecutor; this.asyncTasks = new ArrayList<>(); + this.allowWatchingPathsOutsideWorkspace = allowWatchingPathsOutsideWorkspace; } public boolean ensureNoPendingAsyncTasks(EventHandler eventHandler, boolean forSuccessfulFetch) { @@ -194,8 +207,12 @@ protected void registerAsyncTask(AsyncTask task) { protected abstract String getIdentifyingStringForLogging(); /** Returns the file digests used by this context object so far. */ - public ImmutableMap getAccumulatedFileDigests() { - return ImmutableMap.copyOf(accumulatedFileDigests); + public ImmutableMap getRecordedFileInputs() { + return ImmutableMap.copyOf(recordedFileInputs); + } + + public ImmutableMap getRecordedDirentsInputs() { + return ImmutableMap.copyOf(recordedDirentsInputs); } /** Returns set of environment variable keys encountered so far. */ @@ -1127,17 +1144,14 @@ public StarlarkPath path(Object path) throws EvalException, InterruptedException protected StarlarkPath getPath(String method, Object path) throws EvalException, InterruptedException { if (path instanceof String) { - PathFragment pathFragment = PathFragment.create(path.toString()); - return new StarlarkPath( - pathFragment.isAbsolute() - ? workingDirectory.getFileSystem().getPath(pathFragment) - : workingDirectory.getRelative(pathFragment)); + return new StarlarkPath(this, workingDirectory.getRelative(path.toString())); } else if (path instanceof Label) { return getPathFromLabel((Label) path); } else if (path instanceof StarlarkPath) { return (StarlarkPath) path; } else { - throw Starlark.errorf("%s can only take a string or a label.", method); + // This can never happen because we check it in the Starlark interpreter. + throw new IllegalArgumentException("expected string or label for path"); } } @@ -1146,22 +1160,38 @@ protected StarlarkPath getPath(String method, Object path) doc = "Reads the content of a file on the filesystem.", useStarlarkThread = true, parameters = { - @Param( - name = "path", - allowedTypes = { - @ParamType(type = String.class), - @ParamType(type = Label.class), - @ParamType(type = StarlarkPath.class) - }, - doc = "path of the file to read from."), + @Param( + name = "path", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = StarlarkPath.class) + }, + doc = "path of the file to read from."), + @Param( + name = "watch", + defaultValue = "'auto'", + positional = false, + named = true, + doc = + "whether to watch the file. Can be the string 'yes', 'no', " + + "or 'auto'. Passing 'yes' is equivalent to immediately invoking the " + + "watch() 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 watch() docs for more " + + "information.") }) - public String readFile(Object path, StarlarkThread thread) + public String readFile(Object path, String watch, StarlarkThread thread) throws RepositoryFunctionException, EvalException, InterruptedException { StarlarkPath p = getPath("read()", path); WorkspaceRuleEvent w = WorkspaceRuleEvent.newReadEvent( 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) { @@ -1169,6 +1199,154 @@ public String readFile(Object path, StarlarkThread thread) } } + /** + * Converts a regular {@link Path} to a {@link RepoCacheFriendlyPath} based on {@link + * ShouldWatch}. If the path shouldn't be watched for whatever reason, returns null. If it's + * illegal to watch the path in the current context, but the user still requested a watch, throws + * an exception. + */ + @Nullable + protected RepoCacheFriendlyPath toRepoCacheFriendlyPath(Path path, ShouldWatch shouldWatch) + throws EvalException { + if (shouldWatch == ShouldWatch.NO) { + return null; + } + if (path.startsWith(workingDirectory)) { + // The path is under the working directory. Don't watch it, as it would cause a dependency + // cycle. + if (shouldWatch == ShouldWatch.AUTO) { + return null; + } + throw Starlark.errorf("attempted to watch path under working directory"); + } + if (path.startsWith(directories.getWorkspace())) { + // The file is under the workspace root. + PathFragment relPath = path.relativeTo(directories.getWorkspace()); + return RepoCacheFriendlyPath.createInsideWorkspace(RepositoryName.MAIN, relPath); + } + Path outputBaseExternal = + directories.getOutputBase().getRelative(LabelConstants.EXTERNAL_REPOSITORY_LOCATION); + if (path.startsWith(outputBaseExternal)) { + PathFragment relPath = path.relativeTo(outputBaseExternal); + if (!relPath.isEmpty()) { + // The file is under a repo root. + String repoName = relPath.getSegment(0); + PathFragment repoRelPath = + relPath.relativeTo(PathFragment.createAlreadyNormalized(repoName)); + return RepoCacheFriendlyPath.createInsideWorkspace( + RepositoryName.createUnvalidated(repoName), repoRelPath); + } + } + // The file is just under a random absolute path. + if (!allowWatchingPathsOutsideWorkspace) { + if (shouldWatch == ShouldWatch.AUTO) { + return null; + } + throw Starlark.errorf( + "attempted to watch path outside workspace, but it's prohibited in the current context"); + } + return RepoCacheFriendlyPath.createOutsideWorkspace(path.asFragment()); + } + + /** Whether to watch a path. See {@link #readFile} for semantics */ + protected enum ShouldWatch { + YES, + NO, + AUTO; + + static ShouldWatch fromString(String s) throws EvalException { + switch (s) { + case "yes": + return YES; + case "no": + return NO; + case "auto": + return AUTO; + default: + throw Starlark.errorf( + "bad value for 'watch' parameter; want 'yes', 'no', or 'auto', got %s", s); + } + } + } + + protected void maybeWatch(StarlarkPath starlarkPath, ShouldWatch shouldWatch) + throws EvalException, RepositoryFunctionException, InterruptedException { + RepoCacheFriendlyPath repoCacheFriendlyPath = + toRepoCacheFriendlyPath(starlarkPath.getPath(), shouldWatch); + if (repoCacheFriendlyPath == null) { + return; + } + RootedPath rootedPath = repoCacheFriendlyPath.getRootedPath(env, directories); + if (rootedPath == null) { + throw new NeedsSkyframeRestartException(); + } + try { + FileValue fileValue = + (FileValue) env.getValueOrThrow(FileValue.key(rootedPath), IOException.class); + if (fileValue == null) { + throw new NeedsSkyframeRestartException(); + } + + recordedFileInputs.put( + new RepoRecordedInput.File(repoCacheFriendlyPath), + RepoRecordedInput.File.fileValueToMarkerValue(fileValue)); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); + } + } + + protected void maybeWatchDirents(Path path, ShouldWatch shouldWatch) + throws EvalException, RepositoryFunctionException, InterruptedException { + RepoCacheFriendlyPath repoCacheFriendlyPath = toRepoCacheFriendlyPath(path, shouldWatch); + if (repoCacheFriendlyPath == null) { + return; + } + RootedPath rootedPath = repoCacheFriendlyPath.getRootedPath(env, directories); + if (env.valuesMissing()) { + throw new NeedsSkyframeRestartException(); + } + if (env.getValue(DirectoryListingValue.key(rootedPath)) == null) { + throw new NeedsSkyframeRestartException(); + } + try { + recordedDirentsInputs.put( + new RepoRecordedInput.Dirents(repoCacheFriendlyPath), + RepoRecordedInput.Dirents.getDirentsMarkerValue(path)); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); + } + } + + @StarlarkMethod( + name = "watch", + doc = + "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.

\"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 not " + + "include changes to any files under the directory if the path is a directory. For " + + "that, use path.readdir() " + + "instead.

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", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = StarlarkPath.class) + }, + doc = "path of the file to watch."), + }) + public void watchForStarlark(Object path) + throws RepositoryFunctionException, EvalException, InterruptedException { + maybeWatch(getPath("watch()", path), ShouldWatch.YES); + } + // Create parent directories for the given path protected static void makeDirectories(Path path) throws IOException { Path parent = path.getParentDirectory(); @@ -1516,7 +1694,7 @@ private StarlarkPath findCommandOnPath(String program) throws IOException { // root?). Path path = workingDirectory.getFileSystem().getPath(fragment).getChild(program.trim()); if (path.exists() && path.isFile(Symlinks.FOLLOW) && path.isExecutable()) { - return new StarlarkPath(path); + return new StarlarkPath(this, path); } } } @@ -1526,26 +1704,12 @@ private StarlarkPath findCommandOnPath(String program) throws IOException { // Resolve the label given by value into a file path. protected StarlarkPath getPathFromLabel(Label label) throws EvalException, InterruptedException { RootedPath rootedPath = RepositoryFunction.getRootedPathFromLabel(label, env); - SkyKey fileSkyKey = FileValue.key(rootedPath); - FileValue fileValue; + StarlarkPath starlarkPath = new StarlarkPath(this, rootedPath.asPath()); try { - fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class); - } catch (IOException e) { - throw Starlark.errorf("%s", e.getMessage()); - } - - if (fileValue == null) { - throw new NeedsSkyframeRestartException(); - } - if (!fileValue.isFile() || fileValue.isSpecialFile()) { - throw Starlark.errorf("Not a regular file: %s", rootedPath.asPath().getPathString()); - } - - try { - accumulatedFileDigests.put(label, RepositoryFunction.fileValueToMarkerValue(fileValue)); - } catch (IOException e) { - throw Starlark.errorf("%s", e.getMessage()); + maybeWatch(starlarkPath, ShouldWatch.AUTO); + } catch (RepositoryFunctionException e) { + throw Starlark.errorf("%s", e.getCause().getMessage()); } - return new StarlarkPath(rootedPath.asPath()); + return starlarkPath; } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPath.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPath.java index dc70f674cd0a27..3e676ecfdb8d78 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPath.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPath.java @@ -16,9 +16,12 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.docgen.annot.DocCategory; +import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkBaseExternalContext.ShouldWatch; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import java.io.IOException; import javax.annotation.Nullable; import net.starlark.java.annot.Param; @@ -27,24 +30,27 @@ import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkValue; import net.starlark.java.eval.Tuple; /** - * A Path object to be used into Starlark remote repository. + * A Path object to be used in repo rules and module extensions. * *

This path object enable non-hermetic operations from Starlark and should not be returned by - * something other than a StarlarkRepositoryContext. + * something other than a StarlarkBaseExternalContext. */ @Immutable @StarlarkBuiltin( name = "path", category = DocCategory.BUILTIN, doc = "A structure representing a file to be used inside a repository.") -final class StarlarkPath implements StarlarkValue { +public final class StarlarkPath implements StarlarkValue { + private final StarlarkBaseExternalContext ctx; private final Path path; - StarlarkPath(Path path) { + StarlarkPath(StarlarkBaseExternalContext ctx, Path path) { + this.ctx = ctx; this.path = path; } @@ -77,14 +83,41 @@ public String getBasename() { @StarlarkMethod( name = "readdir", - structField = false, - doc = "The list of entries in the directory denoted by this path.") - public ImmutableList readdir() throws IOException { - ImmutableList.Builder builder = ImmutableList.builder(); - for (Path p : path.getDirectoryEntries()) { - builder.add(new StarlarkPath(p)); + doc = + "Returns the list of entries in the directory denoted by this path. Each entry is a " + + "path object itself.", + parameters = { + @Param( + name = "watch", + defaultValue = "'auto'", + positional = false, + named = true, + doc = + "whether Bazel should watch the list of entries in this directory and refetch the " + + "repository or re-evaluate the module extension next time when any changes " + + "are detected. Changes to detect include entry creation, deletion, and " + + "renaming. Note that this doesn't watch the contents of any entries " + + "in the directory.

Can be the string 'yes', 'no', or 'auto'. If set to " + + "'auto', Bazel will only watch this directory when it is legal to do so (see " + + "repository_ctx.watch() " + + "docs for more information)."), + }) + public ImmutableList readdir(String watch) + throws EvalException, RepositoryFunctionException, InterruptedException { + if (!isDir()) { + throw Starlark.errorf("can't readdir(), not a directory: %s", path); + } + ctx.maybeWatchDirents(path, ShouldWatch.fromString(watch)); + try { + ImmutableList.Builder builder = ImmutableList.builder(); + for (Path p : path.getDirectoryEntries()) { + builder.add(new StarlarkPath(ctx, p)); + } + return builder.build(); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); } - return builder.build(); } @StarlarkMethod( @@ -95,7 +128,7 @@ public ImmutableList readdir() throws IOException { @Nullable public StarlarkPath getDirname() { Path parentPath = path.getParentDirectory(); - return parentPath == null ? null : new StarlarkPath(parentPath); + return parentPath == null ? null : new StarlarkPath(ctx, parentPath); } @StarlarkMethod( @@ -109,6 +142,7 @@ public StarlarkPath getDirname() { + "added as needed.")) public StarlarkPath getChild(Tuple relativePaths) throws EvalException { return new StarlarkPath( + ctx, path.getRelative( String.join( Character.toString(PathFragment.SEPARATOR_CHAR), @@ -118,11 +152,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.

Note that " + + "accessing this field does not 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 watch() 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.

Note that accessing this field does " + + "not 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 " + + "watch() method on the context object.") + public boolean isDir() { + return path.isDirectory(); + } + @StarlarkMethod( name = "realpath", structField = true, @@ -130,7 +180,7 @@ public boolean exists() { "Returns the canonical path for this path by repeatedly replacing all symbolic links " + "with their referents.") public StarlarkPath realpath() throws IOException { - return new StarlarkPath(path.resolveSymbolicLinks()); + return new StarlarkPath(ctx, path.resolveSymbolicLinks()); } @Override 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 246228875f8788..764c35e14435d7 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 @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.docgen.annot.DocCategory; +import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.bazel.debug.WorkspaceRuleEvent; import com.google.devtools.build.lib.bazel.repository.DecompressorDescriptor; import com.google.devtools.build.lib.bazel.repository.DecompressorValue; @@ -32,14 +33,18 @@ import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.repository.RepositoryFetchProgress; 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.RepoRecordedInput.RepoCacheFriendlyPath; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; +import com.google.devtools.build.lib.skyframe.DirectoryTreeDigestValue; import com.google.devtools.build.lib.util.StringUtilities; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; @@ -47,6 +52,7 @@ import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.nio.file.InvalidPathException; +import java.util.HashMap; import java.util.Map; import javax.annotation.Nullable; import net.starlark.java.annot.Param; @@ -74,10 +80,10 @@ public class StarlarkRepositoryContext extends StarlarkBaseExternalContext { private final Rule rule; private final RepositoryName repoName; private final PathPackageLocator packageLocator; - private final Path workspaceRoot; private final StructImpl attrObject; private final ImmutableSet ignoredPatterns; private final SyscallCache syscallCache; + private final HashMap recordedDirTreeInputs = new HashMap<>(); /** * Create a new context (repository_ctx) object for a Starlark repository rule ({@code rule} @@ -96,23 +102,24 @@ public class StarlarkRepositoryContext extends StarlarkBaseExternalContext { StarlarkSemantics starlarkSemantics, @Nullable RepositoryRemoteExecutor remoteExecutor, SyscallCache syscallCache, - Path workspaceRoot) + BlazeDirectories directories) throws EvalException { super( outputDirectory, + directories, environment, env, downloadManager, timeoutScaling, processWrapper, starlarkSemantics, - remoteExecutor); + remoteExecutor, + /* allowWatchingPathsOutsideWorkspace= */ true); this.rule = rule; this.repoName = RepositoryName.createUnvalidated(rule.getName()); this.packageLocator = packageLocator; this.ignoredPatterns = ignoredPatterns; this.syscallCache = syscallCache; - this.workspaceRoot = workspaceRoot; WorkspaceAttributeMapper attrs = WorkspaceAttributeMapper.of(rule); ImmutableMap.Builder attrBuilder = new ImmutableMap.Builder<>(); for (String name : attrs.getAttributeNames()) { @@ -130,6 +137,10 @@ protected String getIdentifyingStringForLogging() { return RepositoryFetchProgress.repositoryFetchContextString(repoName); } + public ImmutableMap getRecordedDirTreeInputs() { + return ImmutableMap.copyOf(recordedDirTreeInputs); + } + @StarlarkMethod( name = "name", structField = true, @@ -143,7 +154,7 @@ public String getName() { structField = true, doc = "The path to the root workspace of the bazel invocation.") public StarlarkPath getWorkspaceRoot() { - return new StarlarkPath(workspaceRoot); + return new StarlarkPath(this, directories.getWorkspace()); } @StarlarkMethod( @@ -197,9 +208,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 watch the symlink target. Can be the string " + + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking " + + "the watch() 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 watch() 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); @@ -210,6 +233,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()); @@ -268,12 +292,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 watch the template file. Can be the string " + + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking " + + "the watch() 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 watch() docs for more " + + "information."), }) public void createFileFromTemplate( Object path, Object template, Dict substitutions, // expected Boolean executable, + String watchTemplate, StarlarkThread thread) throws RepositoryFunctionException, EvalException, InterruptedException { StarlarkPath p = getPath("template()", path); @@ -289,6 +326,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()); @@ -384,8 +425,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 watch the patch file. Can be the string " + + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking " + + "the watch() 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 watch() 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); @@ -396,6 +449,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) { @@ -456,12 +513,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 watch the archive file. Can be the string " + + "'yes', 'no', or 'auto'. Passing 'yes' is equivalent to immediately invoking " + + "the watch() 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 watch() docs for more " + + "information."), }) public void extract( Object archive, Object output, String stripPrefix, Dict renameFiles, // expected + String watchArchive, StarlarkThread thread) throws RepositoryFunctionException, InterruptedException, EvalException { StarlarkPath archivePath = getPath("extract()", archive); @@ -470,6 +540,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); @@ -502,6 +576,54 @@ public void extract( env.getListener().post(new ExtractProgress(outputPath.getPath().toString())); } + @StarlarkMethod( + name = "watch_tree", + doc = + "Tells Bazel to watch for changes to any files or directories transitively under the " + + "given path. Any changes to the contents of files, the existence of files or " + + "directories, file names or directory names, will cause this repo to be " + + "refetched.

Note that attempting to watch paths inside the repo currently being " + + "fetched will result in an error. ", + parameters = { + @Param( + name = "path", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Label.class), + @ParamType(type = StarlarkPath.class) + }, + doc = "path of the directory tree to watch."), + }) + public void watchTree(Object path) + throws EvalException, InterruptedException, RepositoryFunctionException { + StarlarkPath p = getPath("watch_tree()", path); + if (!p.isDir()) { + throw Starlark.errorf("can't call watch_tree() on non-directory %s", p); + } + RepoCacheFriendlyPath repoCacheFriendlyPath = + toRepoCacheFriendlyPath(p.getPath(), ShouldWatch.YES); + if (repoCacheFriendlyPath == null) { + return; + } + RootedPath rootedPath = repoCacheFriendlyPath.getRootedPath(env, directories); + if (rootedPath == null) { + throw new NeedsSkyframeRestartException(); + } + try { + DirectoryTreeDigestValue digestValue = + (DirectoryTreeDigestValue) + env.getValueOrThrow(DirectoryTreeDigestValue.key(rootedPath), IOException.class); + if (digestValue == null) { + throw new NeedsSkyframeRestartException(); + } + + recordedDirTreeInputs.put( + new RepoRecordedInput.DirTree(repoCacheFriendlyPath), digestValue.hexDigest()); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); + } + } + @Override public String toString() { return "repository_ctx[" + rule.getLabel() + "]"; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java index 1137d9d9bc8c5f..ba2bb1b083f4c7 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java @@ -39,6 +39,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.RepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.rules.repository.WorkspaceFileHelper; @@ -56,8 +57,6 @@ import com.google.devtools.build.skyframe.SkyKey; import java.io.IOException; import java.util.Map; -import java.util.Objects; -import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import javax.annotation.Nullable; @@ -70,8 +69,6 @@ /** A repository function to delegate work done by Starlark remote repositories. */ public final class StarlarkRepositoryFunction extends RepositoryFunction { - static final String SEMANTICS = "STARLARK_SEMANTICS"; - private final DownloadManager downloadManager; private double timeoutScaling = 1.0; @Nullable private ExecutorService workerExecutorService = null; @@ -99,26 +96,6 @@ public void setWorkerExecutorService(@Nullable ExecutorService workerExecutorSer this.workerExecutorService = workerExecutorService; } - static String describeSemantics(StarlarkSemantics semantics) { - // Here we use the hash code provided by AutoValue. This is unique, as long - // as the number of bits in the StarlarkSemantics is small enough. We will have to - // move to a longer description once the number of flags grows too large. - return "" + semantics.hashCode(); - } - - @Override - protected boolean verifySemanticsMarkerData(Map markerData, Environment env) - throws InterruptedException { - StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); - if (starlarkSemantics == null) { - // As it is a precomputed value, it should already be available. If not, returning - // false is the safe thing to do. - return false; - } - - return describeSemantics(starlarkSemantics).equals(markerData.get(SEMANTICS)); - } - @Override protected void setupRepoRootBeforeFetching(Path repoRoot) throws RepositoryFunctionException { // DON'T delete the repo root here if we're using a worker thread, since when this SkyFunction @@ -144,7 +121,7 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws RepositoryFunctionException, InterruptedException { if (workerExecutorService == null @@ -155,7 +132,7 @@ public RepositoryDirectoryValue.Builder fetch( // Fortunately, no Skyframe restarts should happen during error bubbling anyway, so this // shouldn't be a performance concern. See https://github.com/bazelbuild/bazel/issues/21238 // for more context. - return fetchInternal(rule, outputDirectory, directories, env, markerData, key); + return fetchInternal(rule, outputDirectory, directories, env, recordedInputValues, key); } var state = env.getState(RepoFetchingSkyKeyComputeState::new); var workerFuture = state.workerFuture; @@ -169,7 +146,12 @@ public RepositoryDirectoryValue.Builder fetch( () -> { try { return fetchInternal( - rule, outputDirectory, directories, workerEnv, state.markerData, key); + rule, + outputDirectory, + directories, + workerEnv, + state.recordedInputValues, + key); } finally { state.signalQueue.put(Signal.DONE); } @@ -186,7 +168,7 @@ public RepositoryDirectoryValue.Builder fetch( case DONE: try { RepositoryDirectoryValue.Builder result = workerFuture.get(); - markerData.putAll(state.markerData); + recordedInputValues.putAll(state.recordedInputValues); return result; } catch (ExecutionException e) { Throwables.throwIfInstanceOf(e.getCause(), RepositoryFunctionException.class); @@ -217,7 +199,7 @@ private RepositoryDirectoryValue.Builder fetchInternal( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws RepositoryFunctionException, InterruptedException { @@ -225,15 +207,13 @@ private RepositoryDirectoryValue.Builder fetchInternal( env.getListener().post(new StarlarkRepositoryDefinitionLocationEvent(rule.getName(), defInfo)); StarlarkCallable function = rule.getRuleClassObject().getConfiguredTargetFunction(); - if (declareEnvironmentDependencies(markerData, env, getEnviron(rule)) == null) { + if (declareEnvironmentDependencies(recordedInputValues, env, getEnviron(rule)) == null) { return null; } StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); if (env.valuesMissing()) { return null; } - markerData.put(SEMANTICS, describeSemantics(starlarkSemantics)); - markerData.put("ARCH:", CPU.getCurrent().getCanonicalName()); PathPackageLocator packageLocator = PrecomputedValue.PATH_PACKAGE_LOCATOR.get(env); if (env.valuesMissing()) { @@ -274,7 +254,7 @@ private RepositoryDirectoryValue.Builder fetchInternal( starlarkSemantics, repositoryRemoteExecutor, syscallCache, - directories.getWorkspace()); + directories); if (starlarkRepositoryContext.isRemotable()) { // If a rule is declared remotable then invalidate it if remote execution gets @@ -330,26 +310,28 @@ private RepositoryDirectoryValue.Builder fetchInternal( env.getListener().handle(Event.debug(defInfo)); } - // Modify marker data to include the files used by the rule's implementation function. - for (Map.Entry entry : - starlarkRepositoryContext.getAccumulatedFileDigests().entrySet()) { - // A label does not contain spaces so it's safe to use as a key. - markerData.put("FILE:" + entry.getKey(), entry.getValue()); - } + // Modify marker data to include the files/dirents used by the rule's implementation function. + recordedInputValues.putAll(starlarkRepositoryContext.getRecordedFileInputs()); + recordedInputValues.putAll(starlarkRepositoryContext.getRecordedDirentsInputs()); + recordedInputValues.putAll(starlarkRepositoryContext.getRecordedDirTreeInputs()); // Ditto for environment variables accessed via `getenv`. for (String envKey : starlarkRepositoryContext.getAccumulatedEnvKeys()) { - markerData.put("ENV:" + envKey, clientEnvironment.get(envKey)); + recordedInputValues.put( + new RepoRecordedInput.EnvVar(envKey), clientEnvironment.get(envKey)); } - for (Table.Cell repoMappings : - repoMappingRecorder.recordedEntries().cellSet()) { - markerData.put( - "REPO_MAPPING:" - + repoMappings.getRowKey().getName() - + "," - + repoMappings.getColumnKey(), - repoMappings.getValue().getName()); + // For repos defined in Bzlmod, record any used repo mappings in the marker file. + // Repos defined in WORKSPACE are impossible to verify given the chunked loading (we'd have to + // record which chunk the repo mapping was used in, and ain't nobody got time for that). + if (!isWorkspaceRepo(rule)) { + for (Table.Cell repoMappings : + repoMappingRecorder.recordedEntries().cellSet()) { + recordedInputValues.put( + new RepoRecordedInput.RecordedRepoMapping( + repoMappings.getRowKey(), repoMappings.getColumnKey()), + repoMappings.getValue().getName()); + } } env.getListener().post(resolved); @@ -402,47 +384,6 @@ private static ImmutableSet getEnviron(Rule rule) { return ImmutableSet.copyOf((Iterable) rule.getAttr("$environ")); } - /** - * Verify marker data previously saved by {@link #declareEnvironmentDependencies(Map, Environment, - * Set)} and/or {@link #fetchInternal(Rule, Path, BlazeDirectories, Environment, Map, SkyKey)} (on - * behalf of {@link StarlarkBaseExternalContext#getEnvironmentValue(String, Object)}). - */ - @Override - protected boolean verifyEnvironMarkerData( - Map markerData, Environment env, Set keys) - throws InterruptedException { - /* - * We can ignore `keys` and instead only verify what's recorded in the marker file, because - * any change to `keys` between builds would be caused by a change to a .bzl file, and that's - * covered by RepositoryDelegatorFunction.DigestWriter#areRepositoryAndMarkerFileConsistent. - */ - - ImmutableSet markerKeys = - markerData.keySet().stream() - .filter(s -> s.startsWith("ENV:")) - .collect(ImmutableSet.toImmutableSet()); - - ImmutableMap environ = - getEnvVarValues( - env, - markerKeys.stream() - .map(s -> s.substring(4)) // ENV:FOO -> FOO - .collect(ImmutableSet.toImmutableSet())); - if (environ == null) { - return false; - } - - for (String key : markerKeys) { - String markerValue = markerData.get(key); - String envKey = key.substring(4); // ENV:FOO -> FOO - if (!Objects.equals(markerValue, environ.get(envKey))) { - return false; - } - } - - return true; - } - @Override protected boolean isLocal(Rule rule) { return (Boolean) rule.getAttr("$local"); @@ -463,6 +404,7 @@ public static boolean isConfigureRule(Rule rule) { return rule.getRuleClassObject().isStarlark() && ((Boolean) rule.getAttr("$configure")); } + @Nullable @Override public Class getRuleDefinition() { return null; // unused so safe to return null diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java index 0a2af050d0881a..d2f3771043e341 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; import com.google.devtools.build.lib.skyframe.DirectoryListingValue; @@ -60,6 +61,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Map; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; @@ -75,7 +77,7 @@ public class AndroidNdkRepositoryFunction extends AndroidRepositoryFunction { private static final ImmutableSet PATH_ENV_VAR_AS_SET = ImmutableSet.of(PATH_ENV_VAR); - private static String getDefaultCrosstool(Integer majorRevision) { + private static String getDefaultCrosstool(int majorRevision) { // From NDK 17, libc++ replaces gnu-libstdc++ as the default STL. return majorRevision <= 16 ? GnuLibStdCppStlImpl.NAME : LibCppStlImpl.NAME; } @@ -181,7 +183,7 @@ private static String createCcToolchainRule( // but the gcc tool will actually be clang. ToolPath gcc = null; for (ToolPath toolPath : toolchain.getToolPathList()) { - if ("gcc".equals(toolPath.getName())) { + if (toolPath.getName().equals("gcc")) { gcc = toolPath; } } @@ -258,13 +260,17 @@ public boolean isLocal(Rule rule) { } @Override - public boolean verifyMarkerData(Rule rule, Map markerData, Environment env) + public boolean verifyRecordedInputs( + Rule rule, + BlazeDirectories directories, + Map recordedInputValues, + Environment env) throws InterruptedException { WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule); if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_SET); + return super.verifyRecordedInputs(rule, directories, recordedInputValues, env); } @Override @@ -274,11 +280,11 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws InterruptedException, RepositoryFunctionException { Map environ = - declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET); + declareEnvironmentDependencies(recordedInputValues, env, PATH_ENV_VAR_AS_SET); if (environ == null) { return null; } @@ -521,7 +527,7 @@ private ImmutableList generateBzlConfigFor(String version, CToolchain to tp -> String.format( " %s_path = '%s'", - tp.getName().toLowerCase().replaceAll("-", "_"), tp.getPath())) + tp.getName().toLowerCase(Locale.ROOT).replaceAll("-", "_"), tp.getPath())) .collect(ImmutableList.toImmutableList())); return bigConditional.add("").build(); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java index 04c955c2ddc965..09d9654b79660e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidSdkRepositoryFunction.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.io.InconsistentFilesystemException; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.Type; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.WorkspaceAttributeMapper; import com.google.devtools.build.lib.skyframe.DirectoryListingValue; @@ -177,13 +178,17 @@ public boolean isLocal(Rule rule) { } @Override - public boolean verifyMarkerData(Rule rule, Map markerData, Environment env) + public boolean verifyRecordedInputs( + Rule rule, + BlazeDirectories directories, + Map recordedInputValues, + Environment env) throws InterruptedException { WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule); if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyEnvironMarkerData(markerData, env, PATH_ENV_VAR_AS_SET); + return super.verifyRecordedInputs(rule, directories, recordedInputValues, env); } @Override @@ -193,11 +198,11 @@ public RepositoryDirectoryValue.Builder fetch( final Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws RepositoryFunctionException, InterruptedException { Map environ = - declareEnvironmentDependencies(markerData, env, PATH_ENV_VAR_AS_SET); + declareEnvironmentDependencies(recordedInputValues, env, PATH_ENV_VAR_AS_SET); if (environ == null) { return null; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD index cab8c2ca49eb8b..0071154ae013a1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/BUILD @@ -46,6 +46,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception", "//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/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", diff --git a/src/main/java/com/google/devtools/build/lib/rules/BUILD b/src/main/java/com/google/devtools/build/lib/rules/BUILD index 9764394f6e2789..96acbd10969530 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -307,6 +307,7 @@ java_library( deps = [ ":repository/new_local_repository_rule", ":repository/new_repository_file_handler", + ":repository/repo_recorded_input", ":repository/repository_directory_value", ":repository/repository_function", ":repository/resolved_file_value", @@ -345,6 +346,7 @@ java_library( name = "repository/new_repository_file_handler", srcs = ["repository/NewRepositoryFileHandler.java"], deps = [ + ":repository/repo_recorded_input", ":repository/repository_function", ":repository/workspace_attribute_mapper", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", @@ -360,6 +362,32 @@ java_library( ], ) +java_library( + name = "repository/repo_recorded_input", + srcs = ["repository/RepoRecordedInput.java"], + deps = [ + ":repository/repository_directory_value", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator", + "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", + "//src/main/java/com/google/devtools/build/lib/util", + "//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/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:auto_value", + "//third_party:guava", + "//third_party:jsr305", + ], +) + java_library( name = "repository/repository_directory_value", srcs = ["repository/RepositoryDirectoryValue.java"], @@ -389,6 +417,7 @@ java_library( ], deps = [ ":repository/local_repository_rule", + ":repository/repo_recorded_input", ":repository/repository_directory_value", ":repository/resolved_file_value", ":repository/workspace_attribute_mapper", @@ -399,7 +428,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel:resolved_event", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/cmdline:LabelValidator", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", @@ -414,7 +442,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", - "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_value_dirtiness_checker", "//src/main/java/com/google/devtools/build/lib/util", diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java index 811b274677475c..2b5482ea82e3f9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/LocalRepositoryFunction.java @@ -43,7 +43,7 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws InterruptedException, RepositoryFunctionException { String userDefinedPath = RepositoryFunction.getPathAttr(rule); diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java index e265e3c1685538..7bc6c444cebd88 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java @@ -59,7 +59,7 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws InterruptedException, RepositoryFunctionException { @@ -161,7 +161,7 @@ public RepositoryDirectoryValue.Builder fetch( return null; } - fileHandler.finishFile(rule, outputDirectory, markerData); + fileHandler.finishFile(rule, outputDirectory, recordedInputValues); env.getListener().post(resolve(rule)); return RepositoryDirectoryValue.builder() diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java index 37c15b1c3fdf9e..aad74b69b07a8f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java @@ -60,10 +60,11 @@ public boolean prepareFile(Rule rule, Environment env) return true; } - public void finishFile(Rule rule, Path outputDirectory, Map markerData) + public void finishFile( + Rule rule, Path outputDirectory, Map recordedInputValues) throws RepositoryFunctionException { - this.workspaceFileHandler.finishFile(rule, outputDirectory, markerData); - this.buildFileHandler.finishFile(rule, outputDirectory, markerData); + this.workspaceFileHandler.finishFile(rule, outputDirectory, recordedInputValues); + this.buildFileHandler.finishFile(rule, outputDirectory, recordedInputValues); } /** @@ -139,14 +140,19 @@ public boolean prepareFile(Rule rule, Environment env) * @throws IllegalStateException if {@link #prepareFile} was not called before this, or if * {@link #prepareFile} failed and this was called. */ - public void finishFile(Rule rule, Path outputDirectory, Map markerData) + public void finishFile( + Rule rule, Path outputDirectory, Map recordedInputValues) throws RepositoryFunctionException { if (fileValue != null) { // Link x/FILENAME to /x.FILENAME. symlinkFile(fileValue, filename, outputDirectory); - String fileKey = getFileAttributeAsLabel(rule).toString(); try { - markerData.put("FILE:" + fileKey, RepositoryFunction.fileValueToMarkerValue(fileValue)); + Label label = getFileAttributeAsLabel(rule); + recordedInputValues.put( + new RepoRecordedInput.File( + RepoRecordedInput.RepoCacheFriendlyPath.createInsideWorkspace( + label.getRepository(), label.toPathFragment())), + RepoRecordedInput.File.fileValueToMarkerValue(fileValue)); } catch (IOException e) { throw new RepositoryFunctionException(e, Transience.TRANSIENT); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java new file mode 100644 index 00000000000000..4bef44e2efed2b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepoRecordedInput.java @@ -0,0 +1,658 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.rules.repository; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Preconditions; +import com.google.common.base.Splitter; +import com.google.common.io.BaseEncoding; +import com.google.devtools.build.lib.actions.FileValue; +import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.cmdline.LabelValidator; +import com.google.devtools.build.lib.cmdline.RepositoryName; +import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; +import com.google.devtools.build.lib.skyframe.ClientEnvironmentValue; +import com.google.devtools.build.lib.skyframe.DirectoryListingValue; +import com.google.devtools.build.lib.skyframe.DirectoryTreeDigestValue; +import com.google.devtools.build.lib.skyframe.PrecomputedValue; +import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; +import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.SkyFunction.Environment; +import com.google.devtools.build.skyframe.SkyKey; +import java.io.IOException; +import java.util.Comparator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import javax.annotation.Nullable; + +/** + * Represents a "recorded input" of a repo fetch. We define the "input" of a repo fetch as any + * entity that could affect the output of the repo fetch (i.e. the repo contents). A "recorded + * input" is thus any input we can record during the fetch and thus know about only after the fetch. + * This contrasts with "predeclared inputs", which are known before fetching the repo, and + * "undiscoverable inputs", which are used during the fetch but is not recorded or recordable. + * + *

Recorded inputs are of particular interest, since in order to determine whether a fetched repo + * is still up-to-date, the identity of all recorded inputs need to be stored in addition to their + * values. This contrasts with predeclared inputs; the whole set of predeclared inputs are known + * before the fetch, so we can simply hash all predeclared input values. + * + *

Recorded inputs and their values are stored in marker files for repos. Each recorded + * input is stored as a string, with a prefix denoting its type, followed by a colon, and then the + * information identifying that specific input. + */ +public abstract class RepoRecordedInput implements Comparable { + /** Represents a parser for a specific type of recorded inputs. */ + public abstract static class Parser { + /** + * The prefix that identifies the type of the recorded inputs: for example, the {@code ENV} part + * of {@code ENV:MY_ENV_VAR}. + */ + public abstract String getPrefix(); + + /** + * Parses a recorded input from the post-colon substring that identifies the specific input: for + * example, the {@code MY_ENV_VAR} part of {@code ENV:MY_ENV_VAR}. + */ + public abstract RepoRecordedInput parse(String s); + } + + private static final Comparator COMPARATOR = + Comparator.comparing((RepoRecordedInput rri) -> rri.getParser().getPrefix()) + .thenComparing(RepoRecordedInput::toStringInternal); + + /** + * Parses a recorded input from its string representation. + * + * @param s the string representation + * @return The parsed recorded input object, or {@code null} if the string representation is + * invalid + */ + @Nullable + public static RepoRecordedInput parse(String s) { + List parts = Splitter.on(':').limit(2).splitToList(s); + for (Parser parser : + new Parser[] { + File.PARSER, Dirents.PARSER, DirTree.PARSER, EnvVar.PARSER, RecordedRepoMapping.PARSER + }) { + if (parts.get(0).equals(parser.getPrefix())) { + return parser.parse(parts.get(1)); + } + } + return null; + } + + /** + * Returns whether all values are still up-to-date for each recorded input. If Skyframe values are + * missing, the return value should be ignored; callers are responsible for checking {@code + * env.valuesMissing()} and triggering a Skyframe restart if needed. + */ + public static boolean areAllValuesUpToDate( + Environment env, + BlazeDirectories directories, + Map recordedInputValues) + throws InterruptedException { + env.getValuesAndExceptions( + recordedInputValues.keySet().stream() + .map(RepoRecordedInput::getSkyKey) + .filter(Objects::nonNull) + .collect(toImmutableSet())); + if (env.valuesMissing()) { + return false; + } + for (Map.Entry recordedInputValue : + recordedInputValues.entrySet()) { + if (!recordedInputValue + .getKey() + .isUpToDate(env, directories, recordedInputValue.getValue())) { + return false; + } + } + return true; + } + + @Override + public abstract boolean equals(Object obj); + + @Override + public abstract int hashCode(); + + @Override + public final String toString() { + return getParser().getPrefix() + ":" + toStringInternal(); + } + + @Override + public int compareTo(RepoRecordedInput o) { + return COMPARATOR.compare(this, o); + } + + /** + * Returns the post-colon substring that identifies the specific input: for example, the {@code + * MY_ENV_VAR} part of {@code ENV:MY_ENV_VAR}. + */ + public abstract String toStringInternal(); + + /** Returns the parser object for this type of recorded inputs. */ + public abstract Parser getParser(); + + /** + * Returns the {@link SkyKey} that is necessary to determine {@link #isUpToDate}. Can be null if + * no SkyKey is needed. + */ + @Nullable + public abstract SkyKey getSkyKey(); + + /** + * Returns whether the given {@code oldValue} is still up-to-date for this recorded input. This + * method can assume that {@link #getSkyKey()} is already evaluated; it can request further + * Skyframe evaluations, and if any values are missing, this method can return any value (doesn't + * matter what) and will be reinvoked after a Skyframe restart. + */ + public abstract boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException; + + /** + * Represents a filesystem path stored in a way that is repo-cache-friendly. That is, if the path + * happens to point inside the current Bazel workspace (in either the main repo or an external + * repo), we store the appropriate repo name and the path fragment relative to the repo root, + * instead of the entire absolute path. + * + *

This is almost like storing a label, but includes the extra corner case of files + * inside a repo but not within any package due to missing BUILD files. For example, the file + * {@code @@foo//:abc.bzl} is addressable by a label if the file {@code @@foo//:BUILD} exists. But + * if the BUILD file doesn't exist, the {@code abc.bzl} file should still be "watchable"; it's + * just that {@code @@foo//:abc.bzl} is technically not a valid label. + * + *

Of course, when the path is outside the current Bazel workspace, we just store the absolute + * path. + */ + @AutoValue + public abstract static class RepoCacheFriendlyPath { + public abstract Optional repoName(); + + public abstract PathFragment path(); + + public static RepoCacheFriendlyPath createInsideWorkspace( + RepositoryName repoName, PathFragment path) { + Preconditions.checkArgument( + !path.isAbsolute(), "the provided path should be relative to the repo root"); + return new AutoValue_RepoRecordedInput_RepoCacheFriendlyPath(Optional.of(repoName), path); + } + + public static RepoCacheFriendlyPath createOutsideWorkspace(PathFragment path) { + Preconditions.checkArgument( + path.isAbsolute(), "the provided path should be absolute in the filesystem"); + return new AutoValue_RepoRecordedInput_RepoCacheFriendlyPath(Optional.empty(), path); + } + + @Override + public final String toString() { + // We store `@@foo//abc/def:ghi.bzl` as just `@@foo//abc/def/ghi.bzl`. See class javadoc for + // more context. + return repoName().map(repoName -> repoName + "//" + path()).orElse(path().toString()); + } + + public static RepoCacheFriendlyPath parse(String s) { + if (LabelValidator.isAbsolute(s)) { + int doubleSlash = s.indexOf("//"); + int skipAts = s.startsWith("@@") ? 2 : s.startsWith("@") ? 1 : 0; + return createInsideWorkspace( + RepositoryName.createUnvalidated(s.substring(skipAts, doubleSlash)), + PathFragment.create(s.substring(doubleSlash + 2))); + } + return createOutsideWorkspace(PathFragment.create(s)); + } + + /** + * If resolving this path requires getting a {@link RepositoryDirectoryValue}, this method + * returns the appropriate key; otherwise it returns null. + */ + @Nullable + public final RepositoryDirectoryValue.Key getRepoDirSkyKeyOrNull() { + if (repoName().isEmpty() || repoName().get().isMain()) { + return null; + } + return RepositoryDirectoryValue.key(repoName().get()); + } + + public final RootedPath getRootedPath(Environment env, BlazeDirectories directories) + throws InterruptedException { + Root root; + if (repoName().isEmpty()) { + root = Root.absoluteRoot(directories.getOutputBase().getFileSystem()); + } else if (repoName().get().isMain()) { + root = Root.fromPath(directories.getWorkspace()); + } else { + RepositoryDirectoryValue repoDirValue = + (RepositoryDirectoryValue) env.getValue(getRepoDirSkyKeyOrNull()); + if (repoDirValue == null || !repoDirValue.repositoryExists()) { + return null; + } + root = Root.fromPath(repoDirValue.getPath()); + } + return RootedPath.toRootedPath(root, path()); + } + } + + /** + * Represents a file input accessed during the repo fetch. Despite being named just "file", this + * can represent a file or a directory on the filesystem, and it does not need to exist. The value + * of the input contains whether this is a file or a directory or nonexistent, and if it's a file, + * the digest of its contents. + */ + public static final class File extends RepoRecordedInput { + public static final Parser PARSER = + new Parser() { + @Override + public String getPrefix() { + return "FILE"; + } + + @Override + public RepoRecordedInput parse(String s) { + return new File(RepoCacheFriendlyPath.parse(s)); + } + }; + + private final RepoCacheFriendlyPath path; + + public File(RepoCacheFriendlyPath path) { + this.path = path; + } + + @Override + public Parser getParser() { + return PARSER; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof File)) { + return false; + } + File that = (File) o; + return Objects.equals(path, that.path); + } + + @Override + public int hashCode() { + return path.hashCode(); + } + + @Override + public String toStringInternal() { + return path.toString(); + } + + /** + * Convert to a {@link com.google.devtools.build.lib.actions.FileValue} to a String appropriate + * 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 { + 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) { + // Fast digest not available, or it would have been in the FileValue. + digest = fileValue.realRootedPath().asPath().getDigest(); + } + return BaseEncoding.base16().lowerCase().encode(digest); + } + + @Override + @Nullable + public SkyKey getSkyKey() { + return path.getRepoDirSkyKeyOrNull(); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException { + RootedPath rootedPath = path.getRootedPath(env, directories); + if (rootedPath == null) { + return false; + } + SkyKey fileKey = FileValue.key(rootedPath); + try { + FileValue fileValue = (FileValue) env.getValueOrThrow(fileKey, IOException.class); + if (fileValue == null) { + return false; + } + return oldValue.equals(fileValueToMarkerValue(fileValue)); + } catch (IOException e) { + return false; + } + } + } + + /** Represents the list of entries under a directory accessed during the fetch. */ + public static final class Dirents extends RepoRecordedInput { + public static final Parser PARSER = + new Parser() { + @Override + public String getPrefix() { + return "DIRENTS"; + } + + @Override + public RepoRecordedInput parse(String s) { + return new Dirents(RepoCacheFriendlyPath.parse(s)); + } + }; + + private final RepoCacheFriendlyPath path; + + public Dirents(RepoCacheFriendlyPath path) { + this.path = path; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof Dirents)) { + return false; + } + Dirents that = (Dirents) o; + return Objects.equals(path, that.path); + } + + @Override + public int hashCode() { + return path.hashCode(); + } + + @Override + public String toStringInternal() { + return path.toString(); + } + + @Override + public Parser getParser() { + return PARSER; + } + + @Nullable + @Override + public SkyKey getSkyKey() { + return path.getRepoDirSkyKeyOrNull(); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException { + RootedPath rootedPath = path.getRootedPath(env, directories); + if (rootedPath == null) { + return false; + } + if (env.getValue(DirectoryListingValue.key(rootedPath)) == null) { + return false; + } + try { + return oldValue.equals(getDirentsMarkerValue(rootedPath.asPath())); + } catch (IOException e) { + return false; + } + } + + public static String getDirentsMarkerValue(Path path) throws IOException { + Fingerprint fp = new Fingerprint(); + fp.addStrings( + path.getDirectoryEntries().stream() + .map(Path::getBaseName) + .sorted() + .collect(toImmutableList())); + return fp.hexDigestAndReset(); + } + } + + /** + * Represents an entire directory tree accessed during the fetch. Anything under the tree changing + * (including adding/removing/renaming files or directories and changing file contents) will cause + * it to go out of date. + */ + public static final class DirTree extends RepoRecordedInput { + public static final Parser PARSER = + new Parser() { + @Override + public String getPrefix() { + return "DIRTREE"; + } + + @Override + public RepoRecordedInput parse(String s) { + return new DirTree(RepoCacheFriendlyPath.parse(s)); + } + }; + + private final RepoCacheFriendlyPath path; + + public DirTree(RepoCacheFriendlyPath path) { + this.path = path; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof DirTree)) { + return false; + } + DirTree that = (DirTree) o; + return Objects.equals(path, that.path); + } + + @Override + public int hashCode() { + return path.hashCode(); + } + + @Override + public String toStringInternal() { + return path.toString(); + } + + @Override + public Parser getParser() { + return PARSER; + } + + @Nullable + @Override + public SkyKey getSkyKey() { + return path.getRepoDirSkyKeyOrNull(); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException { + RootedPath rootedPath = path.getRootedPath(env, directories); + if (rootedPath == null) { + return false; + } + DirectoryTreeDigestValue value = + (DirectoryTreeDigestValue) env.getValue(DirectoryTreeDigestValue.key(rootedPath)); + if (value == null) { + return false; + } + return oldValue.equals(value.hexDigest()); + } + } + + /** Represents an environment variable accessed during the repo fetch. */ + public static final class EnvVar extends RepoRecordedInput { + static final Parser PARSER = + new Parser() { + @Override + public String getPrefix() { + return "ENV"; + } + + @Override + public RepoRecordedInput parse(String s) { + return new EnvVar(s); + } + }; + + final String name; + + public EnvVar(String name) { + this.name = name; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof EnvVar)) { + return false; + } + EnvVar envVar = (EnvVar) o; + return Objects.equals(name, envVar.name); + } + + @Override + public int hashCode() { + return name.hashCode(); + } + + @Override + public Parser getParser() { + return PARSER; + } + + @Override + public String toStringInternal() { + return name; + } + + @Override + public SkyKey getSkyKey() { + return ActionEnvironmentFunction.key(name); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException { + String v = PrecomputedValue.REPO_ENV.get(env).get(name); + if (v == null) { + v = ((ClientEnvironmentValue) env.getValue(getSkyKey())).getValue(); + } + // Note that `oldValue` can be null if the env var was not set. + return Objects.equals(oldValue, v); + } + } + + /** Represents a repo mapping entry that was used during the repo fetch. */ + public static final class RecordedRepoMapping extends RepoRecordedInput { + static final Parser PARSER = + new Parser() { + @Override + public String getPrefix() { + return "REPO_MAPPING"; + } + + @Override + public RepoRecordedInput parse(String s) { + List parts = Splitter.on(',').limit(2).splitToList(s); + return new RecordedRepoMapping( + RepositoryName.createUnvalidated(parts.get(0)), parts.get(1)); + } + }; + + final RepositoryName sourceRepo; + final String apparentName; + + public RecordedRepoMapping(RepositoryName sourceRepo, String apparentName) { + this.sourceRepo = sourceRepo; + this.apparentName = apparentName; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof RecordedRepoMapping)) { + return false; + } + RecordedRepoMapping that = (RecordedRepoMapping) o; + return Objects.equals(sourceRepo, that.sourceRepo) + && Objects.equals(apparentName, that.apparentName); + } + + @Override + public int hashCode() { + return Objects.hash(sourceRepo, apparentName); + } + + @Override + public Parser getParser() { + return PARSER; + } + + @Override + public String toStringInternal() { + return sourceRepo.getName() + ',' + apparentName; + } + + @Override + public SkyKey getSkyKey() { + // Since we only record repo mapping entries for repos defined in Bzlmod, we can request the + // WORKSPACE-less version of the main repo mapping (as no repos defined in Bzlmod can see + // stuff from WORKSPACE). + return sourceRepo.isMain() + ? RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS + : RepositoryMappingValue.key(sourceRepo); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) + throws InterruptedException { + RepositoryMappingValue repoMappingValue = (RepositoryMappingValue) env.getValue(getSkyKey()); + return repoMappingValue != RepositoryMappingValue.NOT_FOUND_VALUE + && RepositoryName.createUnvalidated(oldValue) + .equals(repoMappingValue.getRepositoryMapping().get(apparentName)); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java index 87046be6d5c732..4d8c46f8d5faa9 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.repository; -import static com.google.devtools.build.lib.cmdline.LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.annotations.VisibleForTesting; @@ -93,7 +92,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 = 4; + private static final int MARKER_FILE_VERSION = 7; // Mapping of rule class name to RepositoryFunction. private final ImmutableMap handlers; @@ -165,7 +164,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) String.format("'%s' is not a repository rule", repositoryName.getCanonicalForm())); } - DigestWriter digestWriter = new DigestWriter(directories, repositoryName, rule); + DigestWriter digestWriter = + new DigestWriter(directories, repositoryName, rule, starlarkSemantics); if (shouldUseVendorRepos(env, handler, rule)) { RepositoryDirectoryValue repositoryDirectoryValue = tryGettingValueUsingVendoredRepo( @@ -202,7 +202,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) // only recreate it after fetching is done to prevent this scenario. DigestWriter.clearMarkerFile(directories, repositoryName); RepositoryDirectoryValue.Builder builder = - fetchRepository(skyKey, repoRoot, env, digestWriter.getMarkerData(), handler, rule); + fetchRepository( + skyKey, repoRoot, env, digestWriter.getRecordedInputValues(), handler, rule); if (builder == null) { return null; } @@ -392,12 +393,9 @@ private boolean shouldUseVendorRepos(Environment env, RepositoryFunction handler } private boolean shouldExcludeRepoFromVendoring(RepositoryFunction handler, Rule rule) { - return handler.isLocal(rule) || handler.isConfigure(rule) || isWorkspaceRepo(rule); - } - - private boolean isWorkspaceRepo(Rule rule) { - // All workspace repos are under //external, while bzlmod repo rules are not - return rule.getPackage().getPackageIdentifier().equals(EXTERNAL_PACKAGE_IDENTIFIER); + return handler.isLocal(rule) + || handler.isConfigure(rule) + || RepositoryFunction.isWorkspaceRepo(rule); } @Nullable @@ -405,7 +403,7 @@ private RepositoryDirectoryValue.Builder fetchRepository( SkyKey skyKey, Path repoRoot, Environment env, - Map markerData, + Map recordedInputValues, RepositoryFunction handler, Rule rule) throws InterruptedException, RepositoryFunctionException { @@ -417,7 +415,7 @@ private RepositoryDirectoryValue.Builder fetchRepository( RepositoryDirectoryValue.Builder repoBuilder; try { - repoBuilder = handler.fetch(rule, repoRoot, directories, env, markerData, skyKey); + repoBuilder = handler.fetch(rule, repoRoot, directories, env, recordedInputValues, skyKey); } catch (RepositoryFunctionException e) { // Upon an exceptional exit, the fetching of that repository is over as well. env.getListener().post(RepositoryFetchProgress.finished(repoName)); @@ -584,25 +582,31 @@ static String unescape(String str) { } private static class DigestWriter { + private final BlazeDirectories directories; private final Path markerPath; private final Rule rule; // not just Map<> to signal that iteration order must be deterministic - private final TreeMap markerData; + private final TreeMap recordedInputValues; private final String ruleKey; - DigestWriter(BlazeDirectories directories, RepositoryName repositoryName, Rule rule) { - ruleKey = computeRuleKey(rule); + DigestWriter( + BlazeDirectories directories, + RepositoryName repositoryName, + Rule rule, + StarlarkSemantics starlarkSemantics) { + this.directories = directories; + ruleKey = computeRuleKey(rule, starlarkSemantics); markerPath = getMarkerPath(directories, repositoryName.getName()); this.rule = rule; - markerData = Maps.newTreeMap(); + recordedInputValues = Maps.newTreeMap(); } byte[] writeMarkerFile() throws RepositoryFunctionException { StringBuilder builder = new StringBuilder(); builder.append(ruleKey).append("\n"); - for (Map.Entry data : markerData.entrySet()) { - String key = data.getKey(); - String value = data.getValue(); + for (Map.Entry recordedInput : recordedInputValues.entrySet()) { + String key = recordedInput.getKey().toString(); + String value = recordedInput.getValue(); builder.append(escape(key)).append(" ").append(escape(value)).append("\n"); } String content = builder.toString(); @@ -639,14 +643,14 @@ byte[] areRepositoryAndMarkerFileConsistent( return null; } - Map markerData = new TreeMap<>(); + Map recordedInputValues = new TreeMap<>(); String content; try { content = FileSystemUtils.readContent(markerPath, UTF_8); - String markerRuleKey = readMarkerFile(content, markerData); + String markerRuleKey = readMarkerFile(content, recordedInputValues); boolean verified = false; if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) { - verified = handler.verifyMarkerData(rule, markerData, env); + verified = handler.verifyRecordedInputs(rule, directories, recordedInputValues, env); if (env.valuesMissing()) { return null; } @@ -662,12 +666,13 @@ byte[] areRepositoryAndMarkerFileConsistent( } } - Map getMarkerData() { - return markerData; + Map getRecordedInputValues() { + return recordedInputValues; } @Nullable - private String readMarkerFile(String content, Map markerData) { + private static String readMarkerFile( + String content, Map recordedInputValues) { String markerRuleKey = null; Iterable lines = Splitter.on('\n').split(content); @@ -678,22 +683,26 @@ private String readMarkerFile(String content, Map markerData) { firstLine = false; } else { int sChar = line.indexOf(' '); - String key = line; - String value = ""; if (sChar > 0) { - key = unescape(line.substring(0, sChar)); - value = unescape(line.substring(sChar + 1)); + RepoRecordedInput input = RepoRecordedInput.parse(unescape(line.substring(0, sChar))); + if (input == null) { + // ignore invalid entries. + continue; + } + recordedInputValues.put(input, unescape(line.substring(sChar + 1))); } - markerData.put(key, value); } } return markerRuleKey; } - private String computeRuleKey(Rule rule) { + private String computeRuleKey(Rule rule, StarlarkSemantics starlarkSemantics) { return new Fingerprint() .addBytes(RuleFormatter.serializeRule(rule).build().toByteArray()) .addInt(MARKER_FILE_VERSION) + // TODO: Using the hashCode() method for StarlarkSemantics here is suboptimal as + // it doesn't include any default values. + .addInt(starlarkSemantics.hashCode()) .hexDigestAndReset(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java index ac0e193dabf893..591211ce441f77 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java @@ -15,20 +15,15 @@ package com.google.devtools.build.lib.rules.repository; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.devtools.build.lib.cmdline.LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.io.BaseEncoding; -import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; -import com.google.devtools.build.lib.cmdline.LabelSyntaxException; -import com.google.devtools.build.lib.cmdline.LabelValidator; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -41,7 +36,6 @@ import com.google.devtools.build.lib.skyframe.PackageLookupFunction; import com.google.devtools.build.lib.skyframe.PackageLookupValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; -import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -52,12 +46,8 @@ import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.SkyframeLookupResult; import java.io.IOException; -import java.util.Collection; -import java.util.LinkedHashMap; import java.util.Map; -import java.util.Objects; import java.util.Set; import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; @@ -148,6 +138,11 @@ public static void setupRepoRoot(Path repoRoot) throws RepositoryFunctionExcepti } } + public static boolean isWorkspaceRepo(Rule rule) { + // All workspace repos are under //external, while bzlmod repo rules are not + return rule.getPackage().getPackageIdentifier().equals(EXTERNAL_PACKAGE_IDENTIFIER); + } + protected void setupRepoRootBeforeFetching(Path repoRoot) throws RepositoryFunctionException { setupRepoRoot(repoRoot); } @@ -178,8 +173,8 @@ public void reportSkyframeRestart(Environment env, RepositoryName repoName) { *

The {@code markerData} argument can be mutated to augment the data to write to the * repository marker file. If any data in the {@code markerData} change between 2 execute of the * {@link RepositoryDelegatorFunction} then this should be a reason to invalidate the repository. - * The {@link #verifyMarkerData} method is responsible for checking the value added to that map - * when checking the content of a marker file. + * The {@link #verifyRecordedInputs} method is responsible for checking the value added to that + * map when checking the content of a marker file. */ @ThreadSafe @Nullable @@ -188,130 +183,22 @@ public abstract RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws InterruptedException, RepositoryFunctionException; - @SuppressWarnings("unchecked") - private static ImmutableSet getEnviron(Rule rule) { - if (rule.isAttrDefined("$environ", Type.STRING_LIST)) { - return ImmutableSet.copyOf((Collection) rule.getAttr("$environ")); - } - return ImmutableSet.of(); - } - /** * Verify the data provided by the marker file to check if a refetch is needed. Returns true if * the data is up to date and no refetch is needed and false if the data is obsolete and a refetch * is needed. */ - public boolean verifyMarkerData(Rule rule, Map markerData, Environment env) - throws InterruptedException { - return verifyEnvironMarkerData(markerData, env, getEnviron(rule)) - && verifySemanticsMarkerData(markerData, env) - && verifyRepoMappingMarkerData(markerData, env) - && verifyMarkerDataForFiles(rule, markerData, env); - } - - protected boolean verifySemanticsMarkerData(Map markerData, Environment env) - throws InterruptedException { - return true; - } - - private static boolean verifyRepoMappingMarkerData( - Map markerData, Environment env) throws InterruptedException { - ImmutableSet skyKeys = - markerData.keySet().stream() - .filter(k -> k.startsWith("REPO_MAPPING:")) - // grab the part after the 'REPO_MAPPING:' and before the comma - .map(k -> k.substring("REPO_MAPPING:".length(), k.indexOf(','))) - .map(k -> RepositoryMappingValue.key(RepositoryName.createUnvalidated(k))) - .collect(toImmutableSet()); - SkyframeLookupResult result = env.getValuesAndExceptions(skyKeys); - if (env.valuesMissing()) { - return false; - } - for (Map.Entry entry : markerData.entrySet()) { - if (!entry.getKey().startsWith("REPO_MAPPING:")) { - continue; - } - int commaIndex = entry.getKey().indexOf(','); - RepositoryName fromRepo = - RepositoryName.createUnvalidated( - entry.getKey().substring("REPO_MAPPING:".length(), commaIndex)); - String apparentRepoName = entry.getKey().substring(commaIndex + 1); - RepositoryMappingValue repoMappingValue = - (RepositoryMappingValue) result.get(RepositoryMappingValue.key(fromRepo)); - if (repoMappingValue == RepositoryMappingValue.NOT_FOUND_VALUE - || !RepositoryName.createUnvalidated(entry.getValue()) - .equals(repoMappingValue.getRepositoryMapping().get(apparentRepoName))) { - return false; - } - } - return true; - } - - private static boolean verifyLabelMarkerData(Rule rule, String key, String value, Environment env) + public boolean verifyRecordedInputs( + Rule rule, + BlazeDirectories directories, + Map recordedInputValues, + Environment env) throws InterruptedException { - Preconditions.checkArgument(key.startsWith("FILE:")); - try { - RootedPath rootedPath; - String fileKey = key.substring(5); - if (LabelValidator.isAbsolute(fileKey)) { - rootedPath = getRootedPathFromLabel(Label.parseCanonical(fileKey), env); - } else { - // TODO(pcloudy): Removing checking absolute path, they should all be absolute label. - PathFragment filePathFragment = PathFragment.create(fileKey); - Path file = rule.getPackage().getPackageDirectory().getRelative(filePathFragment); - rootedPath = - RootedPath.toRootedPath( - Root.fromPath(file.getParentDirectory()), PathFragment.create(file.getBaseName())); - } - - SkyKey fileSkyKey = FileValue.key(rootedPath); - FileValue fileValue = (FileValue) env.getValueOrThrow(fileSkyKey, IOException.class); - - if (fileValue == null || !fileValue.isFile() || fileValue.isSpecialFile()) { - return false; - } - - return Objects.equals(value, fileValueToMarkerValue(fileValue)); - } catch (LabelSyntaxException e) { - throw new IllegalStateException( - "Key " + key + " is not a correct file key (should be in form FILE:label)", e); - } catch (IOException | EvalException e) { - // Consider those exception to be a cause for invalidation - return false; - } - } - - /** - * 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. - */ - public static String fileValueToMarkerValue(FileValue fileValue) throws IOException { - Preconditions.checkArgument(fileValue.isFile() && !fileValue.isSpecialFile()); - // Return the file content digest in hex. fileValue may or may not have the digest available. - byte[] digest = fileValue.realFileStateValue().getDigest(); - if (digest == null) { - // Fast digest not available, or it would have been in the FileValue. - digest = fileValue.realRootedPath().asPath().getDigest(); - } - return BaseEncoding.base16().lowerCase().encode(digest); - } - - static boolean verifyMarkerDataForFiles( - Rule rule, Map markerData, Environment env) throws InterruptedException { - for (Map.Entry entry : markerData.entrySet()) { - if (entry.getKey().startsWith("FILE:")) { - if (!verifyLabelMarkerData(rule, entry.getKey(), entry.getValue(), env)) { - return false; - } - } - } - return true; + return RepoRecordedInput.areAllValuesUpToDate(env, directories, recordedInputValues); } public static RootedPath getRootedPathFromLabel(Label label, Environment env) @@ -343,7 +230,7 @@ public static RootedPath getRootedPathFromLabel(Label label, Environment env) */ @Nullable protected Map declareEnvironmentDependencies( - Map markerData, Environment env, Set keys) + Map recordedInputValues, Environment env, Set keys) throws InterruptedException { if (keys.isEmpty()) { return ImmutableMap.of(); @@ -354,7 +241,9 @@ protected Map declareEnvironmentDependencies( return null; } // Add the dependencies to the marker file - keys.forEach(key -> markerData.put("ENV:" + key, envDep.get(key))); + for (String key : keys) { + recordedInputValues.put(new RepoRecordedInput.EnvVar(key), envDep.get(key)); + } return envDep; } @@ -382,57 +271,6 @@ public static ImmutableMap getEnvVarValues(Environment env, Set< return repoEnv.buildKeepingLast(); } - /** - * Verify marker data previously saved by {@link #declareEnvironmentDependencies(Map, Environment, - * Iterable)}. This function is to be called from a {@link #verifyMarkerData(Rule, Map, - * Environment)} function to verify the values for environment variables. - */ - protected boolean verifyEnvironMarkerData( - Map markerData, Environment env, Set keys) - throws InterruptedException { - if (keys.isEmpty()) { - return true; - } - - ImmutableMap environ = ActionEnvironmentFunction.getEnvironmentView(env, keys); - if (env.valuesMissing()) { - return false; // Returns false so caller knows to return immediately - } - - Map repoEnvOverride = PrecomputedValue.REPO_ENV.get(env); - if (repoEnvOverride == null) { - return false; - } - - // Only depend on --repo_env values that are specified in the "environ" attribute. - Map repoEnv = new LinkedHashMap<>(environ); - for (String key : keys) { - String value = repoEnvOverride.get(key); - if (value != null) { - repoEnv.put(key, value); - } - } - - // Verify that all environment variable in the marker file are also in keys - for (String key : markerData.keySet()) { - if (key.startsWith("ENV:") && !keys.contains(key.substring(4))) { - return false; - } - } - - // Now verify the values of the marker data - for (String key : keys) { - if (!markerData.containsKey("ENV:" + key)) { - return false; - } - String markerValue = markerData.get("ENV:" + key); - if (!Objects.equals(markerValue, repoEnv.get(key))) { - return false; - } - } - return true; - } - /** * Whether fetching is done using local operations only. * diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index bc14ce30656cb0..ecd48b636ac24d 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -125,6 +125,7 @@ java_library( ":diff_awareness_manager", ":directory_listing_function", ":directory_listing_state_value", + ":directory_tree_digest_function", ":exclusive_test_build_driver_value", ":execution_finished_event", ":file_function", @@ -1259,6 +1260,33 @@ java_library( ], ) +java_library( + name = "directory_tree_digest_function", + srcs = ["DirectoryTreeDigestFunction.java"], + deps = [ + ":directory_listing_value", + ":directory_tree_digest_value", + "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:guava", + "//third_party:jsr305", + ], +) + +java_library( + name = "directory_tree_digest_value", + srcs = ["DirectoryTreeDigestValue.java"], + deps = [ + ":sky_functions", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:auto_value", + ], +) + java_library( name = "dirents", srcs = ["Dirents.java"], diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java new file mode 100644 index 00000000000000..ae23c3721b3379 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunction.java @@ -0,0 +1,148 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.skyframe; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.actions.FileValue; +import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.Dirent; +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.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.SkyframeLookupResult; +import java.io.IOException; +import java.util.stream.StreamSupport; +import javax.annotation.Nullable; + +/** A {@link SkyFunction} for {@link DirectoryTreeDigestValue}s. */ +public final class DirectoryTreeDigestFunction implements SkyFunction { + @Override + @Nullable + public SkyValue compute(SkyKey skyKey, Environment env) + throws InterruptedException, DirectoryTreeDigestFunctionException { + RootedPath rootedPath = (RootedPath) skyKey.argument(); + DirectoryListingValue dirListingValue = + (DirectoryListingValue) env.getValue(DirectoryListingValue.key(rootedPath)); + if (dirListingValue == null) { + return null; + } + + // Get the names of entries directly in this directory, and sort them. This sets the basis for + // subsequent digests. + ImmutableSet sortedDirents = + StreamSupport.stream(dirListingValue.getDirents().spliterator(), /* parallel= */ false) + .map(Dirent::getName) + .sorted() + .collect(toImmutableSet()); + + // Turn each entry into a FileValue. + ImmutableList fileValues = getFileValues(env, sortedDirents, rootedPath); + if (fileValues == null) { + return null; + } + + // For each entry that is a directory (or a symlink to a directory), find its own + // DirectoryTreeDigestValue. + ImmutableList subDirTreeDigests = getSubDirTreeDigests(env, fileValues); + if (subDirTreeDigests == null) { + return null; + } + + // Finally, we're ready to digest everything together! + Fingerprint fp = new Fingerprint(); + fp.addStrings(sortedDirents); + fp.addStrings(subDirTreeDigests); + try { + for (FileValue fileValue : fileValues) { + fp.addInt(fileValue.realFileStateValue().getType().ordinal()); + if (fileValue.isFile()) { + byte[] digest = fileValue.realFileStateValue().getDigest(); + if (digest == null) { + // Fast digest not available, or it would have been in the FileValue. + digest = fileValue.realRootedPath().asPath().getDigest(); + } + fp.addBytes(digest); + } + } + } catch (IOException e) { + throw new DirectoryTreeDigestFunctionException(e); + } + + return DirectoryTreeDigestValue.of(fp.hexDigestAndReset()); + } + + @Nullable + private static ImmutableList getFileValues( + Environment env, ImmutableSet sortedDirents, RootedPath rootedPath) + throws InterruptedException { + ImmutableSet fileValueKeys = + sortedDirents.stream() + .map( + dirent -> + FileValue.key( + RootedPath.toRootedPath( + rootedPath.getRoot(), + rootedPath.getRootRelativePath().getRelative(dirent)))) + .collect(toImmutableSet()); + SkyframeLookupResult result = env.getValuesAndExceptions(fileValueKeys); + if (env.valuesMissing()) { + return null; + } + ImmutableList fileValues = + fileValueKeys.stream() + .map(result::get) + .map(FileValue.class::cast) + .collect(toImmutableList()); + if (env.valuesMissing()) { + return null; + } + return fileValues; + } + + @Nullable + private static ImmutableList getSubDirTreeDigests( + Environment env, ImmutableList fileValues) throws InterruptedException { + ImmutableSet dirTreeDigestValueKeys = + fileValues.stream() + .filter(FileValue::isDirectory) + .map(fv -> DirectoryTreeDigestValue.key(fv.realRootedPath())) + .collect(toImmutableSet()); + SkyframeLookupResult result = env.getValuesAndExceptions(dirTreeDigestValueKeys); + if (env.valuesMissing()) { + return null; + } + ImmutableList dirTreeDigests = + dirTreeDigestValueKeys.stream() + .map(result::get) + .map(DirectoryTreeDigestValue.class::cast) + .map(DirectoryTreeDigestValue::hexDigest) + .collect(toImmutableList()); + if (env.valuesMissing()) { + return null; + } + return dirTreeDigests; + } + + private static final class DirectoryTreeDigestFunctionException extends SkyFunctionException { + public DirectoryTreeDigestFunctionException(IOException e) { + super(e, Transience.TRANSIENT); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestValue.java new file mode 100644 index 00000000000000..246abf09872020 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestValue.java @@ -0,0 +1,51 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.skyframe; + +import com.google.auto.value.AutoValue; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.AbstractSkyKey; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyValue; + +/** + * Contains information about the recursive digest of a directory tree, including all transitive + * descendant files and their contents. + */ +@AutoValue +public abstract class DirectoryTreeDigestValue implements SkyValue { + public abstract String hexDigest(); + + public static DirectoryTreeDigestValue of(String hexDigest) { + return new AutoValue_DirectoryTreeDigestValue(hexDigest); + } + + public static Key key(RootedPath path) { + return new Key(path); + } + + /** Key type for {@link DirectoryTreeDigestValue}. */ + public static class Key extends AbstractSkyKey { + + private Key(RootedPath rootedPath) { + super(rootedPath); + } + + @Override + public SkyFunctionName functionName() { + return SkyFunctions.DIRECTORY_TREE_DIGEST; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 4fad5031b3cf3c..a082489a227836 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -30,6 +30,8 @@ public final class SkyFunctions { SkyFunctionName.createNonHermetic("DIRECTORY_LISTING_STATE"); public static final SkyFunctionName DIRECTORY_LISTING = SkyFunctionName.createHermetic("DIRECTORY_LISTING"); + public static final SkyFunctionName DIRECTORY_TREE_DIGEST = + SkyFunctionName.createHermetic("DIRECTORY_TREE_DIGEST"); // Hermetic even though package lookups secretly access the set of deleted packages, because // SequencedSkyframeExecutor deletes any affected PACKAGE_LOOKUP nodes when that set changes. public static final SkyFunctionName PACKAGE_LOOKUP = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index d41dba2b218e34..af132abef92e76 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -572,6 +572,7 @@ private ImmutableMap skyFunctions() { new FileSymlinkInfiniteExpansionUniquenessFunction()); map.put(FileValue.FILE, new FileFunction(pkgLocator, directories)); map.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()); + map.put(SkyFunctions.DIRECTORY_TREE_DIGEST, new DirectoryTreeDigestFunction()); map.put( SkyFunctions.PACKAGE_LOOKUP, new PackageLookupFunction( 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 ab616727be0ce1..d7ab59b2b0a14b 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 @@ -21,6 +21,7 @@ java_library( "//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", @@ -28,10 +29,8 @@ java_library( "//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/local_repository_rule", "//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:sky_functions", "//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", @@ -39,7 +38,6 @@ java_library( "//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", - "//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", @@ -69,6 +67,7 @@ java_test( "//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", @@ -76,10 +75,8 @@ java_test( "//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/local_repository_rule", "//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:sky_functions", "//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", @@ -87,7 +84,6 @@ java_test( "//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", - "//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", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPathTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPathTest.java index e6ebb596d7ebda..138c170ea9bb77 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPathTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkPathTest.java @@ -35,16 +35,20 @@ public class StarlarkPathTest { private final FileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256); private final Path wd = FileSystemUtils.getWorkingDirectory(fs); + private static StarlarkPath makePath(Path path) { + return new StarlarkPath(/* ctx= */ null, path); + } + @Before public void setup() throws Exception { - ev.update("wd", new StarlarkPath(wd)); + ev.update("wd", makePath(wd)); } @Test public void testStarlarkPathGetChild() throws Exception { - assertThat(ev.eval("wd.get_child()")).isEqualTo(new StarlarkPath(wd)); - assertThat(ev.eval("wd.get_child('foo')")).isEqualTo(new StarlarkPath(wd.getChild("foo"))); + assertThat(ev.eval("wd.get_child()")).isEqualTo(makePath(wd)); + assertThat(ev.eval("wd.get_child('foo')")).isEqualTo(makePath(wd.getChild("foo"))); assertThat(ev.eval("wd.get_child('a','b/c','/d/')")) - .isEqualTo(new StarlarkPath(wd.getRelative("a/b/c/d"))); + .isEqualTo(makePath(wd.getRelative("a/b/c/d"))); } } 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 48beb80df74720..974349a563e1fa 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 @@ -25,6 +25,9 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.io.CharStreams; +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.RepositoryMapping; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -80,6 +83,7 @@ public final class StarlarkRepositoryContextTest { private Scratch scratch; + private Path outputBase; private Path outputDirectory; private Root root; private Path workspaceFile; @@ -92,6 +96,7 @@ public final class StarlarkRepositoryContextTest { @Before public void setUp() throws Exception { scratch = new Scratch("/"); + outputBase = scratch.dir("/outputBase"); outputDirectory = scratch.dir("/outputDir"); root = Root.fromPath(scratch.dir("/wsRoot")); workspaceFile = scratch.file("/wsRoot/WORKSPACE"); @@ -156,6 +161,12 @@ private void setUpContextForRule( outputDirectory, ImmutableList.of(root), BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY); + BlazeDirectories directories = + new BlazeDirectories( + new ServerDirectories(root.asPath(), outputBase, root.asPath()), + root.asPath(), + /* defaultSystemJavabase= */ null, + AnalysisMock.get().getProductName()); context = new StarlarkRepositoryContext( rule, @@ -166,11 +177,11 @@ private void setUpContextForRule( envVariables, downloader, 1.0, - /*processWrapper=*/ null, + /* processWrapper= */ null, starlarkSemantics, repoRemoteExecutor, SyscallCache.NO_CACHE, - root.asPath()); + directories); } private void setUpContextForRule(String name) throws Exception { @@ -301,7 +312,7 @@ public void testRead() throws Exception { setUpContextForRule("test"); context.createFile(context.path("foo/bar"), "foobar", true, true, thread); - String content = context.readFile(context.path("foo/bar"), thread); + String content = context.readFile(context.path("foo/bar"), "auto", thread); assertThat(content).isEqualTo("foobar"); } @@ -313,7 +324,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"); } @@ -324,7 +335,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) @@ -347,7 +358,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) @@ -368,7 +379,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) @@ -445,7 +456,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")); @@ -465,8 +476,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()).containsExactly( - context.path("/my/folder/a"), context.path("/my/folder/b"), context.path("/my/folder/c")); + assertThat(context.path("/my/folder").readdir("no")) + .containsExactly( + context.path("/my/folder/a"), + context.path("/my/folder/b"), + context.path("/my/folder/c")); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD index bea028bcb01688..ba165c318ed011 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/BUILD @@ -34,6 +34,7 @@ java_library( "//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/local_repository_rule", + "//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_compile", diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java index d8f56d8eaae238..84bb5478ce15ee 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryFunctionTest.java @@ -59,7 +59,7 @@ public RepositoryDirectoryValue.Builder fetch( Path outputDirectory, BlazeDirectories directories, SkyFunction.Environment env, - Map markerData, + Map recordedInputValues, SkyKey key) throws InterruptedException { return null; @@ -127,7 +127,7 @@ public void testFileValueToMarkerValue() throws Exception { // Digest should be returned if the FileStateValue has it. FileStateValue fsv = new RegularFileStateValueWithDigest(3, new byte[] {1, 2, 3, 4}); FileValue fv = new RegularFileValue(path, fsv); - assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo("01020304"); + assertThat(RepoRecordedInput.File.fileValueToMarkerValue(fv)).isEqualTo("01020304"); // Digest should also be returned if the FileStateValue doesn't have it. FileStatus status = Mockito.mock(FileStatus.class); @@ -136,6 +136,6 @@ public void testFileValueToMarkerValue() throws Exception { fsv = new RegularFileStateValueWithContentsProxy(3, FileContentsProxy.create(status)); fv = new RegularFileValue(path, fsv); String expectedDigest = BaseEncoding.base16().lowerCase().encode(path.asPath().getDigest()); - assertThat(RepositoryFunction.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest); + assertThat(RepoRecordedInput.File.fileValueToMarkerValue(fv)).isEqualTo(expectedDigest); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index f088aa2e0c7cba..3c21607a83f8b5 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -219,6 +219,8 @@ java_test( "//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness_manager", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_state_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_function", + "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value", "//src/main/java/com/google/devtools/build/lib/skyframe:file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:fileset_entry_function", "//src/main/java/com/google/devtools/build/lib/skyframe:fileset_entry_key", @@ -237,6 +239,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value", "//src/main/java/com/google/devtools/build/lib/skyframe:package_progress_receiver", "//src/main/java/com/google/devtools/build/lib/skyframe:package_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:prepare_deps_of_pattern_value", "//src/main/java/com/google/devtools/build/lib/skyframe:prepare_deps_of_patterns_value", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunctionTest.java new file mode 100644 index 00000000000000..7dd4eed5221952 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/DirectoryTreeDigestFunctionTest.java @@ -0,0 +1,226 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.devtools.build.lib.skyframe; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +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.clock.BlazeClock; +import com.google.devtools.build.lib.pkgcache.PathPackageLocator; +import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; +import com.google.devtools.build.lib.testutil.FoundationTestCase; +import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; +import com.google.devtools.build.lib.vfs.FileStateKey; +import com.google.devtools.build.lib.vfs.Root; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.lib.vfs.SyscallCache; +import com.google.devtools.build.skyframe.EvaluationContext; +import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator; +import com.google.devtools.build.skyframe.MemoizingEvaluator; +import com.google.devtools.build.skyframe.RecordingDifferencer; +import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyFunctionName; +import com.google.devtools.build.skyframe.SkyKey; +import java.util.concurrent.atomic.AtomicReference; +import net.starlark.java.eval.StarlarkSemantics; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DirectoryTreeDigestFunctionTest extends FoundationTestCase { + + private RecordingDifferencer differencer; + private ImmutableMap skyFunctions; + private EvaluationContext evaluationContext; + + @Before + public void setup() throws Exception { + differencer = new SequencedRecordingDifferencer(); + evaluationContext = + EvaluationContext.newBuilder().setParallelism(8).setEventHandler(reporter).build(); + AtomicReference packageLocator = + new AtomicReference<>( + new PathPackageLocator( + outputBase, + ImmutableList.of(Root.fromPath(rootDirectory)), + BazelSkyframeExecutorConstants.BUILD_FILES_BY_PRIORITY)); + BlazeDirectories directories = + new BlazeDirectories( + new ServerDirectories(rootDirectory, outputBase, rootDirectory), + rootDirectory, + /* defaultSystemJavabase= */ null, + AnalysisMock.get().getProductName()); + ExternalFilesHelper externalFilesHelper = + ExternalFilesHelper.createForTesting( + packageLocator, + ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, + directories); + + skyFunctions = + ImmutableMap.builder() + .put(FileValue.FILE, new FileFunction(packageLocator, directories)) + .put( + FileStateKey.FILE_STATE, + new FileStateFunction( + Suppliers.ofInstance(new TimestampGranularityMonitor(BlazeClock.instance())), + SyscallCache.NO_CACHE, + externalFilesHelper)) + .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) + .put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction()) + .put( + SkyFunctions.DIRECTORY_LISTING_STATE, + new DirectoryListingStateFunction(externalFilesHelper, SyscallCache.NO_CACHE)) + .put(SkyFunctions.DIRECTORY_TREE_DIGEST, new DirectoryTreeDigestFunction()) + .buildOrThrow(); + + PrecomputedValue.STARLARK_SEMANTICS.set(differencer, StarlarkSemantics.DEFAULT); + PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, packageLocator.get()); + } + + private String getTreeDigest(String path) throws Exception { + RootedPath rootedPath = + RootedPath.toRootedPath(Root.absoluteRoot(fileSystem), scratch.resolve(path)); + SkyKey key = DirectoryTreeDigestValue.key(rootedPath); + MemoizingEvaluator evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer); + var result = evaluator.evaluate(ImmutableList.of(key), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + return ((DirectoryTreeDigestValue) result.get(key)).hexDigest(); + } + + @Test + public void basic() throws Exception { + scratch.file("a", "a"); + scratch.file("b/b", "b"); + scratch.file("c", "c"); + String oldDigest = getTreeDigest("/"); + + scratch.overwriteFile("b/b", "something else"); + assertThat(getTreeDigest("/")).isNotEqualTo(oldDigest); + } + + @Test + public void addFile() throws Exception { + scratch.file("a", "a"); + scratch.file("b/b", "b"); + scratch.file("c", "c"); + String oldDigest = getTreeDigest("/"); + + scratch.file("b/d", "something else"); + assertThat(getTreeDigest("/")).isNotEqualTo(oldDigest); + } + + @Test + public void removeFile() throws Exception { + scratch.file("a", "a"); + scratch.file("b/b", "b"); + scratch.file("c", "c"); + String oldDigest = getTreeDigest("/"); + + scratch.deleteFile("b/b"); + assertThat(getTreeDigest("/")).isNotEqualTo(oldDigest); + } + + @Test + public void renameFile() throws Exception { + scratch.file("a", "a"); + scratch.file("b/b", "b"); + scratch.file("c", "c"); + String oldDigest = getTreeDigest("/"); + + scratch.deleteFile("b/b"); + scratch.file("b/b1", "b"); + assertThat(getTreeDigest("/")).isNotEqualTo(oldDigest); + } + + @Test + public void swapDirAndFile() throws Exception { + scratch.file("a", "a"); + scratch.file("b", "b"); + scratch.file("c/inner", "inner"); + String oldDigest = getTreeDigest("/"); + + scratch.resolve("c").deleteTree(); + scratch.deleteFile("b"); + scratch.file("b/inner", "inner"); + scratch.file("c", "b"); + assertThat(getTreeDigest("/")).isNotEqualTo(oldDigest); + } + + @Test + public void changeMtime() throws Exception { + scratch.file("a", "a"); + scratch.file("b", "b"); + scratch.file("c", "c"); + String oldDigest = getTreeDigest("/"); + + // We don't digest mtimes so this shouldn't affect anything. + scratch.resolve("c").setLastModifiedTime(2024L); + assertThat(getTreeDigest("/")).isEqualTo(oldDigest); + } + + @Test + public void symlink() throws Exception { + scratch.file("dir/a", "a"); + scratch.resolve("dir/b").createSymbolicLink(scratch.resolve("otherdir")); + scratch.file("dir/c", "c"); + scratch.file("otherdir/b", "b"); + scratch.file("otherdir/sub/sub", "sub"); + String oldDigest = getTreeDigest("dir"); + + scratch.deleteFile("dir/b"); + scratch.resolve("dir/b").createSymbolicLink(scratch.resolve("yetotherdir")); + scratch.file("yetotherdir/crazy", "stuff"); + assertThat(getTreeDigest("dir")).isNotEqualTo(oldDigest); + } + + @Test + public void danglingSymlink() throws Exception { + scratch.file("dir/a", "a"); + scratch.resolve("dir/b").createSymbolicLink(scratch.resolve("otherdir")); + scratch.file("dir/c", "c"); + String oldDigest = getTreeDigest("dir"); + + scratch.file("otherdir/b", "b"); + assertThat(getTreeDigest("dir")).isNotEqualTo(oldDigest); + } + + @Test + public void symlinkPointingToSameContents() throws Exception { + scratch.file("dir/a", "a"); + scratch.file("dir/b/b", "b"); + scratch.file("dir/b/sub/sub", "sub"); + scratch.file("dir/c", "c"); + String oldDigest = getTreeDigest("dir"); + + // replace dir/b with a symlink pointing to otherdir/, which contains the same contents. + // this shouldn't affect the tree digest. + scratch.resolve("dir/b").deleteTree(); + scratch.resolve("dir/b").createSymbolicLink(scratch.resolve("otherdir")); + scratch.file("otherdir/b", "b"); + scratch.file("otherdir/sub/sub", "sub"); + assertThat(getTreeDigest("dir")).isEqualTo(oldDigest); + } +} diff --git a/src/test/py/bazel/bzlmod/bazel_fetch_test.py b/src/test/py/bazel/bzlmod/bazel_fetch_test.py index 53ff1d3a486c69..072caedba197e0 100644 --- a/src/test/py/bazel/bzlmod/bazel_fetch_test.py +++ b/src/test/py/bazel/bzlmod/bazel_fetch_test.py @@ -234,7 +234,7 @@ def testForceFetch(self): 'extension.bzl', [ 'def _repo_rule_impl(ctx):', - ' file_content = ctx.read("' + file_path + '").strip()', + ' file_content = ctx.read("' + file_path + '", watch="no")', ' print(file_content)', ' ctx.file("BUILD")', 'repo_rule = repository_rule(implementation=_repo_rule_impl)', diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index d94bd55675a8cc..21ca3706cc50d5 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -16,6 +16,7 @@ import json import os +import pathlib import tempfile from absl.testing import absltest from src.test.py.bazel import test_base @@ -1934,6 +1935,177 @@ def testReproducibleExtensionInLockfileErrorMode(self): stderr, ) + def testWatchingFileUnderWorkingDirectoryFails(self): + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' ctx.file("hello", "repo")', + ' repo_rule(name=ctx.read("hello", watch="yes"))', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + _, _, stderr = self.RunBazel(['build', '@repo'], allow_failure=True) + self.assertIn( + 'attempted to watch path under working directory', '\n'.join(stderr) + ) + + def testWatchingFileOutsideWorkspaceFails(self): + outside_file = pathlib.Path(tempfile.mkdtemp(dir=self._temp)).joinpath( + 'hello' + ) + scratchFile(outside_file, ['repo']) + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'lala\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' repo_rule(name=ctx.read(%s, watch="yes"))' + % repr(str(outside_file)), + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + _, _, stderr = self.RunBazel(['build', '@repo'], allow_failure=True) + self.assertIn( + 'attempted to watch path outside workspace', '\n'.join(stderr) + ) + + def testPathReaddirWatchesDirents(self): + self.ScratchFile('some_dir/foo') + self.ScratchFile('some_dir/bar') + self.ScratchFile('some_dir/baz') + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'repo\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' print("I see: " + ",".join(sorted([e.basename for e in' + ' ctx.path(%s).readdir()])))' + % repr(self.Path('some_dir')), + ' repo_rule(name="repo")', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + _, _, stderr = self.RunBazel(['build', '@repo']) + self.assertIn('I see: bar,baz,foo', '\n'.join(stderr)) + + # adding and removing entries should cause a reevaluation. + os.remove(self.Path('some_dir/baz')) + self.ScratchFile('some_dir/quux') + _, _, stderr = self.RunBazel(['build', '@repo']) + self.assertIn('I see: bar,foo,quux', '\n'.join(stderr)) + + # but changing file contents shouldn't. + self.ScratchFile('some_dir/bar', ['hulloooo']) + _, _, stderr = self.RunBazel(['build', '@repo']) + self.assertNotIn('I see:', '\n'.join(stderr)) + + def testPathReaddirOutsideWorkspaceDoesNotWatchDirents(self): + outside_dir = pathlib.Path(tempfile.mkdtemp(dir=self._temp)) + scratchFile(outside_dir.joinpath('whatever'), ['repo']) + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'repo\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' print("I see: " + ",".join(sorted([e.basename for e in' + ' ctx.path(%s).readdir()])))' + % repr(str(outside_dir)), + ' repo_rule(name="repo")', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + _, _, stderr = self.RunBazel(['build', '@repo']) + self.assertIn('I see: whatever', '\n'.join(stderr)) + + # since the directory in question is outside the workspace, adding and + # removing entries shouldn't cause a reevaluation. + os.remove(outside_dir.joinpath('whatever')) + scratchFile(outside_dir.joinpath('anything'), ['kek']) + _, _, stderr = self.RunBazel(['build', '@repo']) + self.assertNotIn('I see:', '\n'.join(stderr)) + + def testForceWatchingDirentsOutsideWorkspaceFails(self): + outside_dir = pathlib.Path(tempfile.mkdtemp(dir=self._temp)) + scratchFile(outside_dir.joinpath('whatever'), ['repo']) + self.ScratchFile( + 'MODULE.bazel', + [ + 'ext = use_extension("extension.bzl", "ext")', + 'use_repo(ext, "repo")', + ], + ) + self.ScratchFile('BUILD.bazel') + self.ScratchFile( + 'extension.bzl', + [ + 'def _repo_rule_impl(ctx):', + ' ctx.file("BUILD", "filegroup(name=\'repo\')")', + 'repo_rule = repository_rule(implementation=_repo_rule_impl)', + '', + 'def _ext_impl(ctx):', + ' print("I see: " + ",".join(sorted([e.basename for e in' + ' ctx.path(%s).readdir(watch="yes")])))' + % repr(str(outside_dir)), + ' repo_rule(name="repo")', + 'ext = module_extension(implementation=_ext_impl)', + ], + ) + + _, _, stderr = self.RunBazel(['build', '@repo'], allow_failure=True) + self.assertIn( + 'attempted to watch path outside workspace', '\n'.join(stderr) + ) + if __name__ == '__main__': absltest.main() diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 7b23eddab73211..f0423297d4a67c 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -501,7 +501,7 @@ function test_starlark_repository_environ() { def _impl(repository_ctx): print(repository_ctx.os.environ["FOO"]) # Symlink so a repository is created - repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path("")) + repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path(""), watch_target="no") repo = repository_rule(implementation=_impl, local=False) EOF @@ -544,7 +544,7 @@ EOF def _impl(repository_ctx): print(repository_ctx.os.environ["BAR"]) # Symlink so a repository is created - repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path("")) + repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path(""), watch_target="no") repo = repository_rule(implementation=_impl, local=True) EOF BAR=BEZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" @@ -556,7 +556,7 @@ EOF def _impl(repository_ctx): print(repository_ctx.os.environ["BAZ"]) # Symlink so a repository is created - repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path("")) + repository_ctx.symlink(repository_ctx.path("$repo2"), repository_ctx.path(""), watch_target="no") repo = repository_rule(implementation=_impl, local=True) EOF BAZ=BOZ bazel build @foo//:bar >& $TEST_log || fail "Failed to build" @@ -2660,10 +2660,12 @@ bazel_dep(name="bar") local_path_override(module_name="bar", path="bar") EOF touch BUILD + echo 'load("@r//:r.bzl", "pi"); print(pi)' > WORKSPACE.bzlmod cat > r.bzl < WORKSPACE.bzlmod cat > r.bzl < MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: nothing" + + # Running Bazel again shouldn't cause a refetch, despite "data.txt" + # having been written to after the read. + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" +} + +function test_file_watching_inside_working_dir_forcing_error() { + # when reading a file inside the working directory (where the repo + # is to be fetched), we shouldn't watch it. Forcing the watch should + # result in an error. + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log && fail "expected bazel to fail" + expect_log "attempted to watch path under working directory" +} + +function test_file_watching_outside_workspace() { + # when reading a file outside the Bazel workspace, we should watch it. + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + mkdir -p "${outside_dir}" + echo nothing > ${outside_dir}/data.txt + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: nothing" + + # Running Bazel again shouldn't cause a refetch. + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # But changing the outside file should cause a refetch. + echo something > ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: something" +} + +function test_file_watching_in_other_repo() { + # when reading a file in another repo, we should watch it. + local outside_dir="${TEST_TMPDIR}/outside_dir" + mkdir -p "${outside_dir}" + echo nothing > ${outside_dir}/data.txt + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: nothing" + + # Running Bazel again shouldn't cause a refetch. + bazel build @foo >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # But changing the file inside @bar should cause @foo to refetch. + cat > MODULE.bazel <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: something" +} + +function test_file_watching_in_other_repo_cycle() { + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log && fail "expected bazel to fail!" + expect_log "Circular definition of repositories" +} + +function test_watch_file_status_change() { + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + mkdir -p "${outside_dir}" + echo something > ${outside_dir}/data.txt + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: something" + + # test that all kinds of transitions between file, dir, and noent are watched + + rm ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see nothing" + + mkdir ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see a directory" + + rm -r ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see nothing" + + echo something again > ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: something again" + + rm ${outside_dir}/data.txt + mkdir ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see a directory" + + rm -r ${outside_dir}/data.txt + echo something yet again > ${outside_dir}/data.txt + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: something yet again" +} + +function test_watch_file_status_change_dangling_symlink() { + if "$is_windows"; then + # symlinks on Windows... annoying + return + fi + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + mkdir -p "${outside_dir}" + ln -s ${outside_dir}/pointee ${outside_dir}/pointer + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see nothing" + + echo haha > ${outside_dir}/pointee + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: haha" + + rm ${outside_dir}/pointee + mkdir ${outside_dir}/pointee + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see a directory" +} + +function test_watch_file_status_change_symlink_parent() { + if "$is_windows"; then + # symlinks on Windows... annoying + return + fi + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + mkdir -p "${outside_dir}/a" + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see nothing" + + mkdir -p ${outside_dir}/a/b + echo blah > ${outside_dir}/a/b/c + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: blah" + + rm -rf ${outside_dir}/a/b + ln -s ${outside_dir}/d ${outside_dir}/a/b + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see nothing" + + mkdir ${outside_dir}/d + echo bleh > ${outside_dir}/d/c + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: bleh" +} + +function test_path_readdir_watches_dirents() { + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + touch ${outside_dir}/foo + touch ${outside_dir}/bar + touch ${outside_dir}/baz + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: bar,baz,foo" + + # changing the contents of a file under there shouldn't trigger a refetch. + echo haha > ${outside_dir}/foo + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # adding a file should trigger a refetch. + touch ${outside_dir}/quux + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: bar,baz,foo,quux" + + # removing a file should trigger a refetch. + rm ${outside_dir}/baz + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I see: bar,foo,quux" + + # changing a file to a directory shouldn't trigger a refetch. + rm ${outside_dir}/bar + mkdir ${outside_dir}/bar + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" + + # changing entries in subdirectories shouldn't trigger a refetch. + touch ${outside_dir}/bar/inner + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I see:" +} + +function test_watch_tree() { + local outside_dir=$(mktemp -d "${TEST_TMPDIR}/testXXXXXX") + touch ${outside_dir}/foo + touch ${outside_dir}/bar + touch ${outside_dir}/baz + + create_new_workspace + cat > MODULE.bazel < r.bzl <& $TEST_log || fail "expected bazel to succeed" + expect_log "I'm running!" + + # changing the contents of a file under there should trigger a refetch. + echo haha > ${outside_dir}/foo + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I'm running!" + + # adding a file should trigger a refetch. + touch ${outside_dir}/quux + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I'm running!" + + # just touching an existing file shouldn't cause a refetch. + touch ${outside_dir}/bar + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_not_log "I'm running!" + + # changing a file to a directory should trigger a refetch. + rm ${outside_dir}/baz + mkdir ${outside_dir}/baz + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I'm running!" + + # changing entries in subdirectories should trigger a refetch. + touch ${outside_dir}/baz/inner + bazel build @r >& $TEST_log || fail "expected bazel to succeed" + expect_log "I'm running!" +} + run_suite "local repository tests" diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index 5942e288d89676..2aab31fafdda8b 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1,5 +1,5 @@ { - "lockFileVersion": 4, + "lockFileVersion": 6, "moduleFileHash": "0e3e315145ac7ee7a4e0ac825e1c5e03c068ec1254dd42c3caaecb27e921dc4d", "flags": { "cmdRegistries": [ @@ -1037,7 +1037,8 @@ "@@apple_support~//crosstool:setup.bzl%apple_cc_configure_extension": { "general": { "bzlTransitiveDigest": "pMLFCYaRPkgXPQ8vtuNkMfiHfPmRBy6QJfnid4sWfv0=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_apple_cc": { @@ -1063,7 +1064,8 @@ "@@bazel_tools//tools/android:android_extensions.bzl%remote_android_tools_extensions": { "general": { "bzlTransitiveDigest": "S0n86BFe4SJ3lRaZiRA5D46oH52UO2hP1T50t/zldOw=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "android_tools": { @@ -1089,7 +1091,8 @@ "@@bazel_tools//tools/cpp:cc_configure.bzl%cc_configure_extension": { "general": { "bzlTransitiveDigest": "PHpT2yqMGms2U4L3E/aZ+WcQalmZWm+ILdP3yiLsDhA=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_cc": { @@ -1115,7 +1118,8 @@ "@@bazel_tools//tools/osx:xcode_configure.bzl%xcode_configure_extension": { "general": { "bzlTransitiveDigest": "Qh2bWTU6QW6wkrd87qrU4YeY+SG37Nvw3A0PR4Y0L2Y=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_xcode": { @@ -1133,7 +1137,8 @@ "@@bazel_tools//tools/sh:sh_configure.bzl%sh_configure_extension": { "general": { "bzlTransitiveDigest": "hp4NgmNjEg5+xgvzfh6L83bt9/aiiWETuNpwNuF1MSU=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_sh": { @@ -1148,7 +1153,8 @@ "@@bazel_tools//tools/test:extensions.bzl%remote_coverage_tools_extension": { "general": { "bzlTransitiveDigest": "l5mcjH2gWmbmIycx97bzI2stD0Q0M5gpDc0aLOHKIm8=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "remote_coverage_tools": { @@ -1168,7 +1174,8 @@ "@@buildozer~//:buildozer_binary.bzl%buildozer_binary": { "general": { "bzlTransitiveDigest": "EleDU/FQ1+e/RgkW3aIDmdaxZEthvoWQhsqFTxiSgMI=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "buildozer_binary": { @@ -1192,7 +1199,8 @@ "@@rules_java~//java:extensions.bzl%toolchains": { "general": { "bzlTransitiveDigest": "04zUmtTZlr1dV4uomFuJ3vSbBSlCcm5xmKcp/Fy7bGw=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "remotejdk21_linux_toolchain_config_repo": { @@ -1696,9 +1704,10 @@ "@@rules_jvm_external~//:extensions.bzl%maven": { "general": { "bzlTransitiveDigest": "v8HssW6WP6B8s0BwuAMJuQCz6cQ9jlhOfx4dKBtPYB4=", - "accumulatedFileDigests": { - "@@rules_jvm_external~//:rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" + "recordedFileInputs": { + "@@rules_jvm_external~//rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" }, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "org_slf4j_slf4j_api_1_7_30": { @@ -2718,7 +2727,8 @@ "@@rules_jvm_external~//:non-module-deps.bzl%non_module_deps": { "general": { "bzlTransitiveDigest": "DqBh3ObkOvjDFKv8VTy6J2qr7hXsJm9/sES7bha7ftA=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "io_bazel_rules_kotlin": { @@ -2744,7 +2754,8 @@ "@@rules_python~//python/extensions:python.bzl%python": { "general": { "bzlTransitiveDigest": "31xtOi5rmBJ3jSHeziLzV7KKKgCc6tMnRUZ1BQLBeao=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pythons_hub": { @@ -2772,7 +2783,8 @@ "@@rules_python~//python/extensions/private:internal_deps.bzl%internal_deps": { "general": { "bzlTransitiveDigest": "fUb5iKCtPgjhclraX+//BnJ+LOcG6I6+O9UUxT+gZ50=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, + "recordedDirentsInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pypi__coverage_cp39_aarch64-apple-darwin": {