[libcxx-commits] [PATCH] D93582: [libc++] [libc++abi] Correctly quote source and target names in tests. NFC.
Saleem Abdulrasool via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jan 5 08:46:34 PST 2021
compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.
================
Comment at: libcxx/test/CMakeLists.txt:85
"TargetInfo to use when setting up test environment.")
-set(LIBCXX_EXECUTOR "${Python3_EXECUTABLE} ${CMAKE_CURRENT_LIST_DIR}/../utils/run.py" CACHE STRING
+set(LIBCXX_EXECUTOR "${Python3_EXECUTABLE} '${CMAKE_CURRENT_LIST_DIR}/../utils/run.py'" CACHE STRING
"Executor to use when running tests.")
----------------
I believe that this doesn't work properly on Windows. The quoting rules on Windows are quite different. Using `"` instead of `'` would work. However, depending on how this is used, CMake should be able to handle the quoting appropriately per platform.
================
Comment at: libcxx/utils/libcxx/test/dsl.py:98
out, err, exitCode, timeoutInfo = _executeScriptInternal(test, ['%{build}'])
- _executeScriptInternal(test, ['rm %t.exe'])
+ _executeScriptInternal(test, ['rm "%t.exe"'])
return exitCode == 0
----------------
Please use `pipes.quote` for the argument quoting.
================
Comment at: libcxx/utils/libcxx/test/format.py:73
_checkBaseSubstitutions(substitutions)
- substitutions.append(('%{build}', '%{cxx} %s %{flags} %{compile_flags} %{link_flags} -o %t.exe'))
- substitutions.append(('%{run}', '%{exec} %t.exe'))
+ substitutions.append(('%{build}', '%{cxx} "%s" %{flags} %{compile_flags} %{link_flags} -o "%t.exe"'))
+ substitutions.append(('%{run}', '%{exec} "%t.exe"'))
----------------
Please use `pipes.quote` for the quoted parameters (and the same throughout the rest of the lit configuration).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93582/new/
https://reviews.llvm.org/D93582
More information about the libcxx-commits
mailing list