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

"cycle in dependency graph" when importing a go_repository #1322

Closed
ensonic opened this issue Feb 13, 2018 · 10 comments
Closed

"cycle in dependency graph" when importing a go_repository #1322

ensonic opened this issue Feb 13, 2018 · 10 comments

Comments

@ensonic
Copy link

ensonic commented Feb 13, 2018

in my WORKSPACE:

go_repository(
    name = "github_com_grpc_ecosystem_grpc_gateway",
    commit = "0a810034e45df6b5a119b0cd3a56150c459e3f9a",
    importpath = "github.com/grpc-ecosystem/grpc-gateway"
)

and in my BUILD file

proto_lang_toolchain(
  name = "swagger_toolchain",
  command_line = "--swagger_out=$(OUT)",
  runtime = "@github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:protoc-gen-swagger",
  visibility = ["//visibility:public"],
)

Now building:

$ bazel build //src/proto:swagger_toolchain
ERROR: $HOME/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/protoc-gen-swagger/options/BUILD.bazel:30:1: in go_library rule @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:go_default_library: cycle in dependency graph:
    //src/proto:swagger_toolchain
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:protoc-gen-swagger
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:go_default_library
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/genswagger:go_default_library
.-> @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:go_default_library
|   @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:options_go_proto
`-- @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:go_default_library
This cycle occurred because of a configuration option
ERROR: Analysis of target '//src/proto:swagger_toolchain' failed; build aborted

$ cat §HOME/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/protoc-gen-swagger/options/BUILD.bazel
oad("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
    name = "options_proto",
    srcs = [
        "annotations.proto",
        "openapiv2.proto",
    ],
    visibility = ["//visibility:public"],
    deps = [
        ":options_proto",
        "@com_google_protobuf//:any_proto",
        "@com_google_protobuf//:descriptor_proto",
    ],
)

go_grpc_library(
    name = "options_go_proto",
    importpath = "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options",
    proto = ":options_proto",
    visibility = ["//visibility:public"],
    deps = [
        ":go_default_library",
        "@com_github_golang_protobuf//protoc-gen-go/descriptor:go_default_library",
        "@com_github_golang_protobuf//ptypes/any:go_default_library",
    ],
)

go_library(
    name = "go_default_library",
    embed = [":options_go_proto"],
    importpath = "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options",
    visibility = ["//visibility:public"],
)
@ensonic
Copy link
Author

ensonic commented Feb 13, 2018

FYI. if I add build_file_proto_mode = "legacy", (or disable) to the go_repository in the WORKSPACe, it builds, but then the tool won't work obviously.

Too bad that go_repository does not have a 'build_file' attribute like {new_,}http_archive.

@jayconrod
Copy link
Contributor

I couldn't reproduce this. Are you using a recent version of rules_go and Gazelle? Here's the build file I see:

load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

proto_library(
    name = "options_proto",
    srcs = ["options.proto"],
    visibility = ["//visibility:public"],
    deps = ["@com_google_protobuf//:descriptor_proto"],
)

go_proto_library(
    name = "options_go_proto",
    importpath = "github.com/grpc-ecosystem/grpc-gateway/options",
    proto = ":options_proto",
    visibility = ["//visibility:public"],
    deps = ["@com_github_golang_protobuf//protoc-gen-go/descriptor:go_default_library"],
)

go_library(
    name = "go_default_library",
    embed = [":options_go_proto"],
    importpath = "github.com/grpc-ecosystem/grpc-gateway/options",
    visibility = ["//visibility:public"],
)

I'm working on an alternative to go_repository which lets you specify a set of checked-in build files instead of generating them at load time. It will be useful in cases where Gazelle doesn't quite do the right thing (especially around protos). Take a peak at https://github.com/bazelbuild/bazel-gazelle/blob/master/internal/overlay_repository.bzl. The rules should be ready this week, but Gazelle will still need to support generating and updating build files for these repositories.

@ensonic
Copy link
Author

ensonic commented Feb 13, 2018

We're using bazel 0.10. and rules_go-0.9.0. FYI: this is what we have in the WORKSPACE:

# Go rules and proto support
http_archive(
    name = "io_bazel_rules_go",
    sha256 = "4d8d6244320dd751590f9100cf39fd7a4b75cd901e1f3ffdfd6f048328883695",
    urls = [
        "https://github.com/bazelbuild/rules_go/releases/download/0.9.0/rules_go-0.9.0.tar.gz",
    ],
)

load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_repository")

and in our top-level BUILD.bazel:

load("@io_bazel_rules_go//go:def.bzl", "gazelle", "go_prefix")

# Required by go_proto_library.
go_prefix(prefix = "<our-prefix>")

gazelle(
    name = "gazelle",
    args = [
        "-build_file_name",
        "BUILD.bazel",
    ],
    prefix = "<our-prefix>",
)

We haven't switched to https://github.com/bazelbuild/bazel-gazelle, I'll try this next.

Overriding the BUILD files for cases like this will be very useful to unblock people.

@ensonic
Copy link
Author

ensonic commented Feb 14, 2018

After switching to use gazelle from https://github.com/bazelbuild/bazel-gazelle it fails differently:

bazel build //...
ERROR: $HOME/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/third_party/googleapis/google/api/BUILD.bazel:17:1: no such package '@github_com_grpc_ecosystem_grpc_gateway//google/api': BUILD file not found on package path and referenced by '@github_com_grpc_ecosystem_grpc_gateway//third_party/googleapis/google/api:annotations_go_proto'
ERROR: Analysis of target '//src/proto:registry_swagger' failed; build aborted: no such package '@github_com_grpc_ecosystem_grpc_gateway//google/api': BUILD file not found on package path
INFO: Elapsed time: 1.460s
FAILED: Build did NOT complete successfully (14 packages loaded)
cat --number /usr/local/google/home/ensonic/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/third_party/googleapis/google/api/BUILD.bazel
     1	load("@io_bazel_rules_go//go:def.bzl", "go_library")
     2	load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
     3	
     4	proto_library(
     5	    name = "annotations_proto",
     6	    srcs = [
     7	        "annotations.proto",
     8	        "http.proto",
     9	    ],
    10	    visibility = ["//visibility:public"],
    11	    deps = [
    12	        "//google/api:api_proto",
    13	        "@com_google_protobuf//:descriptor_proto",
    14	    ],
    15	)
    16	
    17	go_proto_library(
    18	    name = "annotations_go_proto",
    19	    importpath = "google.golang.org/genproto/googleapis/api/annotations",
    20	    proto = ":annotations_proto",
    21	    visibility = ["//visibility:public"],
    22	    deps = [
    23	        "//google/api:go_default_library",
    24	        "@com_github_golang_protobuf//protoc-gen-go/descriptor:go_default_library",
    25	    ],
    26	)
    27	
    28	go_library(
    29	    name = "go_default_library",
    30	    embed = [":annotations_go_proto"],
    31	    importpath = "google.golang.org/genproto/googleapis/api/annotations",
    32	    visibility = ["//visibility:public"],
    33	)

The dependency on line 23 looks wrong.

@jayconrod
Copy link
Contributor

Unfortunately, this is a known issue with vendored proto deps. See bazelbuild/bazel-gazelle#33. Bazel interprets import paths in protos as being relative to the repository root. This means when a bunch of protos are vendored, they can't import each other. Related Bazel issues: bazelbuild/bazel#3867 and bazelbuild/bazel#4544.

Until that's fixed, there's not much Gazelle can do to help. Gazelle will look for a proto_library that provides sources the given import paths. Not finding any match, it will be do a simple translation of the import path into a label, which is incorrect in this case.

@ensonic
Copy link
Author

ensonic commented Mar 8, 2018

I've been retrying it (with the gazelle part of rules_go and still at 0.9.0) - there is definitely a bug, it now creates a self edge:

bazel build //src/proto/registry:swagger
ERROR: /usr/local/google/home/ensonic/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/protoc-gen-swagger/options/BUILD.bazel:4:1: in proto_library rule @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:options_proto: cycle in dependency graph:
    //src/proto/registry:swagger
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:protoc-gen-swagger (host)
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger:go_default_library (host)
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/genswagger:go_default_library (host)
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:go_default_library (host)
    @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:options_go_proto (host)
.-> @github_com_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options:options_proto (host) [self-edge]
`--
This cycle occurred because of a configuration option

