Support with_java for scala_proto_toolchain

[dthong]

Can we add back the option 'with_java=True' for scalapb proto? I am using the previous version of rules_scala which supports this (commit 7ea9079f15ba85df21e442f29cd0e37424b09ae7), but seems like the option was removed in the latest version.

[johnynek]

@ianoc is this something easy to add to the aspect based implementation?

[ianoc]

@johnynek not super easy no :( I was asking something about this on the bazel-discuss list yesterday. The scala aspect needs to depend on the java aspects produced provider to have it as a dep. The java aspect of course is in java :/ . It seems though that it will produce a JavaInfo which we could use though it is kinda lurky(since it feels like its an ambiguous provider?). I also then don't know how to make the java provider optional in the aspect -- i believe it would need to be added https://github.com/bazelbuild/rules_scala/blob/master/scala_proto/private/scalapb_aspect.bzl#L220 which i don't think you can configure with a toolchain.

[amit-traiana]

Any news about that? Are you guys run the ScalaPb standalone compiler? or your wrapping it as Bazel plug-in? the standalone compiler doesn't need Java code pre-generated. It will compile it for you into folder you specify.

[ianoc]

@amit-traiana the problem is that the scalpb and javapb are both aspects, and I don't know of a provider/means in bazel to (optionally) say that the generated target for proto food depends on the generated java target. We can't generate it in scalpb since you would have duplicate classes in the repo for when the java ones kick in. Which can cause lots of subtle problems.

The only answer I know of here which I don't really know how to do is to have the aspect target configured in the tool chain depend on a provided dependency from the java proto rule. Maybe the capability is there in bazel/or has been added since I looked at this, but I couldn't figure out how to do it last time I looked.

[amit-traiana]

@ianoc I think you're right and currently there is no way to share the kind of information between aspects, I didn't know you also generating Java classes along with the Scala classes. That indeed can cause a pain when the generated Java Code your create and the one ScalaPB doesn't match.

java_conversion is pretty critical on the project I'm currently working with, so I'm assuming that I'll either have to use one of the variants that call protoc directly (like stackb/rules_proto, or to use pre-aspect version of scala-rules. Any reason you can think about I shouldn't be sticking with the the older scala-rules versions? (besides that fact I am no longer getting dependencies from the Aspects?)

Thanks @ianoc!

[ittaiz]

Also maybe we should revive https://github.com/bazelbuild/rules_scala/pull/763 ? Maybe needs to be redone completely due to the phases architecture but that's work @amit-traiana can decide to take on. @simuons would love your thoughts here as well if you see any problems with having some thing like #763 in (I don't think so but you're much more into the proto details than me)

[amit-traiana]

Doesn't creating "another rule which would give full flexibility in invoking the scalapb to cover these use cases." technically means forking the logic? which in turn makes adding new features or just re-factoring more complex? I'm not familiar with the code, but I think that optimally = ScalaPb should generate both the Java and Scala code - so you won't have to manually glue them later on or being worried about versions.

EDIT: Removed a section regarding dependencies and aspects, because I think I found a solution. More to follow if relevant :-)

[ianoc]

@amit-traiana The intent here in that issue #763 was to share the implementation almost entirely just having some logic to wrap it into a aspect or be directly callable in a rule. Mixing the too would definitely be iffy, though there would be some advanced usages like maybe wix where that would be the right call.

A solution that doesn't help the general purpose (scalapb tossed in lots of capabilities I don't think are in the other proto lang systems for better or worse depending on how badly you like them), but would help @amit-traiana here I suspect is a toolchain option with some warnings but something like: "Enable this and scalpb will generate the java code and the conversion mappings" since it would be all in one with the compile I think it would still work though mixing with the java stuff/other edge cases could be odd, but possibly rare enough since the java generation is built into protoc and I don't think has much in the way of options.

We could query upstream with bazel/propose a patch to add a provider for the JavaInfo output under a specific name from the proto java rules to help with part of this too... that could be dependended on by the aspect I believe?

Scalapb is scala specific and so I don't think in the general case you would have it rule them all, once you consider all the google projects/java projects that will refer to the normal built in rules scalapb looks like an outlier and not going to be the norm.

