[libcxx-commits] [PATCH] D111202: [libc++] Add from-scratch testing configs for Windows

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/configs/llvm-libc++-mingw.cfg.in:14
+config.substitutions.append(('%{link_flags}',
+    '%{user_link_flags} -nostdlib++ -L %{lib} -lc++'
+))
----------------
ldionne wrote:
> There's a weird character here.
Oops, will fix.


================
Comment at: libcxx/utils/libcxx/test/format.py:24
     """
-    command = "%{{cxx}} -xc++ {} -Werror -fsyntax-only -Xclang -verify-ignore-unexpected".format(os.devnull)
+    command = "%{{cxx}} %{{flags}} -xc++ {} -Werror -fsyntax-only -Xclang -verify-ignore-unexpected".format(os.devnull)
     command = lit.TestRunner.applySubstitutions([command], config.substitutions,
----------------
ldionne wrote:
> If we do this, I think it should then be `%{{flags}} %{{compile_flags}}`.
Yes, that's probably most consistent, and that substitution shouldn't have anything that we can't use here.


================
Comment at: libcxx/utils/libcxx/test/newconfig.py:14-17
+def quote(s):
+    if platform.system() == 'Windows':
+        return lit.TestRunner.quote_windows_command([s])
+    return pipes.quote(s)
----------------
ldionne wrote:
> I suggest we simply move that to `lit.TestRunner`. It's too general purpose to be included in `newconfig.py`, even though I know that is convenient.
Ok, I'll see about that.


================
Comment at: libcxx/utils/libcxx/test/newconfig.py:50
+
+  sys.stderr.flush()  # Force flushing to avoid broken output on Windows
----------------
ldionne wrote:
> If this is necessary, IMO this should be part of `lit_config.note()` itself.
Ok, I'll try that too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111202