Page MenuHomePhabricator

[test][Driver] Fix Clang :: Driver/cl-response-file.c
ClosedPublic

Authored by ro on Jun 20 2019, 6:35 AM.

Details

Summary

Clang :: Driver/cl-response-file.c currently FAILs on Solaris:

Command Output (stderr):
--
/vol/llvm/src/clang/dist/test/Driver/cl-response-file.c:10:11: error: CHECK: expected string not found in input
// CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"
          ^

Looking at the generated response file reveals that this is no surprise:

/I/vol/llvm/src/clang/dist/test/Driver\Inputs

with no newline at the end. The echo command used to create it boils down to

echo 'a\cb'

However, one cannot expect \c to be emitted literally: e.g. bash's builtin echo has

\c        suppress further output

I've tried various combinations of builtin echo, /usr/bin/echo, GNU echo if different,
the same for printf, and the backslash unescaped and quoted (a\cb and a\\cb). The
only combination that worked reliably on Solaris, Linux, and macOS was

printf 'a\\cb'

so this is what this patch uses. Tested on amd64-pc-solaris2.11 and x86_64-pc-linux-gnu.
Ok for trunk?

Diff Detail

Repository
rL LLVM

Event Timeline

ro created this revision.Jun 20 2019, 6:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2019, 6:35 AM
jkorous accepted this revision.Jun 20 2019, 2:19 PM
jkorous added a subscriber: jkorous.

LGTM

This revision is now accepted and ready to land.Jun 20 2019, 2:19 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 20 2019, 2:29 PM
rnk added a subscriber: rnk.Jun 21 2019, 11:26 AM

This causes the test to fail on Windows:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/7712

There seems to be something wrong with the blamelist, so it didn't send email. I see this in the log from the previous build on that bot:

Updating llvm to 363241 at ...

But buildbot thinks the previous build was testing r364000, so it didn't send any notifications.

printf seems like the wrong command to use to test backslash handling, since it interprets them as escapes. %S expands to a path containing backslashes, and the test logs seem consistent with that theory. I'll go ahead and revert this.

dyung added a subscriber: dyung.
In D63600#1554029, @rnk wrote:

This causes the test to fail on Windows:
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/7712

There seems to be something wrong with the blamelist, so it didn't send email. I see this in the log from the previous build on that bot:

Updating llvm to 363241 at ...

But buildbot thinks the previous build was testing r364000, so it didn't send any notifications.

printf seems like the wrong command to use to test backslash handling, since it interprets them as escapes. %S expands to a path containing backslashes, and the test logs seem consistent with that theory. I'll go ahead and revert this.

i have been working on a fix for this test with Rainer and we think we have one that works for both Windows and Solaris, but it still uses printf, although I'm not sure I understand your concern with using printf, so I don't know if it solves the problem you mention or not. But I have posted it as D63678.