-
-
Notifications
You must be signed in to change notification settings - Fork 260
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
Document how to deal with 'Found duplicate artifact versions' #1303
base: master
Are you sure you want to change the base?
Conversation
This PR is actually the opposite of the convention that we've been using: always use This came up recently in bazelbuild/bazel-worker-api#7 and #1295 was an attempt to make the logs a little clearer. //cc @shs96c |
Here's the mental model that I use. Projects that are expected to be non-root modules (eg. rulesets and libraries) should define a The key benefit of this is that non-root modules that know that they absolutely must contribute to the default resolution can add things to the default namespace, and it'll work nicely. At the same time, most users won't need to think about which namespace their dependencies are coming from (since we always default to As a concrete example, OTOH, in the Selenium project (which we always expect to be a root module), we use the default namespace. So, for the majority of users, I suspect that the default namespace will be just fine, but we should emphasise that projects which are expected to be non-root modules shouldn't be using |
From what (little, I admit) I understand, as-is currently not all "non-root modules (eg. rulesets and libraries" comply with what's outlined above (notably gRPC & Proto?), yet - correct? Ergo, the documentation as-is currently (without something like this) isn't as helpful as it could be - agreed? What I could do is amend this PR by adding something based on the comment above - would that be helpful? |
You're correct. Essentially anything in the BCR is likely to be a non-root module, and that includes both grpc and proto. Amending the PR would be really helpful -- it's a source of confusion for our users, so making it clearer would be a boon. Thank you for engaging in the discussion, and thank you for your work too! |
maven.install( | ||
your_maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven") | ||
your_maven.install( | ||
# NB: Do *ALWAYS* specify a name, and NEVER use the (default) name = "maven"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't tell people to workaround things and then cause me extra grief as users get duplicate (and not identical) classes in their class path.
This all stems from java_proto_library. Its entire existence it has brought in java_library()s from @com_google_protobuf
. That means you need overrides to prevent Maven Central dependencies bringing in a different copy of protobuf and put it also on your classpath. Today you can't use the protobuf Maven Central binaries.
Reusing the "maven" name is on purpose and is the only functional way today. Also, since protobuf depends on Maven Central binaries itself, you need a single resolution with the application. See https://stackoverflow.com/questions/54119248/how-to-upgrade-a-bazel-aware-librarys-maven-dependency where I tried to get this all working before maven_install existed. With both maven_install() and maven.install() we need a single resolution of versions to take place.
Note that that is all very separate from rules needing maven.install() for their own processing. That should indeed be a different name because those dependencies won't ever be part of the user's classpath.
Re. #708 & #916
This did the trick, at least for me.