[llvm] [FileCheck] Use FILECHECK_DUMP_INPUT_ON_FAILURE only when non-empty

[mgorny]

Summary

Enable dumping output only if FILECHECK_DUMP_INPUT_ON_FAILURE is set to
a non-empty value. This is necessary to support disabling it via
POSIX-compliant env(1) that does not support '-u' argument,
and therefore fix regression caused by r366980.

[jdenny]

LGTM as is.

A few notes we should keep in mind anyway: FILECHECK_DUMP_INPUT_ON_FAILURE is deprecated, so I agree that this simple change is probably sufficient (we don't need to check for 0, for example). I wonder if anyone still uses it. If so, this patch might break some uses, but I think users could easily adjust. If not, we should consider removing it.

Thanks.

[mgorny]

We are using it to force verbose output on buildbot ;-). But setting to an empty value should be very uncommon. The uses I've been able to grep in LLVM all set it to 1.

[jdenny]

In D65334#1602675, @mgorny wrote:

We are using it to force verbose output on buildbot ;-).

Good to know. By the way, you'd get roughly the same output with FILECHECK_OPTS=-dump-input=fail, which is not deprecated. Adding -v or -vv will provide much more info.

But setting to an empty value should be very uncommon.

Agreed.

The uses I've been able to grep in LLVM all set it to 1.

You mean the FileCheck test suite? Or are you grepping some user code?

[mgorny]

In D65334#1602696, @jdenny wrote:

In D65334#1602675, @mgorny wrote:

We are using it to force verbose output on buildbot ;-).

Good to know. By the way, you'd get roughly the same output with FILECHECK_OPTS=-dump-input=fail, which is not deprecated. Adding -v or -vv will provide much more info.

Hm, indeed. However, when grepping help I've noticed FILECHECK_DUMP_INPUT_ON_FAILURE but not FILECHECK_OPTS ;-).

But setting to an empty value should be very uncommon.

Agreed.

The uses I've been able to grep in LLVM all set it to 1.

You mean the FileCheck test suite? Or are you grepping some user code?

The former.

[jdenny]

In D65334#1602703, @mgorny wrote:

Hm, indeed. However, when grepping help I've noticed FILECHECK_DUMP_INPUT_ON_FAILURE but not FILECHECK_OPTS ;-).

It's in FileCheck.rst, but it should probably be in --help as well. I'll make a note to look into that. Thanks!

llvm/trunk/utils/FileCheck/FileCheck.cpp

 static cl::opt<bool> VerboseVerbose(
     "vv", cl::init(false),
     cl::desc("Print information helpful in diagnosing internal FileCheck\n"
              "issues, or add it to the input dump if enabled.  Implies\n"
              "-v.\n"));
 static const char * DumpInputEnv = "FILECHECK_DUMP_INPUT_ON_FAILURE";
 
 static cl::opt<bool> DumpInputOnFailure(
-    "dump-input-on-failure", cl::init(std::getenv(DumpInputEnv)),
+    "dump-input-on-failure",
+    cl::init(std::getenv(DumpInputEnv) && *std::getenv(DumpInputEnv)),
     cl::desc("Dump original input to stderr before failing.\n"
              "The value can be also controlled using\n"
              "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"
              "This option is deprecated in favor of -dump-input=fail.\n"));
 
 enum DumpInputValue {
   DumpInputDefault,
   DumpInputHelp,