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

2019-06-21 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment.

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 
.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63600/new/

https://reviews.llvm.org/D63600



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-06-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

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.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63600/new/

https://reviews.llvm.org/D63600



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-06-20 Thread Rainer Orth via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363985: [test][Driver] Fix Clang :: 
Driver/cl-response-file.c (authored by ro, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63600?vs=205793&id=205903#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63600/new/

https://reviews.llvm.org/D63600

Files:
  cfe/trunk/test/Driver/cl-response-file.c


Index: cfe/trunk/test/Driver/cl-response-file.c
===
--- cfe/trunk/test/Driver/cl-response-file.c
+++ cfe/trunk/test/Driver/cl-response-file.c
@@ -4,7 +4,7 @@
 
 
 
-// RUN: echo '/I%S\Inputs\cl-response-file\ /DFOO=2' > %t.rsp
+// RUN: printf '/I%S\Inputs\\cl-response-file\ /DFOO=2' > %t.rsp
 // RUN: %clang_cl /c -### @%t.rsp -- %s 2>&1 | FileCheck %s
 
 // CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"


Index: cfe/trunk/test/Driver/cl-response-file.c
===
--- cfe/trunk/test/Driver/cl-response-file.c
+++ cfe/trunk/test/Driver/cl-response-file.c
@@ -4,7 +4,7 @@
 
 
 
-// RUN: echo '/I%S\Inputs\cl-response-file\ /DFOO=2' > %t.rsp
+// RUN: printf '/I%S\Inputs\\cl-response-file\ /DFOO=2' > %t.rsp
 // RUN: %clang_cl /c -### @%t.rsp -- %s 2>&1 | FileCheck %s
 
 // CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-06-20 Thread Jan Korous via Phabricator via cfe-commits
jkorous accepted this revision.
jkorous added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: dexonsmith.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63600/new/

https://reviews.llvm.org/D63600



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2019-06-20 Thread Rainer Orth via Phabricator via cfe-commits
ro created this revision.
ro added a reviewer: rsmith.
Herald added a subscriber: fedor.sergeev.
Herald added a project: clang.

`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

  \csuppress 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?


Repository:
  rC Clang

https://reviews.llvm.org/D63600

Files:
  test/Driver/cl-response-file.c


Index: test/Driver/cl-response-file.c
===
--- test/Driver/cl-response-file.c
+++ test/Driver/cl-response-file.c
@@ -4,7 +4,7 @@
 
 
 
-// RUN: echo '/I%S\Inputs\cl-response-file\ /DFOO=2' > %t.rsp
+// RUN: printf '/I%S\Inputs\\cl-response-file\ /DFOO=2' > %t.rsp
 // RUN: %clang_cl /c -### @%t.rsp -- %s 2>&1 | FileCheck %s
 
 // CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"


Index: test/Driver/cl-response-file.c
===
--- test/Driver/cl-response-file.c
+++ test/Driver/cl-response-file.c
@@ -4,7 +4,7 @@
 
 
 
-// RUN: echo '/I%S\Inputs\cl-response-file\ /DFOO=2' > %t.rsp
+// RUN: printf '/I%S\Inputs\\cl-response-file\ /DFOO=2' > %t.rsp
 // RUN: %clang_cl /c -### @%t.rsp -- %s 2>&1 | FileCheck %s
 
 // CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits