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

bzlmod compatibility #522

Closed
wants to merge 4 commits into from
Closed

bzlmod compatibility #522

wants to merge 4 commits into from

Conversation

Attempt3035
Copy link

@Attempt3035 Attempt3035 commented Nov 17, 2023

Hi all!

I'm setting up some build systems with Bazel's new bzlmod system, and finding it quite hard to get dependencies that use the workspace system to work with it, due to the changes in how transitive dependencies are made available.

I recently helped add support to hedronvision's bazel-compile-commands-extractor (Thanks @cpsauer for the help!)

I couldn't find a way to get this project to work with bzlmod out of the box at all, due to the fact that the actual boost dependency is loaded in via macro, but is a dependency of this project, and therefore isn't made available to any project that includes this one in any way as far as I can tell.

Therefore, with some testing, this is the start of the solution I've come up with. Basically, we first add the MODULE.bazel and via extensions load in the dependencies the same way it is currently done. Then, we add the repos so they are avilable to this project, and in the BUILD.bazel, we make aliases which alias each of the boost functions publicly as targets of the current repository. This works really well. It works properly even for multi-repo bazel setups (which I use), as the top layer repo brings in the dependency and overrides it with git or locally:

bazel_dep(name = "boost")

# Local Override
local_path_override(module_name = "boost", path = "/Users/UserName/Desktop/rules_boost")

# Git Override with this fork
git_override(
    module_name = "boost",
    remote = "https://github.com/Attempt3035/rules_boost.git",
    commit = "a5a8d76dddbeca8eb8663d007988c119212bc9e6",
)

Then, other repos can do the same, or just simply declare the dep (assuming they won't ever be built on their own, if they are just libraries) bazel_dep(name = "boost") and then use any of it's targets, ie

deps = [
    "@boost//:filesystem",
    "@boost//:log",
    "@boost//:algorithm"
],

Now, for the unfortunate part.
Firstly, this seems kind of hacky. I'm not sure if it would be a mammoth job to properly integrate bzlmod with the project so we don't need aliases, but I kind of couldn't figure out how it would work considering there's other deps that need to be built and added and it all looks very complicated to me. It also seems like it would be a nightmare to maintain two systems, so maybe aliasing is the best.
Secondly, I had a hell of a time getting around issues related to lzma build using COPTS paths that didn't end up going the right place (using overrides for dep definition adds extra stuff to the path). I think this issue is related. The temporary hacky fix is including them using the includes [] section, as it works relative to the workspace, but that obviously pollutes the includes of the whole project which isn't great. I couldn't find a good way to dynamically get the path. Possibly a custom Make Variable, or maybe I've missed an obvious solution. Either way, I'd appreciate some help solving that bit properly!

Notes:
I used the repo_name feature in the module definition, as it allows us to avoid the repo name collision with the currently defined boost include via http, but I wanted to ensure when users include this module, they use the name boost, so that targets are @boost as it makes more sense and could be swapped out with another method of getting the boost dependency one day.
I had to use the hacky fix for the platforms bug:

bazel_dep(
    name = "platforms",
    version = "0.0.7",
)

I believe it's a known bug Here Too, but this fix worked for me.
Also, I only wrote three aliases as proof of concept for testing. There may be a better way to do this (I hope!), possibly through an extension.

I'd really appreciate any help or comments at all, especially if I've overlooked anything or am on the wrong path. I'd like to keep working on this to get it to be a proper solution, as bzlmod is what I and likely more people in the future will be using, and boost is such an essential part of so much of our c++ code 😝

Cheers!

…pendent private targets, meaning hdrs use workspace based pathing (no issues when being used via override), no project "includes" pollution and strip include prefix for each path.
@Attempt3035
Copy link
Author

Hi all!

Status update:
Found a permanent solution for the lzma headers includes issue that now works regardless of overrides or the paths that everything ends up at. Solves the include scoping issues, as well as reducing visibility of the api file to be just includable when lzma is a direct dependency. (Before it was exposed to everything via includes [].) There are also 5 other cases where COPTS -I is used, these don't seem to be breaking my build scenario yet (I haven't tested projects that use those libs), but I'm guessing I'll see the same issues arise, so confirmation on if this style solution is appropriate would be great!

