Skip to content

Commit

Permalink
Always check $config_dependencies visibility at use
Browse files Browse the repository at this point in the history
All dependencies for `select` expressions are collected in an implicit `$config_dependencies` attribute. Even with
`--incompatible_visibility_private_attributes_at_definition`, visibility for this attribute is now checked relative to the use rather than the definition of the rule.

Fixes #19126

Closes #19139.

PiperOrigin-RevId: 553830083
Change-Id: Ic4af0f5ad9a0c627c973cd5ce88dd1e1bf51474e
  • Loading branch information
fmeum authored and copybara-github committed Aug 4, 2023
1 parent 3b6f805 commit 5ca89d4
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
Expand Down Expand Up @@ -103,6 +104,7 @@ private void validateDirectPrerequisiteVisibility(

if (!toolCheckAtDefinition
|| !attribute.isImplicit()
|| attribute.getName().equals(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE)
|| rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null) {
// Default check: The attribute must be visible from the target.
if (!context.isVisible(prerequisite.getConfiguredTarget())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,42 @@ public void testDataVisibilityPrivateCheckPrivateAtUse() throws Exception {
assertThrows(ViewCreationFailedException.class, () -> update("//use:world"));
}

@Test
public void testConfigSettingVisibilityAlwaysCheckedAtUse() throws Exception {
scratch.file(
"BUILD",
"load('//build_defs:defs.bzl', 'my_rule')",
"my_rule(",
" name = 'my_target',",
" value = select({",
" '//config_setting:my_setting': 'foo',",
" '//conditions:default': 'bar',",
" }),",
")");
scratch.file("build_defs/BUILD");
scratch.file(
"build_defs/defs.bzl",
"def _my_rule_impl(ctx):",
" pass",
"my_rule = rule(",
" implementation = _my_rule_impl,",
" attrs = {",
" 'value': attr.string(mandatory = True),",
" },",
")");
scratch.file(
"config_setting/BUILD",
"config_setting(",
" name = 'my_setting',",
" values = {'cpu': 'does_not_matter'},",
" visibility = ['//:__pkg__'],",
")");
useConfiguration("--incompatible_visibility_private_attributes_at_definition");

update("//:my_target");
assertThat(hasErrors(getConfiguredTarget("//:my_target"))).isFalse();
}

void setupFilesScenario(String wantRead) throws Exception {
scratch.file("src/source.txt", "source");
scratch.file("src/BUILD", "exports_files(['source.txt'], visibility=['//pkg:__pkg__'])");
Expand Down

0 comments on commit 5ca89d4

Please sign in to comment.