search

bazelbuild / bazel-gazelle

Gazelle is a Bazel build file generator for Bazel projects. It natively supports Go and protobuf, and it may be extended to support new languages and custom rule sets.
Apache License 2.0
1.18k stars 374 forks source link

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

Open ash2k opened 2 years ago

ash2k commented 2 years ago

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 commented 2 years ago

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

ash2k commented 2 years ago

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

robfig commented 2 years ago

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: https://github.com/bazelbuild/bazel-gazelle/issues/619 https://github.com/bazelbuild/bazel-gazelle/issues/960 https://github.com/bazelbuild/bazel-gazelle/issues/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 commented 2 years ago

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.