Skip to content

Commit

Permalink
Disable workspace(managed_directories).
Browse files Browse the repository at this point in the history 
This change implements the design doc "Deprecating managed_directories".

If your build is broken by this change, you can temporarily re-enable the
functionality using the `--noincompatible_disable_managed_directories` command
line option.

Tracking bug: #15463

RELNOTES[INC]: workspace(managed_directories=) is not available anymore.

PiperOrigin-RevId: 449469507
  • Loading branch information
@lberki @copybara-github
lberki authored and copybara-github committed May 18, 2022
1 parent 44fef49 commit ab24880
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 97 deletions.
12 changes: 11 additions & 1 deletion src/main/java/com/google/devtools/build/lib/packages/WorkspaceGlobals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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;
Expand Down Expand Up @@ -102,12 +103,21 @@ public void workspace(
builder.addRepositoryMappingEntry(
RepositoryName.MAIN, RepositoryName.createUnvalidated(name), RepositoryName.MAIN);
parseManagedDirectories(
thread.getSemantics().getBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES),
Dict.cast(managedDirectories, String.class, Object.class, "managed_directories"));
}

private void parseManagedDirectories(
Map<String, ?> managedDirectories) // <String, Sequence<String>>
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());
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,15 @@ 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 @@ -603,6 +612,7 @@ 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 @@ -671,6 +681,8 @@ 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
1 change: 1 addition & 0 deletions .../google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ private BuilderRunner bazel(boolean watchFs, boolean trackIncrementalState) {
context()
.bazel()
.withEnv("DEBUG_ID", String.valueOf(currentDebugId))
.withFlags("--noincompatible_disable_managed_directories")
.withFlags(
"--watchfs=" + watchFs, "--track_incremental_state=" + trackIncrementalState);
if (DEBUG) {
Expand Down
194 changes: 98 additions & 96 deletions src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
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;
Expand Down Expand Up @@ -198,108 +199,109 @@ public void testRepositoryMappingInChunks() throws Exception {

@Test
public void setTestManagedDirectoriesKnowledge() throws Exception {
StarlarkSemantics semantics = getStarlarkSemantics();
StarlarkSemantics semantics =
getStarlarkSemantics().toBuilder()
.setBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES, false)
.build();
Injectable injectable = getSkyframeExecutor().injectable();
try {
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);
} finally {
PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semantics);
}
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();
StarlarkSemantics semantics =
getStarlarkSemantics().toBuilder()
.setBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_MANAGE_DIRECTORIES, false)
.build();
Injectable injectable = getSkyframeExecutor().injectable();
try {
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 '@'.");
} finally {
PrecomputedValue.STARLARK_SEMANTICS.set(injectable, semantics);
}
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 {
Expand Down

0 comments on commit ab24880

Please sign in to comment.