golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.79k stars 1.58k forks source link

v1.33.0 breaks bazel build #1611

Closed debkanchan closed 6 months ago

debkanchan commented 6 months ago

What version of protobuf and what language are you using? v1.33.0

What did you do? initialize a bazel+gazelle+go repo (bazel version 7 but using workspace not bzlmod) generate go files from protobuf run bazel run //:gazelle-update-repos-go run bazel run //:gazelle run bazel build //...

What did you expect to see?

$ bazel build //...                                                                                                  
INFO: Analyzed 19 targets (312 packages loaded, 1892 targets configured).
INFO: Found 19 targets...
INFO: Elapsed time: 0.838s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

What did you see instead?

$ bazel build //...                                                                                                  ✹
ERROR: no such package '@@com_google_protobuf//': The repository '@@com_google_protobuf' could not be resolved: Repository '@@com_google_protobuf' is not defined
ERROR: /private/var/tmp/_bazel_debkanchan/bded771e0a6e18fcb92113ecbcaef68f/external/org_golang_google_protobuf/types/gofeaturespb/BUILD.bazel:5:14: no such package '@@com_google_protobuf//': The repository '@@com_google_protobuf' could not be resolved: Repository '@@com_google_protobuf' is not defined and referenced by '@@org_golang_google_protobuf//types/gofeaturespb:gofeaturespb_proto'
ERROR: Analysis of target '//internal/repositories/maps:maps' failed; build aborted: Analysis failed
INFO: Elapsed time: 1.697s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

Make sure you include information that can help us debug (full error message, exception listing, stack trace, logs).

Anything else we should know about your project / environment? I was able to bisect the issue to a specific commit which is f9eb6c7

repo with the issue for easier repro (please don't fork or commit. It's just for debugging)

debkanchan commented 6 months ago

peeking into the generated bazel file it looks like the problem is in the .proto file itself

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

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

go_proto_library(
    name = "gofeaturespb_go_proto",
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    proto = ":gofeaturespb_proto",
    visibility = ["//visibility:public"],
)

go_library(
    name = "gofeaturespb",
    embed = [":gofeaturespb_go_proto"],
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    visibility = ["//visibility:public"],
)

alias(
    name = "go_default_library",
    actual = ":gofeaturespb",
    visibility = ["//visibility:public"],
)
debkanchan commented 6 months ago

Ok this is not just one file. Everywhere there is a .proto file, there is this issue in the build file.

debkanchan commented 6 months ago

I used a commit before the guilty commit and it still produces similar build files for protos but it still works

My intuition is before although similar build rules were generated because no proto files were directly imported in my code it was not a target my build files were depending on hence the build passes

But in that guilty commit the .proto file was moved into the go code directory so the build files are merging into one. So when Bazel is trying to build my code it encounters the proto_library rule causing the build to fail

puellanivis commented 6 months ago

Is this an issue with Bazel or with protoc_gen_go ?

debkanchan commented 6 months ago

protoc-gen-go mostly but also protocol buffers go library. Each component is doing what they should but it ends up creating this error

puellanivis commented 6 months ago

I’m asking because these errors are not originating from protoc-gen-go.

This might however be related to the change we made for https://github.com/golang/protobuf/issues/1608 which moved where this specific file was located.

It’s possible that the bazel file located at /private/var/tmp/_bazel_debkanchan/bded771e0a6e18fcb92113ecbcaef68f/external/org_golang_google_protobuf/types/gofeaturespb/BUILD.bazel still assumes the older filename and/or path (both of which no longer exist). I don’t know where this bazel file actually lives, though, as it’s not a file maintained by us?

debkanchan commented 6 months ago

The Bazel file assumes the correct location. That is not the problem. The problem is since both proto and go file now live in the same directory which causes Bazel to generate a single build file. The build file has a proto_library rule with a dependency to com_google_protobuf. Which causes build to fail.

debkanchan commented 6 months ago

Please also check this issue

lfolger commented 6 months ago

On the flip side, we released the v1.34.0 today which moved the proto and go file into different directories again. Does this help?