Completed aliasing all targets via a macro, which seems very tidy and effective. I am not sure if we need lower level targets aliased too or if what I've done with just top level proves satisfactory.

It looks like everything works properly for bzlmod now, further testing is needed though. I am aware that the aliasing system currently would add unneccesary aliases to @com_github_nelhage_rules_boost for the WORKSPACE system, any suggestions on the best way to avoid that and keep everything completely compatible with existing workspace setups (if i've accidentally made any other possibly breaking changes) would be great!

@Attempt3035 Attempt3035 changed the title Testing attempt at bzlmod support. Working, but has flaws that need addressing. bzlmod compatibility Nov 18, 2023
@nelhage
Copy link
Owner

nelhage commented Nov 20, 2023

Thanks for this! I will try to make time to take a look, but tagging in also @Vertexwahn, who'd previously given this a go in #482

@Attempt3035
Copy link
Author

Thanks! I hadn't seen that previous attempt. Also checked out https://github.com/bazelboost/registry, I think that project is even more on the right track. That particular implementation in my opinion has the problem that it requires a separately cloned and maintained repo of every boost module, with a MODULE and BUILD file added to it, which sounds a lot like an ongoing maintenance nightmare to me. However, I have found a way that we could adapt this idea of providing a bzlmod registry and using patches to patch in the extra files, meaning it's linked straight to the official boost repos, and could be easily versioned alongside that. It's a completely different approach to the aliasing one I've taken in this PR, and would require a complete repo restructure, but the core logic would all stay the same and I definitely think it's the way forward. I'll branch my fork of this project and have a go at it over the coming week, please still check out the aliasing solution above, I'll see what I can do taking ideas of making this a boost module registry :)

@nelhage
Copy link
Owner

nelhage commented Nov 26, 2023

Took a look. I haven't been personally using bazel for a while so I haven't yet played with the bzlmod system.

This approach seems fine to me, although I confess I haven't closely dug in to make sure I have a full model of what's going on. The bazelboost solution with a repository-per-boost-library does seem like much more of a pain…

I'd still be curious if @Vertexwahn has any thoughts since he's given this more thought (and one attempt already!) than I have. That said, I feel pretty open to merging this as-is.

@Attempt3035
Copy link
Author

Hey Nelson! Thank's for taking the time to check it out!

I've been doing lots of work after getting a lot of inspiration from @Vertexwahn's attempt, and I think it's actually a far better strategt going forward. I've spent a ton of time trying to get things moved over to the modular bazel system, redoing a lot of the dependencies between modules so it all lines up with how boost configures their repos, but it's looking like it's going to work. The bzlmod system will definitely manage the interdependencies better, and with a couple of helper scripts, I think future maintenance will become much easier. I'm still working through the solution i've got in mind, but I'll link in the fork hopefully in a week or so once it's working smoothly and you can test it and let me know what you think. While this PR does achieve the goal, I think this new method will be much better :)
Will keep posted!

boost_libs = use_extension("//:boost/boost.bzl", "boost_deps_extension")

use_repo(boost_libs, "boost")
use_repo(boost_libs, "bazel_skylib")
Copy link

Choose a reason for hiding this comment

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

