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

feat: port project to bzlmod. #553

Merged
merged 3 commits into from
May 25, 2024
Merged

Conversation

finn-ball
Copy link
Contributor

@finn-ball finn-ball commented Mar 26, 2024

Ported to bzlmod:
#546

It is now possible to depend on this project with:

bazel_dep(name = "com_github_nelhage_rules_boost")

@finn-ball finn-ball changed the title Port project to bzlmod. feat: port project to bzlmod. Mar 26, 2024
@finn-ball
Copy link
Contributor Author

finn-ball commented Mar 26, 2024

UPDATE: Solved.

@Attempt3035
Copy link

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!

@Attempt3035
Copy link

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!

@finn-ball
Copy link
Contributor Author

finn-ball commented May 16, 2024

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:

bazel_dep(name = "com_github_nelhage_rules_boost")
local_path_override(
    module_name = "com_github_nelhage_rules_boost",
    path = "..",
)

non_module_boost_repositories = use_extension("@com_github_nelhage_rules_boost//:boost/repositories.bzl", "non_module_dependencies")
use_repo(
    non_module_boost_repositories,
    "boost",
)

I couldn't reproduce the error in CI, so couldn't solve the error and no one else showed interest.

@finn-ball
Copy link
Contributor Author

Rebased and updated. To be clear this port will be necessary as WORKSPACE becomes deprecated in favour of MODULE files @nelhage

@finn-ball
Copy link
Contributor Author

finn-ball commented May 16, 2024

Using the bazel_dep of zlib generates this error in CI which I could not reproduce locally:

external/com_github_nelhage_rules_boost~~non_module_dependencies~boost/libs/iostreams/src/zlib.cpp:20:10: error: use of private header from outside its module: 'zlib.h' [-Wprivate-header]
#include "zlib.h"   // Jean-loup Gailly's and Mark Adler's "zlib.h" header.
         ^
1 error generated.

If this is merged, an issue needs to be created to resolve this. I have commented out the use of the BCR zlib for now.

@finn-ball
Copy link
Contributor Author

finn-ball commented May 16, 2024

@nelhage - this now passes CI. Please feel free to merge.

@Attempt3035
Copy link

Looks great, good job @finn-ball!🙌

@finn-ball
Copy link
Contributor Author

Any thoughts @Vertexwahn ?

@Vertexwahn
Copy link
Collaborator

@finn-ball Once we merge this this the old approach of using WORKSPACE files will not working anymore - personally for me no problem ;) - @cpsauer Are you fine with moving over to Bzlmod and dropping the WORKSPACE approach?

@finn-ball
Copy link
Contributor Author

You'll be forced to use bzlmod eventually as the WORKSPACE will be depreciated in the next major release iirc. With bazel 7, bzlmod is on by default.

sha256 = "7ce152bdce1b85344cc36c6b255aab36905d39187c2c2f797a69d5ad220076ee",
strip_prefix = "boringssl-0f1a639954dd7ab86f5f4ddd8b4e2edbea492acd",
http_archive(
name = "zlib",
Copy link

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

Copy link
Contributor Author

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.

Copy link
Contributor Author

@finn-ball finn-ball May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: #567
PR: #565

See here for the currently failing build when we switch zlib to BCR

@Vertexwahn Vertexwahn merged commit 9d45a1b into nelhage:master May 25, 2024
2 checks passed
@Vertexwahn
Copy link
Collaborator

@finn-ball Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants