Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Replace flatbuffers dependency by Planus #732

Merged
merged 3 commits into from
Jan 14, 2022
Merged

Replace flatbuffers dependency by Planus #732

merged 3 commits into from
Jan 14, 2022

Conversation

jorgecarleitao
Copy link
Owner

@jorgecarleitao jorgecarleitao commented Jan 4, 2022

This PR will stay here for some time. It replaces the flatbuffers crate by planus, a recently released replacement to flatbuffers. See #725 for details.

This switches one for the other. All tests and integration tests pass, showing that it does indeed work as advertised 💯

@codecov
Copy link

codecov bot commented Jan 4, 2022

Codecov Report

Merging #732 (f64209d) into main (276d670) will increase coverage by 0.09%.
The diff coverage is 84.03%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #732      +/-   ##
==========================================
+ Coverage   71.02%   71.11%   +0.09%     
==========================================
  Files         315      316       +1     
  Lines       16950    16782     -168     
==========================================
- Hits        12038    11934     -104     
+ Misses       4912     4848      -64     
Impacted Files Coverage Δ
src/ffi/ffi.rs 83.67% <ø> (ø)
src/io/flight/mod.rs 0.00% <0.00%> (ø)
src/io/ipc/compression.rs 92.85% <ø> (ø)
src/io/ipc/mod.rs 0.00% <0.00%> (ø)
src/io/ipc/read/array/binary.rs 43.24% <ø> (+2.70%) ⬆️
src/io/ipc/read/array/boolean.rs 44.44% <ø> (+3.70%) ⬆️
src/io/ipc/read/array/dictionary.rs 47.61% <ø> (ø)
src/io/ipc/read/array/fixed_size_binary.rs 44.44% <ø> (+3.70%) ⬆️
src/io/ipc/read/array/fixed_size_list.rs 40.00% <ø> (ø)
src/io/ipc/read/array/list.rs 25.64% <ø> (+2.56%) ⬆️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 276d670...f64209d. Read the comment docs.

@jorgecarleitao
Copy link
Owner Author

Added MIRI check to a round-trip over IPC. All still green.

@jorgecarleitao
Copy link
Owner Author

I though that this would take longer, but it is essentially done: tests pass, MIRI checks pass, simpler to use and a safer design.

One step missing is benchmarks of read and write, to check if there is no major regression.

@sundy-li , @ritchie46 , @houqp, @yjshen , any thoughts on this?

My opinion is that flatbuffers is unmaintained and has soundness issues, which is a concern because IO is an obvious attack vector - it does not mean that planus won't become unmaintained (maybe @TethysSvensson or @kristoff3r could chip in here wrt to their plans?).

@TethysSvensson
Copy link

TethysSvensson commented Jan 8, 2022

@jorgecarleitao While I am not going to give any guarantees (this being a hobby, open source project after all), my current plan is to maintain and improve upon the library for at least the next year, since we are planning to use the library in product at skybox.gg.

What happens after that is too far into the future for me to predict and also depends on whether there is any community adoption.

@jorgecarleitao jorgecarleitao force-pushed the planus branch 2 times, most recently from a40b725 to 30aca17 Compare January 13, 2022 21:38
@jorgecarleitao jorgecarleitao force-pushed the planus branch 2 times, most recently from 4594ca7 to 792df39 Compare January 14, 2022 20:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants