bazel: use BCR for c-ares and libxml2#28975
bazel: use BCR for c-ares and libxml2#28975dotnwat wants to merge 3 commits intoredpanda-data:devfrom
Conversation
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Also updates to 2.15 Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Fixes this:
RuntimeError: Node <ducktape.cluster.cluster.ClusterNode object at 0x7f8e3c01f580> joined cluster,
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the c-ares and libxml2 dependencies from custom Bazel build files to the Bazel Central Registry (BCR), simplifying dependency management. Additionally, it fixes a minor logging issue in the test suite.
Key changes:
- Removed custom Bazel build configurations for c-ares and libxml2 in favor of BCR versions
- Updated all import paths from custom thirdparty wrappers to direct library includes
- Fixed test logging to use
old_node.nameinstead ofold_nodefor proper string formatting
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
MODULE.bazel |
Added c-ares 1.34.5 and libxml2 2.15.1 as BCR dependencies; removed from non_module_dependencies |
bazel/repositories.bzl |
Removed custom http_archive definitions for c-ares and libxml2 |
bazel/thirdparty/c-ares.BUILD |
Deleted custom c-ares build configuration |
bazel/thirdparty/libxml2.BUILD |
Deleted custom libxml2 build configuration |
src/v/thirdparty/c-ares/ |
Removed custom c-ares wrapper files (BUILD, ares.h) |
src/v/thirdparty/libxml2/ |
Removed custom libxml2 wrapper files (BUILD, parser.h, xmlwriter.h) |
src/v/kafka/client/broker.cc |
Updated include from thirdparty/c-ares/ares.h to <ares.h> |
src/v/kafka/client/BUILD |
Changed dependency from //src/v/thirdparty/c-ares to @c-ares//:ares |
src/v/cloud_storage_clients/xml_sax_parser.h |
Updated include from thirdparty/libxml2/parser.h to <libxml/parser.h> |
src/v/cloud_storage_clients/BUILD |
Changed dependency from //src/v/thirdparty/libxml2 to @libxml2 |
src/v/test_utils/boost_result_redirect.cc |
Updated include from thirdparty/libxml2/xmlwriter.h to <libxml/xmlwriter.h> |
src/v/test_utils/BUILD |
Changed dependency from //src/v/thirdparty/libxml2 to @libxml2 |
bazel/thirdparty/seastar.BUILD |
Updated seastar's c-ares dependency reference to @c-ares//:ares |
tests/rptest/tests/cluster_features_test.py |
Fixed logging to use old_node.name for proper string representation |
|
How different is this from our current build? https://github.com/bazelbuild/bazel-central-registry/blob/main/modules/c-ares/1.34.5.bcr.3/overlay/include/config_linux/ares_config.h |
|
Also missing zlib, I don't know if that matters: https://github.com/bazelbuild/bazel-central-registry/blob/a156a966abe6fc077f25176c9b21b77629c568ed/modules/libxml2/2.15.1/overlay/include/libxml/xmlversion.h#L316 |
CI test resultstest results on build#77828
|
hmm, yeh looks like libxml will automatically inflate/deflate files if needed. not a clue if we actually depend on this. seems like maybe only the xml_sax_parser would be affected. @nvartolomei do you know if we depend on gzip? |
|
@dotnwat I never seen compression used in our xml processing. grepped the code for mentions and for |
|
This good to merge @dotnwat ? |
looks like the zlib question is resolved. i looked at c-ares and there are a bunch of autoconf-derived configs that are hard coded. they all look fine for 64bit machines. its a bit tedious to examine i think the only concern is if whoever published the BCR there had any baked in x86 assumptions |
@travisdowns oh yeh that sounds good. feel free to close this PR! |
|
The libxml2 side of this has been cherry-picked over to #30224. |
Thanks @dotnwat , I just pulled out the libxml2 side of things and put it up separately, but this can stay open for c-ares side (which based on my tests seemed hermetic but obviously moving to BCR has other benefits). |
Backports Required
Release Notes