search

djkoloski / rust_serialization_benchmark

Benchmarks for rust serialization frameworks
514 stars 48 forks source link

change build-time codegen to by-feature only #68

Closed mumbleskates closed 1 month ago

mumbleskates commented 1 month ago

instead of requiring a feature to not perform codegen at build time, we only enable that codegen by specific features. this allows using committed generated code for protocol buffers, flatbuffers, and cap'n proto by default, while still allowing tests or developers to assert that the relevant generated code is up-to-date automatically.

note: i'm actually not certain whether the CI change here is correct or completely desired, but it seems like the philosophically correct thing to do here. this is how we do it at $work, since github's workflows for diffing generated files are so terrible slash nonexistent (and the codegen situation, for this repo in particular, is so cowboy-styled that it probably is a good idea regardless. much better to just commit all the generated code when the alternative is to make everyone fetch exactly the right codegen tools every single time).

mumbleskates commented 1 month ago

ok well i got it to correctly complain when the codegen is out of date. whatever the CI is doing when it regenerates the code it's making like 6000 lines of diffs, not sure what the best course of action here is.

mumbleskates commented 1 month ago

the massive diff in the last commit there is actually because of line separators; the committed flatbuffers files have CRLF at the moment. it's possible that just changing git settings (always commit "\n") will make this a non-issue in windows

djkoloski commented 1 month ago

Re: generated files - I briefly considered having CI regenerate the files and check them in, but I think your approach is better. Having CI check in files makes things a lot more complex and the gain is very minimal.

Thanks for the PR, this is a big improvement to managing codegen for the frameworks that aren't derive-based!

mumbleskates commented 1 month ago

you're welcome!

yeah i still think the ideal way is to have CI/build system do all the codegen. but when there's no artifact caching service like bazel, and/or the codegen process is an obstacle to people who want to build the library, and/or your code review system has absolutely no way to reason about or show you diffs in generated code... it's really not that much worse to just check it in and make the CI do it exactly right and then yell at you when the code is out of sync :)