scalapb-validate integration with akka-grpc

[thesamet]

akka-grpc defaults to compiling with flat_package=true. This results in user's generated code to reference the flat-package location of Scala sources generated by validate.proto. However, the validate.proto shipped with scalapb-validate, through common-protos defaults to flat_package=false.

The goal of this ticket is to come up with a straightforward way for akka-grpc users to use scalapb-validate.

[bjaglin]

Thanks for opening this to centralize the discussion. I have started thinking about this, see scalapb/ScalaPB#873 (comment) & scalapb/protoc-bridge#158.

[thesamet]

Idea 1: One workaround that would work with current versions is to have users throw in a proto file into their project that gives package-scoped option to disable flat_package for validate:

package validate;
import "scalapb/scalapb.proto";
option (scalapb.options) = {
  scope: PACKAGE
  flat_package: false
};

Idea 2: Building on idea 1, we can have a generator option that would have the same impact over specific packages, so akka-grpc users can exclude validate.

Idea 3: make a special case for validate package in the generator like we already do for google.protobuf and scalapb: https://github.com/scalapb/ScalaPB/blob/05b5aea520e621d0946ca08acc760bf306546199/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala#L909-L910

[bjaglin]

We have settled on a variant of idea 1 (since we didn't want to change all source code): disable flat_package at the generator level, but enable it selectively for our packages.

There is a gotcha with that, as Akka gRPC generators don't seem to handle package-level flat_package so flat_package should be disabled only for the ScalaPB gen. I was planning to open a PR on akka-grpc to fix that (I assume DescriptorImplicits is not used properly), update bjaglin/sbt-protoc@53dbd6a#diff-bbd8a5163e4909c406d2fd2b12cca4345f91859fae3676fd8ee189d121e2c9f1R2-R10 (which is broken at the moment), and write a scripted there.

[bjaglin]

Idea 1 does not seem to work as activating flat_package through the generator option takes precedence over the file/package-level setting: https://github.com/scalapb/ScalaPB/blob/05b5aea520e621d0946ca08acc760bf306546199/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala#L917

[thesamet]

https://github.com/scalapb/ScalaPB/blob/05b5aea520e621d0946ca08acc760bf306546199/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala#L909-L910

I think we should change to make it possible for the the file-level option to override with a false value.

[bjaglin]

There is a gotcha with that, as Akka gRPC generators don't seem to handle package-level flat_package

ScalaPB extensions are currently not registered in akka-grpc so the compiler plugin does not see them, I am sending a PR to akka-grpc shortly.

[thesamet]

ScalaPB extensions are currently not registered in akka-grpc so the compiler plugin does not see them, I am sending a PR to akka-grpc shortly

Thanks! Does it impact the solution? akka-grpc might erroneously think that 'validate' is compiled with flat_package - but does the code it generates references it?

[bjaglin]

It makes what I described in #70 (comment) easier (the scripted is precisely what we are using to workaround this issue).

I believe it does not impact in any way the solution you are implementing.

[bjaglin]

I think we can close this now that akka/akka-grpc#1264 verifies the integration with sbt-protoc 1.0.1 / scalapb-validate 0.2.1. Thanks for making the integration seamless!

[bjaglin]

Actually there is still one thing missing: usage of scalapb/validate.proto to customize scalapb-validate through file/package-level options is not possible out of the box from akka-grpc.

import "scalapb/scalapb.proto";
import "scalapb/validate.proto";

option (scalapb.options) = {
    scope: FILE
    [scalapb.validate.file] {
        validate_at_construction: true
    }
};

[info] [error] /tmp/sbt_70a87d09/target/scala-2.12/akka-grpc/main/example/myapp/helloworld/grpc/HelloworldProto.scala:11:22: object ValidateProto is not a member of package scalapb.validate
[info] [error]     scalapb.validate.ValidateProto,

From my understanding, we should either

1. have scalapb-validate-core expose its own option proto just like scalapb/common-protos@7c9595a
2. update https://github.com/scalapb/ScalaPB/blob/05b5aea520e621d0946ca08acc760bf306546199/compiler-plugin/src/main/scala/scalapb/compiler/DescriptorImplicits.scala#L909-L910

     private def isNonFlatDependency =
-      (file.getPackage == "google.protobuf") || (file.getPackage == "scalapb")
+      (file.getPackage == "google.protobuf") || (file.getPackage.startsWith("scalapb"))

Happy to open a PR for (2) on ScalaPB as it looks like the easiest.

[thesamet]

v0.2.2 is released with the manifest option - can you try it out?

[bjaglin]

v0.2.2 is released with the manifest option - can you try it out?

I just verified the fix with akka/akka-grpc@e4a39c6, thanks for the quick turnaround!

And this time, I think we are good to close this.