bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.37k stars 649 forks source link

Enable generate pb.go outputs as Bazel targets #3664

Closed tingilee closed 1 year ago

tingilee commented 1 year ago

What version of rules_go are you using?

0.40.0

What version of gazelle are you using?

0.28.0

What version of Bazel are you using?

6.3.0

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

Yes

What operating system and processor architecture are you using?

MacOS M1 Ventura and Linux AMD

Any other potentially useful information about your toolchain?

What did you do?

What did you expect to see?

We have mockgen rules that we need to refer to the generated pb.go files, sometimes only for one specific file service.pb.go among the many generated pb.go files from go_proto_library. I'd like to request for adding a list of output_list to expose //package:service.pb.go file from other Bazel targets. For files in this list, go_proto_library will declare targets for us to reference the generated file.

This will allow us to declare:

mockgen(
    name = "retriever_service_mock",
    package = "clients",
    source = "//retriever/v1:service.pb.go",
)

What did you see instead?

Today, this feature is not supported.

fmeum commented 1 year ago

Would something like skylib's select_file rule work here? I'm hesitant to add an output_list purely for the sake of providing labels for the output files since the compiler is still in control of what the output file names are.

tingilee commented 1 year ago

I'm not sure if select_file would be able to support the need here. https://github.com/bazelbuild/rules_go/blob/2e821f66bb9fe1e16ea42743d30936248c1fa11a/proto/def.bzl#L137-L150 Select_file seems to only able to get the target's DefaultOutput and not OutputGroupInfo

Error in fail: Can not find specified file in [bazel-out/darwin_arm64-fastbuild/bin/retriever/proto/v1_go_proto.a]

Is it possible to gather OutputGroupInfo in ctx in rule implementations?

tingilee commented 1 year ago

I was able to reference the pb.go file by adding the following:

for file_ in ctx.attr.srcs[OutputGroupInfo].go_generated_srcs.to_list():
        if file_.path.replace("\\", "/").endswith(canonical):
            out = file_
            break

after this: https://github.com/bazelbuild/bazel-skylib/blob/6fcbad3991638ca5882e64ec53143ac316b17a7e/rules/select_file.bzl#L27-L30

With this approach, the output would looks like: bazel-bin/retriever/v1/v1_go_proto_/github.com/repo/retriever/v1/service.pb.go And we won't be able to refer to this file since it's in bazel-bin instead of in the current Bazel workspace.

I agree that adding output_list in the rule seems weird given the proto compilers control the output. The output_list would be decided by the srcs input service.proto along with the compiler, ie. grpc service compiler.

Do you have other ideas of how we might be able to integrate this?

fmeum commented 1 year ago

How about we add an OutputGroupInfo to go_proto_library with one output group per output? Then you can grab yours with filegroup(output_group = ...). That does seem low effort and low footprint to me. What do you think?

tingilee commented 1 year ago

@fmeum Great idea! I've put up a diff, please review.

tingilee commented 1 year ago

Instead of pursuring with this change https://github.com/bazelbuild/rules_go/pull/3667, which seems like an anti-pattern, according to https://bazel.build/extending/rules#requesting_output_files

Note that OutputGroupInfo generally shouldn't be used to convey specific sorts of files from a target to the actions of its consumers. Define rule-specific providers for that instead.

I'm created a rule for us to get the generated resources instead.

def _find_matching_src(package, srcs, file_name):
    match = None
    for src in srcs:
        if file_name == src.basename:
            if src.owner.package != package:
                fail("select_proto_go_file can only be used in the same Bazel package as the go_proto_library itself")
            if match:
                fail("select_proto_go_file found duplicate matches for {}".format(file_name))
            match = src
    if not match:
        fail("select_proto_go_file did not find any matches for {}".format(file_name))
    return match

def _select_proto_go_file_impl(ctx):
    """
    Exposese generated proto Go files from go_proto_library()
    """
    files = [
        _find_matching_src(ctx.label.package, ctx.attr.src[GoSource].srcs, file_name)
        for file_name in ctx.attr.file_names
    ]
    return [DefaultInfo(
        files = depset(files),
    )]

select_proto_go_file = rule(
    implementation = _select_proto_go_file_impl,
    attrs = dict(
        src = attr.label(
            doc = "The GoSource provider, specifically for go_proto_library targets",
            providers = [GoSource],
        ),
        file_names = attr.string_list(
            doc = "The set of src file names to find in the GoSource",
        ),
    ),
)
alexeagle commented 1 year ago

Hi, how about my solution to this earlier duplicate issue https://github.com/bazelbuild/rules_go/issues/3658#issuecomment-1678046338

I don't think you need any new custom rules, nor any new providers, so long as you're willing to repeat the filenames from the existing OutputGroup that should be copied back to the source tree.