Page MenuHomePhabricator

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

Authored by mgorny on Jul 26 2019, 7:05 AM.

Details

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.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Jul 26 2019, 7:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 26 2019, 7:05 AM
Herald added a subscriber: thopre. · View Herald Transcript
jdenny accepted this revision.Jul 26 2019, 7:38 AM

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.

This revision is now accepted and ready to land.Jul 26 2019, 7:38 AM

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.

This revision was automatically updated to reflect the committed changes.

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?

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.

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!