Skip to content

build: add isolated v2 FlatBuffers protocol library - #22

Merged
sgn4sangar merged 5 commits into
mainfrom
feature/v2_bazel_flatbuffers
Sep 9, 2026
Merged

build: add isolated v2 FlatBuffers protocol library#22
sgn4sangar merged 5 commits into
mainfrom
feature/v2_bazel_flatbuffers

Conversation

@tkilias

@tkilias tkilias commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • compile udf_protocol.fbs with Bazel
  • provide the v2 protocol library with an isolated FlatBuffers runtime namespace
  • add generated-code and ELF symbol-leak regression tests

Validation

  • bazel test --lockfile_mode=off //:udf_protocol_test //:udf_protocol_symbol_leak_test
  • bazel build --lockfile_mode=off //:udf_protocol //:udf_protocol_shared

Both were run with Bazel 8.3.1.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e02f8551e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread udf-runner-cpp/v2/udf_protocol.hpp
Comment thread udf-runner-cpp/v2/BUILD.bazel
@sgn4sangar
sgn4sangar force-pushed the feature/v2_bazel_arrow_core branch from 5c750f0 to dc4f843 Compare September 8, 2026 10:59
Base automatically changed from feature/v2_bazel_arrow_core to main September 8, 2026 13:45
@sgn4sangar
sgn4sangar force-pushed the feature/v2_bazel_flatbuffers branch from fb798ee to b70e698 Compare September 8, 2026 15:00
}

void verify_symbols(const std::string& archive_path) {
const std::string command = "nm -g --defined-only -- '" + archive_path + "'";

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this we need to fix later, the AI tried to cheat

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i do agree; tried implementing some code that really use the flatbuffer symbols, but it seems a lot of changes; therefore, i leave it as it is for this PR and create an issue for this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

created an issue to track this

}

void verify_symbols(const std::string& library_path) {
const std::string command = "nm -D --defined-only -- '" + library_path + "'";

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this we need to fix later, the AI tried to cheat

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

created an issue for this

@sgn4sangar
sgn4sangar merged commit 3c8b830 into main Sep 9, 2026
22 checks passed
@sgn4sangar
sgn4sangar deleted the feature/v2_bazel_flatbuffers branch September 9, 2026 06:16
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.

2 participants