Difference between files and runfiles providers is unclear

[jmillikin-stripe]

Description of the problem / feature request

Given the following three files (BUILD, rule.bzl, the_test.sh) I would expect the output of the test to include the generated data file. However, the output does not -- it seems like files included in the DefaultInfo provider of a Skylark rule do not get used in the sh_test data or runfiles sets.

@dslomov's comment on an ancient issue from 2016 (#1305 (comment)) implies this used to work, or at least is expected to work. However, it doesn't seem to in contemporary Bazel versions.

### BUILD
load(":rule.bzl", "custom_rule")
custom_rule(name="the_data")
sh_test(
    name = "the_test",
    srcs = ["the_test.sh"],
    data = [":the_data"],
)

### rule.bzl
def _impl(ctx):
  out = ctx.actions.declare_file(ctx.label.name + "/out_file")
  ctx.actions.write(out, "data")
  return [DefaultInfo(files=depset([out]))]

custom_rule = rule(implementation=_impl)

### the_test.sh
#!/bin/bash
find .
exit 1

$ bazel test //:the_test --test_output=errors
INFO: Analysed target //:the_test (0 packages loaded).
INFO: Found 1 test target...
FAIL: //:the_test (see /private/var/tmp/_bazel_jmillikin/21ae14480478de0e5d7efd7aa0b3e856/execroot/__main__/bazel-out/darwin-fastbuild/testlogs/the_test/test.log)
INFO: From Testing //:the_test:
==================== Test output for //:the_test:
.
./the_test
./the_test.sh
================================================================================

What operating system are you running Bazel on?

MacOS

What's the output of bazel info release?

release 0.9.0-homebrew

[sergiocampama]

Shouldn't these be propagated using the runfiles provider?

return struct(
      runfiles=ctx.runfiles(
          files=depset([out]),
      ),
  )

[benjaminp]

I think this has always been the behavior. At least, I can't see that this behavior has changed recently looking at the code.

[laurentlb]

cc @buchgr

[szakmary]

Still seems to be relevant in version 3.4.0.

[JaredNeil]

This keeps biting me. It seems very strange to me, that a rule that generates a generic file has to provide it as both files and runfiles, just in the off-chance that sometime later a sh_binary might want to use a target of that rule type as a data input. I keep having to go edit upstream rules to provide runfiles even though they aren't generating a binary, which just seems wrong.
It also affects py_binary, which is where I usually run into it.

[brandjon]

For the difference between "files" ("FilesToBuild" in the Java code) and "runfiles", see this comment. Perhaps we could elevate that information to documentation, if not already present. I'll admit, I was confused at first by the example.

One could also interpret this issue as a feature request to treat the runfiles as being, by default if not specified, the same as the files.

[q3k]

We just also hit this (target using a custom rule outputting DefaultInfo(files=...) that could not be consumed by an sh_test specifying that target in its data). The documentation on this is a bit sparse and the behavior is quite confusing at first.

I think the confusion mostly comes from an assumption that runfiles in DefaultInfo means “anything that files=[...] will need if it's ever ran” - or at least that was my intepretation of it, and the documentation didn't contradict me on that. As in, if you output a binary foo.elf which wants to access some runtime stuff, foo.elf should live in files= and all its runtime deps should live in runfiles=. I've blissfully lived in this wrong interpretation for years.

In reality, runfiles= seems to mean “anything that is required to run any other target that depends on this target as its runfiles, including the actual output itself, as non-runfiles will not be included”. Or, in other words, that runfiles is used when the target is depended on as runfiles/data, while files is used when the target is used as a build input. Which is a fair explanation, but should IMO be more explicitly stated.

In addition, maybe some code changes could be made to make this a bit more obvious - maybe if a target's output is directly included as the runfiles of another target, Bazel should warn when there is a direct inclusion of an output with empty DefaultInfo.runfiles, and that is probably not what the user meant? Defaulting to runfiles=files if not set otherwise is tempting, but I'd be afraid of breakage this would cause. And it would probably muddle the distinction even further.

[uri-canva]

#15052 changes the behaviour to reflect the understanding that a lot of people had, that runfiles are additional files needed at runtime by the files declared as regular outputs, so files that are declared as default outputs shouldn't be declared in runfiles.

[fmeum]

As @uri-canva explained, this has been fixed by #15052, which is in Bazel 6. Just a nitpick: "... files that are declared as default outputs shouldn't be declared in runfiles." should be "... files that are declared as default outputs don't have to be declared in runfiles."

@sgowroji Could you close this issue as completed?

[sgowroji]

We are closing this issue w.r.t above comment. Feel free to reach us for any further queries. Thanks !