Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a java_grpc_library() Skylark extension #2756

Closed
cgrushko opened this issue Feb 26, 2017 · 18 comments
Closed

Provide a java_grpc_library() Skylark extension #2756

cgrushko opened this issue Feb 26, 2017 · 18 comments
Assignees
Milestone

Comments

@cgrushko
Copy link
Contributor

Bazel users can now build protos using the built-in proto_library and {cc,java,javalite}_proto_library rules.

These rules do not include gRPC support intentionally; this FR is about providing a Skylark extension to allow users to build gRPC bindings.

@ejona86

@lukaszx0
Copy link
Collaborator

Ah nice, I've been playing with bazel recently and lack of first class support for protos/grpc was a bit problematic.

I'd be happy to own this task if it's not very high priority as I have already few things on my plate.

For posterity, this is related to #2458

@cgrushko
Copy link
Contributor Author

@ejona86 already wrote such extension, but it might be complicated to open-source it.

@ejona86
Copy link
Member

ejona86 commented Feb 28, 2017

Main hold-up is figuring out all the dependencies (and set up Bazel for the first time :-D). This is a modified dump of what I've got.

def _gensource_impl(ctx):
  if len(ctx.attr.srcs) > 1:
    fail("Only one src value supported", "srcs")
  for s in ctx.attr.srcs:
    if s.label.package != ctx.label.package:
      print(("in srcs attribute of {0}: Proto source with label {1} should be in "
             + "same package as consuming rule").format(ctx.label, s.label))
  # Use .jar since .srcjar makes protoc think output will be a directory
  srcdotjar = ctx.new_file(ctx.label.name + "_src.jar")

  srcs = [f for dep in ctx.attr.srcs for f in dep.proto.direct_sources]
  includes = [f for dep in ctx.attr.srcs for f in dep.proto.transitive_imports]
  flavor = ctx.attr.flavor
  if flavor == "normal":
    flavor = ""
  ctx.action(
      inputs = [ctx.executable._java_plugin] + srcs + includes,
      outputs = [srcdotjar],
      executable = ctx.executable._protoc,
      arguments = [
          "--plugin=protoc-gen-grpc-java=" + ctx.executable._java_plugin.path,
          "--grpc-java_out={0},enable_deprecated={1}:{2}"
            .format(flavor, str(ctx.attr.enable_deprecated).lower(), srcdotjar.path)]
          + ["-I{0}={1}".format(include.short_path, include.path) for include in includes]
          + [src.short_path for src in srcs])
  ctx.action(
      command = "cp $1 $2",
      inputs = [srcdotjar],
      outputs = [ctx.outputs.srcjar],
      arguments = [srcdotjar.path, ctx.outputs.srcjar.path])

_gensource = rule(
    attrs = {
        "srcs": attr.label_list(
            mandatory = True,
            non_empty = True,
            providers = ["proto"],
        ),
        "flavor": attr.string(
            values = [
                "normal",
                "lite",
            ],
            default = "normal",
        ),
        "enable_deprecated": attr.bool(
            default = False,
        ),
        "_protoc": attr.label(
            default = Label("@com_google_protobuf//:protoc"),
            executable = True,
            cfg = "host",
        ),
        "_java_plugin": attr.label(
            default = Label("@io_grpc_java//:TODO_plugin"),
            executable = True,
            cfg = "host",
        ),
    },
    outputs = {
        "srcjar": "%{name}.srcjar",
    },
    implementation = _gensource_impl,
)

def java_grpc_library(name, srcs, deps, flavor=None,
                      enable_deprecated=None, visibility=None,
                      **kwargs):
  """Generates and compiles gRPC Java sources for services defined in a proto
  file. This rule is compatible with java_proto_library and java_lite_proto_library.

  Do note that this rule only scans through the proto file for RPC services. It
  does not generate Java classes for proto messages. You will need a separate
  java_proto_library or java_lite_proto_library rule.

  Args:
    name: (str) A unique name for this rule. Required.
    srcs: (list) a single proto_library target that contains the schema of the
        service. Required.
    deps: (list) a single java_proto_library target for the proto_library in
        srcs.  Required.
    flavor: (str) "normal" (default) for normal proto runtime. "lite"
        for the lite runtime.
    visibility: (list) the visibility list
    **kwargs: Passed through to generated targets
  """
  if flavor == None:
    flavor = "normal"

  if len(deps) > 1:
    print("Multiple values in 'deps' is deprecated in " + name)

  gensource_name = name + "__do_not_reference__srcjar"
  _gensource(
      name = gensource_name,
      srcs = srcs,
      flavor = flavor,
      enable_deprecated = enable_deprecated,
      visibility = ["//visibility:private"],
      **kwargs
  )

  added_deps = [
      "@io_grpc_java//:TODO_core",
      "@io_grpc_java//:TODO_stub",
      "@TODO_guava//:util_concurrent",
  ]
  if flavor == "normal":
    added_deps += ["@com_google_protobuf_java//:java_toolchain"]
  elif flavor == "lite":
    added_deps += ["@com_google_protobuf_java_lite_TODO//:java_lite_toolchain"]
  else:
    fail("Unknown flavor type", "flavor")

  native.java_library(
      name = name,
      srcs = [gensource_name],
      visibility = visibility,
      deps = [
          "@TODO//:jsr305_annotations",
      ] + deps + added_deps,
      **kwargs
  )

