-
Notifications
You must be signed in to change notification settings - Fork 121
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
FEAT: use maven_server and maven_jar to fetch artifacts. #239
base: master
Are you sure you want to change the base?
FEAT: use maven_server and maven_jar to fetch artifacts. #239
Conversation
a9562a8
to
8295d2a
Compare
Thanks for the PR. I’m on vacation today and tomorrow. One thing I’m concerned about is this is almost an exact revert of a previous PR. So this implies there are at least two different constituencies. One question: why isn’t this a small change to your own repo: I.e. configure your own repo’s callback to use maven_jar? Why do we need to change the default (especially considering I think bazel has deprecated native maven_jar). |
@johnynek Thanks for the review! Feel free to answer after you come back from vacation. This is my motivation for the change:
Let me know what you think! Enjoy the rest of your vacation 💯 |
so @hsyed and @rdnetto4 have taken stabs at this problem, but we still aren't where we think it is solved. This PR reverts the previous work of @hsyed #181 where the jar_artifact rule was made the new default callback: https://github.com/johnynek/bazel-deps/pull/181/files#diff-2af0d4e7b4b60cc36e83ad2149e68f51R50 What I'm concerned with is that it seems people have bespoke needs, they often make a PR to change the defaults to match their needs, then they move on and the next person repeats. My question: why can't we accomplish using maven_jar using the existing callback mechanism? In that approach, you write a starlark function in your repo, and pass that function to maven_dependencies in order to control how you load the maven dependencies? That was the purpose of the callback architecture. Your change moves us back to how things used to be, by the way, but maven_jar didn't support sha256, so others wanted to change the default. Note, at stripe, we override this function, so I don't tend to worry too much, except that I clearly just accepting the most recent version of this and flipflopping around isn't an approach that makes sense. What I'd like to do is outline:
I'd love for others to chime in so we can make some progress. Thanks for bearing with me. |
There are also issues that bazel will blindly invoke a bazillion maven_jars in parallel and bring a machine to its knees: bazelbuild/bazel#5452 |
This PR is an extension to: #215, and aims to close #95. It replaces our custom download code, with
maven_server
andmaven_jar
. This allows users to download artifacts from authenticated servers by just having$HOME/.m2/settings.xml
properly set up. Feedback welcomed!