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

v0.24.0 issue with internal package import in third party dependency #1117

Open
ash2k opened this issue Oct 17, 2021 · 4 comments
Open

v0.24.0 issue with internal package import in third party dependency #1117

ash2k opened this issue Oct 17, 2021 · 4 comments

Comments

@ash2k
Copy link
Contributor

ash2k commented Oct 17, 2021

What version of gazelle are you using?

v0.24.0

What version of rules_go are you using?

v0.29.0

What version of Bazel are you using?

4.2.1

Does this issue reproduce with the latest releases of all the above?

Yes.

What operating system and processor architecture are you using?

macOS amd64

What did you do?

Upgraded Gazelle to v0.24.0 from v0.23.0.

What did you expect to see?

All working as before.

What did you see instead?

With v0.24.0:

cd  /private/var/tmp/_bazel_mikhail/6a0c1de97b81a8c5de5b6b9aec394679/external/com_google_cloud_go
cat internal/version/BUILD.bazel
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
    name = "version",
    srcs = ["version.go"],
    importpath = "cloud.google.com/go/internal/version",
    visibility = ["//:__subpackages__"],
)

alias(
    name = "go_default_library",
    actual = ":version",
    visibility = ["//:__subpackages__"],
)

go_test(
    name = "version_test",
    srcs = ["version_test.go"],
    embed = [":version"],
)

With v0.23.0:

cd /private/var/tmp/_bazel_mikhail/6a0c1de97b81a8c5de5b6b9aec394679/external/com_google_cloud_go
cat internal/version/BUILD.bazel
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
    name = "version",
    srcs = ["version.go"],
    importpath = "cloud.google.com/go/internal/version",
    visibility = [
        "//:__subpackages__",
        "@com_google_cloud_go_bigquery//:__subpackages__",
        "@com_google_cloud_go_datastore//:__subpackages__",
        "@com_google_cloud_go_firestore//:__subpackages__",
        "@com_google_cloud_go_monitoring//:__subpackages__",
        "@com_google_cloud_go_profiler//:__subpackages__",
        "@com_google_cloud_go_pubsub//:__subpackages__",
        "@com_google_cloud_go_storage//:__subpackages__",
        "@com_google_cloud_go_trace//:__subpackages__",
    ],
)

alias(
    name = "go_default_library",
    actual = ":version",
    visibility = [
        "//:__subpackages__",
        "@com_google_cloud_go_bigquery//:__subpackages__",
        "@com_google_cloud_go_datastore//:__subpackages__",
        "@com_google_cloud_go_firestore//:__subpackages__",
        "@com_google_cloud_go_monitoring//:__subpackages__",
        "@com_google_cloud_go_profiler//:__subpackages__",
        "@com_google_cloud_go_pubsub//:__subpackages__",
        "@com_google_cloud_go_storage//:__subpackages__",
        "@com_google_cloud_go_trace//:__subpackages__",
    ],
)

go_test(
    name = "version_test",
    srcs = ["version_test.go"],
    embed = [":version"],
)

As you can see, v0.24.0 does not generate visibility configuration for the other dependencies/packages that import the internal package. It's unfortunate but that internal package is imported by other modules in the same repo: https://github.com/googleapis/google-cloud-go/blob/45fd2594d99ef70c776df26866f0a3b537e7e69e/profiler/profiler.go#L52

The error I'm getting is:

ERROR: /Volumes/ramdisk/6a0c1de97b81a8c5de5b6b9aec394679/external/com_google_cloud_go_profiler/BUILD.bazel:3:11: in go_library rule @com_google_cloud_go_profiler//:profiler: alias '@com_google_cloud_go//internal/version:go_default_library' referring to target '@com_google_cloud_go//internal/version:version' is not visible from target '@com_google_cloud_go_profiler//:profiler'. Check the visibility declaration of the former target if you think the dependency is legitimate
ERROR: Analysis of target '//internal/module/google_profiler/server:server' failed; build aborted: Analysis of target '@com_google_cloud_go_profiler//:profiler' failed

external/com_google_cloud_go_profiler/BUILD.bazel looks like this:

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