bazel_skylib, zlib, bzip2, zstd, openssl are all availble in the official registry. How about using those versions? (Didn't look yet into the details how they are built TBH.)

@Attempt3035 Any plans finishing this PR?

Copy link
Author

@Attempt3035 Attempt3035 Jan 7, 2024

Choose a reason for hiding this comment

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

Hey @mvukov!

This PR does work as it stands, however:
I've been doing a bunch of work over the last couple of months trying to get the entirety of boost into bzlmod. It's still a bit of a mess and work in progress, but made a PR so you can check it out here: bazelbuild/bazel-central-registry#1280
Hopefully that will be the way forward! Will try and get it tidied up and some readme instructions so you can give it a go, most of the modules are working, just adding tests!

Ideally, we'd all be working on the boost stuff in the same place. Any thoughts on the transition to bzlmod for this entire repository? Would we expect users to just switch when they turn their projects to bzlmod instead of workspace, or would it be valuable to have a module adaptor that adds aliases to the project and requires all boost libraries, so that effectively, users here could just add that bzlmod module to their project and all the targets would still work? Food for thought anyway...

Copy link

Choose a reason for hiding this comment

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

Took a look at bazelbuild/bazel-central-registry#1280. How do you maintain all those patches? (I really hope not by hand). Unless I am mistaken, for each boost module a separate archive is downloaded. What is the benefit compared to a single archive downloaded here?

Didn't dive into all the details, but this PR here and approach of this repo seem much more maintainable than bazelbuild/bazel-central-registry#1280 TBH. If you go bazelbuild/bazel-central-registry#1280 route, does that mean this repo becomes deprecated?

Any objection to finishing up this PR? seems almost ready.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@mvukov Off course, you need some scripts to generate all this stuff - we could place those scripts in this repo here. I also started for other libs to have some helper scripts to be able to switch to newer versions without all the boilerplate. Some people only want to use a single header library of boost - why would you then download 300 MB complete boost? I see many people only using boost.asio. Also in the long term, we could try to bring Bazel support directly to the boost repo itself. Then we could easily copy & paste our BUILD and MODULE since boost itself is organized in this "multirepo" approach. Smaller modules == less CI build traffic, less disk usage, etc. - there are some benefits

@Vertexwahn
Copy link
Collaborator

Vertexwahn commented Jan 10, 2024

That particular implementation in my opinion has the problem that it requires a separately cloned and maintained repo of every boost module, with a MODULE and BUILD file added to it, which sounds a lot like an ongoing maintenance nightmare to me.

That was also my observation.

For me, it would make sense to introduce a "registry" folder in this repo, i.e. the registry folder should act as a Bazel Registry. There we could mirror the stuff that is located on the Bazel Central Registry - e.g. the stuff that is in the bazelbuild/bazel-central-registry#1280 PR.

Another approach would be to have instead of a separate module for each individual boost lib only one "boost" module (as this PR does) - that has then only different targets (instead of modules). Not sure what the better approach is here. Also not sure what happens if one starts to mix boost versions.

It seems we have currently three variants:

  • traditional WORKSPACE approach
  • a single-module approach (many targets)
  • a multiple-module approach (one target)

The traditional WORKSPACE approach will die in the long-term.

From a user perspective in the single-module approach you only write

bazel_dep(name = "boost", version = "1.83.0.bzl.1")

wheres in the multiple-module approach you have to list maybe several modules, e.g.:

bazel_dep(name = "boost.predef", version = "1.83.0.bzl.1") ...

also the multiple-module approach requires regarding your pull request more than 700 files to setup where many files are very similar

The question is what is better maintainable in the long term single-module order multiple-module? Or should we offer both approaches at the same time? What happens if someone mixes both approaches... uff...

@nelhage @Attempt3035

We should go with the multiple-module approach - we should have in this repo some tooling that one can use when a new boost version is released that generates all files for the Bazel Central Registry. At the same time this repo should also act as a Bazel Registry (having a folder registry with all the relevant models). I would reject the single-module approach as suggested in the PR

@Attempt3035
Copy link
Author

Attempt3035 commented Jan 13, 2024

Hey everyone!
Thanks so much @Vertexwahn and @mvukov for the comments! I've just committed the tools I used to create this, now that I've got them compiled nicely and with a followable readme. They are boost.rules and boost.rules.tools

To answer all the questions in a direct form:

General Approach

Boost is in multirepo format, and it makes sense to follow that. Secondly, that allows potential for upstream into boost, and it would be as simple as applying the patches and making a PR to each boost module. Thirdly, it keeps modules and how they interconnect properly isolated, with the right headers exposed, on a module based level. No targets can mistakingly be depended on, no accidental circular dependencies that shouldn't be there.

Secondly, using modules in this way, previous boost versions are still available and can be easily maintained. The versioning system of BZLMOD is great. As far as compatibility, the tool will increment the compatibility level value every boost release version, so no mix and match version conflicts will exist. If someone has a way to confirm inter-compatibility between two boost releases for all libraries (ie mix and match boost 1.83.0 and 1.84.0), then we can simply leave the compatibility level the same between those versions and it will all work seamlessly. - @fmeum suggested a great way to solve this using an umbrella module. This makes sense considering the single boost target is already made for easy inclusion. As a bonus, this means I was able to remove 2k of dependency lines, and the automatic dependency tool has half the work to do, as all module files just depend on the boost module which aliases to all the other libraries. Win Win!

Single-Module concept

Putting all the build scripts and targets into a single module isn't very good, because it's long and doesn't really adhere to Bazel's best practices in isolating and versioning everything.
However, myself and many other users love to have boost included with a single tidy include.

Solution:

boost.rules - which will be the "boost" target in the BCR, aliases and depends on ALL the boost modules. This means, a user can simply bazel_dep(name = "boost", version = "1.83.0.bcr.#") and then use ANY boost target with the syntax @boost//:LibraryName. It can also be mix and matched with direct access (say if someone needed a non-default target that's part of the same package of a lib) and bazel will still link to the same dependency perfectly.

Download Size

Downloading all the boost archives individually as PR #1280 does is the exact same size as downloading in bulk like this repo does. However, Bazel (even with the "@boost" all boost targets module will ONLY download the modules needed (and their deps). Meaning the PR will ALWAYS be more efficient, and at the very worst, if a user needs every single library, will be identical.
You'll only ever download and compile the parts you need. Win Win for both users and the infrastructure.

Automated Approach - boost.rules.tools

Everything has been automatically generated with python scripts. I've compiled this into a single, nice to use supertool that boost package maintainers can use. It lives alongside the tools.bzl extension file which is used by all the boost modules to keep the build files with as little duplicated content as possible. Read the README for details, or jump in to finish the automations that still need work!
It has automation for:

  • All the patching & hash generation
  • Downloading all the sources, so you run the tool once and only ever edit the boost module source files. They are nicely patched exactly how bazel will receive them, and the script converts your changes straight back into the patch when you're done!
  • Autogen Deps in both MODULE and BUILD target - This is still being put into the tool, I used some hacky python scripts that used boostdep-report to do the initial lot, which worked great but needs some polishing up before being in the tool. Not worried about it yet, but it will be great to upgrade everything to Boost 1.84.0.
  • Cleaning up to leave the repo exactly how it was found. This is completely optional, you can commit without deleting sources. They are ignored by git.

Suuuuper fast turnaround time when you're working on something. Just edit your files in the sources, press patch, test, repeat.

This Repo

A few people have asked me if the new approach obsoletes this repository. It does, as far as I can tell, I've been able to provide all the functionality of this repo plus many more benefits. Maintainability will be improved, the tools are there to remove all the repeated tasks, and WORKSPACE will eventually die out anyway. @Vertexwahn I'm not sure on any benefits of mirroring the modules in this repo, I believe once everything is in the Bazel Central Repository, we just work from there.

Overall, I agree with @Vertexwahn that multi-repo approach is by far the way to go, and we have all the benefits of providing single module through boost.rules as well. The method I proposed in the original PR on this thread isn't sufficient to follow in the growth of Bazel or Boost, and isn't anywhere as efficient or maintainable as we could ultimately be. Finally, thank you so much to everyone who has actually built this repo in the first place, most of the complex module logic in PR #1280 comes straight from here, especially the idea of the boost_library macro.

@Vertexwahn
Copy link
Collaborator

I like boost.rules.tools repo. Maybe in BCR in the future there should be hint that there exists some tooling to generate those modules. I see it similar: Bzlmod marks the end of this repository. Anyways Bazel 6 gets still updates for the next two years, and I guess there are some project struggling to switch to Bzlmod. Therefore, I would suggest to continue the support of rules_boost for the traditional WORKSPACE approach here in this repo. Mirroring of the Bzlmod rules could be done here - but is of course not required.

@Vertexwahn
Copy link
Collaborator

@Attempt3035 Just discoverd ;)

Hey there! Before editing, check out the tool at https://github.com/dynacondev/boost.rules.tools
Note: These modules follow a circular dependency structure on the boost module https://github.com/dynacondev/boost.rules to keep boost releases from mixing!

@Vertexwahn
Copy link
Collaborator

@Attempt3035 Can we closes this PR?

@Attempt3035
Copy link
Author

Yep, I think we've just about got it all sorted as far as boost in BCR, I'll hopefully get some time over the next couple of weeks to finish up getting modules building and tests added, and soon you should see boost for bzlmod 😁

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.

4 participants