So, I think I understand where the original work is coming from, but
fundamentally disagree with its implementation.
The idea was to create "drop in" macros that you can use to easily
create rust pb bindings, abstracting over the varied and complex tooling
that the rust ecosystem requires.
I believe this to be a mistake:
1. Potential for version mismatch for rlib symbols
Anyone who remembers the "bad old days" of tokio 0.x (and futures
0.1, for that matter) knows the pain of attempting to link against
two different versions of the same symbol, because some dep you have
is using an older version of the exported symbol.
The use of "bundled" fallbacks in the varied xxx_dep(s) macro args
invite this same issue.
Instead, callers should made aware of the needed deps and be
required to provide their own, ensuring their projects remain
coherent.
The bundled should *only* be used for creating the needed binaries
for protoc codegen
2. Focus on UX over documentation
The entire //rust:... subsystem of this project is misleadingly
documented, to the point that it is easier to simply read the
entire code base, rather than the docs.
There was an attempt to compensate for this via the examples/ and
ease-of-use in the public macros and rules, however the lack of
documentation leaves users that need to integrate with more complex
monorepos completely confused about how to correctly satisfy the
mandatory args, what expected values for these args are, etc
Therefore, this refactor of the top level *_library macros adds proper
documentation each, and simplifies the args and notifies the caller of
the requirements to call each macro.
Modifies the cc_library call to only forward arguments if they're present in kwargs.
This is vital for bazel 7.0.0 compatibility since the `nocopts` attribute is removed (it's actually been deprecated since 2019 but the documentation was never updated).
Quoting [1]:
> By default proto3 JSON printer should convert the field name to
> lowerCamelCase and use that as the JSON name. An implementation
> may provide an option to use proto field name as the JSON name
> instead.
The serde plugin does this correctly, but when the rules specify
the `preserve_proto_field_names` option, two things happen:
1. The generated serde code preserves the proto field names
2. The user of these bazel rules cannot override the choice
If, instead, rules_proto_grpc avoids specifying this flag, it can
be optionally added to the rust_prost_proto_library invocation in
user code via `options`:
options = {
"//rust:rust_serde_plugin": ["preserve_proto_field_names"],
},
Thus, the better default is to leave out this flag from
rules_proto_grpc and instead allow the user to choose.
[1]: https://protobuf.dev/programming-guides/proto3/#json
The lints are generally appropriate for hand-written code, but
oftentimes the best solution for generated code violates such
guidelines. Typically, therefore, generated code is not subject to
such linting.
The most precise way to disable linting for this generated code in
a bazel environment is to add `#![allow(clippy::all)]` to the top
level of the generated crate.
Paul Stemmet
Adam Liddell