bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.36k stars 635 forks source link

Nogo: cycle in dependency graph when building `genquery` targets #3883

Closed Illedran closed 2 months ago

Illedran commented 3 months ago

What version of rules_go are you using?

v0.46.0

What version of gazelle are you using?

No gazelle.

What version of Bazel are you using?

7.0.2

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

Yes (v0.46.0 and 7.0.2 are latest as of time of writing).

What operating system and processor architecture are you using?

$ uname -srm
Linux 5.15.146.1-microsoft-standard-WSL2 x86_64

Any other potentially useful information about your toolchain?

No bzlmod.

What did you do?

When having nogo enabled, attempting to build a genquery whose expression references go_* rules fails due to a cycle in dependency graph.

WORKSPACE

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "io_bazel_rules_go",
    sha256 = "80a98277ad1311dacd837f9b16db62887702e9f1d1c4c9f796d0121a46c8e184",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip",
        "https://github.com/bazelbuild/rules_go/releases/download/v0.46.0/rules_go-v0.46.0.zip",
    ],
)

load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")

go_rules_dependencies()

go_register_toolchains(version = "1.22.1", nogo = "@io_bazel_rules_go//:tools_nogo")

src/main.go

package main

import "fmt"

func main() {
    fmt.Println("Hello world!")
}

src/BUILD.bazel

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

go_library(
    name = "src_lib",
    srcs = ["main.go"],
)

go_binary(
    name = "src",
    embed = [":src_lib"],
)

genquery(
    name = "src_genquery",
    expression = "deps(//src)",
    scope = [":src"],
)
$ bazel build src:src_genquery
ERROR: /home/illedran/.cache/bazel/_bazel_illedran/a3b41e2226cf2286c26d4e18a5d6a010/external/io_bazel_rules_go/BUILD.bazel:71:16: in go_context_data rule @@io_bazel_rules_go//:go_context_data: cycle in dependency graph:
    //src:src_genquery (c996bfa92bcafc2798baf4dbf854da936ce6f49485d19b67b34b429667416404)
    //src:src_genquery (312a038fde71b14d7680a2d5856ddfa0ee2fb821b1f2d44cc703454abe09284c)
    //src:src
.-> @@io_bazel_rules_go//:go_context_data
|   @@io_bazel_rules_nogo//:nogo
|   @@io_bazel_rules_go//:tools_nogo
|   @@io_bazel_rules_go//:tools_nogo_actual
|   @@io_bazel_rules_go//go/tools/builders:nogo_srcs
|   @@org_golang_x_tools//internal/facts:facts
`-- @@io_bazel_rules_go//:go_context_data
ERROR: Analysis of target '//src:src_genquery' failed; build aborted

Same example as above is also available at https://github.com/Illedran/nogo_genquery_bug.

fmeum commented 3 months ago

I think that we can fix this if we decide to drop compatibility with Bazel 5, see https://github.com/bazelbuild/rules_go/blob/d45e218910fbecb00867e3a4dde3fe14d4a1e00a/go/private/BUILD.bazel#L179-L180

@tyler-french @linzhp What do you think, are we ready for this?

linzhp commented 3 months ago

We ran into this issue when upgrading to rules_go 0.29 in Nov 2021, before Bazel 6 was available. The solution was to create an aspect instead.

I am afraid it's still too early to drop compatibility with Bazel 6, when Bazel 7 was only 3 month old.

fmeum commented 3 months ago

@linzhp Sorry I meant dropping support for Bazel 5, i.e., exclusively supporting Bazel 6 and higher (edited my comment above)

linzhp commented 3 months ago

That sounds more reasonable. I am fine with dropping Bazel 5 support.

philipuvarov commented 2 months ago

@linzhp @fmeum Hello! Running into the same issue here. Is there a known work-around for that? Right now, it, unfortunately, blocks adoption of the nogo rules in my project

fmeum commented 2 months ago

I looked into this a bit more and it turns out to be a known Bazel issue (https://github.com/bazelbuild/bazel/issues/14169). I have pinged it and will try to work on a fix.

The recommended workarounds include using an aspect instead or running bazel query outside of Bazel. As this is not something we can work around on the rules_go side, I will close the rules_go issue.