go_library(
    name = "profiler",
    srcs = [
        "heap.go",
        "mutex.go",
        "profiler.go",
    ],
    importpath = "cloud.google.com/go/profiler",
    visibility = ["//visibility:public"],
    deps = [
        "@com_github_golang_protobuf//proto:go_default_library",
        "@com_github_golang_protobuf//ptypes:go_default_library",
        "@com_github_google_pprof//profile:go_default_library",
        "@com_github_googleapis_gax_go_v2//:go_default_library",
        "@com_google_cloud_go//compute/metadata:go_default_library",
        "@com_google_cloud_go//internal/version:go_default_library",
        "@org_golang_google_api//option:go_default_library",
        "@org_golang_google_api//transport/grpc:go_default_library",
        "@org_golang_google_genproto//googleapis/devtools/cloudprofiler/v2:go_default_library",
        "@org_golang_google_genproto//googleapis/rpc/errdetails:go_default_library",
        "@org_golang_google_grpc//:go_default_library",
        "@org_golang_google_grpc//codes:go_default_library",
        "@org_golang_google_grpc//metadata:go_default_library",
        "@org_golang_google_grpc//status:go_default_library",
    ],
)

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

go_test(
    name = "profiler_test",
    srcs = [
        "heap_test.go",
        "integration_test.go",
        "profiler_example_test.go",
        "profiler_test.go",
    ],
    embed = [":profiler"],
    deps = [
        "//mocks",
        "//testdata",
        "@com_github_golang_mock//gomock:go_default_library",
        "@com_github_golang_protobuf//proto:go_default_library",
        "@com_github_golang_protobuf//ptypes:go_default_library",
        "@com_github_google_pprof//profile:go_default_library",
        "@com_github_googleapis_gax_go_v2//:go_default_library",
        "@com_google_cloud_go//compute/metadata:go_default_library",
        "@com_google_cloud_go//internal/testutil:go_default_library",
        "@org_golang_google_api//option:go_default_library",
        "@org_golang_google_api//transport/grpc:go_default_library",
        "@org_golang_google_genproto//googleapis/devtools/cloudprofiler/v2:go_default_library",
        "@org_golang_google_genproto//googleapis/rpc/errdetails:go_default_library",
        "@org_golang_google_grpc//:go_default_library",
        "@org_golang_google_grpc//codes:go_default_library",
        "@org_golang_google_grpc//metadata:go_default_library",
        "@org_golang_google_grpc//status:go_default_library",
    ] + select({
        "@io_bazel_rules_go//go/platform:android": [
            "//proftest",
            "@org_golang_google_api//compute/v1:go_default_library",
            "@org_golang_x_oauth2//google:go_default_library",
        ],
        "@io_bazel_rules_go//go/platform:linux": [
            "//proftest",
            "@org_golang_google_api//compute/v1:go_default_library",
            "@org_golang_x_oauth2//google:go_default_library",
        ],
        "//conditions:default": [],
    }),
)

What's interesting is that it sometimes works and sometimes gives this error.

@pqn
Copy link

pqn commented Nov 9, 2021

Thanks for creating the issue, we're having a similar problem. Did you ever find a workaround/patch?

@ash2k
Copy link
Contributor Author

ash2k commented Nov 9, 2021

@pqn No, I didn't. I'm using 0.23.0, which works fine.

@robfig
Copy link
Contributor

robfig commented Dec 7, 2021

Cross-module internal packages should be supported. If anyone has the bandwidth to solve this, it would be much appreciated. I would start by trying to add a test case that reproduces the incorrect behavior. Here are some pointers.

This issue sounds similar to these previous issues:
#619
#960
#689

Here is the relevant code that determines visibility:
https://github.com/bazelbuild/bazel-gazelle/blob/master/language/go/generate.go#L660-L698

These two test cases were intended to verify this exact behavior:
https://github.com/bazelbuild/bazel-gazelle/blob/master/cmd/gazelle/integration_test.go#L2994-L3113

I'm not sure what might cause the tests to pass but your usage to fail.

@ash2k
Copy link
Contributor Author

ash2k commented Mar 24, 2022

This comment explains how things work. It explains why it's not deterministic in my case. It's weird that gazelle relies on what it generated before to generate the same thing again correctly.

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

3 participants