expanding tree artifacts that are not inputs of the executing action should be an error

[benjaminp]

The DirectoryExpander interface allows Args map functions to expand directory artifacts arbitrarily nested in custom data structures. If map function expands a tree artfiact that is not declared as an input of the executing action, an empty list will be silently returned. This is demonstrated by the following example:

$ cat example.bzl
def _do_expand(arg, expander):
    return [f.path for f in expander.expand(arg.tree)]

def _example(ctx):
    tree = ctx.actions.declare_directory("dir")
    ctx.actions.run_shell(command="touch $1/a $1/b", arguments=[tree.path], outputs=[tree])
    args = ctx.actions.args()
    args.add_all([struct(tree=tree)], map_each=_do_expand)
    out = ctx.actions.declare_file(ctx.label.name)
    ctx.actions.run_shell(command="echo $@ > " + out.path, arguments=[args], outputs=[out])
    return [DefaultInfo(executable=out)]

example = rule(
    implementation=_example,
)
$ cat BUILD
load(":example.bzl", "example")

example(name = "bug")
$ bazel build //:bug && cat bazel-bin/bug

$

By adding the tree artifact to the expanding action's inputs, the example behaves as expected:

$ patch -p1
--- a/example.bzl
+++ b/example.bzl
@@ -7,7 +7,7 @@
     args = ctx.actions.args()
     args.add_all([struct(tree=tree)], map_each=_do_expand)
     out = ctx.actions.declare_file(ctx.label.name)
-    ctx.actions.run_shell(command="echo $@ > " + out.path, arguments=[args], outputs=[out])
+    ctx.actions.run_shell(command="echo $@ > " + out.path, arguments=[args], outputs=[out], inputs=[tree])
     return [DefaultInfo(executable=out)]
 
 example = rule(
$ bazel build //:bug && cat bazel-bin/bug
bazel-out/k8-fastbuild/bin/dir/a bazel-out/k8-fastbuild/bin/dir/b

It would be useful to flag the first case with an error.

[benjaminp]

b7590a0 gives filesets but not tree artifacts the requested behavior.

[jkrems]

I'd like to add that even when they are inputs to the action, it sometimes still fails (maybe something to do with param files?).

It may be safer to make it required to provide the inputs to expand explicitly as part of args.add_all so the association isn't lost.

[DhanalakshmiDurairaj]

Hi
Greetings to all
list DirectoryExpander.expand(file)

Please help me to get an understanding on DirectoryExpander API which is specified in https://bazel.build/rules/lib/builtins/DirectoryExpander.

How to load this API, to expand directory and get list of files. I have to expand directory and get all the files inside the directory into List variable. Pls help me on this.

Thanks & Regards,
D.Dhanalakshmi

[comius]

Isn’t the same true for regular files? That is, you can add a file to args and not inputs and there won’t be any warning/error?

As there is information and Bazel could provide a message I think this would be an improvement.

[benjaminp]

Isn’t the same true for regular files? That is, you can add a file to args and not inputs and there won’t be any warning/error?

Yes, but at least the path for normal artifacts still goes into the command line. Non-input tree artifacts just expand to nothing, which seems harder in practice to debug than some spawn seeing ENOENT.

[jkrems]

Isn’t the same true for regular files? That is, you can add a file to args and not inputs and there won’t be any warning/error?

I'll also add that it is a feature that it's allowed for regular files. Being able to reference the file paths without creating a dependency on (potentially expensive) other actions is very useful. In some of the rules we maintain I'd go as far as it's a fundamental assumption of our rule designs.

Tree artifacts are special because they are the only case where not having them as inputs makes the file path silently disappear. It's a hack that we have to allow the action to read those "inputs" just to resolve the arguments properly.

[fmeum]

Tree artifacts are special because they are the only case where not having them as inputs makes the file path silently disappear. It's a hack that we have to allow the action to read those "inputs" just to resolve the arguments properly.

You should be able to add their paths without having them as inputs with args.add_all([tree_artifact], expand_directories = False). With DirectoryExpander, you should be able to choose not to expand them.

[jkrems]

You should be able to add their paths without having them as inputs [...]

While that is true, it does make tree artifacts "viral" in the tool chain: Anything interacting with file paths now needs to be able to interact with file paths AND file path prefixes. Which introduced a lot of incidental complexity in practice.

To be clear, I'm not complaining about the tree artifacts being required as dependencies of the action. It's a natural consequence of what the directory expansion has to do. The silent failure when they are missing is the problem because it leads to hard-to-debug issues.

It's also a bit awkward that there's no separation between "list of files required for expanding args" and "list of files needed to be staged for reading by the action". But that's a separate issue and not a pressing one (for us at least).