Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config_setting incorrectly treated as implicit dependency by --incompatible_visibility_private_attributes_at_definition #19126

Closed
dgp1130 opened this issue Jul 29, 2023 · 4 comments
Labels
team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug untriaged

Comments

@dgp1130
Copy link
Contributor

dgp1130 commented Jul 29, 2023

Description of the bug:

When explicitly depending on a config_setting and enabling --incompatible_visibility_private_attributes_at_definition, the setting is incorrectly treated as an implicit dependency which must be visible to the dependent's rule definition, which doesn't make logical sense. The full set of conditions from my understanding is:

  1. A target which depends on a config_setting from a different package through a select key.
  2. The target is an instance of a rule from a another package which does not have visibility into the config_setting.
  3. --incompatible_visibility_private_attributes_at_definition.
# BUILD.bazel

load("//build_defs:defs.bzl", "my_rule")

my_rule(
    name = "my_target",
    value = select({
        "//config_setting:my_setting": "foo",
        "//conditions:default": "bar",
    }),
)

When all three happen it throws an error like this:

$ bazel build //:my_target
ERROR: /home/doug/Source/bazel_visibility/BUILD.bazel:3:8: in my_rule rule //:my_target: target '//config_setting:my_setting' is not visible from target '//build_defs:defs.bzl'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: /home/doug/Source/bazel_visibility/BUILD.bazel:3:8: Analysis of target '//:my_target' failed
ERROR: Analysis of target '//:my_target' failed; build aborted: 
INFO: Elapsed time: 0.064s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (2 packages loaded, 2 targets configured)

This doesn't make sense: target '//config_setting:my_setting' is not visible from target '//build_defs:defs.bzl'. This should not be required, as the config setting is not an implicit dependency.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Minimal reproduction here: https://github.com/dgp1130/bazel-config-setting-visibility-bug/

It does rely on Skylib which probably isn't necessary but I figured that wasn't too significant and I didn't want to try and figure out how to define flags manually.

Which operating system are you running Bazel on?

Ubuntu on WSL

What is the output of bazel info release?

release 6.3.0

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

$ git remote get-url origin; git rev-parse main; git rev-parse HEAD
[email protected]:dgp1130/bazel-config-setting-visibility-bug
d86833d09fb3bcb3eb0429d74eb80364db7e684a
d86833d09fb3bcb3eb0429d74eb80364db7e684a

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

I don't know, however I initially reproduced on version 6.0.0 before upgrading to 6.3.0 to confirm it was still present on latest. The bug is at least that old.

Have you found anything relevant by searching the web?

#12932 feels like it might be relevant, however I found that --incompatible_enforce_config_setting_visibility had no effect, so it could be nothing.

Any other information, logs, or outputs that you want to share?

No response

@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 4, 2023
@iancha1992
Copy link
Member

@bazel-io fork 6.4.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Aug 4, 2023
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Aug 7, 2023
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 bazelbuild#19126

Closes bazelbuild#19139.

PiperOrigin-RevId: 553830083
Change-Id: Ic4af0f5ad9a0c627c973cd5ce88dd1e1bf51474e
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Aug 7, 2023
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 bazelbuild#19126

Closes bazelbuild#19139.

PiperOrigin-RevId: 553830083
Change-Id: Ic4af0f5ad9a0c627c973cd5ce88dd1e1bf51474e
iancha1992 pushed a commit to iancha1992/bazel that referenced this issue Aug 7, 2023
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 bazelbuild#19126

Closes bazelbuild#19139.

PiperOrigin-RevId: 553830083
Change-Id: Ic4af0f5ad9a0c627c973cd5ce88dd1e1bf51474e
iancha1992 added a commit that referenced this issue Aug 7, 2023
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

Co-authored-by: Fabian Meumertzheim <[email protected]>
@keertk
Copy link
Member

keertk commented Sep 20, 2023

A fix for this issue has been included in Bazel 6.4.0 RC1. Please test out the release candidate and report any issues as soon as possible. Thanks!

@dgp1130
Copy link
Contributor Author

dgp1130 commented Oct 14, 2023

I gave 6.4.0rc1 a quick test and confirmed that it does fix the issue in my actual repository, so everything looks good here! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug untriaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants