@com_google_protobuf is used, but never declared as dependency in any of the workspace set-up

[hzeller]

The com_google_protobuf rule is used as a direct dependency in various rules within xls:

find . -name "BUILD" | xargs grep @com_google_protobuf

Yet, it is never declared as such in any of the workspace set-up, but apparently it is only initialized there, assuming it is magically there:

$ grep com_google_protobuf WORKSPACE $(find dependency_support/ -type f)
dependency_support/initialize_external.bzl:load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

It is only initialized, so it looks like it is 'accidentally' imported by some of the other dependencies that this happens to work ?

It would be good to be able to reason what dependencies come into the project., by either explicit way to either explicitly adding them to the dependency files, or declare them otherwise (don't know if there is an alias of sort, but at least a comment in the WORKSPACE file would help # @com_google_protobuf provided from from @mystery_import).

That way it is possible to read and understand and track down weird issues (recent issue: the magically imported google protobuf python rule that was there, but that didn't work).

While at it, maybe there are more rules that are used but never declared ?

find xls -name "BUILD" | xargs grep '"@' | sed 's|.*"@\([^"/]*\).*|\1|p' | sort | uniq

Essentially, we need an IWYU for bazel rules :)

[cdleary]

Is there a particular thing for the XLS project's setup to do/fix here? I may be missing the actionable part. Or is this more of a Bazel bug?

In the project as Bazel users we generally assume that we can use constructs like proto_library/proto_cc_library/the C++ proto libraries/etc and that those do whatever they need to do in the workspace to get set up under an abstraction like protobuf_deps() here:

xls/dependency_support/initialize_external.bzl

Line 29 in 0dd5bee

protobuf_deps()

[hzeller]

The actionable part would be to add all the dependencies needed in the WORKSPACE or load_external.bzl part to not rely on dependencies (such as @com_google_protobuf) that are there 'accidentally'.

The com_google_protobuf is not something that comes with bazel by default, so it maybe is inherit it with something else like grpc (don't know which is is, this is why I stumbled upon it)

[cdleary]

In the process of marking this closed now -- summary of why it actually seems better to let transitive dependency versions float with the way we currently do things is given in #931 (comment) It seems like the right solution that would actually help us detect version conflicts between our needs and transitive dependency needs would be to switch to bzlmod -- the "maybe pull this archive and bind it to name @some_module" expression model that Bazel uses in WORKSPACE files right now doesn't make pinning up front inherently more effective at finding compatibility issues, while introducing more work.

We could list all the things that are pulled directly, but that would get stale because there's no cross checking. I'll leave a one liner here in case it's useful for those sorts of purposes in the future:

$ find -name 'BUILD' | xargs grep '@' | perl -pe 's/.*(@\w+).*/\1/' | sort | uniq

I think I'll close this out with a warning comment that just says "there are more direct dependencies than are listed in this file", see $THIS_GITHUB_ISSUE for details.

[cdleary]

Closed via c37d7ad

[hzeller]

Sounds like a reasonable approach given the WORKSPACE circumstances.