[libcxx-commits] [PATCH] D101550: [libcxx] Use shell quoting for include flag on Windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo accepted this revision.
mstorsjo added a comment.

In D101550#2726500 <https://reviews.llvm.org/D101550#2726500>, @phosek wrote:

> I'm trying to use the runtimes build on Windows and include libc++ in the build which is where I ran into this issue.
>
> Prior to ea12d779bc238c387511fe7462020f4ecf4a8246 <https://reviews.llvm.org/rGea12d779bc238c387511fe7462020f4ecf4a8246> there was only one `/I` and that change only landed yesterday which likely explains why we haven't seen this earlier?

Ah, indeed, that would probably explain it.

> I tried using the joined form but that didn't work but that may have to be because of the quote placement. We need to use quotes around the path because of spaces (we ran into that issue before) but I'm not sure if the correct joined form is `/I"${LIBCXX_GENERATED_INCLUDE_DIR}"` or `"/I${LIBCXX_GENERATED_INCLUDE_DIR}"`, do you know?

On the windows command line quoting level, I think both of them are equally ok, but I'm not sure how this translates from the cmake level here to the actual final command line.

I'm fine with this solution in any case as it seems to be what cmake recommends. But it's a bit weird, because - doesn't this practically pass things as one single arg, containing `/I<space>\path\to\dir`? If that works with the compilers in practice I guess that's ok even though it's weird.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101550