Runfile in go/tools/bazel does not work with binaries

[filipesilva]

Binaries can have their runfiles, but calling bazel.Runfile(file) will return an error about a missing TEST_SRCDIR variable.

To reproduce follow:

git clone https://github.com/filipesilva/rules_go_runfiles_binary
cd rules_go_runfiles_binary
bazel run :binary

You should see

environment variable "TEST_SRCDIR" is not defined, are you running with bazel test

[filipesilva]

I made an attempt to fix this bug but didn't get very far. But I can add some details to what should be a fix, based on what we did in https://github.com/bazelbuild/rules_nodejs.

A macro should be used that wrapps the go_binary executable in a native.sh_binary so that it works on Windows:
https://github.com/bazelbuild/rules_nodejs/blob/ee6f2e3ff1d5806ae864d31d28a7daa99057770f/internal/node/node.bzl#L328-L359

The native.sh_binary uses a .sh script declared in the rule implementation:
https://github.com/bazelbuild/rules_nodejs/blob/ee6f2e3ff1d5806ae864d31d28a7daa99057770f/internal/node/node.bzl#L127-L142

This shell script should import the official runfiles loading script, which will declare the RUNFILES_DIR, RUNFILES_MANIFEST_FILE and others:
https://github.com/bazelbuild/rules_nodejs/blob/ee6f2e3ff1d5806ae864d31d28a7daa99057770f/internal/node/node_launcher.sh#L18-L42

After using the runfiles loading script, it should call the real go binary with all arguments.

The current runfiles resolver should pick up the now declared environment variables:
https://github.com/bazelbuild/rules_go/blob/176c51a40576f040bff6fa24308f91b5b07adde9/go/tools/bazel/runfiles.go#L37-L54

[filipesilva]

I was talking to @laszlocsomor and it looks like the approach I suggested above is not the best one.

Instead, it should be possible to implement the Go runfiles helper without having access to the RUNFILES_* environment variables.

This is what the C++ runfiles helper does:

• https://github.com/bazelbuild/bazel/tree/f75fd8c6bacc4feeeb232a9f29d6343262de48f4/tools/cpp/runfiles
• https://github.com/bazelbuild/bazel/blob/f75fd8c6bacc4feeeb232a9f29d6343262de48f4/tools/cpp/runfiles/runfiles_src.h#L17

[laszlocsomor]

FYI, I have a Go implementation of the runfiles library that mimics the C++ runfiles library's semantics. If anyone is interested, or wanted to add it to @bazel_tools//tools/go/runfiles, the code is here: laszlocsomor/bazel@3db7cf9

[jayconrod]

@laszlocsomor The Go runfile handling library is currently at @io_bazel_rules_go//go/tools/bazel:go_default_library. A PR adding any missing functionality there would be welcome.

Do you think it makes sense to move runfiles functionality to @bazel_tools//tools/go/runfiles? That would introduce a soft dependency from Bazel to rules_go.

My preference would be for Bazel to provide a more official specification on how programs should access runfiles. I couldn't find documentation for the environment variables or the manifest format.

[laszlocsomor]

@jayconrod : That's a good argument about the soft dependency. Having the Go runfiles library in rules_go sounds better than having it in bazel_tools.

There's no documentation on how to implement a runfiles library for a new language. I can explain stuff if anyone's interested and has concrete questions, but in any case the C++ and Java runfiles libraries are good examples of how it's done.