-
Notifications
You must be signed in to change notification settings - Fork 232
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: port project to bzlmod. #553
Conversation
UPDATE: Solved. |
Had an old go at this before, ditched as I was trying to add boost to bzlmod directly but ran out of time for the project. Reinstated in #562 as it might help! I also noted build errors with lzma on my machine, so using LZMA straight from bcr in the PR, but I have no idea if that will be backwards compatible with workspace (or if bazel just lets users use both alongside, if so, this should work fine as it stands!), if you have the time available @finn-ball to test then that PR should be good to go, even though I fully believe boost build rules should be natively rewritten for BCR in the long run, see here for work in progress I'm using this on a mac with bazel 7 and bzlmod which is now default. I haven't tried with workspace or on many other systems just yet, testing needed! Let me know if there's anything further I can help with! |
Ohhh I may have misunderstood your PR @finn-ball, you just upgraded the deps to BCR modules, rather than making this whole project bzlmod compatible? I like the dep upgrade, would be great if this PR could get finished off, would go nicely with #562 anyway! |
I updated some dependencies to use bazel modules but also the changes were compatible for this to be used with BCR. I had some of my own internal bazel projects depend on this as a bazel module. You can also see I updated the tests to use:
I couldn't reproduce the error in CI, so couldn't solve the error and no one else showed interest. |
Rebased and updated. To be clear this port will be necessary as |
Using the
If this is merged, an issue needs to be created to resolve this. I have commented out the use of the BCR |
@nelhage - this now passes CI. Please feel free to merge. |
Looks great, good job @finn-ball!🙌 |
Any thoughts @Vertexwahn ? |
@finn-ball Once we merge this this the old approach of using |
You'll be forced to use bzlmod eventually as the |
sha256 = "7ce152bdce1b85344cc36c6b255aab36905d39187c2c2f797a69d5ad220076ee", | ||
strip_prefix = "boringssl-0f1a639954dd7ab86f5f4ddd8b4e2edbea492acd", | ||
http_archive( | ||
name = "zlib", |
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.
Why do you keep zlib here? It is available in the bazel registry so you could get it via bzlmod
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.
See the above error in this PR! If this is merged, then an issue should be created to resolve this.
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.
@finn-ball Thanks for your contribution! |
Ported to bzlmod:
#546
It is now possible to depend on this project with: