diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 1b9df002f5e0b5..53651646bea42a 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -2741,7 +2741,7 @@ "general": { "bzlTransitiveDigest": "uzRwJ/aaGLzKqN69Hz+DktJFrVeKCjILtM+t4Hirz0M=", "accumulatedFileDigests": { - "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "e591609d4999da0cac2aad19df3ff8a1e42f3f032fb16308037d0d9e555f369f", + "@@//src/test/tools/bzlmod:MODULE.bazel.lock": "2ac47b971c5b12765dc679ce4137864f67f02f867f43ec22f86ecc203b905a1c", "@@//:MODULE.bazel": "f291782aef1d2989f49c0e884b32ec8d7814ae48c598b6f3060870ccb3f5c0b6" }, "envVariables": {}, diff --git a/compile.sh b/compile.sh index d91583d277cba8..3e95db7f75f902 100755 --- a/compile.sh +++ b/compile.sh @@ -67,6 +67,7 @@ bazel_build "src:bazel_nojdk${EXE_EXT}" \ --action_env=PATH \ --host_platform=@local_config_platform//:host \ --platforms=@local_config_platform//:host \ + --lockfile_mode=error \ || fail "Could not build Bazel" bazel_bin_path="$(get_bazel_bin_path)/src/bazel_nojdk${EXE_EXT}" [ -e "$bazel_bin_path" ] \ 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 6ecf0be408019c..a6300dede5c6f6 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/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index 3ae71c57c66135..0855f0a1aa74e2 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 = 5; @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..2a4d28d960cee4 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,20 @@ 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()); + } + }; + public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { return new GsonBuilder() .setPrettyPrinting() @@ -468,6 +483,7 @@ public static Gson createLockFileGson(Path moduleFilePath, Path workspaceRoot) { .registerTypeAdapter(ModuleExtensionId.IsolationKey.class, ISOLATION_KEY_TYPE_ADAPTER) .registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter()) .registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER) + .registerTypeAdapter(RepoRecordedInput.File.class, REPO_RECORDED_INPUT_FILE_TYPE_ADAPTER) .create(); } 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 88cdd5c2e7888d..1540f30917b5e3 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(); @@ -65,7 +65,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 cdddef1c3a1479..52151259200462 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, + /* allowWatchingFilesOutsideWorkspace= */ 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 f709892010c453..ce86264327c3dc 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,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) @@ -291,7 +289,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( + extensionId + "' have changed"); } - if (didFilesChange(env, lockedExtension.getAccumulatedFileDigests())) { + if (didFilesChange(env, directories, lockedExtension.getRecordedFileInputs())) { diffRecorder.record( "One or more files the extension '" + extensionId + "' is using have changed"); } @@ -393,39 +391,13 @@ private static boolean didRepoMappingsChange( } private static boolean didFilesChange( - Environment env, ImmutableMap accumulatedFileDigests) + Environment env, BlazeDirectories directories, 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, directories, 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; } /** @@ -956,7 +928,7 @@ public RunModuleExtensionResult run( } } return RunModuleExtensionResult.create( - moduleContext.getAccumulatedFileDigests(), + moduleContext.getRecordedFileInputs(), threadContext.getGeneratedRepoSpecs(), moduleExtensionMetadata, repoMappingRecorder.recordedEntries()); @@ -992,6 +964,7 @@ private ModuleExtensionContext createContext( rootUsage != null && rootUsage.getHasNonDevUseExtension(); return new ModuleExtensionContext( workingDirectory, + directories, env, clientEnvironmentSupplier.get(), downloadManager, @@ -1016,7 +989,7 @@ static final class SingleExtensionEvalFunctionException extends SkyFunctionExcep @AutoValue abstract static class RunModuleExtensionResult { - abstract ImmutableMap getAccumulatedFileDigests(); + abstract ImmutableMap getRecordedFileInputs(); abstract ImmutableMap getGeneratedRepoSpecs(); @@ -1025,12 +998,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..aaedebc5d51452 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,21 +148,25 @@ 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; + private final boolean allowWatchingFilesOutsideWorkspace; 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 allowWatchingFilesOutsideWorkspace) { 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.allowWatchingFilesOutsideWorkspace = allowWatchingFilesOutsideWorkspace; } public boolean ensureNoPendingAsyncTasks(EventHandler eventHandler, boolean forSuccessfulFetch) { @@ -194,8 +207,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 +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(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 +1160,35 @@ 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)); try { return FileSystemUtils.readContent(p.getPath(), ISO_8859_1); } catch (IOException e) { @@ -1173,6 +1196,116 @@ public String readFile(Object path, StarlarkThread thread) } } + protected Pair toRootedPath(Path path) { + 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. + return Pair.of( + new FileOutsideWorkspace(path.asFragment()), + RootedPath.toRootedPath(Root.absoluteRoot(path.getFileSystem()), path)); + } + + 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; + } + 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 { + if (shouldWatch == ShouldWatch.NO) { + return; + } + Path path = starlarkPath.getPath(); + Pair pair = toRootedPath(path); + if (path.startsWith(workingDirectory)) { + // The file is under the working directory. Don't watch it, as it would cause a dependency + // cycle. + if (shouldWatch == ShouldWatch.AUTO) { + return; + } + throw Starlark.errorf("attempted to watch file under working directory"); + } + if (!allowWatchingFilesOutsideWorkspace && pair.first instanceof FileOutsideWorkspace) { + if (shouldWatch == ShouldWatch.AUTO) { + return; + } + throw Starlark.errorf( + "attempted to watch file outside workspace, but it's prohibited in the current context"); + } + try { + FileValue fileValue = + (FileValue) env.getValueOrThrow(FileValue.key(pair.second), IOException.class); + if (fileValue == null) { + throw new NeedsSkyframeRestartException(); + } + if (!fileValue.isFile() || fileValue.isSpecialFile()) { + throw Starlark.errorf("Not a regular file: %s", pair.second.asPath().getPathString()); + } + + recordedFileInputs.put(pair.first, RepoRecordedInput.File.fileValueToMarkerValue(fileValue)); + } catch (IOException e) { + 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.

Note that attempting to watch files inside the repo " + + "currently being fetched, or inside the working directory of the current module " + + "extension, will result in an error. A module extension attempting to watch a file " + + "outside the current Bazel workspace will also result in an error.", + 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(); @@ -1530,26 +1663,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()); + 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..8f0193410bbb4d 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 @@ -41,7 +41,7 @@ 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 Path path; StarlarkPath(Path path) { 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..8e051c7981f0dd 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,23 +96,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, + /* allowWatchingFilesOutsideWorkspace= */ 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()) { @@ -143,7 +144,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 70dcb78cfa75b2..1800444e73b385 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 @@ -253,7 +253,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 @@ -310,10 +310,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/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java index f0d939bee3939f..8526baeb6bbea9 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 @@ -250,13 +250,16 @@ public boolean isLocal(Rule rule) { @Override public boolean verifyRecordedInputs( - Rule rule, Map recordedInputValues, Environment env) + Rule rule, + BlazeDirectories directories, + Map recordedInputValues, + Environment env) throws InterruptedException { WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule); if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyRecordedInputs(rule, recordedInputValues, env); + return super.verifyRecordedInputs(rule, directories, recordedInputValues, env); } @Override 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 7862cc453c76e5..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 @@ -179,13 +179,16 @@ public boolean isLocal(Rule rule) { @Override public boolean verifyRecordedInputs( - Rule rule, Map recordedInputValues, Environment env) + Rule rule, + BlazeDirectories directories, + Map recordedInputValues, + Environment env) throws InterruptedException { WorkspaceAttributeMapper attributes = WorkspaceAttributeMapper.of(rule); if (attributes.isAttributeValueExplicitlySpecified("path")) { return true; } - return super.verifyRecordedInputs(rule, recordedInputValues, env); + return super.verifyRecordedInputs(rule, directories, recordedInputValues, env); } @Override 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..9008af56c56a3e 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,28 @@ 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: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 +406,6 @@ java_library( srcs = [ "repository/LocalRepositoryFunction.java", "repository/NeedsSkyframeRestartException.java", - "repository/RepoRecordedInput.java", "repository/RepositoryDelegatorFunction.java", "repository/RepositoryDirectoryDirtinessChecker.java", "repository/RepositoryFunction.java", @@ -391,6 +414,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..4a404b36e16f01 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,19 @@ 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.io.BaseEncoding; import com.google.devtools.build.lib.actions.FileValue; -import com.google.devtools.build.lib.cmdline.Label; +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.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.PathFragment; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; @@ -33,6 +35,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 +68,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); } private static final Comparator COMPARATOR = @@ -76,22 +79,55 @@ 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 * @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) { 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)); } } 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(rri -> rri.getSkyKey(directories)) + .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(); @@ -106,26 +142,31 @@ 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(); - /** Returns the {@link SkyKey} that is necessary to determine {@link #isUpToDate}. */ - public abstract SkyKey getSkyKey(); + /** + * Returns the {@link SkyKey} that is necessary to determine {@link #isUpToDate}. Can be null if + * no SkyKey is needed. + */ + @Nullable + public abstract SkyKey getSkyKey(BlazeDirectories directories); /** * 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. + * method can assume that {@link #getSkyKey(BlazeDirectories)} 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, @Nullable String oldValue) + public abstract boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException; /** 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 +174,16 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s) { 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(PathFragment.create(s)); } }; @@ -149,42 +191,94 @@ 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; + } - public LabelFile(Label label) { - this.label = label; + @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); } @Override - String toStringInternal() { - return label.toString(); + public int hashCode() { + return Objects.hash(repoName, pathFragment); } @Override - public SkyKey getSkyKey() { - return PackageLookupValue.key(label.getPackageIdentifier()); + public String toStringInternal() { + // We store `@@foo//abc/def:ghi.bzl` as just `@@foo//abc/def/ghi.bzl`. See class javadoc for + // more context. + return repoName + "//" + pathFragment; } @Override - public boolean isUpToDate(Environment env, @Nullable String oldValue) + @Nullable + public SkyKey getSkyKey(BlazeDirectories directories) { + return repoName.isMain() ? null : RepositoryDirectoryValue.key(repoName); + } + + @Override + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { - PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(getSkyKey()); - if (pkgLookupValue == null || !pkgLookupValue.packageExists()) { - return false; + Root root; + if (repoName.isMain()) { + root = Root.fromPath(directories.getWorkspace()); + } else { + RepositoryDirectoryValue repoDirValue = + (RepositoryDirectoryValue) env.getValue(getSkyKey(directories)); + if (repoDirValue == null || !repoDirValue.repositoryExists()) { + return false; + } + root = Root.fromPath(repoDirValue.getPath()); } - RootedPath rootedPath = - RootedPath.toRootedPath(pkgLookupValue.getRoot(), label.toPathFragment()); + RootedPath rootedPath = RootedPath.toRootedPath(root, 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,35 +286,53 @@ 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 { - final RootedPath path; + public static final class FileOutsideWorkspace extends File { + private final PathFragment path; - public AbsolutePathFile(RootedPath path) { + public FileOutsideWorkspace(PathFragment path) { + Preconditions.checkArgument(path.isAbsolute()); this.path = path; } @Override - String toStringInternal() { - return path.asPath().getPathString(); + 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.getPathString(); } @Override - public SkyKey getSkyKey() { - return FileValue.key(path); + public SkyKey getSkyKey(BlazeDirectories directories) { + return FileValue.key( + RootedPath.toRootedPath( + Root.absoluteRoot(directories.getOutputBase().getFileSystem()), path)); } @Override - public boolean isUpToDate(Environment env, @Nullable String oldValue) + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { try { - FileValue fileValue = (FileValue) env.getValueOrThrow(getSkyKey(), IOException.class); + FileValue fileValue = + (FileValue) env.getValueOrThrow(getSkyKey(directories), 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; } @@ -237,7 +349,7 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s) { return new EnvVar(s); } }; @@ -248,27 +360,41 @@ 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; } @Override - public SkyKey getSkyKey() { + public SkyKey getSkyKey(BlazeDirectories directories) { return ActionEnvironmentFunction.key(name); } @Override - public boolean isUpToDate(Environment env, @Nullable String oldValue) + 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(); + v = ((ClientEnvironmentValue) env.getValue(getSkyKey(directories))).getValue(); } // Note that `oldValue` can be null if the env var was not set. return Objects.equals(oldValue, v); @@ -285,7 +411,7 @@ public String getPrefix() { } @Override - public RepoRecordedInput parse(String s, Path baseDirectory) { + public RepoRecordedInput parse(String s) { List parts = Splitter.on(',').limit(2).splitToList(s); return new RecordedRepoMapping( RepositoryName.createUnvalidated(parts.get(0)), parts.get(1)); @@ -300,25 +426,41 @@ 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; } @Override - public SkyKey getSkyKey() { + public SkyKey getSkyKey(BlazeDirectories directories) { return RepositoryMappingValue.key(sourceRepo); } @Override - public boolean isUpToDate(Environment env, @Nullable String oldValue) + public boolean isUpToDate( + Environment env, BlazeDirectories directories, @Nullable String oldValue) throws InterruptedException { - RepositoryMappingValue repoMappingValue = (RepositoryMappingValue) env.getValue(getSkyKey()); + RepositoryMappingValue repoMappingValue = + (RepositoryMappingValue) env.getValue(getSkyKey(directories)); 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 3839956b709e64..979cbd9df3b7e6 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 @@ -93,7 +93,7 @@ public final class RepositoryDelegatorFunction implements SkyFunction { // The marker file version is inject in the rule key digest so the rule key is always different // when we decide to update the format. - private static final int MARKER_FILE_VERSION = 4; + private static final int MARKER_FILE_VERSION = 5; // Mapping of rule class name to RepositoryFunction. private final ImmutableMap handlers; @@ -589,6 +589,7 @@ 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 @@ -600,6 +601,7 @@ private static class DigestWriter { RepositoryName repositoryName, Rule rule, StarlarkSemantics starlarkSemantics) { + this.directories = directories; ruleKey = computeRuleKey(rule, starlarkSemantics); markerPath = getMarkerPath(directories, repositoryName.getName()); this.rule = rule; @@ -648,15 +650,14 @@ 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, recordedInputValues); boolean verified = false; if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) { - verified = handler.verifyRecordedInputs(rule, recordedInputValues, env); + verified = handler.verifyRecordedInputs(rule, directories, recordedInputValues, env); if (env.valuesMissing()) { return null; } @@ -678,7 +679,7 @@ Map getRecordedInputValues() { @Nullable private static String readMarkerFile( - String content, Path baseDirectory, Map recordedInputValues) { + String content, Map recordedInputValues) { String markerRuleKey = null; Iterable lines = Splitter.on('\n').split(content); @@ -690,8 +691,7 @@ private static String readMarkerFile( } else { int sChar = line.indexOf(' '); if (sChar > 0) { - RepoRecordedInput input = - RepoRecordedInput.parse(unescape(line.substring(0, sChar)), baseDirectory); + RepoRecordedInput input = RepoRecordedInput.parse(unescape(line.substring(0, sChar))); 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..1533eaca58c46e 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,14 +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; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.cmdline.Label; @@ -191,38 +187,12 @@ public abstract RepositoryDirectoryValue.Builder fetch( * is needed. */ public boolean verifyRecordedInputs( - Rule rule, Map recordedInputValues, Environment env) + Rule rule, + BlazeDirectories directories, + 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, directories, recordedInputValues); } public static RootedPath getRootedPathFromLabel(Label label, Environment env) 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/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/py/bazel/bzlmod/bazel_fetch_test.py b/src/test/py/bazel/bzlmod/bazel_fetch_test.py index 2981549af4bdad..5bf97be567499a 100644 --- a/src/test/py/bazel/bzlmod/bazel_fetch_test.py +++ b/src/test/py/bazel/bzlmod/bazel_fetch_test.py @@ -17,6 +17,7 @@ import os import tempfile from absl.testing import absltest + from src.test.py.bazel import test_base from src.test.py.bazel.bzlmod.test_utils import BazelRegistry @@ -234,7 +235,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=False)', ' 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 f8669910295f35..3c2093c6aa2df3 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -16,8 +16,10 @@ import json import os +import pathlib import tempfile from absl.testing import absltest + from src.test.py.bazel import test_base from src.test.py.bazel.bzlmod.test_utils import BazelRegistry from src.test.py.bazel.bzlmod.test_utils import scratchFile @@ -1980,6 +1982,62 @@ 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 file under working directory', + '\n'.join(stderr)) + + def testWatchingFileOutsideWorkspaceFails(self): + outside_file = pathlib.Path(self._temp).joinpath('bleh') + 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 file 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 b7df798d38d5d6..f5826d94f75f86 100755 --- a/src/test/shell/bazel/starlark_repository_test.sh +++ b/src/test/shell/bazel/starlark_repository_test.sh @@ -2704,4 +2704,154 @@ EOF expect_log "I see: @@bar~//: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_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 file under working directory" +} + +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" +} + +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" +} + run_suite "local repository tests" diff --git a/src/test/tools/bzlmod/MODULE.bazel.lock b/src/test/tools/bzlmod/MODULE.bazel.lock index b8c8061982c520..25946507c9318e 100644 --- a/src/test/tools/bzlmod/MODULE.bazel.lock +++ b/src/test/tools/bzlmod/MODULE.bazel.lock @@ -1,5 +1,5 @@ { - "lockFileVersion": 4, + "lockFileVersion": 5, "moduleFileHash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", "flags": { "cmdRegistries": [ @@ -1037,7 +1037,7 @@ "@@apple_support~//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~//:buildozer_binary.bzl%buildozer_binary": { "general": { "bzlTransitiveDigest": "EleDU/FQ1+e/RgkW3aIDmdaxZEthvoWQhsqFTxiSgMI=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "buildozer_binary": { @@ -1192,7 +1192,7 @@ "@@rules_java~//java:extensions.bzl%toolchains": { "general": { "bzlTransitiveDigest": "Bm4ulErUcJjZtKeAt2etkB6sGO8evCgHQxbw4Px8q60=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "remotejdk21_linux_toolchain_config_repo": { @@ -1696,8 +1696,8 @@ "@@rules_jvm_external~//:extensions.bzl%maven": { "general": { "bzlTransitiveDigest": "yXprMX4LqzJwuZlbtT9WNeu7p2iEYw7j4R1NP9pc4Ow=", - "accumulatedFileDigests": { - "@@rules_jvm_external~//:rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" + "recordedFileInputs": { + "@@rules_jvm_external~//rules_jvm_external_deps_install.json": "10442a5ae27d9ff4c2003e5ab71643bf0d8b48dcf968b4173fa274c3232a8c06" }, "envVariables": {}, "generatedRepoSpecs": { @@ -2718,7 +2718,7 @@ "@@rules_jvm_external~//: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~//python/extensions:python.bzl%python": { "general": { "bzlTransitiveDigest": "NGtTMUqs2EEJeXu6mXdpmYRrQGZiJV7S3mxeod3Hm7M=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pythons_hub": { @@ -2772,7 +2772,7 @@ "@@rules_python~//python/extensions/private:internal_deps.bzl%internal_deps": { "general": { "bzlTransitiveDigest": "5c1tkdV6L67SQOZWc9MUoS5ZnsSxeDKsh9urs01jZSM=", - "accumulatedFileDigests": {}, + "recordedFileInputs": {}, "envVariables": {}, "generatedRepoSpecs": { "pypi__coverage_cp39_aarch64-apple-darwin": {