rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

Remove `deps` attribute from `go_proto_library` by automatically inferring them

Open udaya2899 opened this issue 1 year ago • 4 comments

What version of rules_go are you using?

0.37.0

What version of gazelle are you using?

0.31.0

What version of Bazel are you using?

6.2.0

Does this issue reproduce with the latest releases of all the above?

Checking the release notes, the requested feature is not there yet

What operating system and processor architecture are you using?

NA

Any other potentially useful information about your toolchain?

NA

What did you do?

I wrote a go_proto_library rule for a corresponding proto_library rule.

For example,

proto_library(
    name = "foo_proto",
    deps = [":bar_proto"],
)

go_proto_library(
    name = "foo_go_proto",
    protos = [":foo_proto"],
    deps = [":bar_go_proto"],
)

What did you expect to see?

The deps attribute needs the corresponding bar_go_proto to be included. Since the protos attribute has the corresponding proto rules, it would be nice to see the bar_go_proto to be inferred by the rule automatically. In other words, we'd like to see the deps attribute removed altogether in the go_proto_library rule. This is how it works in blaze, and it is implemented using aspects there.

cc_proto_library from rules_cc and py_proto_library from rules_python already does this too.

Expected:

proto_library(
    name = "foo_proto",
    deps = [":bar_proto"],
)

go_proto_library(
    name = "foo_go_proto",
    protos = [":foo_proto"],
)

What did you see instead?

This feature is not part of the releases yet. I can also support in developing this but I'm not sure about the work needed and where.

udaya2899 avatar Aug 22 '23 14:08 udaya2899

This would be a very welcome contribution. I can review PRs and offer support. I haven't worked with proto_common yet, but I would assume that https://github.com/bazelbuild/rules_python/blob/main/python/private/proto/py_proto_library.bzl is a good starting point. cc_proto_library still has some native parts.

CC @linzhp

fmeum avatar Aug 22 '23 14:08 fmeum

Another source of inspiration might be py_proto_library from gRPC: https://github.com/grpc/grpc/blob/master/bazel/python_rules.bzl#L160 as py_proto_library from rules_python is a recent addition.

That said, as someone unfamiliar with the internals of these libraries and aspects in general, I am struggling to reconcile these implementations with go_proto_library. Some comments would not have hurt :)

Very happy if someone more qualified would pick this up.

faximan avatar Aug 24 '23 13:08 faximan

Bump. @fmeum some specific pointers could helpsus. Like @faximan said, I find it hard to understand the implementations comparing to py_proto_library.

More specifically:

  1. Where in proto/def.bzl should I insert the logic that queries other deps and appends along with deps?
  2. Where in py_proto_library does this happen that I can take as a reference?
  3. If possible, could you outline the logic/steps to do for a first contributor like me?

udaya2899 avatar Sep 08 '23 16:09 udaya2899

Sorry, I didn't get to answer your questions yet. There is now a draft PR: https://github.com/bazelbuild/rules_go/pull/3706

fmeum avatar Sep 25 '23 09:09 fmeum