[PATCH] D95246: [SystemZ][z/OS] Fix No such file or directory expression error matching in lit tests - continued

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 27 00:44:51 PST 2021


jhenderson added a comment.

In D95246#2522913 <https://reviews.llvm.org/D95246#2522913>, @abhina.sreeskantharajan wrote:

> Please let me know if there are other guides I will need to update that I'm not aware of.

There's also the lit CommandGuide located at `llvm/docs/CommandGuide/lit.rst`.



================
Comment at: clang/test/Driver/clang-offload-bundler.c:73-74
 
-// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck %s -DFILE=%t.tgt2.notexist --check-prefix CK-ERR5
-// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 | FileCheck %s -DFILE=%t.bundle.i.notexist --check-prefix CK-ERR5
-// CK-ERR5: error: '[[FILE]]': {{N|n}}o such file or directory
+// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck %s -DFILE=%t.tgt2.notexist -DMSG=%errc_ENOENT --check-prefix CK-ERR5
+// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 | FileCheck %s -DFILE=%t.bundle.i.notexist -DMSG=%errc_ENOENT --check-prefix CK-ERR5
+// CK-ERR5: error: '[[FILE]]': [[MSG]]
----------------
Nit: These are probably good candidates to split up over multiple lines, as in the inline edit.


================
Comment at: lld/CMakeLists.txt:115
+if (LLD_BUILT_STANDALONE)
+  set(LLVM_HOST_TRIPLE ${TARGET_TRIPLE})
+endif()
----------------
The target and the host may be two completely different things. This variable setting doesn't look right to me. In a situation where someone is building LLD to run on Windows but targeting Linux, the host triple value will end up being set to a Linux triple, and the tests would fail.

The variable setting can also be moved inside the earlier if about 5 lines up, because that block is hit when `LLD_BUILD_STANDALONE` is set.


================
Comment at: llvm/docs/TestingGuide.rst:545-547
+   Example (%errc_ENOENT): ``No such file or directory``
+
+   Example (%errc_ENOENT): ``no such file or directory``
----------------
This is a slightly different format to the other examples above. I think it should be like the inline edit. this also helps distinguish the difference between the two.

Note: I can't remember which way around the two versions are, so you might need to swap them!


================
Comment at: llvm/utils/lit/lit/llvm/config.py:349-351
+        triple = ""
+        if hasattr(self.config, 'host_triple'):
+            triple = self.config.host_triple
----------------
I'm concerned that someone might start using these substitutions in a project for the first time, and get confused why they don't work on non-windows platforms. Maybe the solution is simply to require LLVM_HOST_TRIPLE to be set in all projects, i.e. go back to what you were doing before, and letting python fail if it isn't set.

Happy to hear other ideas too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246



More information about the llvm-commits mailing list