Skip to content

Commit

Permalink
Fixing Transitive Runfiles
Browse files Browse the repository at this point in the history
Updating the call to `ctx.runfiles()` in the `bats_test()` impl function.
Instead of setting `transitive_files` to the `files` of `ctx.attr.data` and `ctx.attr.dep`, this PR uses `default_runfiles.files` for `deps`.
The effect is that all runfiles (not only the immediate outputs) of `deps` will be copied into the `bats_test` target's runfiles.

The existing issue can be shown when a `bats_test()` target's `dep` target contains `data`, it does not get copied over.
Consider:
```python
cc_binary(
  name = "tool",
  srcs = "main.cc",
  data = ["tool_config.txt"],
)
bats_test(
  name = "tool_test",
  deps = [":tool"],
)
```

In this example, the `bats_test` needs the `:tool` target (to run tests against the binary).
With the current implementation, it will only get the binary.
The `data` (_"tool_config.txt"_) will not be copied over into the test runfiles.
This is not only inconsistent (since running `:tool` would always have the file present), but it may (depending on the binary's functionality) cause the binary to no longer run correctly, as it depends on that file to be present.
`data` files are also missing from any further transitive dependencies further in the build DAG.

This issue is also present with `sh_binary` for both `data` and `deps`, due to both attributes being treated as the same.
This causes any transitive scripts (i.e. `deps`) to be missing in the `bats_test` runfiles.

This PR fixes this by gathering all runfiles (which includes `deps` and `data` + transitives) for each dep defined.
This PR does not modify how `data` is processed, as `data` is moreso intended to explicitly list files (or `filegroups`) instead of targets (which would have their own `deps`/`data`).
This distinction is merely a semantic one and `data` could also be processed in the same way in the future, if desired.
  • Loading branch information
Your Name authored and filmil committed Oct 28, 2023
1 parent dfe3644 commit ccc49bb
Showing 1 changed file with 4 additions and 1 deletion.
5 changes: 4 additions & 1 deletion rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,12 @@ def _bats_test_impl(ctx):
content = content,
)

dep_transitive_files = []
for dep in ctx.attr.deps:
dep_transitive_files.extend(dep.default_runfiles.files.to_list())
runfiles = ctx.runfiles(
files = ctx.files.srcs,
transitive_files = depset(ctx.files.data + ctx.files.deps),
transitive_files = depset(ctx.files.data + dep_transitive_files),
).merge(ctx.attr._bats.default_runfiles)
return [DefaultInfo(runfiles = runfiles)]

Expand Down

0 comments on commit ccc49bb

Please sign in to comment.