I'm not sure if you can build this proto file in general with bazel because of how the dependency on the descriptor protos work. I think what is special here is that this is the first release every that contains a proto file outside of the test protos.

I'm not familiar enough with gazelle/bazel to understand what the best action is here. It might be a general issue about how we layout and import files from the protobuf repository and you might need to adjust the build files by hand. So far it was not our goal that you can build this repository with bazel/gazelle out of the box.

debkanchan commented 6 months ago

Unfortunately it did not help. I got the following error

$ bazel build //...                                                                                                  ✹
zsh: correct '//...' to '//..' [nyae]? n
ERROR: no such package '@@com_google_protobuf//': The repository '@@com_google_protobuf' could not be resolved: Repository '@@com_google_protobuf' is not defined
ERROR: /private/var/tmp/_bazel_debkanchan/bded771e0a6e18fcb92113ecbcaef68f/external/org_golang_google_protobuf/src/google/protobuf/BUILD.bazel:5:14: no such package '@@com_google_protobuf//': The repository '@@com_google_protobuf' could not be resolved: Repository '@@com_google_protobuf' is not defined and referenced by '@@org_golang_google_protobuf//src/google/protobuf:gofeaturespb_proto'
ERROR: Analysis of target '//third-party:third-party' failed; build aborted: Analysis failed
INFO: Elapsed time: 1.009s, Critical Path: 0.04s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

going into the BUILD file it looks like

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

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

go_proto_library(
    name = "gofeaturespb_go_proto",
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    proto = ":gofeaturespb_proto",
    visibility = ["//visibility:public"],
)

go_library(
    name = "gofeaturespb",
    embed = [":gofeaturespb_go_proto"],
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    visibility = ["//visibility:public"],
)

alias(
    name = "go_default_library",
    actual = ":gofeaturespb",
    visibility = ["//visibility:public"],
)

It just moved the error somewhere else.

Changing this build file by hand might not be possible.

lfolger commented 6 months ago

I'm not sure what you are trying to achieve here. go_features.proto is with v1.34 no longer in the types/gofeaturespb directory but under src/google/protobuf. This means the proto_library rule is misplaced there.

Furthermore, that proto depends on the descriptor proto in the protocol buffer (not Go) repository and thus you need to tell bazel how to find that external dependency.

Again this is not a supported use case and I will refrain from engaging further on this issue. The only supported way to build this repository is the go tool. If you want to support other build tools we are happy to accept contributions to fix anything that is fundamentally wrong with the module but this does not mean we accept any contribution that makes the module build out of the box with gazelle/bazel if this means restructuring the module.

debkanchan commented 6 months ago

Firstly the BUILD file mentioned above is not in types/gofeaturespb but along with the proto file has moved to src/google/protobuf so the output of the BUILD file is correct.

Second, You're right @lfolger. I depended on protobuf bazel module directly by adding bazel_dep(name = "protobuf", version = "26.0", repo_name = "com_google_protobuf") in MODULE.bazel. That fixed the original issue.

However since the generated BUILD file also imports rules go it created the following import cycle.

$ bazel build //...
ERROR: /private/var/tmp/_bazel_debkanchan/bded771e0a6e18fcb92113ecbcaef68f/external/org_golang_google_protobuf/cmd/protoc-gen-go/BUILD.bazel:15:10: in go_binary rule @@org_golang_google_protobuf//cmd/protoc-gen-go:protoc-gen-go: cycle in dependency graph:
    //cmd/api-server:_image_index_json (e8f6781f0f0725e0d4d34749386c5dc95f0c24b7d50a283fb1eeef58fb2e30a2)
    //cmd/api-server:_image_index_json (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    //cmd/api-server:image (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    //cmd/api-server:tar_layer (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    //cmd/api-server:api-server (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    //cmd/api-server:api-server_lib (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    //third-party:third-party (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@org_golang_google_api//option:option (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@org_golang_google_api//internal:internal (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@com_github_google_s2a_go//:s2a-go (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@com_github_golang_protobuf//proto:go_default_library (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@com_github_golang_protobuf//proto:proto (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@org_golang_google_protobuf//reflect/protodesc:protodesc (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@org_golang_google_protobuf//types/gofeaturespb:gofeaturespb (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@org_golang_google_protobuf//types/gofeaturespb:gofeaturespb_go_proto (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@io_bazel_rules_go//proto:go_proto (7170974daf0d03f96160241903b2474c2ca6fb0684cc2c015d025084d4129f5a)
    @@io_bazel_rules_go//proto:go_proto_reset_plugin_ (f0f11f9aa5edb12bea15e484f1f24988480676527949682b707eb275031dc7a4)
