Skip to content

Conversation

mmorel-35
Copy link

@mmorel-35 mmorel-35 commented Sep 15, 2025

Description

This setup bazel module support. It follows Bzlmod Migration Guide

Notice:

  • I had to disable --copt="-Werror" and push use of c++17

Closes #136

Comment on lines 14 to 17
git_override(
module_name = "skywalking-data-collect-protocol",
commit = "fb3fb005650e2489164978b7804117c7ade1529a",
remote = "https://github.com/apache/skywalking-data-collect-protocol.git",
)
Copy link
Author

@mmorel-35 mmorel-35 Sep 15, 2025

Choose a reason for hiding this comment

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

Notice, this is pointing to skywalking-data-collect-protocol last commit, to confirm its bazel mode migration and will be working properly when its next tag is published

See https://bazel.build/rules/lib/globals/module#git_override for more

@mmorel-35 mmorel-35 marked this pull request as ready for review September 15, 2025 10:54
@mmorel-35
Copy link
Author

/cc @wbpcode
I believe it's for you ;)

@mmorel-35 mmorel-35 force-pushed the bzlmod branch 2 times, most recently from 148823a to 78828eb Compare September 15, 2025 11:22
@wu-sheng wu-sheng requested review from Copilot and wbpcode September 15, 2025 12:55
@wu-sheng wu-sheng added this to the 0.6.0 milestone Sep 15, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the project from Bazel WORKSPACE to Bzlmod module system, following the official Bzlmod Migration Guide. The changes update dependency management, version pins, and build configuration to work with the new module system.

  • Adds MODULE.bazel file with module dependencies using bazel_dep declarations
  • Updates dependency versions across all external libraries (grpc, protobuf, googletest, etc.)
  • Modifies build configuration to enable bzlmod and enforce C++17 standard

Reviewed Changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
MODULE.bazel New module file defining dependencies with bazel_dep and git_override
WORKSPACE.bzlmod New workspace file for bzlmod compatibility
.bazelrc Enables bzlmod and enforces C++17 standard across platforms
.bazelversion Updates Bazel version from 4.0.0 to 7.6.1
bazel/repositories.bzl Updates dependency versions and URLs to match MODULE.bazel
WORKSPACE Removes rules_proto_toolchains and adds protobuf_deps
cpp2sky/BUILD Updates protobuf import to use new location
bazel/fmtlib.BUILD Renames target from "fmtlib" to "fmt"
bazel/spdlog.BUILD Updates dependency reference to match renamed fmt target
test/tracing_context_test.cc Updates include path for protocol buffer headers

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@mmorel-35
Copy link
Author

mmorel-35 commented Sep 20, 2025

I believe the jobs don’t start because github doesn’t supports Ubuntu 20.04 anymore, I opened #136

I'll have to adapt this PR once the other is merged.

Signed-off-by: Matthieu MOREL <[email protected]>
Signed-off-by: Matthieu MOREL <[email protected]>
@mmorel-35
Copy link
Author

@wbpcode ,
Let's see if it's better with bazel 7.6.1 ?

Signed-off-by: Matthieu MOREL <[email protected]>
Signed-off-by: Matthieu MOREL <[email protected]>
@mmorel-35
Copy link
Author

@wbpcode ,
This getting better, I updated Docker images and provided a cache for bazel so it's going to build faster

Signed-off-by: Matthieu MOREL <[email protected]>
@mmorel-35
Copy link
Author

@wbpcode,

I restored the sudo calls for cmake, we shall be closer to success with that change.

Removed C++ standard specification from cmake commands.
wbpcode
wbpcode previously approved these changes Oct 14, 2025
Copy link
Contributor

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. LGTM. Hope CI is happy.

@mmorel-35 mmorel-35 force-pushed the bzlmod branch 2 times, most recently from f045dd0 to de1e98d Compare October 15, 2025 05:32
@mmorel-35
Copy link
Author

@wbpcode ,

I believe this was related to c++ version being 11 instead of 17.
As the build is quite long I separated bazel and cmake builds so they can have a dedicated workflow and caching system

@mmorel-35 mmorel-35 force-pushed the bzlmod branch 2 times, most recently from 40be5f9 to 72b89e2 Compare October 15, 2025 05:39
@mmorel-35 mmorel-35 force-pushed the bzlmod branch 2 times, most recently from b455b89 to 32e0499 Compare October 16, 2025 05:37
@mmorel-35
Copy link
Author

@wbpcode,
Let's hope there is nothing left ;)

@mmorel-35
Copy link
Author

@wbpcode ,
I believe it !

@wbpcode
Copy link
Contributor

wbpcode commented Oct 17, 2025

🙏

@mmorel-35
Copy link
Author

@wbpcode ,

Working locally, let's hope that's final round on this.

@mmorel-35 mmorel-35 force-pushed the bzlmod branch 4 times, most recently from 3a5afb7 to 2806210 Compare October 17, 2025 18:54
@mmorel-35
Copy link
Author

@wbpcode ,
I remove the unspported --ignore-errors

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.

3 participants