diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Util.java b/src/main/java/com/google/devtools/build/lib/analysis/Util.java index 193cb9396eeabd..d4519d08645963 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Util.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Util.java @@ -95,19 +95,24 @@ public static ImmutableSet findImplicitDeps(RuleContext rul } } } - // Consider toolchain dependencies. - ToolchainContext toolchainContext = ruleContext.getToolchainContext(); - if (toolchainContext != null) { - // This logic should stay up to date with the dep creation logic in - // DependencyResolver#partiallyResolveDependencies. - BuildConfigurationValue targetConfiguration = ruleContext.getConfiguration(); - for (Label toolchain : toolchainContext.resolvedToolchainLabels()) { - maybeImplicitDeps.add( - ConfiguredTargetKey.builder() - .setLabel(toolchain) - .setConfiguration(targetConfiguration) - .setExecutionPlatformLabel(toolchainContext.executionPlatform().label()) - .build()); + + ToolchainCollection toolchainContexts = + ruleContext.getToolchainContexts(); + if (toolchainContexts != null) { + for (ResolvedToolchainContext toolchainContext : toolchainContexts.getContextMap().values()) { + if (toolchainContext != null) { + // This logic should stay up to date with the dep creation logic in + // DependencyResolver#partiallyResolveDependencies. + BuildConfigurationValue targetConfiguration = ruleContext.getConfiguration(); + for (Label toolchain : toolchainContext.resolvedToolchainLabels()) { + maybeImplicitDeps.add( + ConfiguredTargetKey.builder() + .setLabel(toolchain) + .setConfiguration(targetConfiguration) + .setExecutionPlatformLabel(toolchainContext.executionPlatform().label()) + .build()); + } + } } } return ImmutableSet.copyOf(Sets.difference(maybeImplicitDeps, explicitDeps)); diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java index 21b18bc8bf5f6b..f2f8855c3ad37f 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java @@ -259,27 +259,39 @@ public void testNoImplicitDeps_toolchains() throws Exception { // Check for implicit toolchain dependencies assertThat(evalToListOfStrings("deps(//test:my_rule)")) - .containsAtLeastElementsIn( - evalToListOfStrings(explicits + " + " + implicits + " + " + PLATFORM_LABEL)); + .containsAtLeast(explicits, implicits, PLATFORM_LABEL); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); ImmutableList filteredDeps = evalToListOfStrings("deps(//test:my_rule)"); - assertThat(filteredDeps).containsAtLeastElementsIn(evalToListOfStrings(explicits)); - assertThat(filteredDeps).containsNoneIn(evalToListOfStrings(implicits)); + assertThat(filteredDeps).contains(explicits); + assertThat(filteredDeps).doesNotContain(implicits); } - @Test - public void testNoImplicitDeps_starlark_toolchains() throws Exception { + private void writeSimpleToolchain() throws Exception { writeFile( - "test/toolchain.bzl", + "test/toolchain_def.bzl", "def _impl(ctx):", - " toolchain = platform_common.ToolchainInfo()", - " return [toolchain]", + " return [platform_common.ToolchainInfo()]", "test_toolchain = rule(", " implementation = _impl,", ")"); writeFile( - "test/rule.bzl", + "test/BUILD", + "load('//test:toolchain_def.bzl', 'test_toolchain')", + "toolchain_type(name = 'toolchain_type')", + "toolchain(", + " name = 'toolchain',", + " toolchain_type = '//test:toolchain_type',", + " toolchain = ':toolchain_impl'", + ")", + "test_toolchain(name = 'toolchain_impl')"); + } + + @Test + public void testNoImplicitDeps_starlark_toolchains() throws Exception { + writeSimpleToolchain(); + writeFile( + "test/rule/rule.bzl", "def _impl(ctx):", " return []", "implicit_toolchain_deps_rule = rule(", @@ -287,32 +299,24 @@ public void testNoImplicitDeps_starlark_toolchains() throws Exception { " toolchains = ['//test:toolchain_type']", ")"); writeFile( - "test/BUILD", - "load(':toolchain.bzl', 'test_toolchain')", + "test/rule/BUILD", "load(':rule.bzl', 'implicit_toolchain_deps_rule')", "implicit_toolchain_deps_rule(", " name = 'my_rule',", - ")", - "toolchain_type(name = 'toolchain_type')", - "toolchain(", - " name = 'toolchain',", - " toolchain_type = ':toolchain_type',", - " toolchain = ':toolchain_impl',", - ")", - "test_toolchain(name = 'toolchain_impl')"); + ")"); ((PostAnalysisQueryHelper) helper).useConfiguration("--extra_toolchains=//test:toolchain"); String implicits = "//test:toolchain_impl"; - String explicits = "//test:my_rule"; + String explicits = "//test/rule:my_rule"; // Check for implicit toolchain dependencies - assertThat(evalToListOfStrings("deps(//test:my_rule)")) - .containsAtLeastElementsIn(evalToListOfStrings(explicits + " + " + implicits)); + assertThat(evalToListOfStrings("deps(//test/rule:my_rule)")) + .containsAtLeast(explicits, implicits); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); - ImmutableList filteredDeps = evalToListOfStrings("deps(//test:my_rule)"); - assertThat(filteredDeps).containsAtLeastElementsIn(evalToListOfStrings(explicits)); - assertThat(filteredDeps).containsNoneIn(evalToListOfStrings(implicits)); + ImmutableList filteredDeps = evalToListOfStrings("deps(//test/rule:my_rule)"); + assertThat(filteredDeps).contains(explicits); + assertThat(filteredDeps).doesNotContain(implicits); } @Test @@ -366,13 +370,12 @@ public void testNoImplicitDeps_cc_toolchains() throws Exception { // Check for implicit toolchain dependencies assertThat(evalToListOfStrings("deps(//test:my_rule)")) - .containsAtLeastElementsIn( - evalToListOfStrings(explicits + " + " + implicits + " + " + PLATFORM_LABEL)); + .containsAtLeast(explicits, implicits, PLATFORM_LABEL); helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); ImmutableList filteredDeps = evalToListOfStrings("deps(//test:my_rule)"); - assertThat(filteredDeps).containsAtLeastElementsIn(evalToListOfStrings(explicits)); - assertThat(filteredDeps).containsNoneIn(evalToListOfStrings(implicits)); + assertThat(filteredDeps).contains(explicits); + assertThat(filteredDeps).doesNotContain(implicits); } // Regression test for b/148550864 @@ -400,6 +403,74 @@ public void testNoImplicitDeps_platformDeps() throws Exception { assertThat(evalToListOfStrings("deps(//test:my_rule)")).containsExactly("//test:my_rule"); } + // Regression test for b/275502129. + @Test + public void testNoImplicitDepsFromAutoExecGroups_autoExecGroupsEnabled() throws Exception { + writeSimpleToolchain(); + writeFile( + "test/aeg/defs.bzl", + "def _impl(ctx):", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " toolchains = ['//test:toolchain_type'],", + ")"); + writeFile( + "test/aeg/BUILD", + "load('//test/aeg:defs.bzl', 'custom_rule')", + "custom_rule(name = 'custom_rule_name')"); + ((PostAnalysisQueryHelper) helper) + .useConfiguration("--incompatible_auto_exec_groups", "--extra_toolchains=//test:all"); + + String implicits = "//test:toolchain_impl"; + String explicits = "//test/aeg:custom_rule_name"; + + // Check for implicit toolchain dependencies + assertThat(evalToListOfStrings("deps(//test/aeg:custom_rule_name)")) + .containsAtLeast(explicits, implicits); + + helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); + ImmutableList filteredDeps = evalToListOfStrings("deps(//test/aeg:custom_rule_name)"); + assertThat(filteredDeps).contains(explicits); + assertThat(filteredDeps).doesNotContain(implicits); + } + + // Regression test for b/275502129. + @Test + public void testNoImplicitDepsFromCustomExecGroups_autoExecGroupsEnabled() throws Exception { + writeSimpleToolchain(); + writeFile( + "test/aeg/defs.bzl", + "def _impl(ctx):", + " return []", + "custom_rule = rule(", + " implementation = _impl,", + " exec_groups = {", + " 'custom_exec_group': exec_group(", + " toolchains = ['//test:toolchain_type'],", + " ),", + " },", + ")"); + writeFile( + "test/aeg/BUILD", + "load('//test/aeg:defs.bzl', 'custom_rule')", + "custom_rule(name = 'custom_rule_name')"); + ((PostAnalysisQueryHelper) helper) + .useConfiguration("--incompatible_auto_exec_groups", "--extra_toolchains=//test:all"); + + String implicits = "//test:toolchain_impl"; + String explicits = "//test/aeg:custom_rule_name"; + + // Check for implicit toolchain dependencies + assertThat(evalToListOfStrings("deps(//test/aeg:custom_rule_name)")) + .containsAtLeast(explicits, implicits); + + helper.setQuerySettings(Setting.NO_IMPLICIT_DEPS); + ImmutableList filteredDeps = evalToListOfStrings("deps(//test/aeg:custom_rule_name)"); + assertThat(filteredDeps).contains(explicits); + assertThat(filteredDeps).doesNotContain(implicits); + } + @Test public void testNoImplicitDeps_computedDefault() throws Exception { MockRule computedDefaultRule =