nikunjy / golink Goto Github PK

In order for my rule_runner to work (see issue #12), I have to manually add a visibility = ["//visibility:public"], attribute to all go_proto_link.

It would be nice if the Gazelle plugin had the option to emit the visibility attribute.

I'd be happy to submit a pull request for this issue and #12 as well.

Hi,

do you plan to make tags/releases?

we can't really use the latest changes with http_archive and we are forced to use git_repository.

Thank you

Hello!

I recently tried this tool out on Embark's internal bazel repo and we noticed golink seems to generate faulty rules when dealing with a proto file with dependencies.

For example, for the following bazel rule file, you can see that the go_proto_link rule is refering to a go_proto_library that does not actually exist in this package:

load("@golink//proto:proto.bzl", "go_proto_link")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

proto_library(
    name = "inventory_proto",
    srcs = ["inventory_user.proto"],
    visibility = ["//visibility:public"],
    deps = [
        "//services/online-subsystem/proto/inventory/inventory:inventory_proto",
        "@go_googleapis//google/rpc:status_proto",
    ],
)

go_proto_library(
    name = "inventory_go_proto",
    compilers = ["@io_bazel_rules_go//proto:go_grpc"],
    importpath = "github.com/EmbarkStudios/src/services/online-subsystem/proto/inventory/user",
    proto = ":user_proto",
    visibility = ["//visibility:public"],
    deps = ["//services/online-subsystem/proto/inventory/inventory:go_default_library"],
)

go_library(
    name = "go_default_library",
    embed = [":inventory_go_proto"],
    importpath = "github.com/EmbarkStudios/src/services/online-subsystem/proto/inventory/user",
    visibility = ["//visibility:public"],
)

proto_library(
    name = "user_proto",
    srcs = ["inventory_user.proto"],
    visibility = ["//visibility:public"],
    deps = ["//services/online-subsystem/proto/inventory/inventory:inventory_proto"],
)

go_proto_link(
    name = "user_go_proto_link",
    dep = ":user_go_proto",
    version = "v1",
)

Since gazelle has started to support Bzlmod, any plan to support Bzlmod?
Thanks.
WD

I've been using this tool for a while, but in a recent upgrade of Bazel and rules_go something has broken. Only noticeable after I've done a clean copy of the source.

See the output below:

$ bazel run //src/api:messages_go_proto_link
INFO: Invocation ID: a9221f6d-610f-412d-a507-156753398315
INFO: Analyzed target //src/api:messages_go_proto_link (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //src/api:messages_go_proto_link up-to-date:
  bazel-bin/src/api/messages_go_proto_link.sh
INFO: Elapsed time: 0.315s, Critical Path: 0.04s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/src/api/messages_go_proto_link.sh
cp: cannot stat 'bazel-out/k8-fastbuild/bin/src/api/messages_go_proto_/testing.com/messages/subject.pb.go': No such file or directory

Any help would be appreciated, I can push this test workspace I have made if that would help.

go_proto_link currently copies the .pb.go files to the same directory as the .proto file.

However, if the .proto file has a options go_package defined, it does not copy to directory specified in the go_options.

For example, given a file:

proto/mypackage/v1/test.proto

package mypackage.v1;

option go_package = "github.com/org/repo/go/mypackage/v1;mypackagev1";

Ideally, it would copy test.pb.go to go/mypackage/v1 instead of proto/mypackage/v1.

One way that this could be accomplished would be by:

• define a importpath attribute in the go_proto_link rule just like go_proto_library.
• Gazelle plugin copies the importpath from go_proto_library
• go_proto_link rule uses the relevant part of importpath to copy to the correct destination directory, creating the path if needed.

Instead of calling native.sh_binary in def go_proto_link why not just add executable = True to _go_proto_link?

We currently have a rule that is a rule runner, in other words, is capable of aggregating multiple rules in a single bazel run //:copy_protos target, that in turn invokes all go_proto_link rules in the repo.

Something like:

rule_runner(
    name = "copy_proto",
    targets = [
        "//common/v1:commonv1_go_proto_link_copy_gen",
        "//api/v1alpha:apiv1alpha_go_proto_link_copy_gen",
    ],
)

Because of this extra level of indirection using native.sh_binary, I have to create the targets in my rule by appending _copy_gen, for my rule to work.

proto.bzl could be just:

load("//:golink.bzl", "gen_copy_files_script")

def go_proto_link_impl(ctx, **kwargs):
    return gen_copy_files_script(ctx, ctx.attr.dep[OutputGroupInfo].go_generated_srcs.to_list())

_go_proto_link = rule(
    implementation = go_proto_link_impl,
    attrs = {
        "dir": attr.string(),
        "dep": attr.label(),
        "_template": attr.label(
            default = "//:copy_into_workspace.sh",
            allow_single_file = True,
        ),
        # It is not used, just used for versioning since this is experimental
        "version": attr.string(),
    },
    executable = True,
)

def go_proto_link(**kwargs):
    if not "dir" in kwargs:
        dir = native.package_name()
        kwargs["dir"] = dir

    _go_proto_link(**kwargs)