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

Non-public bazel visibility for common/ast #896

Closed
1 of 3 tasks
korylprince opened this issue Feb 16, 2024 · 2 comments
Closed
1 of 3 tasks

Non-public bazel visibility for common/ast #896

korylprince opened this issue Feb 16, 2024 · 2 comments

Comments

@korylprince
Copy link

korylprince commented Feb 16, 2024

Feature request checklist

  • There are no issues that match the desired change
  • The change is large enough it can't be addressed with a simple Pull Request
  • If this is a bug, please file a Bug Report.

Change
Please consider changing the common/ast package's bazel visibility to //visibility:public.

common/ast is not an internal package, so go build and friends have no issue building code that uses the package, but common/ast's BUILD.bazel sets the default_visibility to __subpackages__.

It appears this change was made intentionally, but it causes issues when trying to build code that uses common/ast with bazel/gazelle, because common/ast is not externally visible:

ERROR: <my_code>/BUILD.bazel:3:11: in go_library rule <my_code>: target '@com_github_google_cel_go//common/ast:ast' is not visible from target '<my_code>'. Check the visibility declaration of the former target if you think the dependency is legitimate

Use case
I'm using ast.Visitor to walk the ast and perform extra validation on parsed expressions. There doesn't seem to be any reason to make this very useful library private just for bazel/gazelle builds.

Workarounds

I've found a couple work arounds:

go_repository(
    name = "com_github_google_cel_go",
    build_file_name = "build.bazel",
    build_file_generation = "on",
    build_file_proto_mode = "disable_global",
    importpath = "github.com/google/cel-go",
    sum = "h1:vVgaZoHPBDd1lXCYGQOh5A06L4EtuIfmqQ/qnSXSKiU=",
    version = "v0.19.0",
)

This is a hack that causes gazelle to ignore the BUILD.bazel files in the repo because the case-sensitivity doesn't match the build file names, and generate it's own. Note: build_file_generation = "on" on it's own won't write it's own build files.

The other option is to omit the build_file_name directive from above, and reference the dep as @com_github_google_cel_go//common/ast:go_default_library instead of @com_github_google_cel_go//common/ast.

This works because build_file_generation = "on" adds a :go_default_library -> :ast alias with //visibility:public.

P.S.

Thanks for the great library ❤️

@TristonianJones
Copy link
Collaborator

TristonianJones commented Feb 21, 2024

I would be happy to make it public. I had kept it private until someone asked. ;) Truth be told the package was in development for some time and I didn't want people to use it while I was still actively modifying the internals. It should be ready for public consumption now.

@TristonianJones
Copy link
Collaborator

Closed by #904

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants