Add support for bzlmod #287
Conversation
|
The hardest part of this integration is to pull in the starlark library from bazel, right now some of the bazel build config is duplicated here, although that should be removable. Open question: would you be open to dropping the non-bzlmod based build? I haven't touched that yet here, I think we could make both work at once, but we could also delete the old one if it's not necessary with this. |
|
It turns out that it's easy enough to keep non-bzlmod working, so I made the minor adjustments required there |
| @@ -19,15 +19,15 @@ licenses(["notice"]) | |||
| genrule( | |||
| name = "normalised_buildozer", | |||
| testonly = 1, | |||
| srcs = ["@buildtools//buildozer"], | |||
There was a problem hiding this comment.
buildtools isn't in the bcr yet, this repo vendors the downloads directly from that repo's releases
There was a problem hiding this comment.
Adding links regarding buildtools migration to bzlmod:
There was a problem hiding this comment.
We currently automatically vendor buildtools from HEAD because we had needs to be pretty close to the edge. I wonder if this can be treated like io_bazel?
There was a problem hiding this comment.
the problem is that pulling in buildtools that way would require pulling rules_go and resolving the go.mod transitive dependencies of that repo as well.
There was a problem hiding this comment.
I see. I will look into merging this provided we don't end up with too much version skew for build_tools vs the internal monorepo.
There was a problem hiding this comment.
sounds good, i don't feel like there are a ton of changes there enough to cause major drift
| ) | ||
| maybe( | ||
| maven_install, | ||
| name = "maven", |
There was a problem hiding this comment.
for bzlmod it's better to have a unique name for a repository that might be a dependency of something else, but for building bazel we have to use this name as well, right now these are dup'd but in bzlmod they have a separate set of deps
|
Funnily enough, I have a very similar change in my local repo and ran into exactly the same issue. |
|
we've been using this internally for a while, i think it's good to go |
|
This has been pending for a while, could someone review it? It would be very nice to have it merged. |
|
Can confirm this works in combination with #233. I did this: Then just |
|
Thank you! I will probably remove the wholesale bazel dependency as we only use a tiny part of the codebase that seems suitable for vendoring - just to avoid the churn of the ever growing dependency list. |
|
thanks! |
Fixes #220