Skip to content

Commit

Permalink
Allow clang and gcc builds for `src/e2e_test/vizier/planner:planner_t…
Browse files Browse the repository at this point in the history
…est` (#2077)

Summary: Allow clang and gcc builds for
`src/e2e_test/vizier/planner:planner_test`

As far as I can tell, there isn't a bazel config that matches the tags
of the `src/e2e_test/vizier/planner:planner_test` target. This change
removes the tag preventing clang builds since they appear to run without
problems. In addition to this, a `vispb` proto change was missing from
the earlier differential flamegraph work. This prevented the
`planner_test` from running properly (see comment for more details).

Relevant Issues: N/A

Type of change: /kind bugfix

Test Plan: Verified gcc and clang builds work
- [x] `bazel test --config clang
src/e2e_test/vizier/planner:planner_test` is successful
- [x] `bazel test -c opt --config gcc
src/e2e_test/vizier/planner:planner_test` is successful

---------

Signed-off-by: Dom Del Nano <[email protected]>
  • Loading branch information
ddelnano authored Jan 20, 2025
1 parent e20880f commit 685ed42
Show file tree
Hide file tree
Showing 6 changed files with 218 additions and 126 deletions.
303 changes: 180 additions & 123 deletions src/api/proto/vispb/vis.pb.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions src/api/proto/vispb/vis.proto
Original file line number Diff line number Diff line change
Expand Up @@ -458,4 +458,7 @@ message StackTraceFlameGraph {
string node_column = 8;
// The label for the percentage.
string percentage_label = 9;
// The column containing the stacktrace's delta. This is only relevant for differential
// flamegraphs.
string difference_column = 10;
}
8 changes: 7 additions & 1 deletion src/common/exec/exec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,14 @@

namespace px {

// GCC's ingnored-attribute warning is triggered without wrapping pclose. This is likely do to the
// nonnull attribute.
struct pclose_deleter {
void operator()(FILE* file) const { pclose(file); }
};

StatusOr<std::string> Exec(std::string cmd) {
std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd.c_str(), "r"), pclose);
std::unique_ptr<FILE, pclose_deleter> pipe(popen(cmd.c_str(), "r"));
if (pipe == nullptr) {
return error::Internal("popen() failed!");
}
Expand Down
2 changes: 0 additions & 2 deletions src/e2e_test/vizier/planner/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ pl_go_test(
],
tags = [
"no_asan",
"no_gcc",
"no_libcpp",
"no_msan",
"no_tsan",
],
Expand Down
6 changes: 6 additions & 0 deletions src/ui/src/types/generated/vis_pb.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions src/ui/src/types/generated/vis_pb.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 685ed42

Please sign in to comment.