.-> @@org_golang_google_protobuf//cmd/protoc-gen-go:protoc-gen-go (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@org_golang_google_protobuf//cmd/protoc-gen-go:protoc-gen-go_lib (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@org_golang_google_protobuf//compiler/protogen:protogen (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@org_golang_google_protobuf//reflect/protodesc:protodesc (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@org_golang_google_protobuf//types/gofeaturespb:gofeaturespb (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@org_golang_google_protobuf//types/gofeaturespb:gofeaturespb_go_proto (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@io_bazel_rules_go//proto:go_proto (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
|   @@io_bazel_rules_go//proto:go_proto_reset_plugin_ (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
`-- @@org_golang_google_protobuf//cmd/protoc-gen-go:protoc-gen-go (9332fc02be469cdf56cd60e3c81d9eeaab6b7dd25344f92c78901701d94aa653)
ERROR: Analysis of target '//cmd/api-server:_image_index_json' failed; build aborted
INFO: Elapsed time: 9.455s, Critical Path: 0.81s
INFO: 6 processes: 1 internal, 5 darwin-sandbox.
ERROR: Build did NOT complete successfully

From what I understand, simply put, Directly depending on any proto file as part of the source code (not test protos) will cause bazel build to fail

debkanchan commented 6 months ago

I was able to somewhat figure out the root cause of this.

protobuf-go has file reflect/protodesc/editions.go which imports gofeaturespb "google.golang.org/protobuf/types/gofeaturespb"

It should import types/gofeaturespb in generated BUILD file. But it does not.

Because src/google/protobuf/go_features.proto has go_package option set to "google.golang.org/protobuf/types/gofeaturespb", thus the generated BUILD file looks like the following.

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

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

go_proto_library(
    name = "gofeaturespb_go_proto",
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    proto = ":gofeaturespb_proto",
    visibility = ["//visibility:public"],
)

go_library(
    name = "gofeaturespb",
    embed = [":gofeaturespb_go_proto"],
    importpath = "google.golang.org/protobuf/types/gofeaturespb",
    visibility = ["//visibility:public"],
)

alias(
    name = "go_default_library",
    actual = ":gofeaturespb",
    visibility = ["//visibility:public"],
)

So essentially Bazel is generating a 2 BUILD files that have 2 go_library rules that resolve to same import path of "google.golang.org/protobuf/types/gofeaturespb" and then choosing to import the auto-generated go package from src/google/protobuf directory.

I manually changes the deps path from //src/google/protobuf:gofeaturespb to //types/gofeaturespb and it builds successfully. But that is not reproducible.

As far as I can see the only viable options now are Either somehow remove the proto file or enable bazel in protobuf-go repo and then manually resolve the dependency.

debkanchan commented 6 months ago

Solution found but only works with WORKSPACE. Unable to verify on bzlmod.

Need to add the following line to go_repository rule in go_deps.bzl file where google.golang.org/protobuf import is defined like this

go_deps.bzl

    go_repository(
        name = "org_golang_google_protobuf",
        build_file_proto_mode = "disable_global", # Manually added to fix build. See https://github.com/golang/protobuf/issues/1611
        importpath = "google.golang.org/protobuf",
        sum = "h1:Qo/qEd2RZPCf2nKuorzksSknv0d3ERwp1vFG38gSmH4=",
        version = "v1.34.0",
    )