Bazel problem with opencensus-proto as transitive dependency

briantopping commented 5 years ago

Having a problem declaring opencensus-proto as a transitive dependency in https://github.com/briantopping/freeipa-operator/tree/bazel-transitive-issue (just a Bazel build around the “Create and deploy an app-operator” section at https://github.com/operator-framework/operator-sdk#quick-start). @songy23 thought the at what I have here should work, but it gives the following:

ERROR: Analysis of target '//:freeipa-controller' failed; build aborted: no such package '@com_github_census_instrumentation_opencensus_proto//gen-go/metrics/v1': The repository could not be resolved

Changing the name of this go_repository element to match the previous error changes the error to:

ERROR: /private/var/tmp/_bazel_brian/6d13eea7df44df2d1fc4abebceffdd84/external/com_github_census_instrumentation_opencensus_proto/gen-go/trace/v1/BUILD.bazel:3:1: GoCompile external/com_github_census_instrumentation_opencensus_proto/gen-go/trace/v1/linux_amd64_pure_stripped/go_default_library%/github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1.a failed (Exit 1) sandbox-exec failed: error executing command

Note that the errors indeterminately flip between trace and metrics packages, but it's always the same semantics.

This looks very similar to #105, although Bazel-Go rules and Gazelle have deprecated git_repository since those changes were made.

What I expected

