Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Bazel problem with opencensus-proto as transitive dependency #200

Open
briantopping opened this issue May 9, 2019 · 20 comments · May be fixed by #217
Open

Bazel problem with opencensus-proto as transitive dependency #200

briantopping opened this issue May 9, 2019 · 20 comments · May be fixed by #217

Comments

@briantopping
Copy link

briantopping commented May 9, 2019

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
Copy link
Contributor

songy23 commented May 9, 2019

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
Copy link
Contributor

g-easy commented May 10, 2019

Where does com_github_census_instrumentation_opencensus_proto come from?

Like you said, WORKSPACE declares io_opencensus_proto.

@briantopping
Copy link
Author

briantopping commented May 10, 2019

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

@g-easy
Copy link
Contributor

g-easy commented May 10, 2019

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
Copy link
Contributor

g-easy commented May 10, 2019

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
Copy link
Author

briantopping commented May 10, 2019

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 bazelbuild/bazel-gazelle#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
Copy link
Contributor

g-easy commented May 10, 2019

I read bazelbuild/bazel-gazelle#498 (comment) 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
Copy link

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
Copy link

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
Copy link

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

@bweston92
Copy link

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

@g-easy
Copy link
Contributor

g-easy commented Sep 4, 2019

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

@bweston92
Copy link

bweston92 commented Sep 4, 2019

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
Copy link

autrilla commented Oct 9, 2019

fixed the deps for each subpackage manually

@bweston92 could you explain what that entails?

@derekperkins
Copy link

cc @whizard

@bweston92
Copy link

My working copy attached.
com_github_census_instrumentation_opencensus_proto.zip

@rutsky
Copy link

rutsky commented Mar 16, 2020

Related: bazelbuild/bazel-gazelle#669

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",
)

@ribrdb ribrdb linked a pull request Mar 18, 2020 that will close this issue
@seh
Copy link

seh commented Mar 30, 2020

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 #217 or something like this can fix this once and for all.

@pcj
Copy link

pcj commented May 1, 2020

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
Copy link

nikunjy commented Aug 27, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.