Better diagnostics for when empty digests / captures occur for adhoc_tool, shell_command, and friends

[tdyas]

Background

The Problem

Pants does not error or warn when output digests captured by the shell backend or produced by the "wrap as" target types (e.g., experimental_wrap_as_resources) are empty. This can occur, for example, because a file capture glob failed to capture a missing file for adhoc_tool or shell_command or the user specified an incorrect glob for the outputs field of a "wrap as" target type which was then not matched against the inputs.

Motivation

As reported in #21156, a user recently encountered an issue where a file which they expected to be in the execution sandbox was missing. In their reproduction, the output digest of the relevant shell_command target was the path cpp/hello_cpp (but was captured via the glob "hello_cpp" given the shell_command's working directory was cpp).

The user then tried to use an experimental_wrap_as_resources target to convert that output into a resources target for consumption by a python_test target. The outputs glob was specified as hello_cpp and not cpp/hello_cpp since the user was not aware of how the working directory support on shell_command changes where the output is captured. This incorrect glob caused the experimental_wrap_as_resources to produce an empty digest as output.

Pants did not provide any diagnostics to the user about this case. Arguably, Pants probably should provide some error or warning when globs do not match in these cases since the user clearly expected something to be there. Not doing so results in "silent failures" which are hard to debug if you are not a Pants maintainer. (Indeed, I had to add debug logging to rule code and run from Pants sources in order to track down the issue.)

Ideas

Some ideas for better diagnostics:

1. For adhoc_tool and shell_command target types, Pants should by default verify that all output_files and output_directories globs in fact match captured files / directories from the process execution.

• Add an outputs_match_error_behavior field on adhoc_tool and shell_command which takes the values error, warn, ignore to set what diagnostics occur if globs do not match. error is the default.

• Add an outputs_match_mode field on adhoc_tool and shell_command which takes the values any and all to set whether at least one glob needs to match or whether all globs need to match. all is the default.

2. For "wrap as" target types (e.g., experimental_wrap_as_resources), Pants should by default verify that the outputs globs in fact match files which produced by the input targets.

• Add an outputs_match_error_behavior field on adhoc_tool and shell_command which takes the values error, warn, ignore to set what diagnostics occur if globs do not match. error is the default.
• Add an outputs_match_mode field on adhoc_tool and shell_command which takes the values any and all to set whether at least one glob needs to match or whether all globs need to match. all is the default.

3. Should there be global options to set the defaults for outputs_match_error_behavior and outputs_match_mode? Or just rely on __defaults__ mechanism?
4. Are there better names for these?
5. What other ideas may help for diagnostics?

• Maybe Pants should log the files in the digest involved in adhoc_tool, shell_command, and "wrap_as" targets at debug level?

[huonw]

Better diagnostics is always better; your proposals about flags to control the warning behaviour on the relevant targets sounds good to me.

Related prior example: #18183

Add an outputs_match_mode field on adhoc_tool and shell_command which takes the values any and all to set whether at least one glob needs to match or whether all globs need to match. all is the default.

I wonder whether we could get away with (implicitly) all mode, and use outputs_match_error_behavior to control: I can't imagine people would want any behaviour very often? Thus, just using outputs_match_error_behavior="ignore" in that case seems reasonable?

Should there be global options to set the defaults for outputs_match_error_behavior and outputs_match_mode? Or just rely on defaults mechanism?

Starting with __defaults__ sounds reasonable to me.

Maybe Pants should log the files in the digest involved in adhoc_tool, shell_command, and "wrap_as" targets at debug level?

Sounds like that would help debugging 👍 I could imagine we might want to not print the full list in case its very large, but at least enough to give a sense of what's matching and what's not.