From 28715dedc7a66b2768b5d64d38e9528fe829f232 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 3 Aug 2023 09:28:26 -0700 Subject: [PATCH] Automated rollback of commit 3774f005792283b7ca6f2fa299a665f0c2ce459b. *** Reason for rollback *** Performance regression, b/294105788 *** Original change description *** Implement REPO.bazel - REPO.bazel is a new file at the root of repos that can only contain a `repo()` call with a bunch of package default arguments (similar to `package()`) - A new SkyFunction, RepoFileFunction, evaluates a REPO.bazel file and returns the `PackageArgs` object containing those arguments - PackageFunction for packages in repo `@foo` now depends on RepoFileFunction for `@foo` Fixes https://github.com/bazelbuild/bazel/issues/18077 PiperOrigin-RevId: 553500673 Change-Id: I866e73262ab75635c8927f8c77eb5412c01e16a9 --- .../starlark/StarlarkGlobalsImpl.java | 8 - .../build/lib/cmdline/LabelConstants.java | 1 - .../build/lib/packages/PackageArgs.java | 2 - .../build/lib/packages/RepoCallable.java | 59 ------ .../build/lib/packages/RepoThreadContext.java | 63 ------ .../build/lib/packages/StarlarkGlobals.java | 3 - .../lib/packages/StarlarkNativeModule.java | 1 + .../google/devtools/build/lib/skyframe/BUILD | 36 ---- .../build/lib/skyframe/PackageFunction.java | 24 +-- .../build/lib/skyframe/RepoFileFunction.java | 195 ------------------ .../build/lib/skyframe/RepoFileValue.java | 51 ----- .../build/lib/skyframe/SkyFunctions.java | 1 - .../build/lib/skyframe/SkyframeExecutor.java | 4 - .../packages/AbstractPackageLoader.java | 5 - .../build/lib/skyframe/packages/BUILD | 3 +- src/main/protobuf/failure_details.proto | 1 - .../lib/skyframe/RepoFileFunctionTest.java | 125 ----------- 17 files changed, 4 insertions(+), 578 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java delete mode 100644 src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java delete mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java delete mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java delete mode 100644 src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java index b24c47c18f9658..2a5d42d1e69d79 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.collect.nestedset.Depset; import com.google.devtools.build.lib.packages.BuildGlobals; import com.google.devtools.build.lib.packages.Proto; -import com.google.devtools.build.lib.packages.RepoCallable; import com.google.devtools.build.lib.packages.SelectorList; import com.google.devtools.build.lib.packages.StarlarkGlobals; import com.google.devtools.build.lib.packages.StarlarkNativeModule; @@ -121,11 +120,4 @@ public ImmutableMap getSclToplevels() { env.put("struct", StructProvider.STRUCT); return env.buildOrThrow(); } - - @Override - public ImmutableMap getRepoToplevels() { - ImmutableMap.Builder env = ImmutableMap.builder(); - Starlark.addMethods(env, RepoCallable.INSTANCE); - return env.buildOrThrow(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java b/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java index 3a57971736a3f4..783396f7fe41bd 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/LabelConstants.java @@ -44,7 +44,6 @@ public class LabelConstants { public static final PathFragment WORKSPACE_DOT_BAZEL_FILE_NAME = PathFragment.create("WORKSPACE.bazel"); public static final PathFragment MODULE_DOT_BAZEL_FILE_NAME = PathFragment.create("MODULE.bazel"); - public static final PathFragment REPO_FILE_NAME = PathFragment.create("REPO.bazel"); public static final PathFragment MODULE_LOCKFILE_NAME = PathFragment.create("MODULE.bazel.lock"); diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java b/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java index 53971863fd5013..bacf3d749f4cad 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageArgs.java @@ -37,8 +37,6 @@ */ @AutoValue public abstract class PackageArgs { - public static final PackageArgs EMPTY = PackageArgs.builder().build(); - public static final PackageArgs DEFAULT = PackageArgs.builder() .setDefaultVisibility(RuleVisibility.PRIVATE) diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java b/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java deleted file mode 100644 index a78514be8f5fa5..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/packages/RepoCallable.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.packages; - -import java.util.Map; -import net.starlark.java.annot.Param; -import net.starlark.java.annot.StarlarkMethod; -import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkThread; - -/** Definition of the {@code repo()} function used in REPO.bazel files. */ -public final class RepoCallable { - private RepoCallable() {} - - public static final RepoCallable INSTANCE = new RepoCallable(); - - @StarlarkMethod( - name = "repo", - documented = false, // documented separately - extraKeywords = @Param(name = "kwargs"), - useStarlarkThread = true) - public Object repoCallable(Map kwargs, StarlarkThread thread) - throws EvalException { - RepoThreadContext context = RepoThreadContext.fromOrFail(thread, "repo()"); - if (context.isRepoFunctionCalled()) { - throw Starlark.errorf("'repo' can only be called once in the REPO.bazel file"); - } - context.setRepoFunctionCalled(); - - if (kwargs.isEmpty()) { - throw Starlark.errorf("at least one argument must be given to the 'repo' function"); - } - - PackageArgs.Builder pkgArgsBuilder = PackageArgs.builder(); - for (Map.Entry kwarg : kwargs.entrySet()) { - PackageArgs.processParam( - kwarg.getKey(), - kwarg.getValue(), - "repo() argument '" + kwarg.getKey() + "'", - context.getLabelConverter(), - pkgArgsBuilder); - } - context.setPackageArgs(pkgArgsBuilder.build()); - return Starlark.NONE; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java b/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java deleted file mode 100644 index 25bc13e43af36a..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/packages/RepoThreadContext.java +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.packages; - -import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkThread; - -/** Context object for a Starlark thread evaluating the REPO.bazel file. */ -public class RepoThreadContext { - private final LabelConverter labelConverter; - private PackageArgs packageArgs = PackageArgs.EMPTY; - private boolean repoFunctionCalled = false; - - public static RepoThreadContext fromOrFail(StarlarkThread thread, String what) - throws EvalException { - RepoThreadContext context = thread.getThreadLocal(RepoThreadContext.class); - if (context == null) { - throw Starlark.errorf("%s can only be called from REPO.bazel", what); - } - return context; - } - - public void storeInThread(StarlarkThread thread) { - thread.setThreadLocal(RepoThreadContext.class, this); - } - - public RepoThreadContext(LabelConverter labelConverter) { - this.labelConverter = labelConverter; - } - - public LabelConverter getLabelConverter() { - return labelConverter; - } - - public boolean isRepoFunctionCalled() { - return repoFunctionCalled; - } - - public void setRepoFunctionCalled() { - repoFunctionCalled = true; - } - - public void setPackageArgs(PackageArgs packageArgs) { - this.packageArgs = packageArgs; - } - - public PackageArgs getPackageArgs() { - return packageArgs; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java index 69370613bf0ce1..a27d63087186db 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java @@ -64,7 +64,4 @@ public interface StarlarkGlobals { /** Returns the top-levels for .scl files. */ ImmutableMap getSclToplevels(); - - /** Returns the top-levels for REPO.bazel files. */ - ImmutableMap getRepoToplevels(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java index 9dbb23276648f5..3017d40514159a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkNativeModule.java @@ -68,6 +68,7 @@ import net.starlark.java.syntax.Location; /** The Starlark native module. */ +// TODO(cparsons): Move the definition of native.package() to this class. public class StarlarkNativeModule implements StarlarkNativeModuleApi { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 64b14d683244ea..946a3dea4852c8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -167,8 +167,6 @@ java_library( ":recursive_package_provider_backed_target_pattern_resolver", ":recursive_pkg_function", ":recursive_pkg_value", - ":repo_file_function", - ":repo_file_value", ":repository_mapping_function", ":repository_mapping_value", ":rule_configured_target_value", @@ -2195,40 +2193,6 @@ java_library( ], ) -java_library( - name = "repo_file_function", - srcs = ["RepoFileFunction.java"], - deps = [ - ":precomputed_value", - ":repo_file_value", - ":repository_mapping_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/events", - "//src/main/java/com/google/devtools/build/lib/packages", - "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_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", - "//src/main/java/net/starlark/java/eval", - "//src/main/java/net/starlark/java/syntax", - "//third_party:jsr305", - ], -) - -java_library( - name = "repo_file_value", - srcs = ["RepoFileValue.java"], - deps = [ - ":sky_functions", - "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/packages", - "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", - "//third_party:auto_value", - ], -) - java_library( name = "repository_mapping_function", srcs = ["RepositoryMappingFunction.java"], diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java index f3ca4d71a1a832..00f1ac5b815565 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java @@ -63,7 +63,6 @@ import com.google.devtools.build.lib.server.FailureDetails.PackageLoading.Code; import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException; import com.google.devtools.build.lib.skyframe.GlobValue.InvalidGlobPatternException; -import com.google.devtools.build.lib.skyframe.RepoFileFunction.BadRepoFileException; import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction.BuiltinsFailedException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.Pair; @@ -1241,24 +1240,6 @@ private LoadedPackage loadPackage( IgnoredPackagePrefixesValue repositoryIgnoredPackagePrefixes = (IgnoredPackagePrefixesValue) env.getValue(IgnoredPackagePrefixesValue.key(packageId.getRepository())); - RepoFileValue repoFileValue; - try { - repoFileValue = - (RepoFileValue) - env.getValueOrThrow( - RepoFileValue.key(packageId.getRepository()), - IOException.class, - BadRepoFileException.class); - } catch (IOException | BadRepoFileException e) { - throw PackageFunctionException.builder() - .setType(PackageFunctionException.Type.BUILD_FILE_CONTAINS_ERRORS) - .setPackageIdentifier(packageId) - .setTransience(Transience.PERSISTENT) - .setException(e) - .setMessage("bad REPO.bazel file") - .setPackageLoadingCode(PackageLoading.Code.BAD_REPO_FILE) - .build(); - } if (env.valuesMissing()) { return null; } @@ -1394,9 +1375,8 @@ private LoadedPackage loadPackage( .setFilename(buildFileRootedPath) .setConfigSettingVisibilityPolicy(configSettingVisibilityPolicy); - pkgBuilder - .mergePackageArgsFrom(PackageArgs.builder().setDefaultVisibility(defaultVisibility)) - .mergePackageArgsFrom(repoFileValue.packageArgs()); + pkgBuilder.mergePackageArgsFrom( + PackageArgs.builder().setDefaultVisibility(defaultVisibility)); Set globDepKeys = ImmutableSet.of(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java deleted file mode 100644 index 4d77052f11c935..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileFunction.java +++ /dev/null @@ -1,195 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.skyframe; - -import com.google.devtools.build.lib.actions.FileValue; -import com.google.devtools.build.lib.cmdline.LabelConstants; -import com.google.devtools.build.lib.cmdline.PackageIdentifier; -import com.google.devtools.build.lib.cmdline.RepositoryMapping; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; -import com.google.devtools.build.lib.packages.LabelConverter; -import com.google.devtools.build.lib.packages.PackageArgs; -import com.google.devtools.build.lib.packages.RepoThreadContext; -import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue; -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.skyframe.SkyFunction; -import com.google.devtools.build.skyframe.SkyFunctionException; -import com.google.devtools.build.skyframe.SkyFunctionException.Transience; -import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.SkyValue; -import java.io.IOException; -import javax.annotation.Nullable; -import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Module; -import net.starlark.java.eval.Mutability; -import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkSemantics; -import net.starlark.java.eval.StarlarkThread; -import net.starlark.java.syntax.ParserInput; -import net.starlark.java.syntax.Program; -import net.starlark.java.syntax.StarlarkFile; -import net.starlark.java.syntax.SyntaxError; - -/** The function to evaluate the REPO.bazel file at the root of a repo. */ -public class RepoFileFunction implements SkyFunction { - private final BazelStarlarkEnvironment starlarkEnv; - private final Path workspaceRoot; - - public RepoFileFunction(BazelStarlarkEnvironment starlarkEnv, Path workspaceRoot) { - this.starlarkEnv = starlarkEnv; - this.workspaceRoot = workspaceRoot; - } - - @Nullable - @Override - public SkyValue compute(SkyKey skyKey, Environment env) - throws SkyFunctionException, InterruptedException { - RepositoryName repoName = (RepositoryName) skyKey.argument(); - // First we need to find the REPO.bazel file. How we do this depends on whether this is for the - // main repo or an external repo. - Path repoRoot; - if (repoName.isMain()) { - repoRoot = workspaceRoot; - } else { - RepositoryDirectoryValue repoDirValue = - (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(repoName)); - if (repoDirValue == null) { - return null; - } - repoRoot = repoDirValue.getPath(); - } - RootedPath repoFilePath = - RootedPath.toRootedPath(Root.fromPath(repoRoot), LabelConstants.REPO_FILE_NAME); - FileValue repoFileValue = (FileValue) env.getValue(FileValue.key(repoFilePath)); - if (repoFileValue == null) { - return null; - } - if (!repoFileValue.exists()) { - // It's okay to not have a REPO.bazel file. - return RepoFileValue.of(PackageArgs.EMPTY); - } - - // Now we can actually evaluate the file. - StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env); - RepositoryMappingValue repoMapping = - (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(repoName)); - RepositoryMappingValue mainRepoMapping = - (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN)); - if (env.valuesMissing()) { - return null; - } - StarlarkFile repoFile = readAndParseRepoFile(repoFilePath.asPath(), env); - PackageArgs packageArgs = - evalRepoFile( - repoFile, - repoName, - getDisplayNameForRepo(repoName, mainRepoMapping.getRepositoryMapping()), - repoMapping.getRepositoryMapping(), - starlarkSemantics, - env.getListener()); - - return RepoFileValue.of(packageArgs); - } - - private static StarlarkFile readAndParseRepoFile(Path path, Environment env) - throws RepoFileFunctionException { - byte[] contents; - try { - contents = FileSystemUtils.readWithKnownFileSize(path, path.getFileSize()); - } catch (IOException e) { - throw new RepoFileFunctionException( - new IOException("error reading REPO.bazel file at " + path, e), Transience.TRANSIENT); - } - StarlarkFile starlarkFile = - StarlarkFile.parse(ParserInput.fromUTF8(contents, path.getPathString())); - if (!starlarkFile.ok()) { - Event.replayEventsOn(env.getListener(), starlarkFile.errors()); - throw new RepoFileFunctionException( - new BadRepoFileException("error parsing REPO.bazel file at " + path)); - } - return starlarkFile; - } - - private static String getDisplayNameForRepo( - RepositoryName repoName, RepositoryMapping mainRepoMapping) { - String displayName = repoName.getDisplayForm(mainRepoMapping); - if (displayName.isEmpty()) { - return "the main repo"; - } - return displayName; - } - - private PackageArgs evalRepoFile( - StarlarkFile starlarkFile, - RepositoryName repoName, - String repoDisplayName, - RepositoryMapping repoMapping, - StarlarkSemantics starlarkSemantics, - ExtendedEventHandler handler) - throws RepoFileFunctionException, InterruptedException { - try (Mutability mu = Mutability.create("repo file", repoName)) { - Module predeclared = - Module.withPredeclared( - starlarkSemantics, starlarkEnv.getStarlarkGlobals().getRepoToplevels()); - Program program = Program.compileFile(starlarkFile, predeclared); - // TODO(wyv): check that `program` has no `def`, `if`, etc - StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics); - thread.setPrintHandler(Event.makeDebugPrintHandler(handler)); - RepoThreadContext context = - new RepoThreadContext( - new LabelConverter( - PackageIdentifier.create(repoName, PathFragment.EMPTY_FRAGMENT), repoMapping)); - context.storeInThread(thread); - Starlark.execFileProgram(program, predeclared, thread); - return context.getPackageArgs(); - } catch (SyntaxError.Exception e) { - Event.replayEventsOn(handler, e.errors()); - throw new RepoFileFunctionException( - new BadRepoFileException("error parsing REPO.bazel file for " + repoDisplayName, e)); - } catch (EvalException e) { - handler.handle(Event.error(e.getMessageWithStack())); - throw new RepoFileFunctionException( - new BadRepoFileException("error evaluating REPO.bazel file for " + repoDisplayName, e)); - } - } - - /** Thrown when something is wrong with the contents of the REPO.bazel file of a certain repo. */ - public static class BadRepoFileException extends Exception { - public BadRepoFileException(String message) { - super(message); - } - - public BadRepoFileException(String message, Exception cause) { - super(message, cause); - } - } - - static class RepoFileFunctionException extends SkyFunctionException { - private RepoFileFunctionException(IOException e, Transience transience) { - super(e, transience); - } - - private RepoFileFunctionException(BadRepoFileException e) { - super(e, Transience.PERSISTENT); - } - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java deleted file mode 100644 index 34763fee4f2c50..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/RepoFileValue.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.skyframe; - -import com.google.auto.value.AutoValue; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.PackageArgs; -import com.google.devtools.build.skyframe.AbstractSkyKey; -import com.google.devtools.build.skyframe.SkyFunctionName; -import com.google.devtools.build.skyframe.SkyValue; - -/** Contains information about the REPO.bazel file at the root of a repo. */ -@AutoValue -public abstract class RepoFileValue implements SkyValue { - public static final RepoFileValue EMPTY = of(PackageArgs.EMPTY); - - public abstract PackageArgs packageArgs(); - - public static RepoFileValue of(PackageArgs packageArgs) { - return new AutoValue_RepoFileValue(packageArgs); - } - - public static Key key(RepositoryName repoName) { - return new Key(repoName); - } - - /** Key type for {@link RepoFileValue}. */ - public static class Key extends AbstractSkyKey { - - private Key(RepositoryName repoName) { - super(repoName); - } - - @Override - public SkyFunctionName functionName() { - return SkyFunctions.REPO_FILE; - } - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java index 044ddf9f1db110..b3326be03a9e59 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java @@ -141,7 +141,6 @@ public final class SkyFunctions { SkyFunctionName.createHermetic("RESOLVED_HASH_VALUES"); public static final SkyFunctionName MODULE_FILE = SkyFunctionName.createNonHermetic("MODULE_FILE"); - public static final SkyFunctionName REPO_FILE = SkyFunctionName.createHermetic("REPO_FILE"); public static final SkyFunctionName BUILD_DRIVER = SkyFunctionName.createNonHermetic("BUILD_DRIVER"); public static final SkyFunctionName BAZEL_MODULE_RESOLUTION = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 12c22ca188d231..fd2b03aa885e4b 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -668,10 +668,6 @@ private ImmutableMap skyFunctions() { pkgFactory, directories, bzlLoadFunctionForInliningPackageAndWorkspaceNodes)); - map.put( - SkyFunctions.REPO_FILE, - new RepoFileFunction( - ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace())); map.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction(externalPackageHelper)); map.put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 15b2cb614ea318..dd4b5fd1a511de 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -70,7 +70,6 @@ import com.google.devtools.build.lib.skyframe.PackageValue; import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; -import com.google.devtools.build.lib.skyframe.RepoFileFunction; import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction; @@ -477,10 +476,6 @@ public String getBaseNameForLoadedPackage(PackageIdentifier packageName) { WorkspaceFileValue.WORKSPACE_FILE, new WorkspaceFileFunction( ruleClassProvider, pkgFactory, directories, /* bzlLoadFunctionForInlining= */ null)) - .put( - SkyFunctions.REPO_FILE, - new RepoFileFunction( - ruleClassProvider.getBazelStarlarkEnvironment(), directories.getWorkspace())) .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction(getExternalPackageHelper())) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD index 9ba1d3967aa830..2b42184adf89d1 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD @@ -39,6 +39,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/io:file_symlink_cycle_uniqueness_function", @@ -55,7 +56,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:package_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", - "//src/main/java/com/google/devtools/build/lib/skyframe:repo_file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", @@ -67,7 +67,6 @@ java_library( "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", "//src/main/java/net/starlark/java/eval", "//third_party:caffeine", - "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:jsr305", ], diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index ca793a22f2eb0e..37c922a289e8a8 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -1245,7 +1245,6 @@ message PackageLoading { BUILTINS_INJECTION_FAILURE = 29 [(metadata) = { exit_code: 1 }]; SYMLINK_CYCLE_OR_INFINITE_EXPANSION = 30 [(metadata) = { exit_code: 1 }]; OTHER_IO_EXCEPTION = 31 [(metadata) = { exit_code: 36 }]; - BAD_REPO_FILE = 32 [(metadata) = { exit_code: 1 }]; } Code code = 1; diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java deleted file mode 100644 index a9165512eb5f1d..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RepoFileFunctionTest.java +++ /dev/null @@ -1,125 +0,0 @@ -// Copyright 2023 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.skyframe; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; -import com.google.devtools.build.lib.bazel.bzlmod.BazelLockFileFunction; -import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionFunction; -import com.google.devtools.build.lib.bazel.bzlmod.FakeRegistry; -import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; -import com.google.devtools.build.lib.bazel.bzlmod.YankedVersionsUtil; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.BazelCompatibilityMode; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode; -import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; -import com.google.devtools.build.lib.packages.Type; -import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected; -import com.google.devtools.build.lib.vfs.Path; -import java.io.IOException; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -@RunWith(JUnit4.class) -public class RepoFileFunctionTest extends BuildViewTestCase { - - private Path moduleRoot; - private FakeRegistry registry; - - @Override - protected ImmutableList extraPrecomputedValues() { - try { - moduleRoot = scratch.dir("modules"); - } catch (IOException e) { - throw new IllegalStateException(e); - } - registry = FakeRegistry.DEFAULT_FACTORY.newFakeRegistry(moduleRoot.getPathString()); - return ImmutableList.of( - PrecomputedValue.injected( - ModuleFileFunction.REGISTRIES, ImmutableList.of(registry.getUrl())), - PrecomputedValue.injected(ModuleFileFunction.IGNORE_DEV_DEPS, false), - PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, ImmutableMap.of()), - PrecomputedValue.injected(YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, ImmutableList.of()), - PrecomputedValue.injected( - BazelModuleResolutionFunction.CHECK_DIRECT_DEPENDENCIES, CheckDirectDepsMode.WARNING), - PrecomputedValue.injected( - BazelModuleResolutionFunction.BAZEL_COMPATIBILITY_MODE, BazelCompatibilityMode.ERROR), - PrecomputedValue.injected(BazelLockFileFunction.LOCKFILE_MODE, LockfileMode.UPDATE)); - } - - @Test - public void repoFileInTheMainRepo() throws Exception { - scratch.overwriteFile("REPO.bazel", "repo(default_deprecation='EVERYTHING IS DEPRECATED')"); - scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')"); - assertThat( - getRuleContext(getConfiguredTarget("//abc/def:what")) - .attributes() - .get("deprecation", Type.STRING)) - .isEqualTo("EVERYTHING IS DEPRECATED"); - } - - @Test - public void repoFileInAnExternalRepo() throws Exception { - setBuildLanguageOptions("--enable_bzlmod"); - scratch.overwriteFile("MODULE.bazel", "bazel_dep(name='foo',version='1.0')"); - scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')"); - registry.addModule(createModuleKey("foo", "1.0"), "module(name='foo',version='1.0')"); - scratch.overwriteFile(moduleRoot.getRelative("foo~1.0/WORKSPACE.bazel").getPathString()); - scratch.overwriteFile( - moduleRoot.getRelative("foo~1.0/REPO.bazel").getPathString(), - "repo(default_deprecation='EVERYTHING IS DEPRECATED')"); - scratch.overwriteFile( - moduleRoot.getRelative("foo~1.0/abc/def/BUILD").getPathString(), "filegroup(name='what')"); - - assertThat( - getRuleContext(getConfiguredTarget("//abc/def:what")) - .attributes() - .get("deprecation", Type.STRING)) - .isNull(); - assertThat( - getRuleContext(getConfiguredTarget("@@foo~1.0//abc/def:what")) - .attributes() - .get("deprecation", Type.STRING)) - .isEqualTo("EVERYTHING IS DEPRECATED"); - } - - @Test - public void cantCallRepoTwice() throws Exception { - scratch.overwriteFile( - "REPO.bazel", - "repo(default_deprecation='EVERYTHING IS DEPRECATED')", - "repo(features=['abc'])"); - scratch.overwriteFile("abc/def/BUILD", "filegroup(name='what')"); - reporter.removeHandler(failFastHandler); - assertTargetError("//abc/def:what", "'repo' can only be called once"); - } - - @Test - public void featureMerger() throws Exception { - scratch.overwriteFile("REPO.bazel", "repo(features=['a', 'b', 'c', '-d'])"); - scratch.overwriteFile( - "abc/def/BUILD", - "package(features=['-a','-b','d'])", - "filegroup(name='what', features=['b'])"); - RuleContext ruleContext = getRuleContext(getConfiguredTarget("//abc/def:what")); - assertThat(ruleContext.getFeatures()).containsExactly("b", "c", "d"); - assertThat(ruleContext.getDisabledFeatures()).containsExactly("a"); - } -}