From 9991200c54103fbf91059870d72b8205299f3cf3 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 21 Jun 2022 05:00:30 -0700 Subject: [PATCH] Remove support for managed directories. Fixes #15463. The design doc for the deprecation and the removal is available here: https://docs.google.com/document/d/1u9V5RUc7i6Urh8gGfnSurxpWA7JMRtwCi1Pr5BHeE44/edit RELNOTES: --noincompatible_disable_managed_directories, and with that, workspace(managed_directories=) is not supported anymore. PiperOrigin-RevId: 456228902 Change-Id: I5fcbf96b9a508f47cbcabf9715163cd7120020bf --- .../com/google/devtools/build/lib/bazel/BUILD | 1 - .../lib/bazel/BazelRepositoryModule.java | 28 +- .../lib/bazel/rules/BazelRulesModule.java | 8 + .../build/lib/packages/WorkspaceFactory.java | 6 - .../lib/packages/WorkspaceFileValue.java | 13 +- .../build/lib/packages/WorkspaceGlobals.java | 99 ---- .../semantics/BuildLanguageOptions.java | 12 - .../com/google/devtools/build/lib/rules/BUILD | 16 - .../ManagedDirectoriesKnowledgeImpl.java | 105 ---- .../RepositoryDelegatorFunction.java | 54 +- .../RepositoryDirectoryDirtinessChecker.java | 42 +- .../rules/repository/RepositoryFunction.java | 18 +- .../google/devtools/build/lib/skyframe/BUILD | 16 - .../DirectoryListingStateFunction.java | 3 +- .../lib/skyframe/DirtinessCheckerUtils.java | 27 +- .../lib/skyframe/ExternalFilesHelper.java | 67 +-- .../build/lib/skyframe/FileStateFunction.java | 3 +- .../skyframe/ManagedDirectoriesKnowledge.java | 77 --- .../skyframe/SequencedSkyframeExecutor.java | 166 +----- .../build/lib/skyframe/SkyframeExecutor.java | 9 +- ...yframeExecutorRepositoryHelpersHolder.java | 5 +- .../lib/skyframe/WorkspaceFileFunction.java | 6 +- .../packages/AbstractPackageLoader.java | 7 +- .../build/lib/skyframe/packages/BUILD | 2 - .../skyframe/packages/BazelPackageLoader.java | 2 - .../starlarkbuildapi/WorkspaceGlobalsApi.java | 18 +- .../build/lib/analysis/util/AnalysisMock.java | 2 - .../devtools/build/lib/analysis/util/BUILD | 2 - .../lib/analysis/util/BuildViewTestCase.java | 15 - .../devtools/build/lib/bazel/bzlmod/BUILD | 1 - .../build/lib/bazel/bzlmod/DiscoveryTest.java | 2 - .../bzlmod/ModuleExtensionResolutionTest.java | 2 - .../bazel/bzlmod/ModuleFileFunctionTest.java | 2 - .../build/lib/bazel/repository/starlark/BUILD | 2 - .../StarlarkRepositoryIntegrationTest.java | 4 +- .../devtools/build/lib/blackbox/tests/BUILD | 20 - .../lib/blackbox/tests/manageddirs/BUILD.test | 20 - .../ManagedDirectoriesBlackBoxTest.java | 506 ------------------ .../blackbox/tests/manageddirs/WORKSPACE.test | 24 - .../tests/manageddirs/bazelignore.test | 1 - .../blackbox/tests/manageddirs/package.json | 8 - .../blackbox/tests/manageddirs/test_rule.bzl | 49 -- .../tests/manageddirs/use_node_modules.bzl | 93 ---- .../devtools/build/lib/rules/repository/BUILD | 1 - .../repository/RepositoryDelegatorTest.java | 187 +------ .../google/devtools/build/lib/skyframe/BUILD | 2 - .../ContainingPackageLookupFunctionTest.java | 1 - .../skyframe/DirtinessCheckerUtilsTest.java | 41 +- .../build/lib/skyframe/FileFunctionTest.java | 1 - .../skyframe/PackageLookupFunctionTest.java | 1 - .../skyframe/WorkspaceFileFunctionTest.java | 273 ---------- 51 files changed, 58 insertions(+), 2012 deletions(-) delete mode 100644 src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java delete mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/ManagedDirectoriesKnowledge.java delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/bazelignore.test delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl delete mode 100644 src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/BUILD index 04c42351ec78cd..0e9a318d75844b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/BUILD @@ -42,7 +42,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/events", "//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/managed_directories_knowledge_impl", "//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_function", "//src/main/java/com/google/devtools/build/lib/rules:repository/new_local_repository_rule", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java index 8bf94114d9a670..cc7aae90452280 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java @@ -61,8 +61,6 @@ import com.google.devtools.build.lib.pkgcache.PackageOptions; import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule; -import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl; -import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl.ManagedDirectoriesListener; import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryFunction; import com.google.devtools.build.lib.rules.repository.NewLocalRepositoryRule; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; @@ -134,9 +132,6 @@ public class BazelRepositoryModule extends BlazeModule { private Set outputVerificationRules = ImmutableSet.of(); private FileSystem filesystem; private List registries; - // We hold the precomputed value of the managed directories here, so that the dependency - // on WorkspaceFileValue is not registered for each FileStateValue. - private final ManagedDirectoriesKnowledgeImpl managedDirectoriesKnowledge; private final AtomicBoolean enableBzlmod = new AtomicBoolean(false); private final AtomicBoolean ignoreDevDeps = new AtomicBoolean(false); private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING; @@ -145,24 +140,6 @@ public class BazelRepositoryModule extends BlazeModule { public BazelRepositoryModule() { this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager); this.repositoryHandlers = repositoryRules(); - ManagedDirectoriesListener listener = - repositoryNamesWithManagedDirs -> { - Set conflicting = - overrides.keySet().stream() - .filter(repositoryNamesWithManagedDirs::contains) - .map(RepositoryName::getNameWithAt) - .collect(Collectors.toSet()); - if (!conflicting.isEmpty()) { - String message = - "Overriding repositories is not allowed" - + " for the repositories with managed directories.\n" - + "The following overridden external repositories have managed directories: " - + String.join(", ", conflicting.toArray(new String[0])); - throw new AbruptExitException( - detailedExitCode(message, Code.OVERRIDE_DISALLOWED_MANAGED_DIRECTORIES)); - } - }; - managedDirectoriesKnowledge = new ManagedDirectoriesKnowledgeImpl(listener); } private static DetailedExitCode detailedExitCode(String message, ExternalRepository.Code code) { @@ -214,9 +191,7 @@ public void workspaceInit( if ("bazel".equals(runtime.getProductName())) { builder.setSkyframeExecutorRepositoryHelpersHolder( SkyframeExecutorRepositoryHelpersHolder.create( - managedDirectoriesKnowledge, - new RepositoryDirectoryDirtinessChecker( - directories.getWorkspace(), managedDirectoriesKnowledge))); + new RepositoryDirectoryDirtinessChecker())); } // Create the repository function everything flows through. @@ -227,7 +202,6 @@ public void workspaceInit( isFetch, clientEnvironmentSupplier, directories, - managedDirectoriesKnowledge, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER); RegistryFactory registryFactory = new RegistryFactoryImpl(new HttpDownloader(), clientEnvironmentSupplier); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java index a523f0065648cd..6b80558d32c752 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java @@ -515,6 +515,14 @@ public static final class AllCommandGraveyardOptions extends OptionsBase { effectTags = {OptionEffectTag.NO_OP}, help = "No-op") public boolean keepConfigNodes; + + @Option( + name = "incompatible_disable_managed_directories", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.NO_OP}, + help = "No-op") + public boolean incompatibleDisableManagedDirectories; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 3b69ca01e9bb0a..b041955fcd48a0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -17,7 +17,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; -import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; @@ -26,7 +25,6 @@ import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.PackageLoading; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -398,8 +396,4 @@ public Map getLoadedModules() { public Map getVariableBindings() { return ImmutableMap.copyOf(bindings); } - - public Map getManagedDirectories() { - return workspaceGlobals.getManagedDirectories(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFileValue.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFileValue.java index 8f15a74da22050..8f972a547a8774 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFileValue.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; @@ -104,9 +103,6 @@ public String toString() { private final ImmutableMap loadToChunkMap; private final ImmutableMap> repositoryMapping; - // Mapping of the relative paths of the incrementally updated managed directories - // to the managing external repositories - private final ImmutableMap managedDirectories; /** * Create a WorkspaceFileValue containing the various values necessary to compute the split @@ -124,7 +120,6 @@ public String toString() { * @param idx The index of this part of the split WORKSPACE file (0 for the first one, 1 for the * second one and so on). * @param hasNext Is there a next part in the WORKSPACE file or this part the last one? - * @param managedDirectories Mapping of the relative paths of the incrementally updated managed */ public WorkspaceFileValue( Package pkg, @@ -133,8 +128,7 @@ public WorkspaceFileValue( Map bindings, RootedPath path, int idx, - boolean hasNext, - ImmutableMap managedDirectories) { + boolean hasNext) { this.pkg = Preconditions.checkNotNull(pkg); this.idx = idx; this.path = path; @@ -143,7 +137,6 @@ public WorkspaceFileValue( this.loadedModules = ImmutableMap.copyOf(loadedModules); this.loadToChunkMap = ImmutableMap.copyOf(loadToChunkMap); this.repositoryMapping = pkg.getExternalPackageRepositoryMappings(); - this.managedDirectories = managedDirectories; } /** @@ -224,8 +217,4 @@ public ImmutableMap getLoadToChunkMap() { public ImmutableMap> getRepositoryMapping() { return repositoryMapping; } - - public ImmutableMap getManagedDirectories() { - return managedDirectories; - } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java index f277bdebf7d468..1751bfc899b4dc 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java @@ -18,8 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.devtools.build.lib.cmdline.BazelModuleContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; @@ -30,15 +28,12 @@ import com.google.devtools.build.lib.cmdline.TargetPattern; import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.starlarkbuildapi.WorkspaceGlobalsApi; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; import java.util.Map; -import java.util.TreeMap; import java.util.regex.Pattern; import javax.annotation.Nullable; -import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Module; import net.starlark.java.eval.Sequence; @@ -53,20 +48,15 @@ public class WorkspaceGlobals implements WorkspaceGlobalsApi { private final boolean allowOverride; private final RuleFactory ruleFactory; - // Mapping of the relative paths of the incrementally updated managed directories - // to the managing external repositories - private final TreeMap managedDirectoriesMap; public WorkspaceGlobals(boolean allowOverride, RuleFactory ruleFactory) { this.allowOverride = allowOverride; this.ruleFactory = ruleFactory; - this.managedDirectoriesMap = Maps.newTreeMap(); } @Override public void workspace( String name, - Dict managedDirectories, // StarlarkThread thread) throws EvalException, InterruptedException { if (!allowOverride) { @@ -101,95 +91,6 @@ public void workspace( // Add entry in repository map from "@name" --> "@" to avoid issue where bazel // treats references to @name as a separate external repo builder.addRepositoryMappingEntry(RepositoryName.MAIN, name, RepositoryName.MAIN); - parseManagedDirectories( - thread.getSemantics().getBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES), - Dict.cast(managedDirectories, String.class, Object.class, "managed_directories")); - } - - private void parseManagedDirectories( - boolean disabled, Map managedDirectories) // > - throws EvalException { - if (disabled && !managedDirectories.isEmpty()) { - throw Starlark.errorf( - "managed_directories is disabled. Pass the " - + "--noincompatible_disable_managed_directories command line option to temporarily " - + "re-enable it. For more information, see " - + "https://github.com/bazelbuild/bazel/issues/15463"); - } - - Map nonNormalizedPathsMap = Maps.newHashMap(); - for (Map.Entry entry : managedDirectories.entrySet()) { - RepositoryName repositoryName = createRepositoryName(entry.getKey()); - List paths = - getManagedDirectoriesPaths(entry.getValue(), nonNormalizedPathsMap); - for (PathFragment dir : paths) { - PathFragment floorKey = managedDirectoriesMap.floorKey(dir); - if (dir.equals(floorKey)) { - throw Starlark.errorf( - "managed_directories attribute should not contain multiple" - + " (or duplicate) repository mappings for the same directory ('%s').", - nonNormalizedPathsMap.get(dir)); - } - PathFragment ceilingKey = managedDirectoriesMap.ceilingKey(dir); - boolean isDescendant = floorKey != null && dir.startsWith(floorKey); - if (isDescendant || (ceilingKey != null && ceilingKey.startsWith(dir))) { - throw Starlark.errorf( - "managed_directories attribute value can not contain nested mappings." - + " '%s' is a descendant of '%s'.", - nonNormalizedPathsMap.get(isDescendant ? dir : ceilingKey), - nonNormalizedPathsMap.get(isDescendant ? floorKey : dir)); - } - managedDirectoriesMap.put(dir, repositoryName); - } - } - } - - private static RepositoryName createRepositoryName(String key) throws EvalException { - if (!key.startsWith("@")) { - throw Starlark.errorf( - "Cannot parse repository name '%s'. Repository name should start with '@'.", key); - } - try { - return RepositoryName.create(key.substring(1)); - } catch (LabelSyntaxException e) { - throw Starlark.errorf("%s", e); - } - } - - private static List getManagedDirectoriesPaths( - Object directoriesList, Map nonNormalizedPathsMap) - throws EvalException { - if (!(directoriesList instanceof Sequence)) { - throw Starlark.errorf( - "managed_directories attribute value should be of the type attr.string_list_dict()," - + " mapping repository name to the list of managed directories."); - } - List result = Lists.newArrayList(); - for (Object obj : (Sequence) directoriesList) { - if (!(obj instanceof String)) { - throw Starlark.errorf("Expected managed directory path (as string), but got '%s'.", obj); - } - String path = ((String) obj).trim(); - if (path.isEmpty()) { - throw Starlark.errorf("Expected managed directory path to be non-empty string."); - } - PathFragment pathFragment = PathFragment.create(path); - if (pathFragment.isAbsolute()) { - throw Starlark.errorf( - "Expected managed directory path ('%s') to be relative to the workspace root.", path); - } - if (pathFragment.containsUplevelReferences()) { - throw Starlark.errorf( - "Expected managed directory path ('%s') to be under the workspace root.", path); - } - nonNormalizedPathsMap.put(pathFragment, path); - result.add(pathFragment); - } - return result; - } - - public Map getManagedDirectories() { - return managedDirectoriesMap; } private static RepositoryName getRepositoryName(@Nullable Label label) { diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index eb886b4111153e..83946fd1ff2d9f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -547,15 +547,6 @@ public final class BuildLanguageOptions extends OptionsBase { + "'cfg = \"exec\"' instead.") public boolean incompatibleDisableStarlarkHostTransitions; - @Option( - name = "incompatible_disable_managed_directories", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - help = "If set to true, the workspace(managed_directories=) attribute is disabled.") - public boolean incompatibleDisableManagedDirectories; - /** * An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should * never accumulate a large number of these and being able to shortcut on object identity makes a @@ -626,7 +617,6 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool( INCOMPATIBLE_DISABLE_STARLARK_HOST_TRANSITIONS, incompatibleDisableStarlarkHostTransitions) - .setBool(INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES, incompatibleDisableManagedDirectories) .build(); return INTERNER.intern(semantics); } @@ -697,8 +687,6 @@ public StarlarkSemantics toStarlarkSemantics() { "-incompatible_top_level_aspects_require_providers"; public static final String INCOMPATIBLE_DISABLE_STARLARK_HOST_TRANSITIONS = "-incompatible_disable_starlark_host_transitions"; - public static final String INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES = - "+incompatible_disable_managed_directories"; // non-booleans public static final StarlarkSemantics.Key EXPERIMENTAL_BUILTINS_BZL_PATH = 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 2763e8412795e2..5cadf55e284fb8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/BUILD @@ -34,7 +34,6 @@ java_library( ":repository/bind", ":repository/bind_rule", ":repository/local_repository_rule", - ":repository/managed_directories_knowledge_impl", ":repository/new_local_repository_function", ":repository/new_local_repository_rule", ":repository/new_repository_file_handler", @@ -311,20 +310,6 @@ java_library( ], ) -java_library( - name = "repository/managed_directories_knowledge_impl", - srcs = ["repository/ManagedDirectoriesKnowledgeImpl.java"], - deps = [ - "//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/lib/skyframe:managed_directories_knowledge", - "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", - "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", - "//third_party:guava", - "//third_party:jsr305", - ], -) - java_library( name = "repository/new_local_repository_function", srcs = ["repository/NewLocalRepositoryFunction.java"], @@ -432,7 +417,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/repository:repository_failed_event", "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", "//src/main/java/com/google/devtools/build/lib/skyframe:already_reported_exception", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java b/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java deleted file mode 100644 index fbfd9bac71d872..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/ManagedDirectoriesKnowledgeImpl.java +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.devtools.build.lib.rules.repository; - -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.WorkspaceFileValue; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; -import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.vfs.PathFragment; -import java.util.Comparator; -import java.util.Map; -import java.util.Objects; -import java.util.Set; -import javax.annotation.Nullable; - -/** Managed directories component. {@link ManagedDirectoriesKnowledge} */ -public class ManagedDirectoriesKnowledgeImpl implements ManagedDirectoriesKnowledge { - private final ManagedDirectoriesListener listener; - - private ImmutableSortedMap dirToRepoMap = ImmutableSortedMap.of(); - private ImmutableSortedMap> repoToDirMap = - ImmutableSortedMap.of(); - - public ManagedDirectoriesKnowledgeImpl(ManagedDirectoriesListener listener) { - this.listener = listener; - } - - @Override - @Nullable - public RepositoryName getOwnerRepository(PathFragment relativePathFragment) { - Map.Entry entry = dirToRepoMap.floorEntry(relativePathFragment); - if (entry != null && relativePathFragment.startsWith(entry.getKey())) { - return entry.getValue(); - } - return null; - } - - @Override - public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { - ImmutableSet pathFragments = repoToDirMap.get(repositoryName); - return pathFragments != null ? pathFragments : ImmutableSet.of(); - } - - @Override - public boolean workspaceHeaderReloaded( - @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) - throws AbruptExitException { - if (Objects.equals(oldValue, newValue)) { - listener.onManagedDirectoriesRefreshed(repoToDirMap.keySet()); - return false; - } - Map oldDirToRepoMap = dirToRepoMap; - refreshMappings(newValue); - if (!Objects.equals(oldDirToRepoMap, dirToRepoMap)) { - listener.onManagedDirectoriesRefreshed(repoToDirMap.keySet()); - return true; - } - return false; - } - - private void refreshMappings(@Nullable WorkspaceFileValue newValue) { - if (newValue == null) { - dirToRepoMap = ImmutableSortedMap.of(); - repoToDirMap = ImmutableSortedMap.of(); - return; - } - - dirToRepoMap = ImmutableSortedMap.copyOf(newValue.getManagedDirectories()); - - Map> reposMap = Maps.newHashMap(); - for (Map.Entry entry : dirToRepoMap.entrySet()) { - RepositoryName repositoryName = entry.getValue(); - reposMap.computeIfAbsent(repositoryName, name -> Sets.newTreeSet()).add(entry.getKey()); - } - - ImmutableSortedMap.Builder> reposMapBuilder = - new ImmutableSortedMap.Builder<>(Comparator.comparing(RepositoryName::getNameWithAt)); - for (Map.Entry> entry : reposMap.entrySet()) { - reposMapBuilder.put(entry.getKey(), ImmutableSet.copyOf(entry.getValue())); - } - repoToDirMap = reposMapBuilder.buildOrThrow(); - } - - /** Interface allows {@link BazelRepositoryModule} to react to managed directories refreshes. */ - public interface ManagedDirectoriesListener { - void onManagedDirectoriesRefreshed(Set repositoryNames) - throws AbruptExitException; - } -} 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 dc93c9c57e4b19..00bd8176a80aad 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java @@ -15,15 +15,10 @@ package com.google.devtools.build.lib.rules.repository; -import static com.google.devtools.build.lib.rules.repository.RepositoryDirectoryDirtinessChecker.managedDirectoriesExist; - 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.collect.Iterables; import com.google.common.collect.Maps; -import com.google.common.collect.Ordering; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; @@ -39,7 +34,6 @@ import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.NoRepositoryDirectoryValue; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.AlreadyReportedRepositoryAccessException; import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -53,15 +47,12 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; -import java.util.stream.Collectors; import javax.annotation.Nullable; /** @@ -110,9 +101,6 @@ public final class RepositoryDelegatorFunction implements SkyFunction { private final AtomicBoolean isFetch; private final BlazeDirectories directories; - // Managed directories mappings, pre-calculated and injected by SequencedSkyframeExecutor - // before each command. - private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge; private final ExternalPackageHelper externalPackageHelper; @@ -124,14 +112,12 @@ public RepositoryDelegatorFunction( AtomicBoolean isFetch, Supplier> clientEnvironmentSupplier, BlazeDirectories directories, - ManagedDirectoriesKnowledge managedDirectoriesKnowledge, ExternalPackageHelper externalPackageHelper) { this.handlers = handlers; this.starlarkHandler = starlarkHandler; this.isFetch = isFetch; this.clientEnvironmentSupplier = clientEnvironmentSupplier; this.directories = directories; - this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; this.externalPackageHelper = externalPackageHelper; } @@ -316,27 +302,18 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } - ImmutableSet managedDirectories = - managedDirectoriesKnowledge.getManagedDirectories(repositoryName); - DigestWriter digestWriter = - new DigestWriter( - directories, - repositoryName, - rule, - managedDirectories); + DigestWriter digestWriter = new DigestWriter(directories, repositoryName, rule); // Local repositories are fetched regardless of the marker file because the operation is // generally fast and they do not depend on non-local data, so it does not make much sense to // try to cache them from across server instances. boolean fetchLocalRepositoryAlways = isFetch.get() && handler.isLocal(rule); - if (!fetchLocalRepositoryAlways - && managedDirectoriesExist(directories.getWorkspace(), managedDirectories)) { + if (!fetchLocalRepositoryAlways) { // For the non-local repositories, check if they are already up-to-date: // 1) unconditional fetching is not enabled, AND // 2) unconditional syncing is not enabled or the rule is not a configure rule, AND // 3) repository directory exists, AND - // 4) marker file correctly describes the current repository state, AND - // 5) managed directories, mapped to the repository, exist + // 4) marker file correctly describes the current repository state if (!needsConfiguring && doNotFetchUnconditionally && repoRoot.exists()) { byte[] markerHash = digestWriter.areRepositoryAndMarkerFileConsistent(handler, env); if (env.valuesMissing()) { @@ -393,11 +370,7 @@ && managedDirectoriesExist(directories.getWorkspace(), managedDirectories)) { return RepositoryDirectoryValue.builder() .setPath(repoRoot) .setFetchingDelayed() - .setDigest( - new Fingerprint() - .addIterableStrings( - Iterables.transform(managedDirectories, PathFragment::getPathString)) - .digestAndReset()) + .setDigest(new Fingerprint().digestAndReset()) .build(); } @@ -535,28 +508,16 @@ static String unescape(String str) { } private static class DigestWriter { - private static final String MANAGED_DIRECTORIES_MARKER = "$MANAGED"; private final Path markerPath; private final Rule rule; private final Map markerData; private final String ruleKey; - DigestWriter( - BlazeDirectories directories, - RepositoryName repositoryName, - Rule rule, - ImmutableSet managedDirectories) { + DigestWriter(BlazeDirectories directories, RepositoryName repositoryName, Rule rule) { ruleKey = computeRuleKey(rule); markerPath = getMarkerPath(directories, repositoryName.getName()); this.rule = rule; markerData = Maps.newHashMap(); - - List directoriesList = Ordering.natural().sortedCopy(managedDirectories); - String directoriesString = - directoriesList.stream() - .map(PathFragment::getPathString) - .collect(Collectors.joining(" ")); - markerData.put(MANAGED_DIRECTORIES_MARKER, directoriesString); } byte[] writeMarkerFile() throws RepositoryFunctionException { @@ -602,10 +563,7 @@ byte[] areRepositoryAndMarkerFileConsistent(RepositoryFunction handler, Environm content = FileSystemUtils.readContent(markerPath, StandardCharsets.UTF_8); String markerRuleKey = readMarkerFile(content, markerData); boolean verified = false; - if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey) - && Objects.equals( - markerData.get(MANAGED_DIRECTORIES_MARKER), - this.markerData.get(MANAGED_DIRECTORIES_MARKER))) { + if (Preconditions.checkNotNull(ruleKey).equals(markerRuleKey)) { verified = handler.verifyMarkerData(rule, markerData, env); if (env.valuesMissing()) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java index c1c70d142ec92b..93828ea4aa8194 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java +++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDirectoryDirtinessChecker.java @@ -16,14 +16,9 @@ package com.google.devtools.build.lib.rules.repository; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; -import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.SyscallCache; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; @@ -32,32 +27,13 @@ /** * A dirtiness checker that: * - *