@ittaiz would phases impact these external rules? TIL. Do we have a java.common_compile interface like we could hook into possibly? It might be nice to just pull these rules out of rules_scala entirely (thrift is not probably widely used at all I'd guess).

[ittaiz]

@ianoc what are these external rules? Also we have a java_common.compile interface to compile java code. Do you mean if we have something like this for scala? Not really. Given the phases architecture one can build their own set of rules by swapping out the one phase they think we got wrong (or they got better). Does that answer your question?

[ianoc]

@ittaiz I guess I was viewing things like proto and thrift to be external from Scala compilation itself. (could, and if it wasn't more painful around repos and how often hazel used to become incompatible across versions, should have been separate repos).

More in the non-mono repo/same company case, if you make scala_proto somewhere else, you just want an easy means to invoke scalac like that java_common compile stuff, where in it should likely work across consumers easily. (I've ran into this that the setup around scala_proto for extensibility was actually a mistake since you can only really use this in a leaf repo since its configured on the toolchain. -- so its somewhat tangental for this issue originally but I need to build another scala proto rule in a repo that will be consumed by several repos with different toolchains/rules_scala versions/phaases probably and so on. So was looking to see if there was an API like that common_compile you could use from another set of rules.

[ittaiz]

I think that the current state shows us that they are external to a very certain point. We don't have good decoupling there. Also re the toolchain and such- this is by design in bazel since everything is a mono repo. Maybe you can export something like a "toolchain fragment" and leaves will use that? Re java_common.compile- FR at Bazel? We can't add this ourselves.

@amit-traiana are you interested in reviving #763?

[amit-traiana]

@ittaiz As for now I'm using rules-proto-grpc, which uses the scalapb plug-in directly with protoc. So I have an easier solution to use java_convertions. We currently in a verge of moving our whole build-system into Bazel, and I'm focusing on the 'must'. and since rules_proto_grpc supplies a solution, I will be using it for now. It is something I would like to perhaps tackle later on when things are settled here though. So reviving at least on my part is not urgent as for the moment. Thanks!

[ittaiz]

ok. @ianoc maybe we should close this issue and have a more generic issue which is something like #763?

[gergelyfabian]

Thanks @amit-traiana for the directions on a possible workaround.

I'm posting my workaround here, using rules-proto-grpc, if anyone was wondering in the future how to solve this (outside of rules_scala):

# WORKSPACE:

http_archive(
    name = "rules_proto_grpc",
    sha256 = "28724736b7ff49a48cb4b2b8cfa373f89edfcb9e8e492a8d5ab60aa3459314c8",
    strip_prefix = "rules_proto_grpc-4.0.1",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/archive/4.0.1.tar.gz"],
)

load("@rules_proto_grpc//:repositories.bzl", "rules_proto_grpc_repos", "rules_proto_grpc_toolchains")

rules_proto_grpc_toolchains()

rules_proto_grpc_repos()

load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains")

rules_proto_dependencies()

rules_proto_toolchains()

load("@rules_proto_grpc//scala:repositories.bzl", RULES_PROTO_GRPC_SCALA_MAVEN_ARTIFACTS = "MAVEN_ARTIFACTS", rules_proto_grpc_scala_repos = "scala_repos")

rules_proto_grpc_scala_repos()

load("@io_grpc_grpc_java//:repositories.bzl", "IO_GRPC_GRPC_JAVA_ARTIFACTS", "IO_GRPC_GRPC_JAVA_OVERRIDE_TARGETS", "grpc_java_repositories")

maven_install(
    name = "rules_proto_grpc_java_maven",
    artifacts = IO_GRPC_GRPC_JAVA_ARTIFACTS,
    generate_compat_repositories = True,
    override_targets = IO_GRPC_GRPC_JAVA_OVERRIDE_TARGETS,
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)

load("@rules_proto_grpc_java_maven//:compat.bzl", rules_proto_grpc_java_compat_repositories = "compat_repositories")

# Note: This wasn't well documented, but I needed to include Java compat repositories.
rules_proto_grpc_java_compat_repositories()

grpc_java_repositories()

load("@rules_proto_grpc//java:repositories.bzl", rules_proto_grpc_java_repos = "java_repos")

rules_proto_grpc_java_repos()

# Override proto-grpc-scala artifacts with your own versions (if you need).
#RULES_PROTO_GRPC_SCALA_MAVEN_ARTIFACTS = [...]

maven_install(
    name = "rules_proto_grpc_scala_maven",
    artifacts = RULES_PROTO_GRPC_SCALA_MAVEN_ARTIFACTS,
    maven_install_json = "@//:rules_proto_grpc_scala_maven_install.json",
    repositories = [
        "https://repo1.maven.org/maven2",
    ],
)

load("@rules_proto_grpc_scala_maven//:defs.bzl", rules_proto_grpc_scala_pinned_maven_install = "pinned_maven_install")

rules_proto_grpc_scala_pinned_maven_install()


# BUILD:
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@rules_proto_grpc//scala:defs.bzl", "scala_grpc_library")
load("@rules_proto_grpc//java:defs.bzl", "java_grpc_library")

proto_library(
    name = "proto_lib",
    srcs = [
        "test.proto"
    ],
    deps = [],
)

# You need to generate Java code separately.
java_grpc_library(
    name = "proto_java_lib",
    protos = [":proto_lib"],
)

# Generating scala code with "java_conversions" only creates the converters, but not the Java code itself.
scala_grpc_library(
    name = "proto_scala_lib",
    options = {
        "@rules_proto_grpc//scala:grpc_scala_plugin": [
            "java_conversions",
            "flat_package",
        ],
    },
    protos = [":proto_lib"],
    deps = [":proto_java_lib"],
)