Skip to content

Commit

Permalink
Remove support for managed directories.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lberki authored and copybara-github committed Jun 21, 2022
1 parent 84b626d commit cbf8159
Show file tree
Hide file tree
Showing 51 changed files with 58 additions and 2,012 deletions.
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -134,9 +132,6 @@ public class BazelRepositoryModule extends BlazeModule {
private Set<String> outputVerificationRules = ImmutableSet.of();
private FileSystem filesystem;
private List<String> 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;
Expand All @@ -145,24 +140,6 @@ public class BazelRepositoryModule extends BlazeModule {
public BazelRepositoryModule() {
this.starlarkRepositoryFunction = new StarlarkRepositoryFunction(downloadManager);
this.repositoryHandlers = repositoryRules();
ManagedDirectoriesListener listener =
repositoryNamesWithManagedDirs -> {
Set<String> 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) {
Expand Down Expand Up @@ -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.
Expand All @@ -227,7 +202,6 @@ public void workspaceInit(
isFetch,
clientEnvironmentSupplier,
directories,
managedDirectoriesKnowledge,
BazelSkyframeExecutorConstants.EXTERNAL_PACKAGE_HELPER);
RegistryFactory registryFactory =
new RegistryFactoryImpl(new HttpDownloader(), clientEnvironmentSupplier);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -398,8 +396,4 @@ public Map<String, Module> getLoadedModules() {
public Map<String, Object> getVariableBindings() {
return ImmutableMap.copyOf(bindings);
}

public Map<PathFragment, RepositoryName> getManagedDirectories() {
return workspaceGlobals.getManagedDirectories();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -104,9 +103,6 @@ public String toString() {
private final ImmutableMap<String, Integer> loadToChunkMap;
private final ImmutableMap<RepositoryName, ImmutableMap<String, RepositoryName>>
repositoryMapping;
// Mapping of the relative paths of the incrementally updated managed directories
// to the managing external repositories
private final ImmutableMap<PathFragment, RepositoryName> managedDirectories;

/**
* Create a WorkspaceFileValue containing the various values necessary to compute the split
Expand All @@ -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,
Expand All @@ -133,8 +128,7 @@ public WorkspaceFileValue(
Map<String, Object> bindings,
RootedPath path,
int idx,
boolean hasNext,
ImmutableMap<PathFragment, RepositoryName> managedDirectories) {
boolean hasNext) {
this.pkg = Preconditions.checkNotNull(pkg);
this.idx = idx;
this.path = path;
Expand All @@ -143,7 +137,6 @@ public WorkspaceFileValue(
this.loadedModules = ImmutableMap.copyOf(loadedModules);
this.loadToChunkMap = ImmutableMap.copyOf(loadToChunkMap);
this.repositoryMapping = pkg.getExternalPackageRepositoryMappings();
this.managedDirectories = managedDirectories;
}

/**
Expand Down Expand Up @@ -224,8 +217,4 @@ public ImmutableMap<String, Integer> getLoadToChunkMap() {
public ImmutableMap<RepositoryName, ImmutableMap<String, RepositoryName>> getRepositoryMapping() {
return repositoryMapping;
}

public ImmutableMap<PathFragment, RepositoryName> getManagedDirectories() {
return managedDirectories;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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<PathFragment, RepositoryName> 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, // <String, Object>
StarlarkThread thread)
throws EvalException, InterruptedException {
if (!allowOverride) {
Expand Down Expand Up @@ -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<String, ?> managedDirectories) // <String, Sequence<String>>
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<PathFragment, String> nonNormalizedPathsMap = Maps.newHashMap();
for (Map.Entry<String, ?> entry : managedDirectories.entrySet()) {
RepositoryName repositoryName = createRepositoryName(entry.getKey());
List<PathFragment> 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<PathFragment> getManagedDirectoriesPaths(
Object directoriesList, Map<PathFragment, String> 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<PathFragment> 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<PathFragment, RepositoryName> getManagedDirectories() {
return managedDirectoriesMap;
}

private static RepositoryName getRepositoryName(@Nullable Label label) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
Expand Down
16 changes: 0 additions & 16 deletions src/main/java/com/google/devtools/build/lib/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit cbf8159

Please sign in to comment.