bazelbuild / rules_rust

Rust rules for Bazel
https://bazelbuild.github.io/rules_rust/
Apache License 2.0
671 stars 435 forks source link

Prost Crate Naming Conventions #2889

Open purkhusid opened 2 months ago

purkhusid commented 2 months ago

Wanted to start a discussion around the current naming conventions in the prost rules provided by rules_rust.

The prost rules set the crate name to the name of the proto_library that is being compiled. This approach is very prone to clashes e.g. the following folder structure:

/src
    /protobuf
        service1/
            /models
                models.proto
        service2/
            /models
                models.proto

In this folder structure the common way to name the proto_library target would be models_proto (Which is the convention that gazelle uses) but that causes issues if there is a consumer that needs both service1/models and service2/models .

I think it makes sense to change the naming conventions or make them configurable since this is a very likely issue when you have a repo with a large protobuf codebase.

Options:

purkhusid commented 2 months ago

@UebelAndre You got any opinion on this?

purkhusid commented 1 month ago

Another friendly ping @UebelAndre @matts1 @illicitonion @freeformstu

matts1 commented 1 month ago

Sorry, missed this earlier. Isn't there a crate_name attribute for this exact reason? Could we not either use that for prost, or add the attribute if it doesn't exist to prost_library?

Don't really like the idea of passing a naming convention to the rule. It'd seem more appropriate to create a symbolic macro wrapping the rule to change the naming convention if a user didn't want to write crate_name all the time

Not particularly familiar with prost so may be missing something

purkhusid commented 1 month ago

Sorry, missed this earlier. Isn't there a crate_name attribute for this exact reason? Could we not either use that for prost, or add the attribute if it doesn't exist to prost_library?

Don't really like the idea of passing a naming convention to the rule. It'd seem more appropriate to create a symbolic macro wrapping the rule to change the naming convention if a user didn't want to write crate_name all the time

Not particularly familiar with prost so may be missing something

The crate_name attribute is unfortunately not possible for the prost rules since they are aspect based and aspects can't access properties on the target that invoked the aspect. It would of course be possible to change the rules to not use aspects but that is a fairly large change and will also change the API of the prost rules.

matts1 commented 1 month ago

Unfortunately, I think I'm lacking the knowledge here to comment on a solution. I'll defer to someone more familiar with prost.