[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

Thu Feb 20 16:28:27 PST 2020

ravi-ramaseshan added a comment.

In D74811#1885514 <https://reviews.llvm.org/D74811#1885514>, @scott.linder wrote:

> Multiple build bots were failing with the patch applied, see e.g. http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/4890/steps/test-check-clang/logs/FAIL%3A%20Clang%3A%3Aclang_f_opts.c
>
> Seems like on platforms with a backslash for a directory separator you get `"\\\\with\\ spaces\\\\clang"` rather than `"/with\\ spaces/clang"`, although I'm not completely sure that is what we want? I imagine platforms with \ as a separator have different escaping rules?
>
> I reverted the commit for now; @ravi-ramaseshan is it OK if I just amend the test to look for `"-record-command-line" "{{.+}}with\\ spaces{{.+}}"` and we can worry about whether the other escaping is correct separately?

Hi Scott, thanks for checking. Sorry about not testing it on those platforms. I think "\\\\with\\ spaces\\\\clang" looks correct.
I think amending the commit as you did looks good to me. Will you be able to amend the commit for this or should I post another review with the fix?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74811