generated BUILD file, see first 'options_proto' rule

load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_grpc_library")

proto_library(
    name = "options_proto",
    srcs = [
        "annotations.proto",
        "openapiv2.proto",
    ],
    visibility = ["//visibility:public"],
    deps = [
        ":options_proto",
        "@com_google_protobuf//:any_proto",
        "@com_google_protobuf//:descriptor_proto",
    ],
)

go_grpc_library(
    name = "options_go_proto",
    importpath = "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options",
    proto = ":options_proto",
    visibility = ["//visibility:public"],
    deps = [
        ":go_default_library",
        "@com_github_golang_protobuf//protoc-gen-go/descriptor:go_default_library",
        "@com_github_golang_protobuf//ptypes/any:go_default_library",
    ],
)

go_library(
    name = "go_default_library",
    embed = [":options_go_proto"],
    importpath = "github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options",
    visibility = ["//visibility:public"],
)

I've tried switching to 0.10.1 with external gazelle again, but still have the problem with vendored deps. I've tried to sync my versions to the vendored version to no avail.

@jayconrod
Copy link
Contributor

The self-import bug was fixed in bazelbuild/bazel-gazelle#52. Upgrading past that should resolve it.

I'm hoping bazelbuild/bazel#4544 will let us fix the vendored proto problem. That will take some time though.

