`experimental_shell_command` with non-matching `output_files` leads to weird downstream errors

[huonw]

Describe the bug

When someone specifies explicit output_files or output_directories in experimental_shell_command (esc) that don't match, this is probably an error in the specification of esc (either command or outputs), but this isn't directly caught, and may only pop-up as downstream errors and/or confusion.

Example: https://gist.github.com/huonw/c95933eeb581cdb669324d12a0edd3b2

experimental_shell_command(
    name="missing-output",
    command="true",
    tools=[],
    output_files=["doesnt-exist.txt"]
)
archive(name="archive", files=[":missing-output"], format="zip")
experimental_run_shell_command(
    name="run",
    command="ls -l $CHROOT",
    execution_dependencies=[":missing-output"],
)

Running

git clone https://gist.github.com/huonw/c95933eeb581cdb669324d12a0edd3b2
cd c95933eeb581cdb669324d12a0edd3b2

pants version 
#> ...
#> 2.16.0.dev6

# packaging fails with a fairly obtuse error
pants package //:archive
#> ...
#> ProcessExecutionFailure: Process 'Create archive.zip' failed with exit code 12.
#> stdout:
#>
#> zip error: Nothing to do! (archive.zip)
#> ...

# running doesn't have the expected files (something more interesting than `ls` would likely 'explode')
pants run //:run
#> total 0

Some ideas (for cacheable esc, where filesystem output is all that's relevant, this may not apply for non-cacheable ones, if/when they exist), all the examples are against output_files=["foo.txt", "*.log"]:

1. be loose: at least one element of output_files and output_directories matches (so that the output is non-empty). For example: just having bar.log would be okay.
2. be strict: and require that every element of output_files and output_directories match at least one element. For example: having both foo.txt and bar.log would be okay.
3. something in the middle, like requiring that non-globs match, but globs can match zero. For example: just having foo.txt would be okay.

The same presumably applies to output_directories but there's an additional question: is having only an empty directory acceptable? (For example: output_directories=["dir/"], output_files=[] and dir/ is empty.)

Pants version
2.16.0.dev6

OS
macOS

Additional info
N/A

[stuhood]

Yea, this would be great to fix before stabilization, because defaulting to an error would be good.

[chrisjrn]

There's some tension here in what Pants does under the hood: setting output_directories has Pants ensure that an output directory is present before the process runs, so a process will always finish with each output directory present in the digest.

That fact makes (1) and (2) somewhat moot with respect to output_directories, unless we also enforce that an output directory be non-empty, which is probably fine in 99% of cases, but will probably annoy someone with a highly pathological use case.

I think a reasonable design would be to verify that each non-glob value from output_files is matched. Thoughts?

[huonw]

Ah, I see.

I think a reasonable design would be to verify that each non-glob value from output_files is matched. Thoughts?

That sounds reasonable.

Another option would be a flag strict_output_checks_bikeshed_name_here that defaults to True. True means full strictness (including checking globs match, and directories have contents), but can be switched to False if the user is opting in to a pathological case.