-
Notifications
You must be signed in to change notification settings - Fork 483
Upgrade bazel to v6.5.0, upgrade protobuf, tensorflow and dependent deps #2290
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
Upgrade bazel to v6.5.0, upgrade protobuf, tensorflow and dependent deps #2290
Conversation
8ef72cf to
ff0ed38
Compare
a958b02 to
1d098bb
Compare
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com> (cherry picked from commit f69b9d17010abe2cd8cabea7c42a4d4cc5f46f91)
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com> (cherry picked from commit 929b042996c0fa13871d0d3764e1e82a7bfd45fe)
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
1d098bb to
9b80f0a
Compare
| #else | ||
| constexpr uint64_t kFileSizeLimitMB = 310; | ||
| constexpr uint64_t kFileSizeLimitMB = 325; | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stirling_wrapper binary size exceeds the previous limit from protobuf v27 and later. I tried to split out a smaller version of this into and the stirling specific changes (the absl upgrade results in the src/stirling/source_connectors/socket_tracer/uprobe_manager.h change) in #2296.
Unfortunately these dependencies have hard dependencies on each other, which makes it hard to do without upgrading all of them.
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
|
|
||
| #include <map> | ||
| #include <memory> | ||
| #include <mutex> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After upgrading absl, the #include <absl/synchronization/mutex.h> on line 27 caused the following failure for the gcc build:
root@px-dev-docker-hacksaw:/px/src/px.dev/pixie# bazel build --config gcc src/stirling/source_connectors/socket_tracer:cc_library
INFO: Analyzed target //src/stirling/source_connectors/socket_tracer:cc_library (23 packages loaded, 170 targets configured).
INFO: Found 1 target...
ERROR: /px/src/px.dev/pixie/src/stirling/source_connectors/socket_tracer/BUILD.bazel:21:14: Compiling src/stirling/source_connectors/socket_tracer/uprobe_manager.cc failed: (Exit 1): gcc-12 failed: error executing command (from target //src/stirling/source_connectors/socket_tracer:cc_library) /usr/bin/gcc-12 -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -fno-omit-frame-pointer '-std=c++17' -MD -MF ... (remaining 330 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from src/stirling/source_connectors/socket_tracer/uprobe_manager.cc:19:
./src/stirling/source_connectors/socket_tracer/uprobe_manager.h:608:8: error: 'mutex' in namespace 'std' does not name a type
608 | std::mutex deploy_uprobes_mutex_;
| ^~~~~
./src/stirling/source_connectors/socket_tracer/uprobe_manager.h:44:1: note: 'std::mutex' is defined in header '<mutex>'; did you forget to '#include <mutex>'?
43 | #include "src/stirling/utils/proc_tracker.h"
+++ |+#include <mutex>
44 |
src/stirling/source_connectors/socket_tracer/uprobe_manager.cc: In member function 'void px::stirling::UProbeManager::DeployUProbes(const absl::lts_20250512::flat_hash_set<px::md::UPID>&)':
src/stirling/source_connectors/socket_tracer/uprobe_manager.cc:1010:14: error: 'lock_guard' in namespace 'std' does not name a template type
1010 | const std::lock_guard<std::mutex> lock(deploy_uprobes_mutex_);
| ^~~~~~~~~~
src/stirling/source_connectors/socket_tracer/uprobe_manager.cc:43:1: note: 'std::lock_guard' is defined in header '<mutex>'; did you forget to '#include <mutex>'?
42 | #include "src/stirling/utils/linux_headers.h"
+++ |+#include <mutex>
43 | #include "src/stirling/utils/proc_path_tools.h"
Target //src/stirling/source_connectors/socket_tracer:cc_library failed to build
Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
NickLanam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rubber stamping for the trivial UI changes
Summary: Upgrade Bazel from 6.2.0 to 6.5.0 along with protobuf, tensorflow, and related dependencies. This is a coordinated upgrade since these dependencies have hard interdependencies that make incremental upgrades difficult.
Key changes
protoc-gen-grpc-webwith a Bazel repositoryI attempted to split this into a smaller change in #2296. TensorFlow has a tight dependency on protobuf, which prevented that attempt from working. I'm open to other ideas on how to split this up, but so far this seemed like the best balance with #2293 and #2297 to follow to complete the Bazel 7 migration.
Relevant Issues: #2282
Type of change: /kind cleanup
Test Plan: Build passes