Skip to content

Set up for Bazel module builds. #1597

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

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

bcsgh
Copy link
Contributor

@bcsgh bcsgh commented Jan 26, 2025

Notes:

  1. This likely doesn't require a release note as it shouldn't have any effect on any released product.
  2. The MODULE.bazel is forked from the "bazel-central-registry" and this allows the BCR to quit having to uses patches.
  3. With these changes JsonCpp can be built by Bazel without any changes. Without this, Bazel is unable to locate a correct root to build from and fails.
  4. This effectively supersede Define jsoncpp as Bazel module #1534 ;; Addressing comments from there:
    1. The version in MODULE.bazel should track the version used everywhere else. This just becomes yet-another-copy of that.
    2. WORKSPACE is dead (though one would be needed ... if someone had to JsonCpp, as a root module, with Bazel, and without using modules, which should be nobody ever again).
    3. I'm not including a .bazelrc as things should "just work" (unless your ~/.bazelrc does something annoying).

It would be nice if making updates to BCR became part of the normal release process. It looks like that could be rather easy with the available tool support maybe even automated.

@bcsgh bcsgh marked this pull request as ready for review January 26, 2025 04:58
@bcsgh
Copy link
Contributor Author

bcsgh commented Feb 23, 2025

What else is needed before this is ready to merge?

@baylesj
Copy link
Contributor

baylesj commented Mar 12, 2025

With this patch:

-> % bazel build jsoncpp
INFO: Analyzed target //:jsoncpp (86 packages loaded, 457 targets configured).
INFO: Found 1 target...
Target //:jsoncpp up-to-date:
  bazel-bin/libjsoncpp.a
INFO: Elapsed time: 11.480s, Critical Path: 1.53s
INFO: 8 processes: 4 internal, 4 darwin-sandbox.
INFO: Build completed successfully, 8 total actions

Without this patch:

-> % bazel build jsoncpp
WARNING: Invoking Bazel in batch mode since it is not invoked from within a workspace (below a directory having a WORKSPACE file).
OpenJDK 64-Bit Server VM warning: Options -Xverify:none and -noverify were deprecated in JDK 13 and will likely be removed in a future release.
ERROR: The 'build' command is only supported from within a workspace (below a directory having a WORKSPACE file).
See documentation at https://bazel.build/concepts/build-ref#workspace

I'm satisfied that this patch works and I'm interested in getting a build bot configuration for it.

@baylesj
Copy link
Contributor

baylesj commented Mar 12, 2025

It would be nice to add the tests as a BUILD.bazel target, but that's outside the scope of this patch and could be a followup.

@baylesj baylesj merged commit 037752d into open-source-parsers:master Mar 12, 2025
10 checks passed
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.

2 participants