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

gazelle marks packages as internal for some reason #660

Closed
kyessenov opened this issue Jul 27, 2017 · 5 comments
Closed

gazelle marks packages as internal for some reason #660

kyessenov opened this issue Jul 27, 2017 · 5 comments
Assignees

Comments

@kyessenov
Copy link

Trying to build k8s client-go from HEAD produces the following error:

bazel build @io_k8s_client_go//...
ERROR: /home/kuat/.cache/bazel/_bazel_kuat/9ff0653fb8eda31e5f7b80b7b7a8a577/external/io_k8s_client_go/vendor/golang.org/x/text/language/BUILD.bazel:3:1: Target '@org_golang_x_text//internal/tag:go_default_library' is not visible from target '@io_k8s_client_go//vendor/golang.org/x/text/language:go_default_library'.
Check the visibility declaration of the former target if you think the dependency is legitimate.
ERROR: Analysis of target '@io_k8s_client_go//vendor/golang.org/x/text/language:go_default_library' failed; build aborted.
INFO: Elapsed time: 0.452s

Peeking into BUILD file for x/text, I find this:

load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
    name = "go_default_library",
    srcs = ["tag.go"],
    visibility = ["//:__subpackages__"],
)

Why is visibility not public?

@kyessenov
Copy link
Author

The offending line appears to be this:
https://github.com/bazelbuild/rules_go/blob/master/go/tools/gazelle/rules/generator.go#L162

There's some override happening that rewrites from projectX/vendor/projectY to projectY that fails to resolve due to visibility.

@jayconrod
Copy link
Contributor

"internal" directories are supposed to be private. We should probably actually be more restrictive there, since they should be private to the directory that contains them, not the whole repository.

This seems like an issue with vendoring though. I'll analyze further tomorrow. At the moment, external dependencies and vendoring don't mix well in Gazelle.

@kyessenov
Copy link
Author

Let me know if there's some work around. Here's the workspace and the dependency: https://github.com/istio/pilot/pull/966/files#diff-09498dbadf45966909850dc8a47ebb13R287

@jayconrod
Copy link
Contributor

I don't have a simple fix for this yet, but I'll explain why it's happening, and I can tell you what we're doing to fix it.

The error is coming from a rule in an external repository defined with go_repository, @io_k8s_client_go. BUILD.bazel files are generated by go_repository automatically in external mode. External mode determines how Gazelle converts import paths into Bazel dependencies. Any import that is outside of the repository prefix (k8s.io/client/go) will be converted to an external label. The vendor directory does not get special treatment in this mode. So golang.org/x/text/internal/tag will be resolved as @org_golang_x_text//internal/tag:go_default_library, even though it's imported from the vendor directory, and it could be resolved in the vendor directory.

There is no way to switch Gazelle into vendored mode for go_repository. We have discouraged mixing external repositories and vendoring, though we're probably being unnecessarily rigid. The assumption is that if Gazelle is in external mode, all dependencies should be external.

I think the bug here is that we're even generating rules for packages in vendor directories. Nothing will depend on these packages, and the rules we are generating don't make sense. Gazelle should not recurse into vendor directories in external mode.

In #608 I'm working on a "flat mode" for Gazelle. This will allow you to generate a single BUILD file for an external repository and use that with new_git_repository instead of go_repository. In #460, we plan to remove go_repository once flat mode is working and is easy to use. I expect flat mode to be out in another week or so. This will allow a great deal of customization, and will be an effective workaround for cases where Gazelle doesn't do the right thing.

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jul 28, 2017
In external mode, nothing will depend on rules in vendor
directories. They are not likely to build.

Fixes bazel-contrib#660
@kyessenov
Copy link
Author

Thanks for explanation! I'm using a simple workaround of disabling visibility checks at the moment. Looking forward to flat mode.

jayconrod added a commit that referenced this issue Aug 1, 2017
In external mode, nothing will depend on rules in vendor
directories. They are not likely to build.

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

No branches or pull requests

2 participants