@ejona86 ejona86 self-assigned this Mar 2, 2017
@ejona86
Copy link
Member

ejona86 commented Mar 14, 2017

@ronshapiro
Copy link

Logging a 👍 for this from the Dagger team. We'd love this so that we can release our gRPC support (google/dagger#647) and run our tests in travis.

@lukaszx0
Copy link
Collaborator

lukaszx0 commented Apr 4, 2017

Ha! I was actually planning to follow up on that dagger issue and if bazel is blocking it, I'll try to find some time to work on this.

@buchgr
Copy link
Collaborator

buchgr commented Apr 5, 2017

@ejona86 @lukaszx0 are you working on this? Else, I would be happy to take this over and work on it now as we need it soonish within bazel :-).

@ejona86
Copy link
Member

ejona86 commented Apr 5, 2017

@buchgr, @lukaszx0, no, I've not started on this (other than the snippet above). I have this working on Blaze already. But I've not started on getting Bazel up-and-running.

If someone else works on this, the snippet would need to stay mostly the same, to make maintenance easier. But figuring out deps would be nice.

@bluecmd
Copy link

bluecmd commented Apr 5, 2017

Just making sure you're aware of https://github.com/pubref/rules_protobuf. Works for me, but it's a bit too magical w.r.t. dependencies in my opinion.

@cgrushko
Copy link
Contributor Author

cgrushko commented Apr 5, 2017

@bluecmd do you know if it can interop with Bazel's native protobuf support? (https://bazel.build/blog/2017/02/27/protocol-buffers.html)

@ejona86
Copy link
Member

ejona86 commented Apr 5, 2017

@bluecmd, yeah, I'm aware of it. I've suggested it to some people. I was already working on the Blaze portion then, as the grpc-io announce thread mentioned.

@buchgr
Copy link
Collaborator

buchgr commented Apr 6, 2017

I ll try to add this to bazel then, as we need it for open sourcing another piece of bazel. Unless you wanted to do it @ejona86? I'd obviously attribute the code to you in the commit. Thanks!

@ejona86 ejona86 removed their assignment Apr 7, 2017
@lukaszx0
Copy link
Collaborator

lukaszx0 commented Apr 7, 2017

@buchgr I also haven't started working on it yet and likely won't anytime soon (backlogged with other stuff) so feel free to take it. Looks like it will unblock a bunch of things - grpc-dagger, grpc and bazel itself!

@buchgr buchgr self-assigned this Apr 7, 2017
@simonhorlick
Copy link
Contributor

I'm really interested in seeing official rules for grpc-java so I made a quick attempt in my fork here: master...simonhorlick:master. I've used the same approach to dependencies as rules_closure, where consumers can opt-out of libraries they directly depend on.

You can also see it in action with a slightly modified version of the helloworld example here: https://github.com/simonhorlick/grpc-java-helloworld.

@buchgr
Copy link
Collaborator

buchgr commented May 3, 2017

@simonhorlick hey that's great. If you feel like it works, just open a PR :-). I unfortunately have not had enough time to add this to grpc myself, but I have added it to bazel for bazel-internal grpc. One thing I noticed is that protoc paths work different outside of Google and so in the implementation function of java_grpc_library you need to replace all occurrences of File.short_path by _path_ignoring_repository(File):

def _path_ignoring_repository(f):
  if (len(f.owner.workspace_root) == 0):
    return f.short_path
  return f.path[len(f.owner.workspace_root)+1:]

simonhorlick added a commit to simonhorlick/grpc-java that referenced this issue May 4, 2017
Bazel third party dependencies are specified in repositories.bzl which
gives the consumer the ability to opt-out of any dependencies they use
directly in their own project. Also note that reposititories.bzl
uses java_import_external from rules_closure instead of Bazel's built in
maven_jar. This allows us to specify multiple urls for each jar,
reducing the likelyhood of build failures due to repositories becoming
unavailable and also define dependencies between libraries.

Fixes grpc#2756

References:

bazelbuild/bazel#1952
@simonhorlick
Copy link
Contributor

@buchgr Thanks, I had something similar, but yours is a bit cleaner. I've submitted a PR with the changes.

@dionescu
Copy link

dionescu commented May 11, 2017

What about gRPC C++ support in Bazel?

Edit: Just noticed this is grpc-java repo. I see there is a separate issue tracking C++ support here:
grpc/grpc#9873

simonhorlick added a commit to simonhorlick/grpc-java that referenced this issue Jun 13, 2017
Bazel third party dependencies are specified in repositories.bzl which
gives the consumer the ability to opt-out of any dependencies they use
directly in their own project.

Fixes grpc#2756
ejona86 pushed a commit that referenced this issue Jun 22, 2017
Bazel third party dependencies are specified in repositories.bzl which
gives the consumer the ability to opt-out of any dependencies they use
directly in their own project.

Fixes #2756
@ejona86
Copy link
Member

ejona86 commented Jun 22, 2017

I created #3125 for adding Bazel to CI

@ejona86 ejona86 added this to the 1.5 milestone Jun 22, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Sep 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants