From b4272a02109218d6f4a99d8f1b732057a32e954e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?X=C3=B9d=C5=8Dng=20Y=C3=A1ng?= Date: Tue, 6 Feb 2024 21:11:17 -0500 Subject: [PATCH] Watch arbitrary file in repo rules * Starlark API changes * New method `rctx.watch()` that watches a path. As of right now, said path must exist and be a regular file, but that will change in follow-ups. * Existing method `rctx.read()` gets a new optional boolean parameter `watch` that defaults to True. This causes the file read to be watched as well by default, even if it's not addressed by a label. * Both changes apply to `mctx` as well. * Marker file format changes * `FILE:` marker file entries can now be `FILE:` followed by either an absolute path (starts with a singular '/'), or a label-like ... thing (which is a label but has a slash in place of a colon). This is because we might be watching something inside a repo that is _not_ under a package. * Same change will happen to the `accumulatedFileDigests` sections of the module lockfile. * Code changes * `RepoRecordedInput.File` subclasses are renamed to `FileOutsideWorkspace` (absolute paths) and `FileInsideWorkspace` (label-like things) instead to better reflect their meaning. * Unified the file watching/digest checking logic in SingleExtensionEvalFunction with the RepoRecordedInput stuff. Follow-ups will unify more. * Moved RepoRecordedInput to its own BUILD target as it's being used outside the repo fetching machinery (namely, module extensions) * Since `FileOutsideWorkspace` has to be an absolute path (never relative), we don't need a "base directory" for the RepoRecordedInput parser. What we really need is just the FileSystem object. Refactored code to reflect that. RELNOTES: Added a new method, `repository_ctx.watch()`, which asks Bazel to watch for changes to an arbitrary file. When said file changes, Bazel will refetch the repo. Work towards #20952. --- MODULE.bazel.lock | 2 +- .../lib/bazel/BazelRepositoryModule.java | 2 +- .../devtools/build/lib/bazel/bzlmod/BUILD | 2 + .../bazel/bzlmod/BazelLockFileFunction.java | 13 +- .../lib/bazel/bzlmod/BazelLockFileModule.java | 11 +- .../lib/bazel/bzlmod/BazelLockFileValue.java | 2 +- .../lib/bazel/bzlmod/GsonTypeAdapterUtil.java | 23 +- .../bazel/bzlmod/LockFileModuleExtension.java | 7 +- .../bazel/bzlmod/ModuleExtensionContext.java | 3 + .../bzlmod/SingleExtensionEvalFunction.java | 49 +---- .../devtools/build/lib/bazel/repository/BUILD | 1 + .../build/lib/bazel/repository/starlark/BUILD | 1 + .../starlark/StarlarkBaseExternalContext.java | 154 ++++++++++---- .../starlark/StarlarkRepositoryContext.java | 8 +- .../starlark/StarlarkRepositoryFunction.java | 7 +- .../build/lib/bazel/rules/android/BUILD | 1 + .../com/google/devtools/build/lib/rules/BUILD | 25 ++- .../repository/NewRepositoryFileHandler.java | 6 +- .../rules/repository/RepoRecordedInput.java | 197 ++++++++++++++---- .../RepositoryDelegatorFunction.java | 12 +- .../rules/repository/RepositoryFunction.java | 33 +-- .../build/lib/analysis/util/AnalysisMock.java | 5 +- .../bzlmod/BazelDepGraphFunctionTest.java | 4 +- .../bzlmod/BazelLockFileFunctionTest.java | 7 +- .../BazelModuleResolutionFunctionTest.java | 4 +- .../bzlmod/BzlmodRepoRuleFunctionTest.java | 4 +- .../bzlmod/ModuleExtensionResolutionTest.java | 4 +- .../build/lib/bazel/repository/starlark/BUILD | 2 + .../StarlarkRepositoryContextTest.java | 17 +- .../devtools/build/lib/rules/repository/BUILD | 1 + .../repository/RepositoryDelegatorTest.java | 4 +- .../repository/RepositoryFunctionTest.java | 4 +- .../shell/bazel/starlark_repository_test.sh | 104 +++++++++ src/test/tools/bzlmod/MODULE.bazel.lock | 28 +-- 34 files changed, 542 insertions(+), 205 deletions(-) diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index ac675135666807..d0a789c3b2ae98 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2741,7 +2741,7 @@ "general": { "bzlTransitiveDigest": "wiyY30lgHvAgghminN0h4lxMWexDIg/6r/+eOIA5bPU=", "accumulatedFileDigests": { - "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "c5a21b1716921abcd71d97668caa3dddc331daa5c7a1a80e4e041f2fd0e74767", + "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "053f3fbea3c154dbfa732217ac41090bc35fbd0f7784e5dfb3ec9945b8334621", "@@//:MODULE.bazel": "4d4b578dd607ef6ca79369c6c5d896c09e9aaa0fa4787f7865333fb2303d5b70" }, "envVariables": {}, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 008242a1ae063f..6a848b6b972ee3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -257,7 +257,7 @@ public void workspaceInit( new ModuleFileFunction(registryFactory, directories.getWorkspace(), builtinModules)) .addSkyFunction(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) .addSkyFunction( - SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace())) + SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace(), runtime.getFileSystem())) .addSkyFunction(SkyFunctions.BAZEL_FETCH_ALL, new BazelFetchAllFunction()) .addSkyFunction(SkyFunctions.BAZEL_MOD_TIDY, new BazelModTidyFunction()) .addSkyFunction(SkyFunctions.BAZEL_MODULE_INSPECTION, new BazelModuleInspectorFunction()) 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 ce8d22920e72c1..2586c2a83ec5a8 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:visible-for-serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", @@ -217,6 +218,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/profiler", + "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_failed_exception", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java index 500a5775bd8051..c3e42b9d74fd1f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; @@ -52,6 +53,7 @@ public class BazelLockFileFunction implements SkyFunction { Pattern.compile("\"lockFileVersion\":\\s*(\\d+)"); private final Path rootDirectory; + private final FileSystem runtimeFileSystem; private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS = BzlmodFlagsAndEnvVars.create( @@ -67,8 +69,9 @@ public class BazelLockFileFunction implements SkyFunction { .setModuleExtensions(ImmutableMap.of()) .build(); - public BazelLockFileFunction(Path rootDirectory) { + public BazelLockFileFunction(Path rootDirectory, FileSystem runtimeFileSystem) { this.rootDirectory = rootDirectory; + this.runtimeFileSystem = runtimeFileSystem; } @Override @@ -84,7 +87,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) } try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, "parse lockfile")) { - return getLockfileValue(lockfilePath, rootDirectory); + return getLockfileValue(lockfilePath, rootDirectory, runtimeFileSystem); } catch (IOException | JsonSyntaxException | NullPointerException e) { throw new BazelLockfileFunctionException( ExternalDepsException.withMessage( @@ -96,7 +99,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } - public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath, Path rootDirectory) + public static BazelLockFileValue getLockfileValue( + RootedPath lockfilePath, Path rootDirectory, FileSystem runtimeFileSystem) throws IOException { BazelLockFileValue bazelLockFileValue; try { @@ -110,7 +114,8 @@ public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath, Path .asPath() .getParentDirectory() .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), - rootDirectory) + rootDirectory, + runtimeFileSystem) .fromJson(json, BazelLockFileValue.class); } else { // This is an old version, needs to be updated diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 50fbd82feb5a07..bc489235bc8e26 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.util.AbruptExitException; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; @@ -47,6 +48,7 @@ public class BazelLockFileModule extends BlazeModule { @Nullable private BazelModuleResolutionEvent moduleResolutionEvent; private final Map extensionResolutionEventsMap = new HashMap<>(); + private FileSystem runtimeFileSystem; private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); @@ -57,6 +59,7 @@ public void beforeCommand(CommandEnvironment env) { if (options.lockfileMode.equals(LockfileMode.UPDATE)) { env.getEventBus().register(this); } + runtimeFileSystem = env.getRuntime().getFileSystem(); } @Override @@ -90,7 +93,7 @@ public void afterCommand() throws AbruptExitException { // on the next build, which breaks commands such as `bazel config` that rely on // com.google.devtools.build.skyframe.MemoizingEvaluator#getDoneValues. if (!newLockfile.equals(oldLockfile)) { - updateLockfile(workspaceRoot, newLockfile); + updateLockfile(workspaceRoot, newLockfile, runtimeFileSystem); } this.moduleResolutionEvent = null; this.extensionResolutionEventsMap.clear(); @@ -190,7 +193,8 @@ private boolean shouldKeepExtension( * @param workspaceRoot Root of the workspace where the lockfile is located * @param updatedLockfile The updated lockfile data to save */ - public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updatedLockfile) { + public static void updateLockfile( + Path workspaceRoot, BazelLockFileValue updatedLockfile, FileSystem runtimeFileSystem) { RootedPath lockfilePath = RootedPath.toRootedPath(Root.fromPath(workspaceRoot), LabelConstants.MODULE_LOCKFILE_NAME); try { @@ -202,7 +206,8 @@ public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updated .asPath() .getParentDirectory() .getRelative(LabelConstants.MODULE_DOT_BAZEL_FILE_NAME), - workspaceRoot) + workspaceRoot, + runtimeFileSystem) .toJson(updatedLockfile) + "\n"); } catch (IOException e) { 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 e8563e8f601900..3ae71c57c66135 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 = 3; + public static final int LOCK_FILE_VERSION = 4; @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..e7fc5e7f309a84 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,8 @@ 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.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -444,7 +446,24 @@ public Location read(JsonReader jsonReader) throws IOException { } } - public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { + private static TypeAdapter makeRepoRecordedInputFileTypeAdapter( + FileSystem runtimeFileSystem) { + return 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(), runtimeFileSystem); + } + }; + } + + public static Gson createLockFileGson( + Path moduleFilePath, Path workspaceRoot, FileSystem runtimeFileSystem) { return new GsonBuilder() .setPrettyPrinting() .disableHtmlEscaping() @@ -468,6 +487,8 @@ 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, makeRepoRecordedInputFileTypeAdapter(runtimeFileSystem)) .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 fea4715f7de5e8..eefa813ef7642c 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,7 @@ public static Builder builder() { @SuppressWarnings("mutable") public abstract byte[] getBzlTransitiveDigest(); - public abstract ImmutableMap getAccumulatedFileDigests(); + public abstract ImmutableMap getRecordedFileInputs(); public abstract ImmutableMap getEnvVariables(); @@ -60,7 +60,8 @@ 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 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 491427a6d3263a..c27cc481f89402 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,6 +64,7 @@ protected ModuleExtensionContext( boolean rootModuleHasNonDevDependency) { super( workingDirectory, + directories, env, envVariables, downloadManager, 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 45abfdef90db99..ce45db6b46cc84 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 @@ -30,7 +30,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; @@ -49,6 +48,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; @@ -62,7 +62,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; @@ -72,7 +71,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; @@ -195,7 +193,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) extension.getEvalFactors(), LockFileModuleExtension.builder() .setBzlTransitiveDigest(extension.getBzlTransitiveDigest()) - .setAccumulatedFileDigests(moduleExtensionResult.getAccumulatedFileDigests()) + .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs()) .setEnvVariables(extension.getEnvVars()) .setGeneratedRepoSpecs(generatedRepoSpecs) .setModuleExtensionMetadata(moduleExtensionMetadata) @@ -284,7 +282,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( + extensionId + "' have changed"); } - if (didFilesChange(env, lockedExtension.getAccumulatedFileDigests())) { + if (didFilesChange(env, lockedExtension.getRecordedFileInputs())) { diffRecorder.record( "One or more files the extension '" + extensionId + "' is using have changed"); } @@ -386,39 +384,13 @@ private static boolean didRepoMappingsChange( } private static boolean didFilesChange( - Environment env, ImmutableMap accumulatedFileDigests) + Environment env, ImmutableMap recordedFileInputs) 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, recordedFileInputs); 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; } /** @@ -951,7 +923,7 @@ public RunModuleExtensionResult run( } } return RunModuleExtensionResult.create( - moduleContext.getAccumulatedFileDigests(), + moduleContext.getRecordedFileInputs(), threadContext.getGeneratedRepoSpecs(), moduleExtensionMetadata, repoMappingRecorder.recordedEntries()); @@ -987,6 +959,7 @@ private ModuleExtensionContext createContext( rootUsage != null && rootUsage.getHasNonDevUseExtension(); return new ModuleExtensionContext( workingDirectory, + directories, env, clientEnvironmentSupplier.get(), downloadManager, @@ -1011,7 +984,7 @@ static final class SingleExtensionEvalFunctionException extends SkyFunctionExcep @AutoValue abstract static class RunModuleExtensionResult { - abstract ImmutableMap getAccumulatedFileDigests(); + abstract ImmutableMap getRecordedFileInputs(); abstract ImmutableMap getGeneratedRepoSpecs(); @@ -1020,12 +993,12 @@ abstract static class RunModuleExtensionResult { abstract ImmutableTable getRecordedRepoMappingEntries(); static RunModuleExtensionResult create( - ImmutableMap accumulatedFileDigests, + ImmutableMap recordedFileInputs, ImmutableMap generatedRepoSpecs, Optional moduleExtensionMetadata, ImmutableTable recordedRepoMappingEntries) { return new AutoValue_SingleExtensionEvalFunction_RunModuleExtensionResult( - accumulatedFileDigests, + recordedFileInputs, 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/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 4ce0fabc14a265..5811960f84619b 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,6 +35,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/repository:repository_events", "//src/main/java/com/google/devtools/build/lib/repository:request_repository_information_event", + "//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/rules:repository/workspace_attribute_mapper", 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 bdbdb08a2e5ffb..7084bfc5349235 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,6 +50,9 @@ 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.FileInsideWorkspace; +import com.google.devtools.build.lib.rules.repository.RepoRecordedInput.FileOutsideWorkspace; 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; @@ -54,15 +60,16 @@ import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult; import com.google.devtools.build.lib.skyframe.ActionEnvironmentFunction; import com.google.devtools.build.lib.util.OsUtils; +import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.OutErr; 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.Root; import com.google.devtools.build.lib.vfs.RootedPath; 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 +140,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,13 +148,14 @@ 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 HashSet accumulatedEnvKeys = new HashSet<>(); private final RepositoryRemoteExecutor remoteExecutor; private final List asyncTasks; protected StarlarkBaseExternalContext( Path workingDirectory, + BlazeDirectories directories, Environment env, Map envVariables, DownloadManager downloadManager, @@ -155,6 +164,7 @@ protected StarlarkBaseExternalContext( StarlarkSemantics starlarkSemantics, @Nullable RepositoryRemoteExecutor remoteExecutor) { this.workingDirectory = workingDirectory; + this.directories = directories; this.env = env; this.envVariables = ImmutableMap.copyOf(envVariables); this.osObject = new StarlarkOS(this.envVariables); @@ -194,8 +204,8 @@ 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); } /** Returns set of environment variable keys encountered so far. */ @@ -1131,17 +1141,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(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"); } } @@ -1150,22 +1157,29 @@ 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 = "True", + doc = "whether to watch the file.") }) - public String readFile(Object path, StarlarkThread thread) + public String readFile(Object path, boolean 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); + if (watch) { + watch(p); + } try { return FileSystemUtils.readContent(p.getPath(), ISO_8859_1); } catch (IOException e) { @@ -1173,6 +1187,84 @@ public String readFile(Object path, StarlarkThread thread) } } + @Nullable + protected Pair toRootedPath(Path path) { + if (path.startsWith(workingDirectory)) { + // The file is under the working directory. Don't watch it, as it would cause a dependency + // cycle. + return null; + } + if (path.startsWith(directories.getWorkspace())) { + // The file is under the workspace root. + PathFragment relPath = path.relativeTo(directories.getWorkspace()); + return Pair.of( + new FileInsideWorkspace(RepositoryName.MAIN, relPath), + RootedPath.toRootedPath(Root.fromPath(directories.getWorkspace()), 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 Pair.of( + new FileInsideWorkspace(RepositoryName.createUnvalidated(repoName), repoRelPath), + RootedPath.toRootedPath( + Root.fromPath(outputBaseExternal.getChild(repoName)), repoRelPath)); + } + } + // The file is just under a random absolute path. + RootedPath rootedPath = RootedPath.toRootedPath(Root.absoluteRoot(path.getFileSystem()), path); + return Pair.of(new FileOutsideWorkspace(rootedPath), rootedPath); + } + + protected void watch(StarlarkPath starlarkPath) + throws EvalException, RepositoryFunctionException, InterruptedException { + Pair pair = toRootedPath(starlarkPath.getPath()); + if (pair == null) { + return; + } + try { + FileValue fileValue = + (FileValue) env.getValueOrThrow(FileValue.key(pair.getSecond()), IOException.class); + if (fileValue == null) { + throw new NeedsSkyframeRestartException(); + } + if (!fileValue.isFile() || fileValue.isSpecialFile()) { + throw Starlark.errorf("Not a regular file: %s", pair.getSecond().asPath().getPathString()); + } + + recordedFileInputs.put( + pair.getFirst(), RepoRecordedInput.File.fileValueToMarkerValue(fileValue)); + } catch (IOException e) { + throw new RepositoryFunctionException(e, Transience.TRANSIENT); + } + } + + @StarlarkMethod( + name = "watch", + doc = + "Tells Bazel to watch for changes to the given file. Any changes to the file will " + + "invalidate this repository or module extension, and cause it to be refetched or " + + "re-evaluated next time.", + 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 { + watch(getPath("watch()", path)); + } + // Create parent directories for the given path protected static void makeDirectories(Path path) throws IOException { Path parent = path.getParentDirectory(); @@ -1530,26 +1622,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(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()); + watch(starlarkPath); + } 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/StarlarkRepositoryContext.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContext.java index 246228875f8788..afebbd66d076ad 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; @@ -74,7 +75,6 @@ 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; @@ -96,10 +96,11 @@ public class StarlarkRepositoryContext extends StarlarkBaseExternalContext { StarlarkSemantics starlarkSemantics, @Nullable RepositoryRemoteExecutor remoteExecutor, SyscallCache syscallCache, - Path workspaceRoot) + BlazeDirectories directories) throws EvalException { super( outputDirectory, + directories, environment, env, downloadManager, @@ -112,7 +113,6 @@ public class StarlarkRepositoryContext extends StarlarkBaseExternalContext { 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()) { @@ -143,7 +143,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(directories.getWorkspace()); } @StarlarkMethod( 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 b460b1e4e35d70..4f30919a630bb2 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 @@ -246,7 +246,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 @@ -303,10 +303,7 @@ private RepositoryDirectoryValue.Builder fetchInternal( } // Modify marker data to include the files used by the rule's implementation function. - for (Map.Entry entry : - starlarkRepositoryContext.getAccumulatedFileDigests().entrySet()) { - recordedInputValues.put(new RepoRecordedInput.LabelFile(entry.getKey()), entry.getValue()); - } + recordedInputValues.putAll(starlarkRepositoryContext.getRecordedFileInputs()); // Ditto for environment variables accessed via `getenv`. for (String envKey : starlarkRepositoryContext.getAccumulatedEnvKeys()) { 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 d82dc2c7e4878a..cb55df62f1859f 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,27 @@ 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/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:precomputed_value", + "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value", + "//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:guava", + "//third_party:jsr305", + ], +) + java_library( name = "repository/repository_directory_value", srcs = ["repository/RepositoryDirectoryValue.java"], @@ -382,7 +405,6 @@ java_library( srcs = [ "repository/LocalRepositoryFunction.java", "repository/NeedsSkyframeRestartException.java", - "repository/RepoRecordedInput.java", "repository/RepositoryDelegatorFunction.java", "repository/RepositoryDirectoryDirtinessChecker.java", "repository/RepositoryFunction.java", @@ -391,6 +413,7 @@ java_library( ], deps = [ ":repository/local_repository_rule", + ":repository/repo_recorded_input", ":repository/repository_directory_value", ":repository/resolved_file_value", ":repository/workspace_attribute_mapper", 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 8f2100ae4a6a73..4eee9852198cc8 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 @@ -147,9 +147,11 @@ public void finishFile( // Link x/FILENAME to /x.FILENAME. symlinkFile(fileValue, filename, outputDirectory); try { + Label label = getFileAttributeAsLabel(rule); recordedInputValues.put( - new RepoRecordedInput.LabelFile(getFileAttributeAsLabel(rule)), - RepositoryFunction.fileValueToMarkerValue(fileValue)); + new RepoRecordedInput.FileInsideWorkspace( + 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 index 512335744419e0..b838afcbdecc41 100644 --- 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 @@ -14,17 +14,20 @@ package com.google.devtools.build.lib.rules.repository; +import static com.google.common.collect.ImmutableSet.toImmutableSet; + +import com.google.common.base.Preconditions; import com.google.common.base.Splitter; +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.cmdline.Label; 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.PackageLookupValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; -import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; @@ -33,6 +36,7 @@ import java.io.IOException; import java.util.Comparator; import java.util.List; +import java.util.Map; import java.util.Objects; import javax.annotation.Nullable; @@ -65,7 +69,7 @@ public abstract static class Parser { * 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, Path baseDirectory); + public abstract RepoRecordedInput parse(String s, FileSystem fileSystem); } private static final Comparator COMPARATOR = @@ -76,22 +80,52 @@ public abstract static class Parser { * Parses a recorded input from its string representation. * * @param s the string representation - * @param baseDirectory the path to a base directory that any filesystem paths should be resolved - * relative to + * @param runtimeFileSystem the file system that any paths should be assumed to be in * @return The parsed recorded input object, or {@code null} if the string representation is * invalid */ @Nullable - public static RepoRecordedInput parse(String s, Path baseDirectory) { + public static RepoRecordedInput parse(String s, FileSystem runtimeFileSystem) { List parts = Splitter.on(':').limit(2).splitToList(s); for (Parser parser : new Parser[] {File.PARSER, EnvVar.PARSER, RecordedRepoMapping.PARSER}) { if (parts.get(0).equals(parser.getPrefix())) { - return parser.parse(parts.get(1), baseDirectory); + return parser.parse(parts.get(1), runtimeFileSystem); } } 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, Map recordedInputValues) + throws InterruptedException { + ImmutableSet skyKeys = + recordedInputValues.keySet().stream() + .map(RepoRecordedInput::getSkyKey) + .collect(toImmutableSet()); + env.getValuesAndExceptions(skyKeys); + if (env.valuesMissing()) { + return false; + } + for (Map.Entry recordedInputValue : + recordedInputValues.entrySet()) { + if (!recordedInputValue.getKey().isUpToDate(env, 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(); @@ -106,7 +140,7 @@ public int compareTo(RepoRecordedInput 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}. */ - abstract String toStringInternal(); + public abstract String toStringInternal(); /** Returns the parser object for this type of recorded inputs. */ public abstract Parser getParser(); @@ -125,7 +159,7 @@ public abstract boolean isUpToDate(Environment env, @Nullable String oldValue) /** Represents a file input accessed during the repo fetch. */ public abstract static class File extends RepoRecordedInput { - static final Parser PARSER = + public static final Parser PARSER = new Parser() { @Override public String getPrefix() { @@ -133,15 +167,17 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s, FileSystem fileSystem) { if (LabelValidator.isAbsolute(s)) { - return new LabelFile(Label.parseCanonicalUnchecked(s)); + int doubleSlash = s.indexOf("//"); + int skipAts = s.startsWith("@@") ? 2 : s.startsWith("@") ? 1 : 0; + return new FileInsideWorkspace( + RepositoryName.createUnvalidated(s.substring(skipAts, doubleSlash)), + // For backwards compatibility, treat colons as slashes. + PathFragment.create(s.substring(doubleSlash + 2).replace(':', '/'))); } - Path path = baseDirectory.getRelative(s); - return new AbsolutePathFile( - RootedPath.toRootedPath( - Root.fromPath(path.getParentDirectory()), - PathFragment.create(path.getBaseName()))); + return new FileOutsideWorkspace( + RootedPath.toRootedPath(Root.absoluteRoot(fileSystem), PathFragment.create(s))); } }; @@ -149,42 +185,85 @@ public RepoRecordedInput parse(String s, Path baseDirectory) { public Parser getParser() { return PARSER; } + + /** + * 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); + } } - /** Represents a file input accessed during the repo fetch that is addressable by a label. */ - public static final class LabelFile extends File { - final Label label; + /** + * Represents a file input accessed during the repo fetch that is within the current Bazel + * workspace. + * + *

This is almost like being addressable by 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. + */ + public static final class FileInsideWorkspace extends File { + private final RepositoryName repoName; + private final PathFragment pathFragment; + + public FileInsideWorkspace(RepositoryName repoName, PathFragment pathFragment) { + this.repoName = repoName; + this.pathFragment = pathFragment; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof FileInsideWorkspace)) return false; + FileInsideWorkspace that = (FileInsideWorkspace) o; + return Objects.equals(repoName, that.repoName) + && Objects.equals(pathFragment, that.pathFragment); + } - public LabelFile(Label label) { - this.label = label; + @Override + public int hashCode() { + return Objects.hash(repoName, pathFragment); } @Override - String toStringInternal() { - return label.toString(); + public String toStringInternal() { + // We store `@@foo//abc/def:ghi.bzl` as just `@@foo//abc/def/ghi.bzl`. + return repoName + "//" + pathFragment; } @Override public SkyKey getSkyKey() { - return PackageLookupValue.key(label.getPackageIdentifier()); + return RepositoryDirectoryValue.key(repoName); } @Override public boolean isUpToDate(Environment env, @Nullable String oldValue) throws InterruptedException { - PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(getSkyKey()); - if (pkgLookupValue == null || !pkgLookupValue.packageExists()) { + RepositoryDirectoryValue repoDirValue = (RepositoryDirectoryValue) env.getValue(getSkyKey()); + if (repoDirValue == null || !repoDirValue.repositoryExists()) { return false; } RootedPath rootedPath = - RootedPath.toRootedPath(pkgLookupValue.getRoot(), label.toPathFragment()); + RootedPath.toRootedPath(Root.fromPath(repoDirValue.getPath()), pathFragment); SkyKey fileKey = FileValue.key(rootedPath); try { FileValue fileValue = (FileValue) env.getValueOrThrow(fileKey, IOException.class); if (fileValue == null || !fileValue.isFile() || fileValue.isSpecialFile()) { return false; } - return oldValue.equals(RepositoryFunction.fileValueToMarkerValue(fileValue)); + return oldValue.equals(fileValueToMarkerValue(fileValue)); } catch (IOException e) { return false; } @@ -192,18 +271,31 @@ public boolean isUpToDate(Environment env, @Nullable String oldValue) } /** - * Represents a file input accessed during the repo fetch that is not addressable by a - * label. This most likely means that it's outside any known Bazel workspace. + * Represents a file input accessed during the repo fetch that is outside the current Bazel + * workspace. This file is addressed by its absolute path. */ - public static final class AbsolutePathFile extends File { + public static final class FileOutsideWorkspace extends File { final RootedPath path; - public AbsolutePathFile(RootedPath path) { + public FileOutsideWorkspace(RootedPath path) { this.path = path; } @Override - String toStringInternal() { + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof FileOutsideWorkspace)) return false; + FileOutsideWorkspace that = (FileOutsideWorkspace) o; + return Objects.equals(path, that.path); + } + + @Override + public int hashCode() { + return Objects.hash(path); + } + + @Override + public String toStringInternal() { return path.asPath().getPathString(); } @@ -220,7 +312,7 @@ public boolean isUpToDate(Environment env, @Nullable String oldValue) if (fileValue == null || !fileValue.isFile() || fileValue.isSpecialFile()) { return false; } - return oldValue.equals(RepositoryFunction.fileValueToMarkerValue(fileValue)); + return oldValue.equals(fileValueToMarkerValue(fileValue)); } catch (IOException e) { return false; } @@ -237,7 +329,7 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s, FileSystem fileSystem) { return new EnvVar(s); } }; @@ -248,13 +340,26 @@ 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 Objects.hash(name); + } + @Override public Parser getParser() { return PARSER; } @Override - String toStringInternal() { + public String toStringInternal() { return name; } @@ -285,7 +390,7 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s, FileSystem fileSystem) { List parts = Splitter.on(',').limit(2).splitToList(s); return new RecordedRepoMapping( RepositoryName.createUnvalidated(parts.get(0)), parts.get(1)); @@ -300,13 +405,27 @@ public RecordedRepoMapping(RepositoryName sourceRepo, String apparentName) { 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 - String toStringInternal() { + public String toStringInternal() { return sourceRepo.getName() + ',' + 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 3839956b709e64..8ddbe0d76203a5 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 @@ -45,6 +45,7 @@ import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed; import com.google.devtools.build.lib.util.Fingerprint; +import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -93,7 +94,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 = 5; // Mapping of rule class name to RepositoryFunction. private final ImmutableMap handlers; @@ -594,6 +595,7 @@ private static class DigestWriter { // not just Map<> to signal that iteration order must be deterministic private final TreeMap recordedInputValues; private final String ruleKey; + private final FileSystem runtimeFileSystem; DigestWriter( BlazeDirectories directories, @@ -604,6 +606,7 @@ private static class DigestWriter { markerPath = getMarkerPath(directories, repositoryName.getName()); this.rule = rule; recordedInputValues = Maps.newTreeMap(); + runtimeFileSystem = directories.getOutputBase().getFileSystem(); } byte[] writeMarkerFile() throws RepositoryFunctionException { @@ -648,12 +651,11 @@ byte[] areRepositoryAndMarkerFileConsistent( return null; } - Path baseDirectory = rule.getPackage().getPackageDirectory(); Map recordedInputValues = new TreeMap<>(); String content; try { content = FileSystemUtils.readContent(markerPath, UTF_8); - String markerRuleKey = readMarkerFile(content, baseDirectory, recordedInputValues); + String markerRuleKey = readMarkerFile(content, runtimeFileSystem, recordedInputValues); boolean verified = false; if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) { verified = handler.verifyRecordedInputs(rule, recordedInputValues, env); @@ -678,7 +680,7 @@ Map getRecordedInputValues() { @Nullable private static String readMarkerFile( - String content, Path baseDirectory, Map recordedInputValues) { + String content, FileSystem fileSystem, Map recordedInputValues) { String markerRuleKey = null; Iterable lines = Splitter.on('\n').split(content); @@ -691,7 +693,7 @@ private static String readMarkerFile( int sChar = line.indexOf(' '); if (sChar > 0) { RepoRecordedInput input = - RepoRecordedInput.parse(unescape(line.substring(0, sChar)), baseDirectory); + RepoRecordedInput.parse(unescape(line.substring(0, sChar)), fileSystem); if (input == null) { // ignore invalid entries. continue; 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 9aa7f860412b9e..166f05e2c4e72c 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,12 +15,10 @@ 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 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; @@ -193,36 +191,7 @@ public abstract RepositoryDirectoryValue.Builder fetch( public boolean verifyRecordedInputs( Rule rule, Map recordedInputValues, Environment env) throws InterruptedException { - ImmutableSet skyKeys = - recordedInputValues.keySet().stream() - .map(RepoRecordedInput::getSkyKey) - .collect(toImmutableSet()); - env.getValuesAndExceptions(skyKeys); - if (env.valuesMissing()) { - return false; - } - for (Map.Entry recordedInputValue : recordedInputValues.entrySet()) { - if (!recordedInputValue.getKey().isUpToDate(env, recordedInputValue.getValue())) { - return false; - } - } - return true; - } - /** - * 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); + return RepoRecordedInput.areAllValuesUpToDate(env, recordedInputValues); } public static RootedPath getRootedPathFromLabel(Label label, Environment env) diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index e8c92a94f653f8..64cbad497d456b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -170,7 +170,10 @@ public ImmutableMap getSkyFunctions(BlazeDirectori directories.getWorkspace(), getBuiltinModules(directories))) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(directories.getWorkspace())) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction( + directories.getWorkspace(), directories.getOutputBase().getFileSystem())) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .put( SkyFunctions.SINGLE_EXTENSION_EVAL, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java index 31aacbcbe3da8d..9b049a0edf56ac 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java @@ -126,7 +126,9 @@ public void setup() throws Exception { SkyFunctions.MODULE_FILE, new ModuleFileFunction(registryFactory, rootDirectory, ImmutableMap.of())) .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, resolutionFunctionMock) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction(registryFactory)) diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java index d3f3a9107c8c7a..cb083b92463bfb 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunctionTest.java @@ -154,7 +154,9 @@ public void setup() throws Exception { .put( SkyFunctions.MODULE_FILE, new ModuleFileFunction(registryFactory, rootDirectory, ImmutableMap.of())) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction(registryFactory)) .put( SkyFunctions.MODULE_EXTENSION_REPO_MAPPING_ENTRIES, @@ -189,7 +191,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) .setFlags(flags) .setLocalOverrideHashes(localOverrideHashes) .setModuleDepGraph(key.depGraph()) - .build()); + .build(), + fileSystem); return new SkyValue() {}; } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java index 63fbbd4e09656d..944b81cd5fcb44 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunctionTest.java @@ -113,7 +113,9 @@ public void setup() throws Exception { new ModuleFileFunction(registryFactory, rootDirectory, ImmutableMap.of())) .put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction()) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .put(SkyFunctions.REPO_SPEC, new RepoSpecFunction(registryFactory)) .put( diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java index 010b56d916b593..dffbffa714a6eb 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleFunctionTest.java @@ -125,7 +125,9 @@ public void setup() throws Exception { BzlmodRepoRuleValue.BZLMOD_REPO_RULE, new BzlmodRepoRuleFunction(ruleClassProvider, directories)) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .put( SkyFunctions.MODULE_FILE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index c2916c1efcf8a6..8b1d873cca272d 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -265,7 +265,9 @@ public void setup() throws Exception { .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, new BzlmodRepoRuleFunction(ruleClassProvider, directories)) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .put(SkyFunctions.SINGLE_EXTENSION_USAGES, new SingleExtensionUsagesFunction()) 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..5a4dae694ace81 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", @@ -69,6 +70,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", 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 655e21734d7c40..81bcbf43735667 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; @@ -82,6 +85,7 @@ public final class StarlarkRepositoryContextTest { private Scratch scratch; + private Path outputBase; private Path outputDirectory; private Root root; private Path workspaceFile; @@ -94,6 +98,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"); @@ -159,6 +164,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, @@ -169,11 +180,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 { @@ -304,7 +315,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"), true, thread); assertThat(content).isEqualTo("foobar"); } 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/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 1d387cf05abd00..787bf07903fcac 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -237,7 +237,9 @@ public void setupDelegator() throws Exception { SkyFunctions.MODULE_FILE, new ModuleFileFunction(registryFactory, rootPath, ImmutableMap.of())) .put(SkyFunctions.BAZEL_DEP_GRAPH, new BazelDepGraphFunction()) - .put(SkyFunctions.BAZEL_LOCK_FILE, new BazelLockFileFunction(rootDirectory)) + .put( + SkyFunctions.BAZEL_LOCK_FILE, + new BazelLockFileFunction(rootDirectory, fileSystem)) .put(SkyFunctions.BAZEL_MODULE_RESOLUTION, new BazelModuleResolutionFunction()) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, 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 9a7f175d633e10..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 @@ -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/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh index 12170039ec80a1..13c62f43f7c756 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2704,4 +2704,108 @@ EOF expect_log "I see: @@bar~override//:data" } +function test_file_watching_inside_working_dir() { + # when reading a file inside the working directory (where the repo + # is to be fetched), we shouldn't watch it. + 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, 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_outside_workspace() { + # when reading a file outside the Bazel workspace, 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 @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" +} + run_suite "local repository tests" diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index dcf40eaf6abf1d..7a81e961dc178a 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1,5 +1,5 @@ { - "lockFileVersion": 3, + "lockFileVersion": 4, "moduleFileHash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "flags": { "cmdRegistries": [ @@ -1037,7 +1037,7 @@ "@@apple_support~1.5.0//crosstool:setup.bzl%apple_cc_configure_extension": { "general": { "bzlTransitiveDigest": "pMLFCYaRPkgXPQ8vtuNkMfiHfPmRBy6QJfnid4sWfv0=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_apple_cc": { @@ -1063,7 +1063,7 @@ "@@bazel_tools//tools/android:android_extensions.bzl%remote_android_tools_extensions": { "general": { "bzlTransitiveDigest": "vsrPPBNf8OgywAYLMcIL1oNm2R8WtbCIL9wgQBUecpA=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "android_tools": { @@ -1089,7 +1089,7 @@ "@@bazel_tools//tools/cpp:cc_configure.bzl%cc_configure_extension": { "general": { "bzlTransitiveDigest": "XWy8pzw7/6RclAFWd6/VfUdoXn2SdSpmHOrbfEFJ1ao=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_cc": { @@ -1115,7 +1115,7 @@ "@@bazel_tools//tools/osx:xcode_configure.bzl%xcode_configure_extension": { "general": { "bzlTransitiveDigest": "Qh2bWTU6QW6wkrd87qrU4YeY+SG37Nvw3A0PR4Y0L2Y=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_xcode": { @@ -1133,7 +1133,7 @@ "@@bazel_tools//tools/sh:sh_configure.bzl%sh_configure_extension": { "general": { "bzlTransitiveDigest": "hp4NgmNjEg5+xgvzfh6L83bt9/aiiWETuNpwNuF1MSU=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "local_config_sh": { @@ -1148,7 +1148,7 @@ "@@bazel_tools//tools/test:extensions.bzl%remote_coverage_tools_extension": { "general": { "bzlTransitiveDigest": "AL+K5m+GCP3XRzLPqpKAq4GsjIVDXgUveWm8nih4ju0=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "remote_coverage_tools": { @@ -1168,7 +1168,7 @@ "@@buildozer~6.4.0.2//:buildozer_binary.bzl%buildozer_binary": { "general": { "bzlTransitiveDigest": "EleDU/FQ1+e/RgkW3aIDmdaxZEthvoWQhsqFTxiSgMI=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "buildozer_binary": { @@ -1192,7 +1192,7 @@ "@@rules_java~7.3.2//java:extensions.bzl%toolchains": { "general": { "bzlTransitiveDigest": "Bm4ulErUcJjZtKeAt2etkB6sGO8evCgHQxbw4Px8q60=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "remotejdk21_linux_toolchain_config_repo": { @@ -1696,8 +1696,8 @@ "@@rules_jvm_external~4.4.2//:extensions.bzl%maven": { "general": { "bzlTransitiveDigest": "yXprMX4LqzJwuZlbtT9WNeu7p2iEYw7j4R1NP9pc4Ow=", - "accumulatedFileDigests": { - "@@rules_jvm_external~4.4.2//:rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" + "recordedFileInputs": { + "@@rules_jvm_external~4.4.2//rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" }, "envVariables": {}, "generatedRepoSpecs": { @@ -2718,7 +2718,7 @@ "@@rules_jvm_external~4.4.2//:non-module-deps.bzl%non_module_deps": { "general": { "bzlTransitiveDigest": "Td87llNSs5GZ/kAxu6pAqfEXBZ3HdkSqRmUzvIfbFWg=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "io_bazel_rules_kotlin": { @@ -2744,7 +2744,7 @@ "@@rules_python~0.22.1//python/extensions:python.bzl%python": { "general": { "bzlTransitiveDigest": "NGtTMUqs2EEJeXu6mXdpmYRrQGZiJV7S3mxeod3Hm7M=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pythons_hub": { @@ -2772,7 +2772,7 @@ "@@rules_python~0.22.1//python/extensions/private:internal_deps.bzl%internal_deps": { "general": { "bzlTransitiveDigest": "5c1tkdV6L67SQOZWc9MUoS5ZnsSxeDKsh9urs01jZSM=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pypi__coverage_cp39_aarch64-apple-darwin": {