commit fad21dae0b01d5f9b2274542c89f4c8163c2ff36
author Googler <noreply@google.com> Fri Oct 01 15:53:33 2021 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Oct 01 15:54:50 2021 -0700
tree 1316e36f988c7bbf5d7bbfb6d8cd616bf565cd7d
parent 30f59b0c398fb3ce1e088386fd14c643e493cc5f

Escape analysistest failure message

Currently, the failure message does not output correctly if it contains sequences subject to shell variable expansion (e.g. `$1`) or the limit string used to denote the end of the message (in this case `\nEOF\n`). Those are quite plausible edge-cases, since analysistest targets might assert about the command-line of actions generated with `ctx.actions.run_shell` or the contents of shell files generated with `ctx.actions.write`.

This uses the approach of echoing by line and escaping characters as necessary for both versions. In the sh case, we can baskslash-escape every character. For Windows, `%` is escaped as `%%`. (Other special characters don't seem to cause trouble, though I'm probably missing relevant cases.)

The heredoc style `cat<<EOF\n[...]\nEOF\n` can have shell-escaping disabled by quoting the limit-string (`cat<<'EOF'`) or preceding it with a backslash (`cat<<\EOF`). But that still relies on choosing a limit string that doesn't occur in the contents, and there's no way (whether or not shell expansion is done) of escaping the limit string if it does. While one could choose a more obscure limit string, that would add another hidden implementation detail. Alternately, this could add logic to choose a limit string that doesn't occur in the message, but that seems unnecessarily complex.

Another approach would be to write the message to a file, then read it with cat/type. However, that requires either splitting the change between Starlark and Java code with separate release cycles (awkward), or doing something to insert the message file in the test's runfiles at the last minute (more complicated).

Adds some new test-cases which fail without this change.

Removes an unused return value to make the usage of this function a bit clearer.

https://github.com/bazelbuild/bazel-skylib/pull/320 for the corresponding
change for unittest.

RELNOTES: None.
PiperOrigin-RevId: 400309738