Generating protobufs in the repo

[clayrosenthal]

Generating the protobufs in the build script, then storing them in src/generated to avoid name conflict. Only generate it with the gen feature flag.

[CLAassistant]

All committers have signed the CLA.

[krant]

Thanks for the PR @clayrosenthal , that's a good start, but it will need some work:

• Pre-generated protobuf files has to be inside the repo, so project would build without protoc or event without protobufs submodule - it would greatly simplify building on exotic platforms and first-time experience for wondering users (no need for git clone --recursive, yay!).
• Could you please leave submodule renaming for future PRs, to make this one more atomic? When/if we arrive to separate crate with generated protobufs we handle it there.
• Fixing linting warnings in other parts of library is also better to separate - it would be much easier to review and to track git history.
• Last but not least - hide build-dependencies behind gen feature-flag, so more advanced users would be able to continue using the project as it is now (just by enabling the feature), while others by default will use generated Rust files, enjoy faster build times (no need to get/build prost-build and friends, hey!) and not bother with protoc installing.

Sorry if this could sound like too much, it is totally fine to leave it to someone else, but if you decide to pursue the goal I will be glad to continue reviews and giving the feedback.

[clayrosenthal]

Whoops, I meant to include the generated output in the PR, but I think the submodule renaming borked it. Reworked this to not rename the submodule.

hide build-dependencies behind gen feature-flag,

Which build dependencies are you referring to, and how would I do that? Like add a #[gen] or something?

Edit: Did some research, I put the generation behind the gen feature, and updating the generated-protobufs into the update-gen feature.

[krant]

Yeah, exactly, just like it's done in https://github.com/tikv/rust-prometheus/blob/master/build.rs for example - when gen is enabled, build.rs invokes whole protobuf generating toolchain (like it currently does), but when it is disabled (which would be by default) - main in build.rs would do nothing.
Currently there are 3 build dependencies - prost-build, protoc-bin-vendored and walkdir. When gen is disabled we could omit them completely by making them optional, rust-prometheus repo does something similar for its build deps.

[krant]

generation behind the gen feature, and updating the generated-protobufs into the update-gen

Why not to have updating behind gen too?

[clayrosenthal]

Hmm good point. I'll just have gen generate for building and updating. Fixed now

[clayrosenthal]

Ok squashed the commits to try and keep the history prettier, should be good now

[krant]

Looks good! The only thing which puzzles me is why is there _.rs file? I've deleted it and project seems to build just fine.

[krant]

Couple of nitpicks:

• Is this possible to drop OUT_DIR completely and make gen to put meshtastic.rs over generated-protobufs/meshtastic.rs?
• Could you rename generated-protobufs dir into generated?

[krant]

_.rs file

it comes from nanopb.proto because it misses package specifier. Later I'll create PR with the fix in protobufs repo and when it merge, nanopb code will go into generated meshtastic.rs. Here and now _.rs could be removed form the PR - it must be harmless.

[clayrosenthal]

Yeah in my project I just renamed the package in the build script, to my understanding that proto file is generated by some third party library nanopb/nanopb, but I don't quite get its use.

[caveman99]

Yeah in my project I just renamed the package in the build script, to my understanding that proto file is generated by some third party library nanopb/nanopb, but I don't quite get its use.

The primary consumer of these proto files is the device firmware, and it uses the nanopb bindings. it's a very slim implementation of a protobuf subset. Most other consumers (e.g. apps and libs) use stock protoc with one or two flags to be compatible with the nanopb stuff we use.

[clayrosenthal]

it's a very slim implementation of a protobuf subset.

So if you're using the full protobuf directory you don't need the smaller nanopb?

[caveman99]

Nanopb also uses the full protobuf directory. The protobuf definitions are the same, with a few quirks needed to be performant on embedded systems. Nanopb is the interface library for the generated code. It needs the nanopb.proto to work but if you generate protoc code for other languages you don't need it and you can ignore errors from that file.

[krant]

Thanks for the effort @clayrosenthal, I've gave it run locally and it looks good to me. The only thing I noticed is that if I re-gen meshtastic.rs it differs from git version, I suppose it is because of different order of files in protobufs dir. Could you please add protos.sort(); after walkdir invocation and regenerate? When you push it, I'll re-check meshtastic.rs matches my local generated version and it will be ready.

P.S. Sorry, one more thing - delete _.rs file and add it to .gitignore.

[clayrosenthal]

Ok, added a sort and removed the nanopb file with a description in the gitignore. Thanks for the guidance @krant

[krant]

Thanks a lot @clayrosenthal, now when run cargo build --features gen my meshtastic.rs is the same.
Just perfect. This PR greatly improves project usability.

[krant]

@lukipuki would you mind to review this and merge if it looks good to you?