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 dd261b0954f7dd..63ddfd88cfcce3 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 @@ -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; @@ -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 managedDirectories) // > + 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()); 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 f2b322036eba3e..764f81a186abcd 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 @@ -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 @@ -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); } @@ -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 EXPERIMENTAL_BUILTINS_BZL_PATH = 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 index 086ba144d31546..924155672eab70 100644 --- 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 @@ -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) { 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 bdc4c499001e55..80d7997a09bde5 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 @@ -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; @@ -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 {