Skip to content

Commit bc01887

Browse files
authored
Consolidate all_scripts_test.go to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 (#2311)
Summary: Consolidate `all_scripts_test.go` to use a single CGO target to fix duplicate symbol errors introduced by rules_go v0.58.1 bazel-contrib/rules_go#4438, included in `rules_go` v0.58.1, causes certain statically linked CGO binaries to fail with duplicate symbol errors. This occurs when a binary depends on more than one CGO library that transitively depends on a common set of object files. `all_scripts_test.go` previously depended on two CGO targets: - `//src/carnot/planner` - `//src/e2e_test/vizier/planner/dump_schemas/godumpschemas` This PR solves this issue by removing the src/e2e_test/vizier/planner/dump_schemas/godumpschemas CGO library and instead generate the protobuf export directly in C++, loading it in the main application. This approach mirrors the existing pattern used in [src/vizier/funcs](https://github.com/pixie-io/pixie/blob/a6349a90b1e4b30f0bb13872ad03dff83a53f363/src/vizier/funcs/BUILD.bazel#L50-L66). **Why not fix `rules_go`?** The `rules_go` change that causes the issue explains that it doesn't include the necessary deduplication logic to avoid these duplicate symbol errors (bazel-contrib/rules_go#4438 (comment)). This tradeoff was [deemed acceptable](bazel-contrib/rules_go#4438 (comment)) since it solved the c++ initialization problem with minimal complexity. Relevant Issues: N/A Type of change: /kind cleanup Test Plan: Build should pass Signed-off-by: Dom Del Nano <[email protected]>
1 parent a6349a9 commit bc01887

File tree

9 files changed

+307
-153
lines changed

9 files changed

+307
-153
lines changed

src/e2e_test/vizier/planner/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ pl_go_test(
3535
"//src/carnot/planner/distributedpb:distributed_plan_pl_go_proto",
3636
"//src/carnot/planner/plannerpb:service_pl_go_proto",
3737
"//src/carnot/udfspb:udfs_pl_go_proto",
38-
"//src/e2e_test/vizier/planner/dump_schemas/godumpschemas",
38+
"//src/e2e_test/vizier/planner/dump_schemas/schemas",
3939
"//src/table_store/schemapb:schema_pl_go_proto",
4040
"//src/utils",
4141
"//src/vizier/funcs/go",

src/e2e_test/vizier/planner/all_scripts_test.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import (
4242
"px.dev/pixie/src/carnot/planner/distributedpb"
4343
"px.dev/pixie/src/carnot/planner/plannerpb"
4444
"px.dev/pixie/src/carnot/udfspb"
45-
"px.dev/pixie/src/e2e_test/vizier/planner/dump_schemas/godumpschemas"
45+
"px.dev/pixie/src/e2e_test/vizier/planner/dump_schemas/schemas"
4646
"px.dev/pixie/src/table_store/schemapb"
4747
"px.dev/pixie/src/utils"
4848
funcs "px.dev/pixie/src/vizier/funcs/go"
@@ -207,7 +207,16 @@ func scriptToQueryRequest(s *script) (*plannerpb.QueryRequest, error) {
207207
}
208208

209209
func loadSchemas() (*schemapb.Schema, error) {
210-
return godumpschemas.DumpSchemas()
210+
schema := &schemapb.Schema{}
211+
b, err := schemas.Asset("src/e2e_test/vizier/planner/dump_schemas/data/schema.pb")
212+
if err != nil {
213+
return nil, err
214+
}
215+
err = proto.Unmarshal(b, schema)
216+
if err != nil {
217+
return nil, err
218+
}
219+
return schema, nil
211220
}
212221

213222
func makePEMCarnotInfo(id uuid.UUID) *distributedpb.CarnotInfo {

src/e2e_test/vizier/planner/dump_schemas/BUILD.bazel

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,23 @@
1414
#
1515
# SPDX-License-Identifier: Apache-2.0
1616

17-
load("//bazel:pl_build_system.bzl", "pl_cc_library")
17+
load("//bazel:pl_build_system.bzl", "pl_cc_binary")
1818

1919
package(default_visibility = ["//src/e2e_test/vizier/planner:__subpackages__"])
2020

21-
pl_cc_library(
22-
name = "dump_schemas",
23-
srcs = [
24-
"dump_schemas.cc",
25-
"dump_schemas.h",
26-
],
27-
hdrs = ["dump_schemas.h"],
21+
pl_cc_binary(
22+
name = "export_schemas",
23+
srcs = ["export_schemas.cc"],
24+
stamp = -1,
2825
deps = [
29-
"//src/shared/schema:cc_library",
3026
"//src/stirling:cc_library",
3127
"//src/table_store/schema:cc_library",
3228
],
3329
)
30+
31+
genrule(
32+
name = "schema_pb",
33+
outs = ["data/schema.pb"],
34+
cmd = "$(location :export_schemas) --out_file_path $@ &>/dev/null",
35+
tools = [":export_schemas"],
36+
)

src/e2e_test/vizier/planner/dump_schemas/dump_schemas.h

Lines changed: 0 additions & 30 deletions
This file was deleted.

src/e2e_test/vizier/planner/dump_schemas/dump_schemas.cc renamed to src/e2e_test/vizier/planner/dump_schemas/export_schemas.cc

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,18 @@
1616
* SPDX-License-Identifier: Apache-2.0
1717
*/
1818

19+
#include <fstream>
1920
#include <iostream>
2021
#include <string>
2122

22-
#include "src/e2e_test/vizier/planner/dump_schemas/dump_schemas.h"
23-
#include "src/shared/schema/utils.h"
2423
#include "src/stirling/stirling.h"
2524
#include "src/table_store/schema/schema.h"
2625

27-
char* DumpSchemas(int* resultLen) {
26+
DEFINE_string(out_file_path, "schema.pb", "The file to save the serialized Schema");
27+
28+
int main(int argc, char** argv) {
29+
px::EnvironmentGuard env_guard(&argc, argv);
30+
2831
auto source_registry = px::stirling::CreateProdSourceRegistry();
2932
auto sources = source_registry->sources();
3033

@@ -39,15 +42,14 @@ char* DumpSchemas(int* resultLen) {
3942
rel_map[schema.name()] = relation;
4043
}
4144
}
45+
4246
px::table_store::schemapb::Schema schema_pb;
4347
PX_CHECK_OK(px::table_store::schema::Schema::ToProto(&schema_pb, rel_map));
44-
std::string output;
45-
schema_pb.SerializeToString(&output);
4648

47-
*resultLen = output.size();
48-
char* ret = new char[output.size()];
49-
memcpy(ret, output.data(), output.size());
50-
return ret;
51-
}
49+
std::ofstream out_schema;
50+
out_schema.open(FLAGS_out_file_path);
51+
schema_pb.SerializeToOstream(&out_schema);
52+
out_schema.close();
5253

53-
void SchemaStrFree(char* str) { free(str); }
54+
return 0;
55+
}

src/e2e_test/vizier/planner/dump_schemas/godumpschemas/dumpschemas.go

Lines changed: 0 additions & 54 deletions
This file was deleted.

src/e2e_test/vizier/planner/dump_schemas/godumpschemas/dumpschemas_stub.go

Lines changed: 0 additions & 34 deletions
This file was deleted.

src/e2e_test/vizier/planner/dump_schemas/godumpschemas/BUILD.bazel renamed to src/e2e_test/vizier/planner/dump_schemas/schemas/BUILD.bazel

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,19 +14,21 @@
1414
#
1515
# SPDX-License-Identifier: Apache-2.0
1616

17-
load("//bazel:pl_build_system.bzl", "pl_cgo_library")
17+
load("@io_bazel_rules_go//go:def.bzl", "go_library")
18+
load("//bazel:pl_build_system.bzl", "pl_bindata")
1819

1920
package(default_visibility = ["//src/e2e_test/vizier/planner:__subpackages__"])
2021

21-
# gazelle:ignore
22-
pl_cgo_library(
23-
name = "godumpschemas",
24-
srcs = ["dumpschemas.go"],
25-
cdeps = ["//src/e2e_test/vizier/planner/dump_schemas:dump_schemas"],
26-
cgo = True,
27-
importpath = "px.dev/pixie/src/e2e_test/vizier/planner/dump_schemas/godumpschemas",
28-
deps = [
29-
"//src/table_store/schemapb:schema_pl_go_proto",
30-
"@com_github_gogo_protobuf//proto",
31-
],
22+
pl_bindata(
23+
name = "schema_bindata",
24+
srcs = ["//src/e2e_test/vizier/planner/dump_schemas:schema_pb"],
25+
package = "schemas",
26+
strip_bin_dir = True,
27+
)
28+
29+
go_library(
30+
name = "schemas",
31+
# keep
32+
srcs = [":schema_bindata"],
33+
importpath = "px.dev/pixie/src/e2e_test/vizier/planner/dump_schemas/schemas",
3234
)

0 commit comments

Comments
 (0)