@ensonic
Copy link
Author

ensonic commented Mar 9, 2018

Almost there! Now github_com_grpc_ecosystem_grpc_gateway/third_party/googleapis/google/api/BUILD.bazel contains

oad("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

proto_library(
    name = "annotations_proto",
    srcs = [
        "annotations.proto",
        "http.proto",
    ],
    visibility = ["//visibility:public"],
    deps = [
        "//google/api:api_proto",
        "@com_google_protobuf//:descriptor_proto",
    ],
)

and that yields

ERROR: /usr/local/google/home/ensonic/.cache/bazel/_bazel_ensonic/556f802681b2087c6d9a6bcc3d0d954b/external/github_com_grpc_ecosystem_grpc_gateway/third_party/googleapis/google/api/BUILD.bazel:17:1: no such package '@github_com_grpc_ecosystem_grpc_gateway//google/api': BUILD file not found on package path and referenced by '@github_com_grpc_ecosystem_grpc_gateway//third_party/googleapis/google/api:annotations_go_proto'
ERROR: Analysis of target '//src/proto/registry:proto_service' failed; build aborted: no such package '@github_com_grpc_ecosystem_grpc_gateway//google/api': BUILD file not found on package path
INFO: Elapsed time: 9.523s

This looks like
bazelbuild/bazel-gazelle#33
but that package is not using vendoring, so I am not sure how that applies.

@jayconrod
Copy link
Contributor

This seems like another flavor of the vendored proto problem.

third_party/googleapis/google/api/annotations.proto imports "google/api/http.proto". I think this is meant to refer to http.proto in the same directory, but Bazel (and Gazelle) will interpret this relative to the root of the repository.

In order for this to work correctly, that proto_library rule would need to have an attribute like proto_source_root = "third_party/googleapis", once bazelbuild/bazel#4544 is fixed and Bazel supports that functionality. I don't think Gazelle will be able to figure that out the source root automatically though, so some pre-generated build files might be necessary in this case.

@ensonic
Copy link
Author

ensonic commented Apr 27, 2018

Works now.

@ensonic ensonic closed this as completed Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants