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

Some libraries should have deps #169

Closed
cpovirk opened this issue May 16, 2023 · 3 comments
Closed

Some libraries should have deps #169

cpovirk opened this issue May 16, 2023 · 3 comments
Labels
bug Something isn't working P3

Comments

@cpovirk
Copy link
Member

cpovirk commented May 16, 2023

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:

  • Continue to live with it, adding transitive deps explicitly to the targets in bazel-common users' projects as errors crop up.
  • Use exports (again, "overkill/unhygienic but would at least work").
  • Set srcs to an empty srcjar (either one that we check in, though that would likely require compliance exemptions, or one that we autogenerate at build time). Ideally we'd generate only one such jar and share it across libraries.
  • See if jvm_import_external can set deps (maybe?) or if we should be using something else, like maven_install (which sounds more likely to do this for us). [edit: Err, did something named "maven_install" subsequently become part of rules_jvm_external??] (We may have an issue open in one of our projects about whether maven_install could largely replace bazel-common. Compare what gRPC did.) [edit: The thing we currently use is maven_import, our wrapper around java_import_external.]
@cpovirk cpovirk added bug Something isn't working P3 labels May 16, 2023
@cpovirk
Copy link
Member Author

cpovirk commented Sep 25, 2024

google/flogger#387 has the latest mention of rules_jvm_external, for which we once had #87 open.

@chaoren
Copy link
Member

chaoren commented Oct 2, 2024

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

copybara-service bot pushed a commit that referenced this issue Oct 2, 2024
copybara-service bot pushed a commit that referenced this issue Oct 3, 2024
copybara-service bot pushed a commit that referenced this issue Oct 3, 2024
copybara-service bot pushed a commit that referenced this issue Oct 4, 2024
@chaoren
Copy link
Member

chaoren commented Oct 4, 2024

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

@chaoren chaoren closed this as completed Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P3
Projects
None yet
Development

No branches or pull requests

2 participants