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

Add a MODULE.bazel in order to support for bzlmod #9559

Closed
shs96c opened this issue Sep 22, 2022 · 16 comments · Fixed by #11046
Closed

Add a MODULE.bazel in order to support for bzlmod #9559

shs96c opened this issue Sep 22, 2022 · 16 comments · Fixed by #11046

Comments

@shs96c
Copy link

shs96c commented Sep 22, 2022

Is your feature request related to a problem?

In Bazel 6, a new feature called bzlmod will ship. This is like a "package manager for Bazel rulesets", and using it avoids a lot of setup work being need to be done in a WORKSPACE.

Describe the solution you'd like

When run, bzlmod looks for rulesets on the Bazel Central Registry. Once the MODULE.bazel is added to this project, it can then be registered with the BCR, and then people can start using it from the modularised builds.

Describe alternatives you've considered

There are no alternatives at the moment.

Additional context

While there are workarounds to get things working, it would be nice if this was available before Bazel 6 is released, so projects can migrate to using bzlmod ASAP.

@sanjaypujare
Copy link
Contributor

Support for bazel is on best effort basis. You may want to consider contributing this enhancement so it can then be reviewed.

@ejona86
Copy link
Member

ejona86 commented Sep 26, 2022

Because of the integration with BCR, it looks like we may need to be the one to do the last parts of this. And unclear if we'll need a new part to the release process or something. Looks like protobuf is out-of-date and we'll need to have yet-another copy of all our Maven dependencies (seems to replace maven_install).

This is probably all for the best, but will be annoying during the transition and will need some effort to rework.

@shs96c
Copy link
Author

shs96c commented Jan 10, 2023

You can move the build to use Bazel 6, so you'll only have one version of your maven_deps in the tree (in a MODULE.bazel file). Users of the library won't be impacted by that change.

@ejona86
Copy link
Member

ejona86 commented Jan 10, 2023

You can move the build to use Bazel 6... Users of the library won't be impacted by that change.

If we don't test with Bazel 5, we'll break something. And we'd still need to support the maven_install approach (for Bazel 5 users, and Bazel 6 users that haven't swapped to modules), thus the duplication I noted.

@vorburger
Copy link
Contributor

There IS a https://registry.bazel.build/modules/grpc but it doesn't define @io_grpc_grpc_java and is presumably only for https://github.com/grpc/grpc? I couldn't find any documentation what that module actually provides.

@tmhdgsn
Copy link

tmhdgsn commented Aug 7, 2023

Has there been any movement on this ? Thanks !

@ah-quant
Copy link

ah-quant commented Dec 12, 2023

Bazel 7 is released now and MODULE.bazel is enabled by default. The release of Bazel 8 will disable WORKSPACE by default and Bazel 9 will remove it. Seems like the perfect time to pick this up again.

This issue is tagged with "help wanted" and I experimented a bit but hit a snag. Working on this probably also needs module support in https://github.com/googleapis/googleapis (there's even already a PR, googleapis/googleapis#855)...

I'm not hopeful that a non Googler can do anything in googleapis. I'm not even sure issues and PRs in that repo are even read. Can you tell us a bit about the situation there or nudge someone into looking at it? (edit I misjudged, commits by external contributors seem to exist. But this probably has to be aligned with internal code and will be quite intricate)

This will probably also affect grpc/grpc#35271

@ejona86
Copy link
Member

ejona86 commented Dec 12, 2023

External contributors should be okay, but yeah, I see even a trivial PR updating a broken link has been languishing. @veblush, do you know who best to poke on googleapis to review PRs?

Googleapis is a severe risk for this item. Its dependency situation tends to take some effort in the best of times, and there's circular dependencies between multiple grpc repos and googleapis that seem will impact this. I see the mention of use_repo; I know nothing about it, but it sounds like we may need it here.

@SanjayVas
Copy link

there's circular dependencies between multiple grpc repos and googleapis that seem will impact this

Indeed, comments on the googleapis PR (googleapis/googleapis#855 (comment)) suggest that they're blocked on gRPC for migrating to Bzlmod. I assume this is because googleapis optionally generates per-language gRPC targets. What probably contributes to dependency issues is the fact that googleapis is the canonical open source location for global common components in addition to being the location for Google service APIs.

Separately, I agree that this repo probably was never the right place for the java_grpc_library Bazel rule. IIUC, that's the only reason that Bazel is referenced at all by this repo. I'm on the team that originally contributed the kt_jvm_grpc_library rule to grpc-kotlin, but we switched to maintaining our own version of the rules so that we can use the gRPC library deps from Maven instead of compiling from source (which is important if you want to release your own library as a Maven artifact). I'm currently working on migrating our project repos to Bzlmod and split these rules out as a Bazel module into a separate repo. Note that the rules only support my project's specific requirements, e.g. are only used on Linux x64.

@keith
Copy link
Contributor

keith commented Mar 27, 2024

#11046

@ejona86 ejona86 modified the milestones: Next, 1.64 Apr 4, 2024
@hlawrence-cn
Copy link

Perhaps this is a limitation in bzlmod as much as it is an issue in this repo, but when attempting to migrate from rules_jvm to bzlmod for grpc-java, I hit issues where grpc-java pulls in guava-android, while we need the standard guava (we use some apis not included in the android version). While at compile time, this works fine, at runtime, whether guava-android or guava-jre get pulled in appears to be non-controllable.

When pulling in grpc-java via maven on rules_jvm, we could simply pin to the jre version / exclude the android one. Also, at build time we would receive a warning that multiple versions of guava were pulled in. With this, we do not.

I don't really have a suggested solution here, just worth noting -- and I'm sure others will see this behavior as well. Obviously there's nothing keeping us from continuing to use the maven version, at least for now.

Happy to open an issue for this if it's useful.

@hlawrence-cn
Copy link

Opened #11102

@keith
Copy link
Contributor

keith commented Apr 12, 2024

I think you probably want a maven.override similar to the ones in my PR?

@shs96c
Copy link
Author

shs96c commented Apr 12, 2024

@hlawrence-cn, you should be able to select which version of guava you want by setting it in your maven.install tag. If the one from the root project isn't the one that gets resolved, please file an issue with rules_jvm_external.

@hlawrence-cn
Copy link

Right -- using maven.install I can -- the issue comes when trying to use grpc-java via bzlmod instead of maven.install. Though perhaps the idea would be to pull java sources via maven still, and just use the @grpc-java repo pulled via bzlmod for the grpc service targets?

@hlawrence-cn
Copy link

Probably makes sense to move conversation to #11102

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 12, 2024
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants