cel-go icon indicating copy to clipboard operation
cel-go copied to clipboard

Non-public bazel visibility for common/ast

Open korylprince opened this issue 1 year ago • 1 comments

Feature request checklist

  • [x] 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 ❤️

korylprince avatar Feb 16 '24 02:02 korylprince

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 avatar Feb 21 '24 16:02 TristonianJones

Closed by #904

TristonianJones avatar Apr 30 '24 04:04 TristonianJones