google / bazel-common

Common functionality for Google's open-source libraries that are built with bazel.
Apache License 2.0
85 stars 40 forks source link

Some libraries should have deps #169

Closed cpovirk closed 2 weeks ago

cpovirk commented 1 year ago

I think I almost filed this issue once before. This time, I'm going to do it for real.

As an example, asm-tree should depend on asm, and and asm-commons should depend on both asm-tree and asm. Without those deps, it's possible to get errors like class file for org.objectweb.asm.tree.MethodNode not found.

The trick is that targets like asm-tree are java_library targets, which export jvm_import_external-generated java_import targets. And while java_import targets can have deps, java_library targets can't have deps unless they have srcs. (runtime_deps would be wrong here; exports is overkill/unhygienic but would at least work.)

Possible solutions:

cpovirk commented 3 weeks ago

https://github.com/google/flogger/issues/387 has the latest mention of rules_jvm_external, for which we once had https://github.com/google/bazel-common/pull/87 open.

chaoren commented 2 weeks ago

FYI, rules_jvm_external does produce targets with the correct deps, so this should be obsolete once #165 is finished.

chaoren commented 2 weeks ago

Obsolete after e2204d625d97dda88112af0b0a9e56ed7712d15c. Now some libraries have redundant deps that could be removed instead.