rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

go_test: could not use x_defs to stamp rlocationpath of another binary in data

Open sluongng opened this issue 1 year ago • 7 comments

What version of rules_go are you using?

0.44.2

What version of gazelle are you using?

0.35.0

What version of Bazel are you using?

7.0.2

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

yes

What operating system and processor architecture are you using?

MacOS ARM64

Any other potentially useful information about your toolchain?

Irrelevant

What did you do?

Here is a minimal reproduce https://github.com/sluongng/rules-go-test-xdefs-issue/blob/9360ec82c4f307d0e986f36580b2b90cb6760f1c/BUILD#L10-L12

What did you expect to see?

The x_defs should detect the runfile locations of the target and add it to the Go variable

What did you see instead?

ERROR: /Users/sluongng/work/misc/test-go/BUILD:6:8: in go_test rule //:a_test: label '//liba:liba' in $(location) expression is not a declared prerequisite of this rule
ERROR: /Users/sluongng/work/misc/test-go/BUILD:6:8: Analysis of target '//:a_test' failed

sluongng avatar Feb 05 '24 16:02 sluongng

I think this is a go_test issue and not a Bazel issue because similar x_defs on a go_library target would work.

sluongng avatar Feb 05 '24 17:02 sluongng

It's not clear to me why the test in https://github.com/bazelbuild/rules_go/commit/f5da3bebc4c655d026a5e93587f35a16145f3379 doesn't catch this. Could you try to make a minimal change to it that triggers the issue?

fmeum avatar Feb 06 '24 11:02 fmeum

Let's see https://github.com/bazelbuild/rules_go/pull/3848

sluongng avatar Feb 06 '24 12:02 sluongng

I could reproduce with this diff

diff --git a/tests/core/go_test/x_defs/BUILD.bazel b/tests/core/go_test/x_defs/BUILD.bazel
index 7edc4885..9111d581 100644
--- a/tests/core/go_test/x_defs/BUILD.bazel
+++ b/tests/core/go_test/x_defs/BUILD.bazel
@@ -59,12 +59,21 @@ go_library(
     },
 )

+sh_binary(
+    name = "my_bin",
+    srcs = ["my_bin.sh"],
+)
+
 go_test(
     name = "x_defs_test",
     srcs = ["x_defs_test.go"],
-    data = ["x_defs_test.go"],
+    data = [
+        "x_defs_test.go",
+        ":my_bin",
+    ],
     x_defs = {
         "BinGo": "$(rlocationpath x_defs_test.go)",
+        "temp": "$(rlocationpath :my_bin)",
     },
     deps = [
         ":x_defs_lib",

Perhaps it's only trigger if the input was a label? 🤔

sluongng avatar Feb 06 '24 12:02 sluongng

with

diff --git a/go/private/context.bzl b/go/private/context.bzl
index 38e1d673..1d1bb4ee 100644
--- a/go/private/context.bzl
+++ b/go/private/context.bzl
@@ -291,6 +291,7 @@ def _library_to_source(go, attr, library, coverage_instrumented):
         if "." not in k:
             k = "{}.{}".format(library.importmap, k)
         x_defs[k] = v
+    print("DEBUG: {}".format(x_defs))
     source["x_defs"] = x_defs
     if not source["cgo"]:
         for k in ("cdeps", "cppopts", "copts", "cxxopts", "clinkopts"):

I got

DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"github.com/bazelbuild/rules_go/tests/core/go_test/x_defs/x_defs_lib.LibGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_lib.go"}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"tests/core/go_test/x_defs/x_defs_test.BinGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_test.go", "tests/core/go_test/x_defs/x_defs_test.temp": "$(rlocationpath :my_bin)"}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"tests/core/go_test/x_defs/x_defs_test_test.BinGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_test.go", "tests/core/go_test/x_defs/x_defs_test_test.temp": "$(rlocationpath :my_bin)"}

So I think ctx.expand_location was not working as expected?

sluongng avatar Feb 06 '24 12:02 sluongng

I updated the PR with my latest setup

> bazel shutdown
> bazel test tests/core/go_test/x_defs:x_defs_test
Starting local Bazel server and connecting to it...
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"github.com/bazelbuild/rules_go/tests/core/go_test/x_defs/x_defs_lib.LibGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_lib.go", "github.com/bazelbuild/rules_go/tests/core/go_test/x_defs/x_defs_lib.temp": "io_bazel_rules_go/tests/core/go_test/x_defs/blah.txt"}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"tests/core/go_test/x_defs/x_defs_test.BinGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_test.go", "tests/core/go_test/x_defs/x_defs_test.temp": "io_bazel_rules_go/tests/core/go_test/x_defs/blah.txt"}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {"tests/core/go_test/x_defs/x_defs_test_test.BinGo": "io_bazel_rules_go/tests/core/go_test/x_defs/x_defs_test.go", "tests/core/go_test/x_defs/x_defs_test_test.temp": "$(rlocationpath blah.txt)"}
DEBUG: /Users/sluongng/work/bazelbuild/rules_go/go/private/context.bzl:294:10: DEBUG: {}
ERROR: /Users/sluongng/work/bazelbuild/rules_go/tests/core/go_test/x_defs/BUILD.bazel:72:8: in go_test rule //tests/core/go_test/x_defs:x_defs_test: label '//tests/core/go_test/x_defs:blah.txt' in $(location) expression is not a declared prerequisite of this rule
ERROR: /Users/sluongng/work/bazelbuild/rules_go/tests/core/go_test/x_defs/BUILD.bazel:72:8: Analysis of target '//tests/core/go_test/x_defs:x_defs_test' failed
ERROR: Analysis of target '//tests/core/go_test/x_defs:x_defs_test' failed; build aborted
...

So it looks like the expand_location worked in go_library and not in go_test on a data label target. I wonder if the data label was not included in the final providers of go_test somehow 🤔

sluongng avatar Feb 06 '24 12:02 sluongng

Ok I figured out. We do not pass the full context to external compile action but only a subset of context. That subset context was missing data attribute.

Changed the PR to a fix.

sluongng avatar Feb 06 '24 13:02 sluongng