Skip to content

build: add flatbuffers#2133

Merged
snowp merged 13 commits intomainfrom
fbs
Mar 30, 2022
Merged

build: add flatbuffers#2133
snowp merged 13 commits intomainfrom
fbs

Conversation

@snowp
Copy link
Contributor

@snowp snowp commented Mar 28, 2022

This adds flatbuffers to the build, including a bzl macro for generating Kotlin/Swift/C++ files for a given flatbuffer file. Tests are added that demonstrate that the platform code can consume these generated files

Signed-off-by: Snow Pettersen snowp@lyft.com

Description:
Risk Level: Low, build only
Testing: New tests
Docs Changes: n/a
Release Notes: n/a

Snow Pettersen added 11 commits March 28, 2022 15:04
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp marked this pull request as ready for review March 29, 2022 14:43
@snowp snowp changed the title WIP build: add flatbuffers build: add flatbuffers Mar 29, 2022
@snowp
Copy link
Contributor Author

snowp commented Mar 29, 2022

@keith @Augustyniak ptal

keith
keith previously approved these changes Mar 29, 2022
jpsim
jpsim previously approved these changes Mar 29, 2022
Copy link
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

It'd be good to have some tests validating that a buffer can be read and written across the generated language bindings, but not necessary right now. If we start using flatbuffers in EM directly we can add tests for that specifically.

The java version is 2.0.3 but the C++ and Swift versions are 2.0.0. Are those versions compatible with each other?

name = "{}_fb_swift_srcs".format(name),
srcs = ["test.fbs"],
outs = [
"test_generated.swift",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: why test_generated.swift? Can this be $(name).generated.swift?

Suggested change
"test_generated.swift",
"{}.generated.swift".format(name),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yeah the swift rules here aren't generic, just hard coded to the test.fbs input, let me update

@snowp
Copy link
Contributor Author

snowp commented Mar 30, 2022

I would expect them to be compatible per semver, also from looking into the format a bit I believe the underlying binary format is very stable, most changes to the library are around updating the per language bindings. For example, this seems like one of the patch changes: google/flatbuffers#6785

Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp dismissed stale reviews from jpsim and keith via 3e66b6d March 30, 2022 12:11
Signed-off-by: Snow Pettersen <snowp@lyft.com>
@snowp snowp merged commit 541ea7e into main Mar 30, 2022
@snowp snowp deleted the fbs branch March 30, 2022 15:10
Copy link
Contributor

@Augustyniak Augustyniak left a comment

Choose a reason for hiding this comment

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

+1

jpsim added a commit that referenced this pull request Mar 31, 2022
* origin/main:
  key_value: structure for prefs based key value store (#2120)
  build: add flatbuffers (#2133)
  Bump Lyft Support Rotation (#2131)

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit that referenced this pull request Apr 12, 2022
* main: (59 commits)
  Bump Lyft Support Rotation (#2156)
  add specifying more maven deps (#2151)
  update envoy@e4eaf1b97 (#2146)
  bazel: create symbol mapping file (#2126)
  Bump Lyft Support Rotation (#2148)
  bazel: remove sandbox disable
  build: export flatbuffer jvm dep (#2147)
  Bump Lyft Support Rotation (#2143)
  bazel: Add flatbuffers Swift hack
  key_value: structure for prefs based key value store (#2120)
  build: add flatbuffers (#2133)
  Bump Lyft Support Rotation (#2131)
  envoy: bump upstream Envoy to 419e237 (#2132)
  stats: enable more metrics (#2130)
  Use the right type (envoy_network_t) (#2125)
  Bump Lyft Support Rotation (#2118)
  Update CONTRIBUTING.md to include updating subrepos (#2023)
  ci: create baseline and experimental test app pipelines (#2075)
  config: temporarily hardcode h2 max concurrent streams to 100 (#2124)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
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.

4 participants