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

package(default_visibility=X) differs from visibility=X #13681

Closed
dmivankov opened this issue Jul 14, 2021 · 3 comments
Closed

package(default_visibility=X) differs from visibility=X #13681

dmivankov opened this issue Jul 14, 2021 · 3 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug

Comments

@dmivankov
Copy link
Contributor

Description of the problem / feature request:

package(default_visibility = X) works differently from explicitly setting visibility = X
Namely visibility= can refer to other @repository//some:__pkg__, but default_visibility ignores such entries.

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

A/WORKSPACE

workspace(name = "A")

new_local_repository(
  name = "B",
  path = "../B",
  build_file_content = """
package(default_visibility = ["@A//:__pkg__"]) # line 1
genrule(
  name = "b.gen",
  outs = ["b"],
  cmd = "echo >$@",
#  visibility = ["@A//:__pkg__"], # line 2
)
  """
)

A/BUILD

alias(
  name = "a",
  actual = "@B//:b",
)

genrule( # to also test non-trivial targets
  cmd = "echo >$@",
  name = "aa",
  outs = ["aaa"],
  srcs = ["@B//:b"],
)

B - empty directory

$ cd A
$ bazel build @B//:b
# ok

$ bazel build @A//:a
A/BUILD:1:6: in alias rule //:a: target '@B//:b' is not visible from target '//:a'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: Analysis of target '//:a' failed; build aborted: Analysis of target '//:a' failed

$ bazel build @A//:aaa
A/BUILD:6:8: in genrule rule //:aa: target '@B//:b' is not visible from target '//:aa'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: Analysis of target '//:aaa' failed; build aborted: Analysis of target '//:aa' failed

but if line 1 is commented out in A/WORKSPACE and line 2 uncommented all 3 build commands suddenly start to work

What operating system are you running Bazel on?

NixOS

What's the output of bazel info release?

release 4.1.0- (@non-git)

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

via NixPkgs

@oquenchil oquenchil added team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website type: bug untriaged labels Jul 14, 2021
@philwo philwo added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jul 16, 2021
@brandjon brandjon added team-Build-Language P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website P2 We'll consider working on this in future. (Assignee optional) labels Aug 20, 2021
@brandjon
Copy link
Member

default_visibility is parsed using PackageUtils.getVisibility, which dispatches to PackageGroupsRuleVisibility, which tries to process the arg as a package_group package path string, and failing that considers the arg to be a label of a package_group target.

So the problem is that default_visibility does not recognize the same cases that visibility does. It's instead a bizarre mix of package_group and visibility syntax, that allows //pkg/... and //visibility:public, but not //pkg:__subpackages__.

dmivankov added a commit to dmivankov/rules_jvm_external that referenced this issue Jan 4, 2022
…ependencies to some scopes

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to bazel-contrib#648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)
dmivankov added a commit to dmivankov/rules_jvm_external that referenced this issue Jan 5, 2022
…ependencies to some scopes

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to bazel-contrib#648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)
dmivankov added a commit to dmivankov/rules_jvm_external that referenced this issue Jan 18, 2022
…ependencies to some scopes

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to bazel-contrib#648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)
shs96c pushed a commit to bazel-contrib/rules_jvm_external that referenced this issue Jan 19, 2022
…ependencies to some scopes (#649)

Note that using package default_visibility doesn't always work so explicit visiblity attributes are needed
bazelbuild/bazel#13681

Somewhat related to #648

In combination this for example allows to create a class_name -> maven artifact resolver for
transitive dependency classes while still keeping strict visibility (making transitive dependencies
visible only to the resolve cli tool)
@brandjon brandjon added team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob and removed team-Build-Language labels Nov 4, 2022
Copy link

github-actions bot commented Jan 9, 2024

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.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 9, 2024
Copy link

github-actions bot commented Apr 9, 2024

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. team-Loading-API BUILD file and macro processing: labels, package(), visibility, glob type: bug
Projects
None yet
Development

No branches or pull requests

4 participants