operator-sdk presumably ties into metrics, but as a developer with no direct dependency on opencensus-proto, it's surprising that it's so difficult (I'm five days into this error so far) to get it building under Bazel. I assume this has to do with the combination of it being both a transitive and depending on protobuf.

songy23 commented 5 years ago

I'm less familiar with transitive proto dependencies in Bazel. (We usually recommend Go users to depend on the gen-go files directly.) /cc @g-easy any advice here?

g-easy commented 5 years ago

Where does com_github_census_instrumentation_opencensus_proto come from?

Like you said, WORKSPACE declares io_opencensus_proto.

briantopping commented 5 years ago

I haven't got ~much~ any idea TBH. I generally do a bazel clean --expunge after every change to WORKSPACE...

g-easy commented 5 years ago

I added //gen-go/resource/v1:go_default_library to deps=[...] in $(bazel info output_base)/external/io_opencensus_proto/gen-go/metrics/v1/BUILD.bazel but that file is (AFAICT) autogenerated by go_repository() magic.

With the manual addition, this succeeds:

bazel build @io_opencensus_proto//gen-go/metrics/v1:go_default_library

Now the question is how to properly fix the dep. (sorry, I know very little about golang)

g-easy commented 5 years ago

Ping @jayconrod, any ideas? I don't understand the interplay between Gazelle and go-gen. It looks like metrics/ needs a dependency on resources/ but the autogenerated BUILD file is missing that dep. How do we add it?

briantopping commented 5 years ago

If that's the case @g-easy, I found this yesterday, but didn't know how to use it. It's looks like a pretty recent POC. I didn't know the $(bazel info output_base) magic to get back to the files, but when I do the same thing @apesternikov did here and the build incantation, your build command works.

EDIT: I see that https://github.com/bazelbuild/bazel-gazelle/issues/518 was created for this POC repo and closed by @jayconrod with comments as well.

EDIT 2: Should have tested the outer build though, still the same error. Changes are pushed.

EDIT 3: Combine everything together.. the rename of the go_repository to com_github_census_instrumentation_opencensus_proto AND the patch and everything works. I am not sure what to do at this point...

g-easy commented 5 years ago

I read https://github.com/bazelbuild/bazel-gazelle/issues/498#issuecomment-486387509 but I don't understand what the way forward is.

It looks like a lot of people are running into problems with opencensus-proto + golang + bazel. Should opencensus-proto be doing something different? "Manual" build rules instead of gen-go?

Let's wait a day and see what @jayconrod says.

jayconrod commented 5 years ago

This sounds like it might be the same issue reported in bazelbuild/bazel-gazelle#498? bazelbuild/bazel-gazelle#518 was a dup of that.

go_repository is meant to map Go repositories following go build conventions to Bazel. It fetches them via HTTP, git, or go mod download, then generates build files with Gazelle. By default, Gazelle will also generate proto_library and go_proto_library rules if there are .proto files present, but this can be disabled.

The crux of the issue is that this repository already has some build files. Normally go_repository won't generate build files if it finds BUILD or BUILD.bazel in the repository root directory, but this repository only has build files in the src subdirectory. What's happening is that Gazelle generates a rule for something that imports, for example, github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1. But that import is provided by two targets, @com_github_census_instrumentation_opencensus_proto//src/opencensus/proto/resource/v1:resource_proto_go and @com_github_census_instrumentation_opencensus_proto//gen-go/resource/v1:go_default_library. When Gazelle can't resolve a dependency because of a conflict, it prints a warning and doesn't add an entry to deps. So you'll get a missing strict dependency error during the build.

You also mentioned something about naming go_repository rules. It's easiest if the rule names correspond to the import paths. Dependency resolution describes how Gazelle maps imports to Bazel labels. com_github_census_instrumentation_opencensus_proto would be the best name to use. This follows Bazel convention.

jayconrod commented 5 years ago

It looks like a lot of people are running into problems with opencensus-proto + golang + bazel. Should opencensus-proto be doing something different? "Manual" build rules instead of gen-go?

There are a lot of options here. I think the simplest thing to do to make go_repository work by default would be:

Add a BUILD.bazel file at the repository root. This will prevent go_repository from running Gazelle (same effect as setting build_file_generation = "off"). It's okay if it's empty.
Add BUILD.bazel files in //gen-go/resource/v1 and other directories. You can use These can be generated with Gazelle, but you may need to resolve conflicts with # gazelle:resolve directives in the top-level build file. Alternatively, you can create alias rules that point to the go_proto_library rules in //src.

When Gazelle is run in other repositories, it will assume the existence of com_github_census_instrumentation_opencensus_proto//gen-go/resource/v1:go_default_library and other rules.

bweston92 commented 5 years ago

Any update on a long term fix from opencensus side of things?

bweston92 commented 5 years ago

Managed to get it working but required me to make a local_repository and then fix the build files myself.

g-easy commented 5 years ago

I don't think anyone has the bandwidth to work on this. @bweston92 which approach did you take in your fix?

bweston92 commented 5 years ago

I copied the gen-go folder into third_party/com_github_census_instrumentation_opencensus_proto added a empty WORKSPACE & BUILD file at third_party/com_github_census_instrumentation_opencensus_proto and fixed the deps for each subpackage manually and added the following to my WORKSPACE file.

local_repository(
    name = "com_github_census_instrumentation_opencensus_proto",
    path = "./third_party/com_github_census_instrumentation_opencensus_proto",
)

autrilla commented 5 years ago

fixed the deps for each subpackage manually

@bweston92 could you explain what that entails?

derekperkins commented 5 years ago

cc @whizard

bweston92 commented 5 years ago

My working copy attached. com_github_census_instrumentation_opencensus_proto.zip

rutsky commented 4 years ago

One of the options listed there worked for me. I added to WORKSPACE:

# Workaround for transitive import issues: https://github.com/bazelbuild/bazel-gazelle/issues/669
go_repository(
    name = "io_opencensus_go_contrib_exporter_stackdriver",
    build_directives = [
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/metrics/v1:metrics_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/trace/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/trace/v1:trace_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/stats/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/stats/v1:stats_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/resource/v1:resource_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/agent/common/v1:common_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/agent/metric/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/agent/metric/v1:metric_proto_go",
        "gazelle:resolve go go github.com/census-instrumentation/opencensus-proto/gen-go/agent/trace/v1 @com_github_census_instrumentation_opencensus_proto//opencensus/proto/agent/trace/v1:trace_proto_go",
    ],
    commit = "ab68e2a40809b13b9e06ac2135d2549a6a984d62",
    importpath = "contrib.go.opencensus.io/exporter/stackdriver",
)

# Workaround for transitive import issues: https://github.com/bazelbuild/bazel-gazelle/issues/669
http_archive(
    name = "com_github_census_instrumentation_opencensus_proto",
    strip_prefix = "opencensus-proto-0.2.1/src",
    urls = ["https://github.com/census-instrumentation/opencensus-proto/archive/v0.2.1.tar.gz"],
    sha256 = "bfcefa6093fc2ecdf5c9effea86e6982d0d6f9a5178b17fcf73a62e0f3fb43d0",
)

seh commented 4 years ago

Thank you, @rutsky. I was able to adapt your build_directives attribute to fix the honeycombio/opentelemetry-exporter-go dependency (called "com_github_honeycombio_opentelemetry_exporter_go" by Gazelle, to aid others searching for this solution).

I sure hope census-instrumentation/opencensus-proto#217 or something like this can fix this once and for all.

pcj commented 4 years ago

contour ingress controller also has this transitive dependency via envoy. I was able to compile with the following:

    go_repository(
        name = "com_github_census_instrumentation_opencensus_proto",
        importpath = "github.com/census-instrumentation/opencensus-proto",
        sum = "h1:glEXhBS5PSLLv4IXzLA5yPRVX4bilULVyxxbrfOtDAk=",
        version = "v0.2.1",
        build_extra_args = ["-exclude=src"],  # keep        
    )

nikunjy commented 4 years ago

what @pcj did also worked for me. I tried the solution from @rutsky that also worked. Going with the @pcj cause its shorter

census-instrumentation / opencensus-proto

Bazel problem with opencensus-proto as transitive dependency #200

What I expected