Dirties {@link RepositoryDirectoryValue}s, if either: - * - *

    - *
  • they were produced in a {@code --nofetch} build, so that they are re-created on subsequent - * {@code --fetch} builds. The alternative solution would be to reify the value of the flag as - * a Skyframe value. - *
  • any of their managed directories do not exist. This dirtying allows the user to regenerate - * managed directory contents if they become corrupt, although it is unknown if this is useful - * in practice. (A more correct option here would be to build support for managed directories - * into FilesystemValueChecker, much like {@link - * com.google.devtools.build.lib.skyframe.FilesystemValueChecker#getDirtyActionValues}, so - * that changes to the contents of a managed directory would automatically invalidate the - * generating {@link RepositoryDirectoryValue}.) Changes in which directories are managed are - * handled in {@link ManagedDirectoriesKnowledge#workspaceHeaderReloaded}. - *
+ *

Dirties {@link RepositoryDirectoryValue}s, if they were produced in a {@code --nofetch} build, + * so that they are re-created on subsequent {@code --fetch} builds. The alternative solution would + * be to reify the value of the flag as a Skyframe value. */ @VisibleForTesting public class RepositoryDirectoryDirtinessChecker extends SkyValueDirtinessChecker { - private final Path workspaceRoot; - private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge; - - public RepositoryDirectoryDirtinessChecker( - Path workspaceRoot, ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { - this.workspaceRoot = workspaceRoot; - this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; - } + public RepositoryDirectoryDirtinessChecker() {} @Override public boolean applies(SkyKey skyKey) { @@ -76,7 +52,6 @@ public DirtyResult check( SkyValue skyValue, SyscallCache syscallCache, @Nullable TimestampGranularityMonitor tsgm) { - RepositoryName repositoryName = (RepositoryName) skyKey.argument(); RepositoryDirectoryValue repositoryValue = (RepositoryDirectoryValue) skyValue; if (!repositoryValue.repositoryExists()) { @@ -86,15 +61,6 @@ public DirtyResult check( return DirtyResult.dirty(); } - if (!managedDirectoriesExist( - workspaceRoot, managedDirectoriesKnowledge.getManagedDirectories(repositoryName))) { - return DirtyResult.dirty(); - } return DirtyResult.notDirty(); } - - static boolean managedDirectoriesExist( - Path workspaceRoot, ImmutableSet managedDirectories) { - return managedDirectories.stream().allMatch(pf -> workspaceRoot.getRelative(pf).exists()); - } } 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 10a2f676649520..8b7d27907484b0 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 @@ -234,7 +234,7 @@ private static boolean verifyLabelMarkerData(Rule rule, String key, String value } /** - * Convert to a @{link com.google.devtools.build.lib.skyframe.FileValue} to a String appropriate + * 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. @@ -526,22 +526,6 @@ public static void addExternalFilesDependencies( env.getValue(RepositoryDirectoryValue.key(RepositoryName.createUnvalidated(repositoryName))); } - /** - * For paths that are under managed directories, we require that the corresponding FileStateValue - * or DirectoryListingStateValue is evaluated only after RepositoryDirectoryValue is evaluated. - * This way we guarantee that the repository rule is given a chance to update the managed - * directory before the files under the managed directory are accessed. - * - *

We do not need to require anything else (comparing to dependencies required for external - * repositories files), as overriding external repositories with managed directories is currently - * forbidden; also, we do not have do perform special checks for local_repository targets, since - * such targets cannot have managed directories by definition. - */ - public static void addManagedDirectoryDependencies(RepositoryName repositoryName, Environment env) - throws InterruptedException { - env.getValue(RepositoryDirectoryValue.key(repositoryName)); - } - /** * Sets up a mapping of environment variables to use. */ 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 0b5d9103b0ba53..03399b50674261 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -141,7 +141,6 @@ java_library( ":incremental_artifact_conflict_finder", ":loading_phase_started_event", ":local_repository_lookup_value", - ":managed_directories_knowledge", ":map_as_package_roots", ":metadata_consumer_for_metrics", ":output_store", @@ -323,7 +322,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/util:crash_failure_details", "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code", - "//src/main/java/com/google/devtools/build/lib/util:exit_code", "//src/main/java/com/google/devtools/build/lib/util:resource_usage", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -1526,19 +1524,6 @@ java_library( ], ) -java_library( - name = "managed_directories_knowledge", - srcs = ["ManagedDirectoriesKnowledge.java"], - deps = [ - "//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/lib/util:abrupt_exit_exception", - "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", - "//third_party:guava", - "//third_party:jsr305", - ], -) - java_library( name = "map_as_package_roots", srcs = ["MapAsPackageRoots.java"], @@ -2257,7 +2242,6 @@ java_library( name = "skyframe_executor_repository_helpers_holder", srcs = ["SkyframeExecutorRepositoryHelpersHolder.java"], deps = [ - ":managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//third_party:auto_value", ], diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java index 876ab3ffb00858..e2f121612f4a95 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingStateFunction.java @@ -55,8 +55,7 @@ public DirectoryListingStateValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } - if (fileType == FileType.EXTERNAL_REPO - || fileType == FileType.EXTERNAL_IN_MANAGED_DIRECTORY) { + if (fileType == FileType.EXTERNAL_REPO) { // Do not use syscallCache as files under repositories get generated during the build, // while syscallCache is used independently from Skyframe and generally assumes // the file system is frozen at the beginning of the build command. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java index 3d34f540ed2de7..080598a2f62dd6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtils.java @@ -20,7 +20,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.FileStateValue; -import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.FileType; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.Root; @@ -100,12 +99,9 @@ public SkyValue createNewValue( static final class MissingDiffDirtinessChecker extends BasicFilesystemDirtinessChecker { private final Set missingDiffPackageRoots; - private final ExternalFilesHelper externalFilesHelper; - MissingDiffDirtinessChecker( - Set missingDiffPackageRoots, ExternalFilesHelper externalFilesHelper) { + MissingDiffDirtinessChecker(Set missingDiffPackageRoots) { this.missingDiffPackageRoots = missingDiffPackageRoots; - this.externalFilesHelper = externalFilesHelper; } @Override @@ -113,26 +109,6 @@ public boolean applies(SkyKey key) { return super.applies(key) && missingDiffPackageRoots.contains(((RootedPath) key.argument()).getRoot()); } - - @Override - public DirtyResult check( - SkyKey key, - @Nullable SkyValue oldValue, - SyscallCache syscallCache, - @Nullable TimestampGranularityMonitor tsgm) { - FileType fileType = externalFilesHelper.getAndNoteFileType((RootedPath) key.argument()); - if (fileType != FileType.INTERNAL) { - if (fileType != FileType.EXTERNAL_IN_MANAGED_DIRECTORY) { - BugReport.sendBugReport( - "File %s had unexpected type %s under one of package roots %s", - key, fileType, missingDiffPackageRoots); - } - DirtyResult result = super.check(key, oldValue, SyscallCache.NO_CACHE, tsgm); - // Don't populate Skyframe with new value if it might change. - return result.isDirty() ? DirtyResult.dirty() : result; - } - return super.check(key, oldValue, syscallCache, tsgm); - } } /** @@ -194,7 +170,6 @@ private static boolean isCacheableType(FileType fileType) { case EXTERNAL: case BUNDLED: return true; - case EXTERNAL_IN_MANAGED_DIRECTORY: case EXTERNAL_REPO: case OUTPUT: return false; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java index a4aebf24a5a89c..ca8293f9c6d736 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ExternalFilesHelper.java @@ -51,62 +51,40 @@ public class ExternalFilesHelper { private boolean tooManyNonOutputExternalFilesSeen = false; private boolean anyFilesInExternalReposSeen = false; - // This is a set of external files that are not in managed directories or - // external repositories. + // This is a set of external files that are not in external repositories. private Set nonOutputExternalFilesSeen = Sets.newConcurrentHashSet(); - private final ManagedDirectoriesKnowledge managedDirectoriesKnowledge; private ExternalFilesHelper( AtomicReference pkgLocator, ExternalFileAction externalFileAction, BlazeDirectories directories, - int maxNumExternalFilesToLog, - ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { + int maxNumExternalFilesToLog) { this.pkgLocator = pkgLocator; this.externalFileAction = externalFileAction; this.directories = directories; this.maxNumExternalFilesToLog = maxNumExternalFilesToLog; - this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; } public static ExternalFilesHelper create( AtomicReference pkgLocator, ExternalFileAction externalFileAction, - BlazeDirectories directories, - ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { + BlazeDirectories directories) { return TestType.isInTest() - ? createForTesting(pkgLocator, externalFileAction, directories, managedDirectoriesKnowledge) + ? createForTesting(pkgLocator, externalFileAction, directories) : new ExternalFilesHelper( - pkgLocator, - externalFileAction, - directories, - /*maxNumExternalFilesToLog=*/ 100, - managedDirectoriesKnowledge); + pkgLocator, externalFileAction, directories, /*maxNumExternalFilesToLog=*/ 100); } public static ExternalFilesHelper createForTesting( AtomicReference pkgLocator, ExternalFileAction externalFileAction, BlazeDirectories directories) { - return createForTesting( - pkgLocator, - externalFileAction, - directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES); - } - - private static ExternalFilesHelper createForTesting( - AtomicReference pkgLocator, - ExternalFileAction externalFileAction, - BlazeDirectories directories, - ManagedDirectoriesKnowledge managedDirectoriesKnowledge) { return new ExternalFilesHelper( pkgLocator, externalFileAction, directories, // These log lines are mostly spam during unit and integration tests. - /*maxNumExternalFilesToLog=*/ 0, - managedDirectoriesKnowledge); + /*maxNumExternalFilesToLog=*/ 0); } @@ -144,8 +122,8 @@ public enum FileType { INTERNAL, /** - * A non {@link #EXTERNAL_REPO} or {@link #EXTERNAL_IN_MANAGED_DIRECTORY} path outside the - * package roots about which we may make no other assumptions. + * A non {@link #EXTERNAL_REPO} path outside the package roots about which we may make no other + * assumptions. */ EXTERNAL, @@ -175,17 +153,6 @@ public enum FileType { * RepositoryDirectoryValue is computed. */ EXTERNAL_REPO, - - /** - * A path is under one of the managed directories. Managed directories are user-owned - * directories, which can be incrementally updated by repository rules, so that the updated - * files are visible for the actions in the same build. - * - *

Every such path under the managed directory is generated or updated by the execution of - * the corresponding repository rule, so these paths should not be cached by Skyframe before the - * RepositoryDirectoryValue is computed. {@link ManagedDirectoriesKnowledge} - */ - EXTERNAL_IN_MANAGED_DIRECTORY, } /** @@ -232,11 +199,7 @@ void setExternalFilesKnowledge(ExternalFilesKnowledge externalFilesKnowledge) { ExternalFilesHelper cloneWithFreshExternalFilesKnowledge() { return new ExternalFilesHelper( - pkgLocator, - externalFileAction, - directories, - maxNumExternalFilesToLog, - managedDirectoriesKnowledge); + pkgLocator, externalFileAction, directories, maxNumExternalFilesToLog); } public FileType getAndNoteFileType(RootedPath rootedPath) { @@ -244,12 +207,6 @@ public FileType getAndNoteFileType(RootedPath rootedPath) { } private Pair getFileTypeAndRepository(RootedPath rootedPath) { - RepositoryName repositoryName = - managedDirectoriesKnowledge.getOwnerRepository(rootedPath.getRootRelativePath()); - if (repositoryName != null) { - anyFilesInExternalReposSeen = true; - return Pair.of(FileType.EXTERNAL_IN_MANAGED_DIRECTORY, repositoryName); - } FileType fileType = detectFileType(rootedPath); if (FileType.EXTERNAL == fileType) { if (nonOutputExternalFilesSeen.size() >= MAX_EXTERNAL_FILES_TO_TRACK) { @@ -309,12 +266,6 @@ FileType maybeHandleExternalFile(RootedPath rootedPath, SkyFunction.Environment FileType fileType = Preconditions.checkNotNull(pair.getFirst()); switch (fileType) { - case EXTERNAL_IN_MANAGED_DIRECTORY: - Preconditions.checkState( - externalFileAction == ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, - externalFileAction); - RepositoryFunction.addManagedDirectoryDependencies(pair.getSecond(), env); - break; case BUNDLED: case INTERNAL: break; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java index cba69ee10faa8e..8b534e254fb116 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateFunction.java @@ -60,8 +60,7 @@ public FileStateValue compute(SkyKey skyKey, Environment env) if (env.valuesMissing()) { return null; } - if (fileType == FileType.EXTERNAL_REPO - || fileType == FileType.EXTERNAL_IN_MANAGED_DIRECTORY) { + if (fileType == FileType.EXTERNAL_REPO) { // do not use syscallCache as files under repositories get generated during the build return FileStateValue.create(rootedPath, SyscallCache.NO_CACHE, tsgm.get()); } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ManagedDirectoriesKnowledge.java b/src/main/java/com/google/devtools/build/lib/skyframe/ManagedDirectoriesKnowledge.java deleted file mode 100644 index 6ec3b0bebf8aa3..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ManagedDirectoriesKnowledge.java +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright 2019 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.common.collect.ImmutableSet; -import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.packages.WorkspaceFileValue; -import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.vfs.PathFragment; -import javax.annotation.Nullable; - -/** - * Managed directories are user-owned directories, which can be incrementally updated by repository - * rules, so that the updated files are visible for the actions in the same build. - * - *

File and directory nodes inside managed directories depend on the relevant {@link - * com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue} node, so that they are - * invalidated if the repository definition changes and recomputed based on the contents written by - * the repository rule. - * - *

The repository rule will also be re-run if any of the directories it manages is entirely - * missing. This allows a user to fairly cleanly regenerate managed directories. Whether this is an - * actually used feature is unknown to the Bazel developers. - */ -public interface ManagedDirectoriesKnowledge { - ManagedDirectoriesKnowledge NO_MANAGED_DIRECTORIES = - new ManagedDirectoriesKnowledge() { - @Override - public boolean workspaceHeaderReloaded( - @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) { - return false; - } - - @Nullable - @Override - public RepositoryName getOwnerRepository(PathFragment relativePathFragment) { - return null; - } - - @Override - public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { - return ImmutableSet.of(); - } - }; - - /** - * Returns true if the multi-mapping of repository -> {managed directory} changes. Such changes - * trigger a full wipe of the Skyframe graph, similar to a --package_path flag change. - */ - boolean workspaceHeaderReloaded( - @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) - throws AbruptExitException; - - /** - * Returns the owning repository for the incrementally updated path, or null. - * - * @param relativePathFragment path to check, relative to workspace root - * @return RepositoryName or null if there is no owning repository - */ - @Nullable - RepositoryName getOwnerRepository(PathFragment relativePathFragment); - - /** Returns managed directories for the passed repository. */ - ImmutableSet getManagedDirectories(RepositoryName repositoryName); -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java index 4d33299e12f9c0..57021851d60ef3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -31,7 +31,6 @@ import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.actions.ActionKeyContext; import com.google.devtools.build.lib.actions.CommandLineExpansionException; -import com.google.devtools.build.lib.actions.FileStateType; import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.AspectValue; @@ -44,7 +43,6 @@ import com.google.devtools.build.lib.bazel.repository.RepositoryOptions; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; -import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.concurrent.Uninterruptibles; import com.google.devtools.build.lib.events.Event; @@ -58,17 +56,12 @@ import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; -import com.google.devtools.build.lib.packages.WorkspaceFileValue; -import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey; import com.google.devtools.build.lib.pkgcache.PackageOptions; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.repository.ExternalPackageHelper; -import com.google.devtools.build.lib.server.FailureDetails; -import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.server.FailureDetails.Workspaces; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.DiffAwarenessManager.ProcessableModifiedFileSet; import com.google.devtools.build.lib.skyframe.DirtinessCheckerUtils.BasicFilesystemDirtinessChecker; @@ -83,8 +76,6 @@ import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.rewinding.RewindableGraphInconsistencyReceiver; import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.DetailedExitCode; -import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.ResourceUsage; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; @@ -207,9 +198,6 @@ private SequencedSkyframeExecutor( /*shouldUnblockCpuWorkWhenFetchingDeps=*/ false, new PackageProgressReceiver(), new ConfiguredTargetProgressReceiver(), - repositoryHelpersHolder == null - ? null - : repositoryHelpersHolder.managedDirectoriesKnowledge(), skyKeyStateReceiver, bugReporter); this.diffAwarenessManager = new DiffAwarenessManager(diffAwarenessFactories); @@ -391,15 +379,6 @@ private WorkspaceInfoFromDiff handleDiffs( modifiedFiles.set(0); numSourceFilesCheckedBecauseOfMissingDiffs = 0; - // Read the WORKSPACE file header, but only invalidate if tracking incremental state. Otherwise, - // this command started with a fresh graph already, and attempting to invalidate edgeless nodes - // results in a crash. - boolean managedDirectoriesChanged = - repositoryHelpersHolder != null && refreshWorkspaceHeader(eventHandler); - if (managedDirectoriesChanged && trackIncrementalState) { - invalidateCachedWorkspacePathsStates(); - } - WorkspaceInfoFromDiff workspaceInfo = null; Map modifiedFilesByPathEntry = Maps.newHashMap(); @@ -422,8 +401,7 @@ private WorkspaceInfoFromDiff handleDiffs( } BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class); int fsvcThreads = buildRequestOptions == null ? 200 : buildRequestOptions.fsvcThreads; - handleDiffsWithCompleteDiffInformation( - tsgm, modifiedFilesByPathEntry, managedDirectoriesChanged, fsvcThreads); + handleDiffsWithCompleteDiffInformation(tsgm, modifiedFilesByPathEntry, fsvcThreads); RepositoryOptions repoOptions = options.getOptions(RepositoryOptions.class); handleDiffsWithMissingDiffInformation( eventHandler, @@ -431,42 +409,11 @@ private WorkspaceInfoFromDiff handleDiffs( pathEntriesWithoutDiffInformation, options.getOptions(PackageOptions.class).checkOutputFiles, repoOptions == null || repoOptions.checkExternalRepositoryFiles, - managedDirectoriesChanged, fsvcThreads); handleClientEnvironmentChanges(); return workspaceInfo; } - /** - * Skyframe caches the states of files (FileStateValue) and directory listings - * (DirectoryListingStateValue). In the case when the files/directories are under a managed - * directory or inside an external repository, evaluation of file/directory listing states - * requires that RepositoryDirectoryValue of the owning external repository is evaluated - * beforehand. (So that the repository rule generates the files.) So there is a dependency on - * RepositoryDirectoryValue for files under managed directories and external repositories. This - * dependency is recorded by Skyframe. - * - *

From the other side, by default Skyframe injects the new values of changed files already at - * the stage of checking what files have changed. Only values without any dependencies can be - * injected into Skyframe. - * - *

When the values of managed directories change, whether a file is under a managed directory - * can change. This implies that corresponding file/directory listing states gain the dependency - * (RepositoryDirectoryValue) or they lose this dependency. In both cases, we should prevent - * Skyframe from injecting those new values of file/directory listing states at the stage of - * checking changed files, because the files have not been generated yet. - * - *

The selected approach is to invalidate PACKAGE_LOCATOR_DEPENDENT_VALUES, which includes - * invalidating all cached file/directory listing state values. Additionally, no new - * FileStateValues and DirectoryStateValues should be injected. - */ - private void invalidateCachedWorkspacePathsStates() { - logger.atInfo().log( - "Invalidating cached packages and paths states" - + " because managed directories configuration has changed."); - invalidate(SkyFunctionName.functionIsIn(PACKAGE_LOCATOR_DEPENDENT_VALUES)); - } - /** Invalidates entries in the client environment. */ private void handleClientEnvironmentChanges() { // Remove deleted client environmental variables. @@ -496,7 +443,6 @@ private void handleClientEnvironmentChanges() { private void handleDiffsWithCompleteDiffInformation( TimestampGranularityMonitor tsgm, Map modifiedFilesByPathEntry, - boolean managedDirectoriesChanged, int fsvcThreads) throws InterruptedException, AbruptExitException { for (Root pathEntry : ImmutableSet.copyOf(modifiedFilesByPathEntry.keySet())) { @@ -507,8 +453,7 @@ private void handleDiffsWithCompleteDiffInformation( handleChangedFiles( ImmutableList.of(pathEntry), getDiff(tsgm, modifiedFileSet, pathEntry, fsvcThreads), - /*numSourceFilesCheckedIfDiffWasMissing=*/ 0, - managedDirectoriesChanged); + /*numSourceFilesCheckedIfDiffWasMissing=*/ 0); processableModifiedFileSet.markProcessed(); } } @@ -523,7 +468,6 @@ private void handleDiffsWithMissingDiffInformation( Set> pathEntriesWithoutDiffInformation, boolean checkOutputFiles, boolean checkExternalRepositoryFiles, - boolean managedDirectoriesChanged, int fsvcThreads) throws InterruptedException { @@ -571,8 +515,7 @@ private void handleDiffsWithMissingDiffInformation( Iterables.concat( dirtinessCheckers, ImmutableList.of( - new MissingDiffDirtinessChecker( - diffPackageRootsUnderWhichToCheck, tmpExternalFilesHelper))); + new MissingDiffDirtinessChecker(diffPackageRootsUnderWhichToCheck))); } if (checkExternalRepositoryFiles && repositoryHelpersHolder != null) { dirtinessCheckers = @@ -581,8 +524,7 @@ private void handleDiffsWithMissingDiffInformation( ImmutableList.of(repositoryHelpersHolder.repositoryDirectoryDirtinessChecker())); } if (checkExternalRepositoryFiles) { - fileTypesToCheck = - EnumSet.of(FileType.EXTERNAL_REPO, FileType.EXTERNAL_IN_MANAGED_DIRECTORY); + fileTypesToCheck = EnumSet.of(FileType.EXTERNAL_REPO); } if (externalFilesKnowledge.tooManyNonOutputExternalFilesSeen || !externalFilesKnowledge.nonOutputExternalFilesSeen.isEmpty()) { @@ -614,8 +556,7 @@ private void handleDiffsWithMissingDiffInformation( handleChangedFiles( diffPackageRootsUnderWhichToCheck, batchDirtyResult, - /*numSourceFilesCheckedIfDiffWasMissing=*/ batchDirtyResult.getNumKeysChecked(), - managedDirectoriesChanged); + /*numSourceFilesCheckedIfDiffWasMissing=*/ batchDirtyResult.getNumKeysChecked()); // We use the knowledge gained during the graph scan that just completed. Otherwise, naively, // once an external file gets into the Skyframe graph, we'll overly-conservatively always // think the graph needs to be scanned. @@ -648,10 +589,7 @@ private void handleDiffsWithMissingDiffInformation( new ExternalDirtinessChecker(externalFilesHelper, EnumSet.of(FileType.EXTERNAL))); } handleChangedFiles( - ImmutableList.of(), - batchDirtyResult, - batchDirtyResult.getNumKeysChecked(), - /*managedDirectoriesChanged=*/ false); + ImmutableList.of(), batchDirtyResult, batchDirtyResult.getNumKeysChecked()); } for (Pair pair : pathEntriesWithoutDiffInformation) { @@ -662,21 +600,11 @@ private void handleDiffsWithMissingDiffInformation( private void handleChangedFiles( Collection diffPackageRootsUnderWhichToCheck, Differencer.Diff diff, - int numSourceFilesCheckedIfDiffWasMissing, - boolean managedDirectoriesChanged) { + int numSourceFilesCheckedIfDiffWasMissing) { int numWithoutNewValues = diff.changedKeysWithoutNewValues().size(); Iterable keysToBeChangedLaterInThisBuild = diff.changedKeysWithoutNewValues(); Map changedKeysWithNewValues = diff.changedKeysWithNewValues(); - // If managed directories settings changed, do not inject any new values, just invalidate - // keys of the changed values. {@link #invalidateCachedWorkspacePathsStates()} - if (managedDirectoriesChanged) { - numWithoutNewValues += changedKeysWithNewValues.size(); - keysToBeChangedLaterInThisBuild = - Iterables.concat(keysToBeChangedLaterInThisBuild, changedKeysWithNewValues.keySet()); - changedKeysWithNewValues = ImmutableMap.of(); - } - logDiffInfo( diffPackageRootsUnderWhichToCheck, keysToBeChangedLaterInThisBuild, @@ -1030,86 +958,6 @@ protected void dumpPackages(PrintStream out) { } } - /** - * Calculate the new value of the WORKSPACE file header (WorkspaceFileValue with the index = 0), - * and call the listener, if the value has changed. Needed for incremental update of user-owned - * directories by repository rules. - */ - private boolean refreshWorkspaceHeader(ExtendedEventHandler eventHandler) - throws InterruptedException, AbruptExitException { - Root workspaceRoot = Root.fromPath(directories.getWorkspace()); - RootedPath workspacePath = - RootedPath.toRootedPath(workspaceRoot, LabelConstants.WORKSPACE_FILE_NAME); - WorkspaceFileKey workspaceFileKey = WorkspaceFileValue.key(workspacePath); - - WorkspaceFileValue oldValue = - (WorkspaceFileValue) memoizingEvaluator.getExistingValue(workspaceFileKey); - FileStateValue newFileStateValue = maybeInvalidateWorkspaceFileStateValue(workspacePath); - WorkspaceFileValue newValue = - (WorkspaceFileValue) evaluateSingleValue(workspaceFileKey, eventHandler); - if (newValue != null - && !newValue.getManagedDirectories().isEmpty() - && FileStateType.SYMLINK.equals(newFileStateValue.getType())) { - throw new AbruptExitException( - DetailedExitCode.of( - ExitCode.PARSING_FAILURE, - FailureDetail.newBuilder() - .setMessage( - "WORKSPACE file can not be a symlink if incrementally updated directories" - + " are used.") - .setWorkspaces( - Workspaces.newBuilder() - .setCode( - FailureDetails.Workspaces.Code - .ILLEGAL_WORKSPACE_FILE_SYMLINK_WITH_MANAGED_DIRECTORIES)) - .build())); - } - return repositoryHelpersHolder - .managedDirectoriesKnowledge() - .workspaceHeaderReloaded(oldValue, newValue); - } - - // We only check the FileStateValue of the WORKSPACE file; we do not support the case - // when the WORKSPACE file is a symlink. - private FileStateValue maybeInvalidateWorkspaceFileStateValue(RootedPath workspacePath) - throws InterruptedException, AbruptExitException { - SkyKey workspaceFileStateKey = FileStateValue.key(workspacePath); - SkyValue oldWorkspaceFileState = memoizingEvaluator.getExistingValue(workspaceFileStateKey); - FileStateValue newWorkspaceFileState; - try { - newWorkspaceFileState = - FileStateValue.create(workspacePath, perCommandSyscallCache, tsgm.get()); - } catch (IOException e) { - throw new AbruptExitException( - DetailedExitCode.of( - ExitCode.PARSING_FAILURE, - FailureDetail.newBuilder() - .setMessage("Can not read WORKSPACE file.") - .setWorkspaces( - Workspaces.newBuilder() - .setCode( - FailureDetails.Workspaces.Code - .WORKSPACE_FILE_READ_FAILURE_WITH_MANAGED_DIRECTORIES)) - .build()), - e); - } - if (oldWorkspaceFileState != null && !oldWorkspaceFileState.equals(newWorkspaceFileState)) { - recordingDiffer.invalidate(ImmutableSet.of(workspaceFileStateKey)); - } - return newWorkspaceFileState; - } - - private SkyValue evaluateSingleValue(SkyKey key, ExtendedEventHandler eventHandler) - throws InterruptedException { - EvaluationContext evaluationContext = - newEvaluationContextBuilder() - .setKeepGoing(false) - .setNumThreads(DEFAULT_THREAD_COUNT) - .setEventHandler(eventHandler) - .build(); - return memoizingEvaluator.evaluate(ImmutableSet.of(key), evaluationContext).get(key); - } - public static Builder builder() { return new Builder(); } 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 7ce5c0a94e38b2..9285981072ac7b 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 @@ -418,7 +418,6 @@ protected SkyframeExecutor( boolean shouldUnblockCpuWorkWhenFetchingDeps, @Nullable PackageProgressReceiver packageProgress, @Nullable ConfiguredTargetProgressReceiver configuredTargetProgress, - @Nullable ManagedDirectoriesKnowledge managedDirectoriesKnowledge, SkyKeyStateReceiver skyKeyStateReceiver, BugReporter bugReporter) { // Strictly speaking, these arguments are not required for initialization, but all current @@ -462,13 +461,7 @@ protected SkyframeExecutor( this.skyframeBuildView = new SkyframeBuildView(artifactFactory, this, ruleClassProvider, actionKeyContext); this.externalFilesHelper = - ExternalFilesHelper.create( - pkgLocator, - externalFileAction, - directories, - managedDirectoriesKnowledge != null - ? managedDirectoriesKnowledge - : ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES); + ExternalFilesHelper.create(pkgLocator, externalFileAction, directories); this.crossRepositoryLabelViolationStrategy = crossRepositoryLabelViolationStrategy; this.buildFilesByPriority = buildFilesByPriority; this.externalPackageHelper = externalPackageHelper; diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorRepositoryHelpersHolder.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorRepositoryHelpersHolder.java index a8584787d8604f..f61f7220aba383 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorRepositoryHelpersHolder.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorRepositoryHelpersHolder.java @@ -20,14 +20,11 @@ /** Provides repository-related objects for use by {@link SequencedSkyframeExecutor}. */ @AutoValue public abstract class SkyframeExecutorRepositoryHelpersHolder { - public abstract ManagedDirectoriesKnowledge managedDirectoriesKnowledge(); - public abstract RepositoryDirectoryDirtinessChecker repositoryDirectoryDirtinessChecker(); public static SkyframeExecutorRepositoryHelpersHolder create( - ManagedDirectoriesKnowledge managedDirectoriesKnowledge, RepositoryDirectoryDirtinessChecker repositoryDirectoryDirtinessChecker) { return new AutoValue_SkyframeExecutorRepositoryHelpersHolder( - managedDirectoriesKnowledge, repositoryDirectoryDirtinessChecker); + repositoryDirectoryDirtinessChecker); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java index 3ea17ab4e7f0ec..37ddb85bcac376 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java @@ -250,8 +250,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) /* bindings = */ ImmutableMap.of(), workspaceFile, /* idx = */ 0, // first fragment - /* hasNext = */ false, - ImmutableMap.of()); + /* hasNext = */ false); } // Get the state at the end of the previous chunk. @@ -354,8 +353,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) parser.getVariableBindings(), workspaceFile, key.getIndex(), - key.getIndex() < chunks.size() - 1, - ImmutableMap.copyOf(parser.getManagedDirectories())); + key.getIndex() < chunks.size() - 1); } private static StarlarkFile parseWorkspaceFile( 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 b69533c7993f7f..8f0e64814a02b6 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 @@ -66,7 +66,6 @@ import com.google.devtools.build.lib.skyframe.FileFunction; import com.google.devtools.build.lib.skyframe.FileStateFunction; import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PackageFunction; import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageFunction.GlobbingStrategy; @@ -246,11 +245,7 @@ protected void validate() { public final PackageLoader build() { validate(); externalFilesHelper = - ExternalFilesHelper.create( - pkgLocatorRef, - externalFileAction, - directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES); + ExternalFilesHelper.create(pkgLocatorRef, externalFileAction, directories); return buildImpl(); } 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 bf0855e5cfee13..f704ea2d2da5e2 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 @@ -48,7 +48,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:package_value", "//src/main/java/com/google/devtools/build/lib/skyframe:per_build_syscall_cache", @@ -87,7 +86,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:action_environment_function", "//src/main/java/com/google/devtools/build/lib/skyframe:client_environment_function", "//src/main/java/com/google/devtools/build/lib/skyframe:directory_listing_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java index 3cff84dd4d8939..2f1cf6c404b6e6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/BazelPackageLoader.java @@ -34,7 +34,6 @@ import com.google.devtools.build.lib.skyframe.DirectoryListingStateFunction; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.LocalRepositoryLookupFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PackageFunction.ActionOnIOExceptionReadingBuildFile; import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy; import com.google.devtools.build.lib.skyframe.PrecomputedValue; @@ -130,7 +129,6 @@ public BazelPackageLoader buildImpl() { isFetch, ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, EXTERNAL_PACKAGE_HELPER)) .build()); diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/WorkspaceGlobalsApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/WorkspaceGlobalsApi.java index deae557f0ae1bd..93d27f9c9201c2 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/WorkspaceGlobalsApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/WorkspaceGlobalsApi.java @@ -19,7 +19,6 @@ import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; import net.starlark.java.annot.StarlarkMethod; -import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.NoneType; import net.starlark.java.eval.Sequence; @@ -62,24 +61,9 @@ public interface WorkspaceGlobalsApi { + "letters, numbers, underscores, dashes, and dots.", named = true, positional = false), - @Param( - name = "managed_directories", - named = true, - positional = false, - defaultValue = "{}", - doc = - "Dict (strings to list of strings) for defining the mappings between external" - + " repositories and relative (to the workspace root) paths to directories" - + " they incrementally update." - + "\nManaged directories must be excluded from the source tree by listing" - + " them (or their parent directories) in the .bazelignore file."), }, useStarlarkThread = true) - void workspace( - String name, - Dict managedDirectories, // > - StarlarkThread thread) - throws EvalException, InterruptedException; + void workspace(String name, StarlarkThread thread) throws EvalException, InterruptedException; @StarlarkMethod( name = "register_execution_platforms", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java index 031365543e9d7b..be9468829a1f72 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisMock.java @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.packages.PackageFactoryBuilderWithSkyframeForTesting; import com.google.devtools.build.lib.testutil.TestConstants; @@ -135,7 +134,6 @@ public ImmutableMap getSkyFunctions(BlazeDirectori new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER), SkyFunctions.MODULE_FILE, new ModuleFileFunction( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD index cc5d5ff490c432..4c9bf1c647648b 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD @@ -105,13 +105,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key", "//src/main/java/com/google/devtools/build/lib/skyframe:diff_awareness", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_roots_no_symlink_creation", "//src/main/java/com/google/devtools/build/lib/skyframe:package_value", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", - "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_executor_repository_helpers_holder", "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value", "//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value", "//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_exception", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index cf0fbf74926d93..4de6844534dd2d 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -146,7 +146,6 @@ import com.google.devtools.build.lib.pkgcache.PackageOptions; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction; -import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryDirtinessChecker; import com.google.devtools.build.lib.skyframe.AspectKeyCreator; import com.google.devtools.build.lib.skyframe.AspectKeyCreator.AspectKey; import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; @@ -156,14 +155,12 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import com.google.devtools.build.lib.skyframe.DiffAwareness; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PackageFunction; import com.google.devtools.build.lib.skyframe.PackageRootsNoSymlinkCreation; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.SkyframeExecutor; -import com.google.devtools.build.lib.skyframe.SkyframeExecutorRepositoryHelpersHolder; import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue; import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue; import com.google.devtools.build.lib.testutil.FoundationTestCase; @@ -325,14 +322,6 @@ RepositoryDelegatorFunction.ENABLE_BZLMOD, enableBzlmod())) .setExtraSkyFunctions(analysisMock.getSkyFunctions(directories)) .setPerCommandSyscallCache(SyscallCache.NO_CACHE) .setDiffAwarenessFactories(diffAwarenessFactories); - ManagedDirectoriesKnowledge managedDirectoriesKnowledge = getManagedDirectoriesKnowledge(); - if (managedDirectoriesKnowledge != null) { - builder.setRepositoryHelpersHolder( - SkyframeExecutorRepositoryHelpersHolder.create( - managedDirectoriesKnowledge, - new RepositoryDirectoryDirtinessChecker( - directories.getWorkspace(), managedDirectoriesKnowledge))); - } skyframeExecutor = builder.build(); if (usesInliningBzlLoadFunction()) { injectInliningBzlLoadFunction(skyframeExecutor, pkgFactory, directories); @@ -425,10 +414,6 @@ protected final ConfiguredRuleClassProvider getRuleClassProvider() { return ruleClassProvider; } - protected ManagedDirectoriesKnowledge getManagedDirectoriesKnowledge() { - return null; - } - protected final PackageFactory getPackageFactory() { return pkgFactory; } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 7f3fd0fe74689e..c8f6d644932059 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -53,7 +53,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java index 4d4e12269eed3d..647d787d9a6c89 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java @@ -44,7 +44,6 @@ import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.FileFunction; import com.google.devtools.build.lib.skyframe.FileStateFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -174,7 +173,6 @@ private void setUpWithBuiltinModules(ImmutableMap b new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index 5ebe10cdf99841..18a80ea084e5e7 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.skyframe.FileStateFunction; import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesFunction; import com.google.devtools.build.lib.skyframe.LocalRepositoryLookupFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PackageFunction; import com.google.devtools.build.lib.skyframe.PackageFunction.GlobbingStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; @@ -225,7 +224,6 @@ public void setup() throws Exception { new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 5a390b55b9c2c3..25c70c518ba8b4 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -45,7 +45,6 @@ import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.skyframe.FileFunction; import com.google.devtools.build.lib.skyframe.FileStateFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PrecomputedFunction; import com.google.devtools.build.lib.skyframe.PrecomputedValue; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -144,7 +143,6 @@ private void setUpWithBuiltinModules(ImmutableMap b new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) .put( BzlmodRepoRuleValue.BZLMOD_REPO_RULE, 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 ac60a64711faef..473c28680f825b 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 @@ -29,7 +29,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", @@ -75,7 +74,6 @@ java_test( "//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_and_data", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions", "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java index 8968e41573dc03..7719a9f8eb4628 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryIntegrationTest.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.rules.repository.RepositoryFunction; import com.google.devtools.build.lib.skyframe.BazelSkyframeExecutorConstants; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryBootstrap; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; @@ -58,7 +57,7 @@ public class StarlarkRepositoryIntegrationTest extends BuildViewTestCase { /** * Proxy to the real analysis mock to overwrite {@code #getSkyFunctions(BlazeDirectories)} to * inject the StarlarkRepositoryFunction in the list of SkyFunctions. In Bazel, this function is - * injected by the corresponding @{code BlazeModule}. + * injected by the corresponding {@code BlazeModule}. */ private static class CustomAnalysisMock extends AnalysisMock.Delegate { CustomAnalysisMock(AnalysisMock proxied) { @@ -84,7 +83,6 @@ public ImmutableMap getSkyFunctions( new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER); return ImmutableMap.of(SkyFunctions.REPOSITORY_DIRECTORY, function); } diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD b/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD index 7cfaaf3463bb3c..2978f6fae14680 100644 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD +++ b/src/test/java/com/google/devtools/build/lib/blackbox/tests/BUILD @@ -22,25 +22,6 @@ java_library( ], ) -java_test( - name = "ManagedDirectoriesBlackBoxTest", - timeout = "moderate", - srcs = ["manageddirs/ManagedDirectoriesBlackBoxTest.java"], - resources = [ - "manageddirs/BUILD.test", - "manageddirs/WORKSPACE.test", - "manageddirs/bazelignore.test", - "manageddirs/package.json", - "manageddirs/test_rule.bzl", - "manageddirs/use_node_modules.bzl", - ], - tags = ["black_box_test"], - deps = [ - ":common_deps", - "@com_google_testparameterinjector//:testparameterinjector", - ], -) - java_test( name = "PythonBlackBoxTest", timeout = "moderate", @@ -56,7 +37,6 @@ test_suite( name = "black_box_tests", tags = ["black_box_test"], tests = [ - "ManagedDirectoriesBlackBoxTest", "PythonBlackBoxTest", "//src/test/java/com/google/devtools/build/lib/blackbox/framework:framework_tests", "//src/test/java/com/google/devtools/build/lib/blackbox/junit:TimeoutTestWatcherTest", diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test deleted file mode 100644 index 44b185192f6d00..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/BUILD.test +++ /dev/null @@ -1,20 +0,0 @@ -# Copyright 2019 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. -load(":test_rule.bzl", "test_rule") - -test_rule( - name = "test_generated_deps", - module_source = "@generated_node_modules//:example-module", - version = "0.2.0", -) diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java deleted file mode 100644 index 924155672eab70..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java +++ /dev/null @@ -1,506 +0,0 @@ -// Copyright 2019 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.blackbox.tests.manageddirs; - -import static com.google.common.truth.Truth.assertThat; - -import com.google.devtools.build.lib.blackbox.framework.BuilderRunner; -import com.google.devtools.build.lib.blackbox.framework.PathUtils; -import com.google.devtools.build.lib.blackbox.framework.ProcessResult; -import com.google.devtools.build.lib.blackbox.junit.AbstractBlackBoxTest; -import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.ResourceFileLoader; -import com.google.testing.junit.testparameterinjector.TestParameter; -import com.google.testing.junit.testparameterinjector.TestParameterInjector; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.Arrays; -import java.util.List; -import java.util.Random; -import org.junit.Assume; -import org.junit.Test; -import org.junit.runner.RunWith; - -/** Tests for managed directories. */ -@RunWith(TestParameterInjector.class) -public final class ManagedDirectoriesBlackBoxTest extends AbstractBlackBoxTest { - - // Flip to true to use --host_jvm_debug for all bazel commands. - private static final boolean DEBUG = false; - - private final Random random = new Random(17); - private Integer currentDebugId; - - @Test - public void testBuildProject(@TestParameter boolean trackIncrementalState) throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(/*watchFs=*/ false, trackIncrementalState); - checkProjectFiles(); - } - - @Test - public void testNodeModulesDeleted(@TestParameter boolean watchFs) throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - - Path nodeModules = context().getWorkDir().resolve("node_modules"); - assertThat(nodeModules.toFile().isDirectory()).isTrue(); - PathUtils.deleteTree(nodeModules); - - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - } - - @Test - public void testNodeModulesDeletedAndRecreated() throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - - Path nodeModules = context().getWorkDir().resolve("node_modules"); - assertThat(nodeModules.toFile().isDirectory()).isTrue(); - - Path nodeModulesBackup = context().getWorkDir().resolve("node_modules_backup"); - PathUtils.copyTree(nodeModules, nodeModulesBackup); - PathUtils.deleteTree(nodeModules); - - PathUtils.copyTree(nodeModulesBackup, nodeModules); - - buildExpectRepositoryRuleNotCalled(); - checkProjectFiles(); - } - - @Test - public void testBuildProjectFetchNotRecalled() throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - buildExpectRepositoryRuleNotCalled(); - checkProjectFiles(); - } - - private BuilderRunner bazel() { - return bazel(/*watchFs=*/ false); - } - - private BuilderRunner bazel(boolean watchFs) { - return bazel(watchFs, /*trackIncrementalState=*/ true); - } - - private BuilderRunner bazel(boolean watchFs, boolean trackIncrementalState) { - currentDebugId = random.nextInt(); - BuilderRunner bazel = - context() - .bazel() - .withEnv("DEBUG_ID", String.valueOf(currentDebugId)) - .withFlags("--noincompatible_disable_managed_directories") - .withFlags( - "--watchfs=" + watchFs, "--track_incremental_state=" + trackIncrementalState); - if (DEBUG) { - bazel.enableDebug(); - } - return bazel; - } - - @Test - public void testChangeOfFileTextUnderNodeModules() throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - - Path nodeModules = context().getWorkDir().resolve("node_modules"); - Path modulePackageJson = nodeModules.resolve("example-module/package.json"); - assertThat(modulePackageJson.toFile().exists()).isTrue(); - - // Assert that non-structural changes are not detected. - PathUtils.append(modulePackageJson, "# comment"); - - buildExpectRepositoryRuleNotCalled(); - checkProjectFiles(); - } - - @Test - public void testLoadIsNotCalledForManagedDirectories() throws Exception { - generateProject(); - Path workspaceFile = context().getWorkDir().resolve(WORKSPACE); - PathUtils.append(workspaceFile, "load('@non_existing//:target.bzl', 'some_symbol')"); - - // Test that there is error when loading, so we parsed managed directories successfully. - ProcessResult result = bazel().shouldFail().build("//..."); - assertThat(findPattern(result, "ERROR: Failed to load Starlark extension")).isTrue(); - } - - @Test - public void testWithBazelTools() throws Exception { - generateProject(); - Path workspaceFile = context().getWorkDir().resolve(WORKSPACE); - PathUtils.append( - workspaceFile, - "load(\"@bazel_tools//tools/build_defs/repo:http.bzl\", \"http_archive\", \"http_file\")"); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - } - - @Test - public void testAddManagedDirectoriesLater() throws Exception { - // Start the server, have things cached. - context().write("BUILD", ""); - bazel().build("//..."); - - // Now that we generate the project and have managed directories updated, we are also testing, - // that managed directories are re-read correctly from the changed file. - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - - // Test everything got cached. - buildExpectRepositoryRuleNotCalled(); - checkProjectFiles(); - } - - @Test - public void testFilesUnderManagedDirectoriesRefreshed(@TestParameter boolean watchFs) - throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - - // Now remove the ManagedDirectories, and change the package version - it should still work. - List properWorkspaceText = context().read("WORKSPACE"); - - context() - .write( - "WORKSPACE", - "workspace(name = \"fine_grained_user_modules\")", - "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")", - "generate_fine_grained_node_modules(name = \"generated_node_modules\",", - "package_json = \"//:package.json\",)"); - Path packageJson = - PathUtils.resolve(context().getWorkDir(), "node_modules", "example-module", "package.json"); - assertThat(packageJson.toFile().exists()).isTrue(); - - // Now we are building it without managed directories, both managed directories and - // RepositoryDirectoryValue will be dirty - we expect repository rule to be called again. - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - - // Now change files directly in generated area, and build. - PathUtils.writeFile( - packageJson, - "{", - " \"license\": \"MIT\",", - " \"main\": \"example-module.js\",", - " \"name\": \"example-module\",", - " \"repository\": {", - " \"type\": \"git\",", - " \"url\": \"aaa\",", - " },", - " \"version\": \"7.7.7\"", - "}"); - Path build = context().getWorkDir().resolve("BUILD"); - List oldBuild = PathUtils.readFile(build); - PathUtils.writeFile( - build, - "load(\":test_rule.bzl\", \"test_rule\")", - "test_rule(", - " name = \"test_generated_deps\",", - " module_source = \"@generated_node_modules//:example-module\",", - " version = \"7.7.7\"", - ")"); - - // Test rule inputs has changed, so the build is not cached; however, the repository rule - // is not rerun, since it's inputs (including managed directories settings) were not changed, - // so debug_id is the same. - buildExpectRepositoryRuleNotCalled(); - checkProjectFiles("7.7.7"); - - // And is cached. - buildExpectRepositoryRuleNotCalled(); - - // Now change just the managed directories and see the generated version comes up. - PathUtils.writeFile( - context().getWorkDir().resolve(WORKSPACE), properWorkspaceText.toArray(new String[0])); - PathUtils.writeFile(build, oldBuild.toArray(new String[0])); - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles("0.2.0"); - } - - @Test - public void testManagedDirectoriesSettingsAndManagedDirectoriesFilesChangeSimultaneously( - @TestParameter boolean watchFs) throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - - // Modify managed directories somehow. - context() - .write( - "WORKSPACE", - "workspace(name = \"fine_grained_user_modules\",", - "managed_directories = {'@generated_node_modules': ['node_modules', 'something']})", - "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")", - "generate_fine_grained_node_modules(name = \"generated_node_modules\",", - "package_json = \"//:package.json\",)"); - Path packageJson = - PathUtils.resolve(context().getWorkDir(), "node_modules", "example-module", "package.json"); - assertThat(packageJson.toFile().exists()).isTrue(); - - // Modify generated package.json under the managed directory. - PathUtils.writeFile( - packageJson, - "{", - " \"license\": \"MIT\",", - " \"main\": \"example-module.js\",", - " \"name\": \"example-module\",", - " \"repository\": {", - " \"type\": \"git\",", - " \"url\": \"aaa\",", - " },", - " \"version\": \"7.7.7\"", - "}"); - // Expect files under managed directories be regenerated - // and changes under managed directories be lost. - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - } - - @Test - public void testRepositoryOverrideWithManagedDirectories() throws Exception { - generateProject(); - - Path override = context().getTmpDir().resolve("override"); - PathUtils.writeFile(override.resolve(WORKSPACE)); - // Just define some similar target. - PathUtils.writeFile( - override.resolve("BUILD"), - "genrule(", - " name = \"example-module\",", - " srcs = [],", - " cmd = \"touch $(location package.json)\",", - " outs = [\"package.json\"],", - " visibility = ['//visibility:public'],", - ")"); - - BuilderRunner bazel = - bazel().withFlags("--override_repository=generated_node_modules=" + override); - ProcessResult result = bazel.shouldFail().build("@generated_node_modules//:example-module"); - assertThat(result.errString()) - .contains( - "ERROR: Overriding repositories is not allowed" - + " for the repositories with managed directories." - + "\nThe following overridden external repositories" - + " have managed directories: @generated_node_modules"); - - // Assert the result stays the same even when managed directories has not changed. - result = bazel.shouldFail().build("@generated_node_modules//:example-module"); - assertThat(result.errString()) - .contains( - "ERROR: Overriding repositories is not allowed" - + " for the repositories with managed directories." - + "\nThe following overridden external repositories" - + " have managed directories: @generated_node_modules"); - } - - @Test - public void testRepositoryOverrideChangeToConflictWithManagedDirectories() throws Exception { - generateProject(); - buildExpectRepositoryRuleCalled(); - checkProjectFiles(); - - Path override = context().getTmpDir().resolve("override"); - PathUtils.writeFile(override.resolve(WORKSPACE)); - // Just define some similar target. - PathUtils.writeFile( - override.resolve("BUILD"), - "genrule(", - " name = \"example-module\",", - " srcs = [],", - " cmd = \"touch $(location package.json)\",", - " outs = [\"package.json\"],", - " visibility = ['//visibility:public'],", - ")"); - - // Now the overrides change. - BuilderRunner bazel = - bazel().withFlags("--override_repository=generated_node_modules=" + override); - ProcessResult result = bazel.shouldFail().build("@generated_node_modules//:example-module"); - assertThat(result.errString()) - .contains( - "ERROR: Overriding repositories is not allowed" - + " for the repositories with managed directories." - + "\nThe following overridden external repositories" - + " have managed directories: @generated_node_modules"); - } - - /** - * The test to verify that WORKSPACE file can not be a symlink when managed directories are used. - * - *

The test of the case, when WORKSPACE file is a symlink, but not managed directories are - * used, is in {@link WorkspaceBlackBoxTest#testWorkspaceFileIsSymlink()} - */ - @Test - public void testWorkspaceSymlinkThrowsWithManagedDirectories() throws Exception { - generateProject(); - - Path workspaceFile = context().getWorkDir().resolve(WORKSPACE); - assertThat(workspaceFile.toFile().delete()).isTrue(); - - Path tempWorkspace = Files.createTempFile(context().getTmpDir(), WORKSPACE, ""); - PathUtils.writeFile( - tempWorkspace, - "workspace(name = \"fine_grained_user_modules\",", - "managed_directories = {'@generated_node_modules': ['node_modules']})", - "", - "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")", - "", - "generate_fine_grained_node_modules(", - " name = \"generated_node_modules\",", - " package_json = \"//:package.json\",", - ")"); - Files.createSymbolicLink(workspaceFile, tempWorkspace); - - ProcessResult result = bazel().shouldFail().build("//..."); - assertThat( - findPattern( - result, - "WORKSPACE file can not be a symlink if incrementally updated directories are" - + " used.")) - .isTrue(); - } - - @Test - public void testNoCheckFiles(@TestParameter boolean allSkips, @TestParameter boolean watchFs) - throws Exception { - // On Darwin CI, --watchfs is nondeterministic if this passes or fails - Assume.assumeFalse(OS.DARWIN.equals(OS.getCurrent())); - - generateProject(); - buildExpectRepositoryRuleCalled(watchFs); - checkProjectFiles(); - - Path nodeModules = context().getWorkDir().resolve("node_modules"); - assertThat(nodeModules.toFile().isDirectory()).isTrue(); - PathUtils.deleteTree(nodeModules); - assertThat(nodeModules.toFile().isDirectory()).isFalse(); - - // As compared to testNodeModulesDeleted, we don't check that the external file disappeared with - // this flag so the build is broken - BuilderRunner bazel = - bazel(watchFs).withFlags("--noexperimental_check_external_repository_files"); - if (allSkips) { - bazel = bazel.withFlags("--noexperimental_check_output_files"); - } - - ProcessResult result = bazel.shouldFail().build("//..."); - assertThat(findPattern(result, "Not found package.json")).isTrue(); - - // it doesn't make the file on disk - assertThat(nodeModules.toFile().isDirectory()).isFalse(); - - // In a perfect world we would be able to fix the build by rebuilding here without the flags, - // but we don't - // invalidate the cache correctly so the server would have to shut down - } - - private void generateProject() throws IOException { - writeProjectFile("BUILD.test", "BUILD"); - writeProjectFile("WORKSPACE.test", "WORKSPACE"); - writeProjectFile("bazelignore.test", ".bazelignore"); - writeProjectFile("package.json", "package.json"); - writeProjectFile("test_rule.bzl", "test_rule.bzl"); - writeProjectFile("use_node_modules.bzl", "use_node_modules.bzl"); - } - - private void writeProjectFile(String oldName, String newName) throws IOException { - String text = ResourceFileLoader.loadResource(ManagedDirectoriesBlackBoxTest.class, oldName); - assertThat(text).isNotNull(); - assertThat(text).isNotEmpty(); - context().write(newName, text); - } - - private void checkProjectFiles() throws IOException { - checkProjectFiles("0.2.0"); - } - - private void checkProjectFiles(String version) throws IOException { - Path nodeModules = context().getWorkDir().resolve("node_modules"); - assertThat(nodeModules.toFile().exists()).isTrue(); - assertThat(nodeModules.toFile().isDirectory()).isTrue(); - - Path exampleModule = nodeModules.resolve("example-module"); - assertThat(exampleModule.toFile().exists()).isTrue(); - assertThat(exampleModule.toFile().isDirectory()).isTrue(); - - Path packageJson = exampleModule.resolve("package.json"); - assertThat(packageJson.toFile().exists()).isTrue(); - assertThat(packageJson.toFile().isDirectory()).isFalse(); - - List text = PathUtils.readFile(packageJson); - assertThat(text.stream().anyMatch(s -> s.trim().equals("\"name\": \"example-module\","))) - .isTrue(); - String versionString = String.format("\"version\": \"%s\"", version); - assertThat(text.stream().anyMatch(s -> s.trim().equals(versionString))).isTrue(); - } - - private String getDebugId(BuilderRunner bazel) throws Exception { - Path path = context().resolveExecRootPath(bazel, "external/generated_node_modules/debug_id"); - List lines = PathUtils.readFile(path); - assertThat(lines.size()).isEqualTo(1); - return lines.get(0); - } - - private void buildExpectRepositoryRuleCalled() throws Exception { - buildExpectRepositoryRuleCalled(/*watchFs=*/ false); - } - - private void buildExpectRepositoryRuleCalled(boolean watchFs) throws Exception { - buildExpectRepositoryRuleCalled(watchFs, /*trackIncrementalState=*/ true); - } - - private void buildExpectRepositoryRuleCalled(boolean watchFs, boolean trackIncrementalState) - throws Exception { - BuilderRunner bazel = bazel(watchFs, trackIncrementalState); - ProcessResult result = bazel.build("//..."); - buildSucceeded(result); - debugIdShouldBeUpdated(bazel); - } - - private void buildExpectRepositoryRuleNotCalled() throws Exception { - BuilderRunner bazel = bazel(); - ProcessResult result = bazel.build("//..."); - buildSucceeded(result); - debugIdShouldNotBeUpdated(bazel); - } - - private void debugIdShouldBeUpdated(BuilderRunner bazel) throws Exception { - assertThat(getDebugId(bazel)).isEqualTo(String.valueOf(currentDebugId)); - } - - private void debugIdShouldNotBeUpdated(BuilderRunner bazel) throws Exception { - assertThat(getDebugId(bazel)).isNotEqualTo(String.valueOf(currentDebugId)); - } - - private static void buildSucceeded(ProcessResult result) { - assertThat(findPattern(result, "INFO: Build completed successfully")).isTrue(); - } - - private static boolean findPattern(ProcessResult result, String pattern) { - String[] lines = result.errString().split("\n"); - return Arrays.stream(lines).anyMatch(s -> s.contains(pattern)); - } -} diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test deleted file mode 100644 index 53017a26733dfe..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/WORKSPACE.test +++ /dev/null @@ -1,24 +0,0 @@ -# Copyright 2019 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. -workspace( - name = "fine_grained_user_modules", - managed_directories = {"@generated_node_modules": ["node_modules"]}, -) - -load(":use_node_modules.bzl", "generate_fine_grained_node_modules") - -generate_fine_grained_node_modules( - name = "generated_node_modules", - package_json = "//:package.json", -) diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/bazelignore.test b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/bazelignore.test deleted file mode 100644 index b512c09d476623..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/bazelignore.test +++ /dev/null @@ -1 +0,0 @@ -node_modules \ No newline at end of file diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json deleted file mode 100644 index abcb5a5d1de94d..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/package.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "name": "fine_grained_user_modules", - "version": "0.0.1", - "description": "prototype for using user's node_modules", - "dependencies": { - "example-module": "0.2.0" - } -} diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl deleted file mode 100644 index bd0ca74c911d00..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/test_rule.bzl +++ /dev/null @@ -1,49 +0,0 @@ -# Copyright 2019 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. -""" Test rule for testing results of NPM package manager interaction mock -""" - -def _test_rule(ctx): - out = ctx.actions.declare_file("out.txt") - files = ctx.attr.module_source.files - found = False - for file_ in files.to_list(): - if file_.basename == "package.json": - compare_version(ctx.actions, file_, out, ctx.attr.version) - found = True - break - if not found: - fail("Not found package.json") - return [DefaultInfo(files = depset([out]))] - -test_rule = rule( - implementation = _test_rule, - attrs = { - "module_source": attr.label(), - "version": attr.string(), - }, -) - -def compare_version(action_factory, file_, out, expected_version): - action_factory.run_shell( - mnemonic = "getVersion", - inputs = [file_], - outputs = [out], - command = """result=$(cat ./{file} | grep '"version": "{expected}"' || exit 1) \ -&& echo $result > ./{out}""".format( - file = file_.path, - out = out.path, - expected = expected_version, - ), - ) diff --git a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl b/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl deleted file mode 100644 index 27b42573eb7f0c..00000000000000 --- a/src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/use_node_modules.bzl +++ /dev/null @@ -1,93 +0,0 @@ -# Copyright 2019 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. -""" NPM package manager interaction mock -""" - -def _generate_fine_grained_node_modules(rctx): - print("--fetch--") - package_json_path = rctx.path(rctx.attr.package_json) - - rctx.file( - "script.sh", - """ -root_dir="$1/node_modules" -mkdir $root_dir -mkdir $root_dir/example-module -cat > "$root_dir/example-module/package.json" < debug_id && echo 'ok'") - cmd(rctx, "./script.sh " + str(package_json_path.dirname)) - - node_modules_root = rctx.path(str(package_json_path) + "/../node_modules") - build_file_lines = [] - for path_ in node_modules_root.readdir(): - name = path_.basename - rctx.symlink(path_, rctx.path(name)) - build_file_lines += ["""filegroup(name = "{name}", srcs = glob(["{name}/**"]), visibility = ["//visibility:public"])""".format(name = name)] - - rctx.file("BUILD", "\n".join(build_file_lines)) - -generate_fine_grained_node_modules = repository_rule( - implementation = _generate_fine_grained_node_modules, - attrs = { - "package_json": attr.label(), - }, -) - -def cmd( - repository_ctx, - command, - environment = None): - """Executes a command, returns stdout if succeed and throw an error if it fails. - - Doesn't escape the result! - - Args: - repository_ctx: repository context - command: command parts array - environment: dict with environment variables - - Returns: - process stdout as a string - """ - if environment: - result = repository_ctx.execute(["bash", "-c", command], environment = environment) - else: - result = repository_ctx.execute(["bash", "-c", command]) - if result.return_code != 0: - fail("non-zero exit code: %d, command %s, stderr: (%s)" % ( - result.return_code, - command, - result.stderr, - )) - stripped_stdout = result.stdout.strip() - if not stripped_stdout: - fail( - "empty output from command %s, stderr: (%s)" % (command, result.stderr), - ) - return stripped_stdout 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 96041e35e42e1c..796e12626deb9e 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 @@ -40,7 +40,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe:containing_package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:file_function", "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_function", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_function", "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value", diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 5b5102b07058ab..6c9c56d7b2b2f7 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.repository; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import static com.google.common.truth.Truth.assertThat; import static com.google.devtools.build.lib.bazel.bzlmod.BzlmodTestUtil.createModuleKey; @@ -46,7 +45,6 @@ import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.packages.PackageFactory; -import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.rules.repository.RepositoryDirectoryValue.SuccessfulRepositoryDirectoryValue; @@ -63,7 +61,6 @@ import com.google.devtools.build.lib.skyframe.FileStateFunction; import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesFunction; import com.google.devtools.build.lib.skyframe.LocalRepositoryLookupFunction; -import com.google.devtools.build.lib.skyframe.ManagedDirectoriesKnowledge; import com.google.devtools.build.lib.skyframe.PackageFunction; import com.google.devtools.build.lib.skyframe.PackageFunction.GlobbingStrategy; import com.google.devtools.build.lib.skyframe.PackageLookupFunction; @@ -91,16 +88,12 @@ import com.google.devtools.build.skyframe.SequencedRecordingDifferencer; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunction.Environment; -import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.io.IOException; -import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import javax.annotation.Nullable; import net.starlark.java.eval.StarlarkSemantics; import org.junit.Before; import org.junit.Test; @@ -113,9 +106,7 @@ public class RepositoryDelegatorTest extends FoundationTestCase { private Path overrideDirectory; private MemoizingEvaluator evaluator; - private TestManagedDirectoriesKnowledge managedDirectoriesKnowledge; private RecordingDifferencer differencer; - private TestStarlarkRepositoryFunction testStarlarkRepositoryFunction; private Path rootPath; private FakeRegistry.Factory registryFactory; @@ -130,21 +121,17 @@ public void setupDelegator() throws Exception { rootPath, /* defaultSystemJavabase= */ null, TestConstants.PRODUCT_NAME); - managedDirectoriesKnowledge = new TestManagedDirectoriesKnowledge(); DownloadManager downloader = Mockito.mock(DownloadManager.class); RepositoryFunction localRepositoryFunction = new LocalRepositoryFunction(); - testStarlarkRepositoryFunction = - new TestStarlarkRepositoryFunction(rootPath, downloader, managedDirectoriesKnowledge); ImmutableMap repositoryHandlers = ImmutableMap.of(LocalRepositoryRule.NAME, localRepositoryFunction); RepositoryDelegatorFunction delegatorFunction = new RepositoryDelegatorFunction( repositoryHandlers, - testStarlarkRepositoryFunction, + new StarlarkRepositoryFunction(downloader), /*isFetch=*/ new AtomicBoolean(true), /*clientEnvironmentSupplier=*/ ImmutableMap::of, directories, - managedDirectoriesKnowledge, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER); AtomicReference pkgLocator = new AtomicReference<>( @@ -300,10 +287,8 @@ public void testOverride() throws Exception { @Test public void testRepositoryDirtinessChecker() throws Exception { TimestampGranularityMonitor tsgm = new TimestampGranularityMonitor(new ManualClock()); - TestManagedDirectoriesKnowledge knowledge = new TestManagedDirectoriesKnowledge(); - RepositoryDirectoryDirtinessChecker checker = - new RepositoryDirectoryDirtinessChecker(rootPath, knowledge); + RepositoryDirectoryDirtinessChecker checker = new RepositoryDirectoryDirtinessChecker(); RepositoryName repositoryName = RepositoryName.create("repo"); RepositoryDirectoryValue.Key key = RepositoryDirectoryValue.key(repositoryName); @@ -324,98 +309,6 @@ public void testRepositoryDirtinessChecker() throws Exception { assertThat(checker.check(key, fetchDelayed, SyscallCache.NO_CACHE, tsgm).isDirty()).isTrue(); - RepositoryName managedName = RepositoryName.create("managed"); - RepositoryDirectoryValue.Key managedKey = RepositoryDirectoryValue.key(managedName); - SuccessfulRepositoryDirectoryValue withManagedDirectories = - RepositoryDirectoryValue.builder() - .setPath(rootDirectory.getRelative("c")) - .setDigest(new byte[] {1}) - .build(); - - knowledge.setManagedDirectories(ImmutableMap.of(PathFragment.create("m"), managedName)); - assertThat( - checker - .check(managedKey, withManagedDirectories, SyscallCache.NO_CACHE, tsgm) - .isDirty()) - .isTrue(); - - Path managedDirectoryM = rootPath.getRelative("m"); - assertThat(managedDirectoryM.createDirectory()).isTrue(); - - assertThat( - checker - .check(managedKey, withManagedDirectories, SyscallCache.NO_CACHE, tsgm) - .isDirty()) - .isFalse(); - - managedDirectoryM.deleteTree(); - assertThat( - checker - .check(managedKey, withManagedDirectories, SyscallCache.NO_CACHE, tsgm) - .isDirty()) - .isTrue(); - } - - @Test - public void testManagedDirectoriesCauseRepositoryReFetches() throws Exception { - scratch.file(rootPath.getRelative("BUILD").getPathString()); - scratch.file( - rootPath.getRelative("repo_rule.bzl").getPathString(), - "def _impl(rctx):", - " rctx.file('BUILD', '')", - "fictive_repo_rule = repository_rule(implementation = _impl)"); - scratch.overwriteFile( - rootPath.getRelative("WORKSPACE").getPathString(), - "workspace(name = 'abc')", - "load(':repo_rule.bzl', 'fictive_repo_rule')", - "fictive_repo_rule(name = 'repo1')"); - - // Managed directories from workspace() attribute will not be parsed by this test, since - // we are not calling SequencedSkyframeExecutor. - // That's why we will directly fill managed directories value (the corresponding structure - // is passed to RepositoryDelegatorFunction during construction). - managedDirectoriesKnowledge.setManagedDirectories( - ImmutableMap.of(PathFragment.create("dir1"), RepositoryName.create("repo1"))); - - loadRepo("repo1"); - - assertThat(testStarlarkRepositoryFunction.isFetchCalled()).isTrue(); - testStarlarkRepositoryFunction.reset(); - - loadRepo("repo1"); - // Nothing changed, fetch does not happen. - assertThat(testStarlarkRepositoryFunction.isFetchCalled()).isFalse(); - testStarlarkRepositoryFunction.reset(); - - // Delete managed directory, fetch should happen again. - Path managedDirectory = rootPath.getRelative("dir1"); - managedDirectory.deleteTree(); - loadRepo("repo1"); - assertThat(testStarlarkRepositoryFunction.isFetchCalled()).isTrue(); - testStarlarkRepositoryFunction.reset(); - - // Change managed directories declaration, fetch should happen. - // NB: we are making sure that managed directories exist to check only the declaration changes - // were percepted. - rootPath.getRelative("dir1").createDirectory(); - rootPath.getRelative("dir2").createDirectory(); - - managedDirectoriesKnowledge.setManagedDirectories( - ImmutableMap.of( - PathFragment.create("dir1"), - RepositoryName.create("repo1"), - PathFragment.create("dir2"), - RepositoryName.create("repo1"))); - loadRepo("repo1"); - - assertThat(testStarlarkRepositoryFunction.isFetchCalled()).isTrue(); - testStarlarkRepositoryFunction.reset(); - - managedDirectoriesKnowledge.setManagedDirectories(ImmutableMap.of()); - loadRepo("repo1"); - - assertThat(testStarlarkRepositoryFunction.isFetchCalled()).isTrue(); - testStarlarkRepositoryFunction.reset(); } @Test @@ -582,80 +475,4 @@ private void loadRepo(String strippedRepoName) throws InterruptedException { RepositoryDirectoryValue repositoryDirectoryValue = (RepositoryDirectoryValue) result.get(key); assertThat(repositoryDirectoryValue.repositoryExists()).isTrue(); } - - private static class TestStarlarkRepositoryFunction extends StarlarkRepositoryFunction { - private boolean fetchCalled = false; - private final Path workspaceRoot; - private final TestManagedDirectoriesKnowledge managedDirectoriesKnowledge; - - private TestStarlarkRepositoryFunction( - Path workspaceRoot, - DownloadManager downloader, - TestManagedDirectoriesKnowledge managedDirectoriesKnowledge) { - super(downloader); - this.workspaceRoot = workspaceRoot; - this.managedDirectoriesKnowledge = managedDirectoriesKnowledge; - } - - public void reset() { - fetchCalled = false; - } - - private boolean isFetchCalled() { - return fetchCalled; - } - - @Nullable - @Override - public RepositoryDirectoryValue.Builder fetch( - Rule rule, - Path outputDirectory, - BlazeDirectories directories, - Environment env, - Map markerData, - SkyKey key) - throws RepositoryFunctionException, InterruptedException { - fetchCalled = true; - RepositoryDirectoryValue.Builder builder = - super.fetch(rule, outputDirectory, directories, env, markerData, key); - ImmutableSet managedDirectories = - managedDirectoriesKnowledge.getManagedDirectories((RepositoryName) key.argument()); - try { - for (PathFragment managedDirectory : managedDirectories) { - workspaceRoot.getRelative(managedDirectory).createDirectory(); - } - } catch (IOException e) { - throw new RepositoryFunctionException(e, Transience.PERSISTENT); - } - return builder; - } - } - - private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge { - - private ImmutableMap map = ImmutableMap.of(); - - public void setManagedDirectories(ImmutableMap map) { - this.map = map; - } - - @Nullable - @Override - public RepositoryName getOwnerRepository(PathFragment relativePathFragment) { - return map.get(relativePathFragment); - } - - @Override - public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { - return map.keySet().stream() - .filter(path -> repositoryName.equals(map.get(path))) - .collect(toImmutableSet()); - } - - @Override - public boolean workspaceHeaderReloaded( - @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) { - throw new IllegalStateException(); - } - } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD index 5b1fdd11ef371a..fff5fff9d7cb49 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD @@ -176,7 +176,6 @@ java_test( "//src/main/java/com/google/devtools/build/lib/query2/common:QueryTransitivePackagePreloader", "//src/main/java/com/google/devtools/build/lib/remote/options", "//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule", - "//src/main/java/com/google/devtools/build/lib/rules:repository/managed_directories_knowledge_impl", "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function", "//src/main/java/com/google/devtools/build/lib/rules/cpp", "//src/main/java/com/google/devtools/build/lib/skyframe:action_execution_inactivity_watchdog", @@ -223,7 +222,6 @@ java_test( "//src/main/java/com/google/devtools/build/lib/skyframe:incompatible_view_exception", "//src/main/java/com/google/devtools/build/lib/skyframe:incremental_artifact_conflict_finder", "//src/main/java/com/google/devtools/build/lib/skyframe:local_repository_lookup_value", - "//src/main/java/com/google/devtools/build/lib/skyframe:managed_directories_knowledge", "//src/main/java/com/google/devtools/build/lib/skyframe:metadata_consumer_for_metrics", "//src/main/java/com/google/devtools/build/lib/skyframe:output_store", "//src/main/java/com/google/devtools/build/lib/skyframe:package_error_message_value", diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java index de6712cceae6d7..c07d143fcd7bdb 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java @@ -164,7 +164,6 @@ public final void setUp() throws Exception { new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); RecordingDifferencer differencer = new SequencedRecordingDifferencer(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtilsTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtilsTest.java index 4e517f6b6f401e..920fc96a50dfbc 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtilsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/DirtinessCheckerUtilsTest.java @@ -20,7 +20,6 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; @@ -28,7 +27,6 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories; import com.google.devtools.build.lib.analysis.ServerDirectories; import com.google.devtools.build.lib.cmdline.LabelConstants; -import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction; import com.google.devtools.build.lib.testutil.TestConstants; @@ -49,7 +47,6 @@ import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentMatchers; import org.mockito.Mockito; /** Tests for {@link DirtinessCheckerUtils}. */ @@ -133,39 +130,20 @@ private RootedPath makeExternalRootedPath() { @Test public void skipsSyscallCacheForRepoFile_andDoesntReturnNewValue( - @TestParameter boolean externalChecker, @TestParameter boolean managedDirectory) { + @TestParameter boolean externalChecker) { ExternalFilesHelper externalFilesHelper = this.externalFilesHelper; - RootedPath rootedPath; - if (managedDirectory) { - ManagedDirectoriesKnowledge managedDirectoriesKnowledge = - mock(ManagedDirectoriesKnowledge.class); - externalFilesHelper = - ExternalFilesHelper.create( - pkgLocator, - ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, - directories, - managedDirectoriesKnowledge); - when(managedDirectoriesKnowledge.getOwnerRepository(ArgumentMatchers.any())) - .thenReturn(RepositoryName.MAIN); - rootedPath = RootedPath.toRootedPath(srcRoot, PathFragment.create("srcfile")); - } else { - rootedPath = - RootedPath.toRootedPath( - Root.fromPath(outputBase), - LabelConstants.EXTERNAL_REPOSITORY_LOCATION.getChild("extrepofile")); - } + RootedPath rootedPath = + RootedPath.toRootedPath( + Root.fromPath(outputBase), + LabelConstants.EXTERNAL_REPOSITORY_LOCATION.getChild("extrepofile")); SkyValueDirtinessChecker underTest = externalChecker ? new DirtinessCheckerUtils.ExternalDirtinessChecker( - externalFilesHelper, - EnumSet.of( - ExternalFilesHelper.FileType.EXTERNAL_IN_MANAGED_DIRECTORY, - ExternalFilesHelper.FileType.EXTERNAL_REPO)) - : new DirtinessCheckerUtils.MissingDiffDirtinessChecker( - ImmutableSet.of(srcRoot), externalFilesHelper); + externalFilesHelper, EnumSet.of(ExternalFilesHelper.FileType.EXTERNAL_REPO)) + : new DirtinessCheckerUtils.MissingDiffDirtinessChecker(ImmutableSet.of(srcRoot)); boolean shouldCheck = underTest.applies(rootedPath); - assertThat(shouldCheck).isEqualTo(externalChecker || managedDirectory); + assertThat(shouldCheck).isEqualTo(externalChecker); Assume.assumeTrue("Missing diff checker doesn't apply to external files", shouldCheck); @@ -190,7 +168,6 @@ public void externalDiffChecker_doesntMatchType() { } private DirtinessCheckerUtils.MissingDiffDirtinessChecker createMissingDiffChecker() { - return new DirtinessCheckerUtils.MissingDiffDirtinessChecker( - ImmutableSet.of(srcRoot), externalFilesHelper); + return new DirtinessCheckerUtils.MissingDiffDirtinessChecker(ImmutableSet.of(srcRoot)); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java index ed31a26830d5f4..f03dc071a74cda 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java @@ -228,7 +228,6 @@ private MemoizingEvaluator makeEvaluator(ExternalFileAction externalFileAction) new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)) .build(), differencer); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java index 1f90a6bd540df7..4120b4b946ca5b 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java @@ -175,7 +175,6 @@ public final void setUp() throws Exception { new AtomicBoolean(true), ImmutableMap::of, directories, - ManagedDirectoriesKnowledge.NO_MANAGED_DIRECTORIES, BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER)); differencer = new SequencedRecordingDifferencer(); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java index e689bf9cd75a60..f3d1d3f7b42d30 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java @@ -17,24 +17,16 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.base.Ascii; -import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.RepositoryName; -import com.google.devtools.build.lib.events.NullEventHandler; import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.WorkspaceFileValue; -import com.google.devtools.build.lib.packages.WorkspaceFileValue.WorkspaceFileKey; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; -import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl; -import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledgeImpl.ManagedDirectoriesListener; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.ModifiedFileSet; @@ -43,22 +35,14 @@ import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.skyframe.EvaluationResult; -import com.google.devtools.build.skyframe.Injectable; -import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.Arrays; import java.util.List; -import java.util.Objects; -import java.util.Set; -import javax.annotation.Nullable; -import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.syntax.ParserInput; import net.starlark.java.syntax.StarlarkFile; import net.starlark.java.syntax.Statement; -import org.hamcrest.BaseMatcher; -import org.hamcrest.Description; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -68,15 +52,6 @@ */ @RunWith(JUnit4.class) public class WorkspaceFileFunctionTest extends BuildViewTestCase { - - private TestManagedDirectoriesKnowledge testManagedDirectoriesKnowledge; - - @Override - protected ManagedDirectoriesKnowledge getManagedDirectoriesKnowledge() { - testManagedDirectoriesKnowledge = new TestManagedDirectoriesKnowledge(); - return testManagedDirectoriesKnowledge; - } - @Override protected Iterable getEnvironmentExtensions() { return ImmutableList.of(); @@ -93,25 +68,6 @@ private RootedPath createWorkspaceFile(String... contents) throws IOException { PathFragment.create(workspacePath.getBaseName())); } - // Dummy hamcrest matcher that match the function name of a skykey - static class SkyKeyMatchers extends BaseMatcher { - private final SkyFunctionName functionName; - - public SkyKeyMatchers(SkyFunctionName functionName) { - this.functionName = functionName; - } - @Override - public boolean matches(Object item) { - if (item instanceof SkyKey) { - return ((SkyKey) item).functionName().equals(functionName); - } - return false; - } - - @Override - public void describeTo(Description description) {} - } - private EvaluationResult eval(SkyKey key) throws InterruptedException, AbruptExitException { getSkyframeExecutor() @@ -198,168 +154,6 @@ public void testRepositoryMappingInChunks() throws Exception { .containsEntry(b, ImmutableMap.of("x", y, "good", main)); } - @Test - public void setTestManagedDirectoriesKnowledge() throws Exception { - StarlarkSemantics semantics = - getStarlarkSemantics().toBuilder() - .setBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES, false) - .build(); - Injectable injectable = getSkyframeExecutor().injectable(); - PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semantics); - - TestManagedDirectoriesListener listener = new TestManagedDirectoriesListener(); - ManagedDirectoriesKnowledgeImpl knowledge = new ManagedDirectoriesKnowledgeImpl(listener); - - RepositoryName one = RepositoryName.create("repo1"); - RepositoryName two = RepositoryName.create("repo2"); - RepositoryName three = RepositoryName.create("repo3"); - - PathFragment pf1 = PathFragment.create("dir1"); - PathFragment pf2 = PathFragment.create("dir2"); - PathFragment pf3 = PathFragment.create("dir3"); - - assertThat(knowledge.getManagedDirectories(one)).isEmpty(); - assertThat(knowledge.getOwnerRepository(pf1)).isNull(); - - WorkspaceFileValue workspaceFileValue = createWorkspaceFileValueForTest(); - boolean isChanged = knowledge.workspaceHeaderReloaded(null, workspaceFileValue); - - assertThat(isChanged).isTrue(); - assertThat(listener.getRepositoryNames()).containsExactly(one, two); - - assertThat(knowledge.getManagedDirectories(one)).containsExactly(pf1, pf2); - assertThat(knowledge.getManagedDirectories(two)).containsExactly(pf3); - assertThat(knowledge.getManagedDirectories(three)).isEmpty(); - - assertThat(knowledge.getOwnerRepository(pf1)).isEqualTo(one); - assertThat(knowledge.getOwnerRepository(pf2)).isEqualTo(one); - assertThat(knowledge.getOwnerRepository(pf3)).isEqualTo(two); - - // Nothing changed, let's test the behavior. - listener.reset(); - isChanged = knowledge.workspaceHeaderReloaded(workspaceFileValue, workspaceFileValue); - assertThat(isChanged).isFalse(); - assertThat(listener.getRepositoryNames()).containsExactly(one, two); - - assertThat(knowledge.getManagedDirectories(one)).containsExactly(pf1, pf2); - assertThat(knowledge.getManagedDirectories(two)).containsExactly(pf3); - assertThat(knowledge.getManagedDirectories(three)).isEmpty(); - - assertThat(knowledge.getOwnerRepository(pf1)).isEqualTo(one); - assertThat(knowledge.getOwnerRepository(pf2)).isEqualTo(one); - assertThat(knowledge.getOwnerRepository(pf3)).isEqualTo(two); - } - - @Test - public void testManagedDirectories() throws Exception { - StarlarkSemantics semantics = - getStarlarkSemantics().toBuilder() - .setBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES, false) - .build(); - Injectable injectable = getSkyframeExecutor().injectable(); - PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semantics); - createWorkspaceFileValueForTest(); - - // Test intentionally introduces errors. - reporter.removeHandler(failFastHandler); - - assertManagedDirectoriesParsingError( - "{'@repo1': 'dir1', '@repo2': ['dir3']}", - "managed_directories attribute value should be of the type attr.string_list_dict()," - + " mapping repository name to the list of managed directories."); - - assertManagedDirectoriesParsingError( - "{'@repo1': ['dir1'], '@repo2': ['dir1']}", - "managed_directories attribute should not contain multiple (or duplicate) repository" - + " mappings for the same directory ('dir1')."); - - assertManagedDirectoriesParsingError( - "{'@repo1': ['']}", "Expected managed directory path to be non-empty string."); - assertManagedDirectoriesParsingError( - "{'@repo1': ['/abc']}", - "Expected managed directory path ('/abc') to be relative to the workspace root."); - assertManagedDirectoriesParsingError( - "{'@repo1': ['../abc']}", - "Expected managed directory path ('../abc') to be under the workspace root."); - assertManagedDirectoriesParsingError( - "{'@repo1': ['a/b', 'a/b']}", - "managed_directories attribute should not contain multiple (or duplicate)" - + " repository mappings for the same directory ('a/b')."); - assertManagedDirectoriesParsingError( - "{'@repo1': [], '@repo1': [] }", "dictionary expression has duplicate key: \"@repo1\""); - assertManagedDirectoriesParsingError( - "{'@repo1': ['a/b'], '@repo2': ['a/b/c/..'] }", - "managed_directories attribute should not contain multiple (or duplicate)" - + " repository mappings for the same directory ('a/b/c/..')."); - assertManagedDirectoriesParsingError( - "{'@repo1': ['a'], '@repo2': ['a/b'] }", - "managed_directories attribute value can not contain nested mappings." - + " 'a/b' is a descendant of 'a'."); - assertManagedDirectoriesParsingError( - "{'@repo1': ['a/b'], '@repo2': ['a'] }", - "managed_directories attribute value can not contain nested mappings." - + " 'a/b' is a descendant of 'a'."); - - assertManagedDirectoriesParsingError( - "{'repo1': []}", - "Cannot parse repository name 'repo1'. Repository name should start with '@'."); - } - - private WorkspaceFileValue createWorkspaceFileValueForTest() throws Exception { - WorkspaceFileValue workspaceFileValue = - parseWorkspaceFileValue( - "workspace(", - " name = 'rr',", - " managed_directories = {'@repo1': ['dir1', 'dir2'], '@repo2': ['dir3/dir1/..']}", - ")"); - ImmutableMap managedDirectories = - workspaceFileValue.getManagedDirectories(); - assertThat(managedDirectories).isNotNull(); - assertThat(managedDirectories).hasSize(3); - assertThat(managedDirectories) - .containsExactly( - PathFragment.create("dir1"), RepositoryName.create("repo1"), - PathFragment.create("dir2"), RepositoryName.create("repo1"), - PathFragment.create("dir3"), RepositoryName.create("repo2")); - return workspaceFileValue; - } - - private void assertManagedDirectoriesParsingError( - String managedDirectoriesValue, String expectedError) throws Exception { - parseWorkspaceFileValueWithError( - expectedError, - "workspace(", - " name = 'rr',", - " managed_directories = " + managedDirectoriesValue, - ")"); - } - - private WorkspaceFileValue parseWorkspaceFileValue(String... lines) throws Exception { - WorkspaceFileValue workspaceFileValue = parseWorkspaceFileValueImpl(lines); - Package pkg = workspaceFileValue.getPackage(); - if (pkg.containsErrors()) { - throw new RuntimeException( - Preconditions.checkNotNull(Iterables.getFirst(eventCollector, null)).getMessage()); - } - return workspaceFileValue; - } - - private void parseWorkspaceFileValueWithError(String expectedError, String... lines) - throws Exception { - WorkspaceFileValue workspaceFileValue = parseWorkspaceFileValueImpl(lines); - Package pkg = workspaceFileValue.getPackage(); - assertThat(pkg.containsErrors()).isTrue(); - assertContainsEvent(expectedError); - } - - private WorkspaceFileValue parseWorkspaceFileValueImpl(String[] lines) - throws IOException, InterruptedException, AbruptExitException { - RootedPath workspaceFile = createWorkspaceFile(lines); - WorkspaceFileKey key = WorkspaceFileValue.key(workspaceFile); - EvaluationResult result = eval(key); - return result.get(key); - } - @Test public void testInvalidRepo() throws Exception { // Test intentionally introduces errors. @@ -475,19 +269,6 @@ public void testListBindFunction() throws Exception { assertNoEvents(); } - @Test - public void testWorkspaceFileValueListener() throws Exception { - createWorkspaceFile("workspace(name = 'old')"); - skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE); - assertThat(testManagedDirectoriesKnowledge.getLastWorkspaceName()).isEqualTo("old"); - assertThat(testManagedDirectoriesKnowledge.getCnt()).isEqualTo(1); - - createWorkspaceFile("workspace(name = 'changed')"); - skyframeExecutor.handleDiffsForTesting(NullEventHandler.INSTANCE); - assertThat(testManagedDirectoriesKnowledge.getLastWorkspaceName()).isEqualTo("changed"); - assertThat(testManagedDirectoriesKnowledge.getCnt()).isEqualTo(2); - } - @Test public void testMangledExternalWorkspaceFileIsIgnored() throws Exception { scratch.file("secondary/WORKSPACE", "garbage"); @@ -505,60 +286,6 @@ public void testMangledExternalWorkspaceFileIsIgnored() throws Exception { .containsEntry(secondary, ImmutableMap.of("good", main)); assertNoEvents(); } - - private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge { - private String lastWorkspaceName; - private int cnt = 0; - - @Nullable - @Override - public RepositoryName getOwnerRepository(PathFragment relativePathFragment) { - return null; - } - - @Override - public ImmutableSet getManagedDirectories(RepositoryName repositoryName) { - return null; - } - - @Override - public boolean workspaceHeaderReloaded( - @Nullable WorkspaceFileValue oldValue, @Nullable WorkspaceFileValue newValue) { - if (Objects.equals(oldValue, newValue)) { - return false; - } - ++cnt; - lastWorkspaceName = newValue != null ? newValue.getPackage().getWorkspaceName() : null; - return true; - } - - private String getLastWorkspaceName() { - return lastWorkspaceName; - } - - private int getCnt() { - return cnt; - } - } - - private static class TestManagedDirectoriesListener implements ManagedDirectoriesListener { - @Nullable private Set repositoryNames; - - @Override - public void onManagedDirectoriesRefreshed(Set repositoryNames) { - this.repositoryNames = repositoryNames; - } - - @Nullable - public Set getRepositoryNames() { - return repositoryNames; - } - - public void reset() { - repositoryNames = null; - } - } - // tests of splitChunks, an internal helper function @Test