Add support for rule visibility for private tools/dependencies.

[sergiocampama]

Description of the problem / feature request:

Currently, if a rule has a private tool that it needs to invoke as part of its logic, this tool needs to have public visibility since visibility resolution is per target, not per rule.

Bazel should support a mechanism to allow any target of a specific rule to have access to the private tools/dependencies, but not having to expose these dependencies publicly.

Take for example: https://github.com/bazelbuild/rules_apple/blob/master/tools/codesigningtool/BUILD#L9

There's no available mechanism through which to make this target private to instances of ios_application, since this rule can be created in any package and maintaining a whitelist is a huge burden.

Feature requests: what underlying problem are you trying to solve with this feature?

Lack of visibility enforcement and needing to expose all internals of rules into potential abuse, making it harder to control affected targets and therefore make improvements/changes.

[aiuto]

Is this really Starlark? I think the bulk of the work is analysis.

[aiuto]

I also think we should raise this above P3. Without the mechanism, it is impossible to build a pair of (rule, helper tool) where the implementation of the helper is truly private.

[laurentlb]

Starlark team has no ressources to work on it this year. Feel free to reassign.

[haberman]

Same issue happens with protobuf rules. The protobuf C++ runtime has APIs and .h files that are only meant for use by the generated code. We would like to visibility restrict these so that nobody can #include them from hand-written code. But currently we have to make them public to everybody.

I would like to write visibility restrictions that say that only the implementation of cc_proto_library() can depend on the cc_library(name="runtime_internal") target.

[laurentlb]

Design: https://github.com/bazelbuild/proposals/blob/master/designs/2019-10-15-tool-visibility.md

This can be tested with the flag --incompatible_visibility_private_attributes_at_definition.

[haberman]

This looks great. A couple comments/questions:

1. The design mentions "tools" throughout, but for protobuf it is a cc_library() implicit dep that we want to restrict. Our runtime library is used by the generated code, and it has some interfaces that are meant only for the generated code. Will this design work for cc_library() deps, as well as tools?

2. I know of at least one rule at Google that is attempting to simulate rule visibility, and will be hit by Private attribute as a replacement for rule visibility. Is any mitigation for this planned?

[aiuto]

What I would like to see is a document which gathers some use cases so that we can test solutions against it. Some things which come to my mind are

1. rule with private tool

• rule depends on helper tool.
• helper tool grants visibility to the __pkg__ containing the rule
• this is essentially the tool-visibility proposal

2. macro with private rule

• we have a macro which coordinates one or more rules
• the user API is the macro, not the individual rules

3. Rule use limited to specific packages

• This is actually easy. For rule foo() we add a private attribute _foo_lock of type label, which points to a zero sized file foo_visibility_lock. Use visibility on the exports_files of that to lock it down.

Are there others?
@haberman Can I try to paraphrase what you stated above?

• a macro
  • calls rule which generates some code
  • calls rule (cc_library) that takes generated code and //protobuf:special_hdrs as input
  • You only want //protobuf:special headers to be visible when called this way

Does that get the general sense of what you want?

[haberman]

@aiuto that is close, but I am talking about rules like cc_proto_library(), upb_proto_library(), etc, not macros.

Right now in my aspect-based rules, I have to resort to obnoxious long names as a pseudo-visibility: https://github.com/protocolbuffers/upb/blob/22182e6e54e892f93f26d0522487997d30f604af/bazel/upb_proto_library.bzl#L235

[laurentlb]

Private attribute as a replacement for rule visibility. Is any mitigation for this planned?

That's the reason why the flag is not enabled by default. There's no planned rollout at this time.

[haberman]

That's the reason why the flag is not enabled by default. There's no planned rollout at this time.

Can we let rules specify this behavior in the label itself, so it doesn't need to be a global flag flip?

Right now I have:

_upb_proto_library_aspect = aspect(
    attrs = {
        # [...]
        "_upb": attr.label_list(default = [
            "//:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me",
            "//:upb",
        ]),
    },
    # ...
)

Could I instead write:

_upb_proto_library_aspect = aspect(
    attrs = {
        # [...]
        "_upb": attr.label_list(
            default = [
                "//:generated_code_support",
               "//:upb",
            ]
            # With this flag, the visibility check will pass if these
            # targets are visible from where this aspect is defined.
            inherit_visibility = True,
        ),
    },
    # ...
)

[haberman]

Ping @laurentlb ? I don't see why this needs to be a global flag. If it can be an attribute on label_list, we can get the best of both worlds.

[aiuto]

@brandjon
Does the starlark visibility stuff close this?

[brandjon]

No, this is a separate matter of target visibility. Still on my radar but not this release cycle.

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[fmeum]

This has been fixed in Bazel 7: #19330