On z/OS, the following error message is not matched correctly in lit tests.
EDC5129I No such file or directory.
This patch uses a lit config substitution to check for platform specific error messages.
Differential D95246
[test] Use host platform specific error message substitution in lit tests abhina.sreeskantharajan on Jan 22 2021, 9:37 AM. Authored by
Details
On z/OS, the following error message is not matched correctly in lit tests. EDC5129I No such file or directory. This patch uses a lit config substitution to check for platform specific error messages.
Diff Detail
Event TimelineComment Actions Sorry, could you revert this please. I don't think this is a good fix, as you've reduced coverage within the test, and some changes are completly redundant/add extra noise. I've commented inline with examples. Skimming D94239 suggests that has the same issue. Could you also please explain the messages your system is actually producing so we can provide a better solution than this fix. I'm also concerned that you are doing this to fix this test for a system, yet there are no build bots that report this error. A future contributor is likely to break them in the same way/add new tests with the same issue. If your system is a system that is supposed to be supported by LLVM, there needs to be a build bot. If it isn't supported, you should bring this up on llvm-dev (if you haven't already) to get buy-in for support.
Comment Actions As far I understand, looking on the description of D94239, the message on z/OS looks like "EDC5129I No such file or directory.". Comment Actions Thanks for the feedback. I've reverted my changes from these two patches. We have indicated that we wish to add support for the z/OS platform and we are working on getting a buildbot set up. As grimar noted, this is indeed the correct error message. "EDC5129I No such file or directory." (Note the extra period at the end) '{{.*N|n}}o such file or directory' {{EDC5129I N|N|n}}o such file or directory' Some testcases fail because of the extra period at the end. For those testcases, this is a possible alternative. {{.*N|n}}o such file or directory{{\.?}} Please let me know if there are better alternatives I could look into. Comment Actions Slightly off-the-wall idea: I'm assuming you don't control your system in such a way that you can change the error message to omit the error code?
I think you can just omit the trailing full stop in those cases. If the test isn't using --match-full-lines, it should be fine. If it is, adding {{.?}} seems reasonable. Having the error code explicitly in the pattern looks like the right solution for now, but a thought on that - it seems like tests will still have the fragility problem for when someone else writes a new test that checks the message due to the error code not being present on most systems. Is the error code different for each system error message (I'm guessing it is)? I wonder if we would be better off adding some sort of lit substitution or similar that expands to the right string for the given host OS, which could in turn be fed to FileCheck. It might look a bit like this in practice: # RUN: not do-a-thing %t 2>&1 | FileCheck %s -DMSG=%enoent -DFILE=%t # CHECK: error: '[[FILE]]': [[MSG]] What do you think? Comment Actions Right, I'm not able to change the error message.
I like the lit substitution solution, it will be a lot cleaner compared to a complicated regex. I've noticed there are already different regex for the same error message so this will help the error messages be uniform as well. I can look into implementing this. Comment Actions I've implemented the initial solution suggested by James. I haven't updated all the testcases yet because I wanted to get feedback on the implementation first. Comment Actions I did briefly consider one alternative, which was to build these directly into FileCheck, but it doesn't quite feel like the right approach for FileCheck to need to know system level details. In practice, I think the lit substitution may be the best way forward overall. In the future, it should be easy to expand with more error messages. You should add the new substitution to the lit command guide, so that it's documented. It might also be worth a heads up on llvm-dev to get feedback from those not directly on this review, to see if there are other ideas.
Comment Actions This patch makes the following changes:
I've also created a post on llvm-dev about this patch: https://lists.llvm.org/pipermail/llvm-dev/2021-January/148089.html Comment Actions Fix CI: Other projects like flang also do not specify LLVM_HOST_TRIPLE. If this is not specified, set the triple to an empty string. I also added one more error code %errc_EISDIR and updated the TestingGuide. Comment Actions There's also the lit CommandGuide located at llvm/docs/CommandGuide/lit.rst.
Comment Actions I've changed the host check to use python's sys.platform to determine the host as an alternative to using LLVM_HOST_TRIPLE. It will save us the effort of making sure it's defined in all projects as well. I noticed that llvm/docs/CommandGuide/lit.rst only lists substitutions that are done in TestRunner.py and refers to llvm/docs/TestingGuide.rst for information on the remaining substitutions. I think if we move the error substitution code into TestRunner.py, then it would be appropriate to update the lit.rst. Where do you think is the appropriate place to define the error substitutions, TestRunner.py or config.py?
Comment Actions Address syntax comments, and add a list of supported error code substitutions in the guide. If these changes are ok, I would like to start making changes to all affected testcases. Comment Actions One nit, but otherwise looks good to me, thanks! Please go ahead with the other test updates. Do you plan on doing them in this patch?
Comment Actions This was my initial thought. But if it's preferred to create a second patch to make the remaining changes to affected testcases, I have no issue with doing that. Comment Actions Update all testcases to use -DMSG. Comment Actions This patch causes fails in a bunch of test files. When I roll it back, it passes. Please, pay attention to this. P.S. The similar trouble is in D95808. I've commented there as well. Comment Actions One more mention. There are a lot of popups like this which stops the process untill press OK. That's very annoying. Please revert or fix this patch. Comment Actions Hi Denys, thanks for bringing this to my attention, I'll look into this failure. I do know that the CI test for this patch and the other patch ran on Windows and did not report any errors. https://buildkite.com/llvm-project/premerge-checks/builds/23727#63570ad0-0e74-4825-b1dc-8f53a4d4dad0 Comment Actions
I'd love to help you. I'm using ordinary Win10. Here is my python output: >>> os.name 'nt' >>> sys.platform 'win32' >>> platform.system() 'Windows' >>> platform.release() '10' >>> If you would say what exact info you need I would execute and send you. >>> open('some') Traceback (most recent call last): File "<stdin>", line 1, in <module> FileNotFoundError: [Errno 2] No such file or directory: 'some' >>> Comment Actions Thanks! The following Windows bot (clang-x64-windows-msvc) is also passing http://lab.llvm.org:8011/#/builders/123, I expect it would have the same output in python as your machine. James mentioned that this issue might be caused by a difference in standard library. I was curious if all of these modified testcases are now failing or only the three you mentioned. I also don't know how this change can cause a pop-up to occur, is there more information you can provide? llvm/test/tools/llvm-elfabi/fail-file-write-windows.test I did not modify this but it requires system-windows and has a lower-case error message. I expect if my change causes failures for you, then this testcase should also fail. Comment Actions In D95246#2566417, @abhina.sreeskantharajan wrote:
Here is the list of the tests which fail after the patch (I haven't build LLD so don't have tests as well): Clang :: CodeGen/basic-block-sections.c Clang :: CodeGen/ubsan-blacklist-vfs.c Clang :: Frontend/output-paths.c Clang :: Frontend/stats-file.c LLVM :: DebugInfo/symbolize-missing-file.test LLVM :: Object/archive-extract-dir.test LLVM :: Object/directory.ll LLVM :: tools/llvm-ar/error-opening-directory.test LLVM :: tools/llvm-ar/missing-thin-archive-member.test LLVM :: tools/llvm-ar/move.test LLVM :: tools/llvm-ar/print.test LLVM :: tools/llvm-ar/quick-append.test LLVM :: tools/llvm-ar/replace.test LLVM :: tools/llvm-ar/response.test LLVM :: tools/llvm-cxxdump/trivial.test LLVM :: tools/llvm-libtool-darwin/filelist.test LLVM :: tools/llvm-libtool-darwin/invalid-input-output-args.test LLVM :: tools/llvm-lipo/create-arch.test LLVM :: tools/llvm-lipo/replace-invalid-input.test LLVM :: tools/llvm-lto/error.ll LLVM :: tools/llvm-lto2/X86/stats-file-option.ll LLVM :: tools/llvm-mc/basic.test LLVM :: tools/llvm-mca/invalid_input_file_name.test LLVM :: tools/llvm-ml/basic.test LLVM :: tools/llvm-objcopy/COFF/add-section.test LLVM :: tools/llvm-objcopy/ELF/add-section.test LLVM :: tools/llvm-objcopy/ELF/error-format.test LLVM :: tools/llvm-objcopy/MachO/add-section-error.test LLVM :: tools/llvm-objcopy/redefine-symbols.test LLVM :: tools/llvm-objcopy/wasm/dump-section.test LLVM :: tools/llvm-profdata/weight-instr.test LLVM :: tools/llvm-profdata/weight-sample.test LLVM :: tools/llvm-readobj/ELF/thin-archive-paths.test LLVM :: tools/llvm-readobj/basic.test LLVM :: tools/llvm-readobj/thin-archive.test LLVM :: tools/llvm-size/no-input.test LLVM :: tools/llvm-xray/X86/no-such-file.txt LLVM :: tools/obj2yaml/invalid_input_file.test LLVM :: tools/yaml2obj/output-file.yaml
I got this. The popups is not the fault of this patch. They happened earlier. I'll investigate it later.
Yes, this fails for me too. Didn't check it earlier. Comment Actions @ASDenysPetrov, could you provide your link command-line for one of the tools that produces the failing message, please, e.g. llvm-ar? That may give us a hint about whether different libraries are being used. It's possible we need some additional cmake-time configuration steps to detect this difference. Also are you using cygwin at all? Not that I'm saying we shouldn't try to fix your use-case, but if you aren't using Cygwin, it's worth noting that https://llvm.org/docs/GettingStarted.html#hardware states that only Visual Studio builds (whether via the IDE or some other tool like ninja) are known to work for Windows. Comment Actions
Here is output of running tests in llvm-ar folder:
I'm using MSYS2 instead of Cygwin. I found it as a better alternative. I'm using GCC+MSYS2+Ninja. Comment Actions Sorry, what I meant was, what is the command used to build the llvm-ar executable, not what does it print when used. The command used to build llvm-ar should tell us what libraries are being used, which could impact this behaviour. Comment Actions @jhenderson Comment Actions MSYS2 is based on a modified version of Cygwin according to its description, maybe if we can detect that in config.py this will solve your issue. sysconfig.get_platform() Comment Actions
Sure, >>> sysconfig.get_platform() 'win32' >>> Comment Actions @ASDenysPetrov are you running python inside MSYS bash? that is installed by
I learned that the python should give different output >>> sys.platform msys >>> platform.system() MSYS_NT-10.0-19041 >>> sysconfig.get_platform() msys-3.1.7-x86_64 After taking a look at the CMakeCache.txt, I think a potential workaround would be to check if config.host_cxx contains MSYS2 which is set to CMAKE_CXX_COMPILER:FILEPATH=D:/WORK/Utilities/MSYS2/mingw64/bin/c++.exe if we can't determine MSYS through python. Comment Actions @abhina.sreeskantharajan wrote:
No, I installed Python as usual via Windows msi got from official site. Here is what I've got from Python: >>> sys.platform 'win32' >>> platform.system() 'Windows' >>> sysconfig.get_platform() 'win32' >>> Comment Actions @ASDenysPetrov, if you run "ninja -v llvm-ar", what is the output of the line that actually builds the llvm-ar executable? That'll tell us which libraries are in use, which should hopefully point to the difference to our own environments. For example, the last line of output for this on my Linux machine is: [811/811] : && /usr/bin/c++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -Wl,-rpath-link,/home/binutils/llvm/llvm-project/build/./lib -Wl,-O3 -Wl,--gc-sections tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.o -o bin/llvm-ar -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMAArch64AsmParser.a lib/libLLVMAMDGPUAsmParser.a lib/libLLVMARMAsmParser.a lib/libLLVMAVRAsmParser.a lib/libLLVMBPFAsmParser.a lib/libLLVMHexagonAsmParser.a lib/libLLVMLanaiAsmParser.a lib/libLLVMMipsAsmParser.a lib/libLLVMMSP430AsmParser.a lib/libLLVMPowerPCAsmParser.a lib/libLLVMRISCVAsmParser.a lib/libLLVMSparcAsmParser.a lib/libLLVMSystemZAsmParser.a lib/libLLVMWebAssemblyAsmParser.a lib/libLLVMX86AsmParser.a lib/libLLVMAArch64Desc.a lib/libLLVMAMDGPUDesc.a lib/libLLVMARMDesc.a lib/libLLVMAVRDesc.a lib/libLLVMBPFDesc.a lib/libLLVMHexagonDesc.a lib/libLLVMLanaiDesc.a lib/libLLVMMipsDesc.a lib/libLLVMMSP430Desc.a lib/libLLVMNVPTXDesc.a lib/libLLVMPowerPCDesc.a lib/libLLVMRISCVDesc.a lib/libLLVMSparcDesc.a lib/libLLVMSystemZDesc.a lib/libLLVMWebAssemblyDesc.a lib/libLLVMX86Desc.a lib/libLLVMXCoreDesc.a lib/libLLVMAArch64Info.a lib/libLLVMAMDGPUInfo.a lib/libLLVMARMInfo.a lib/libLLVMAVRInfo.a lib/libLLVMBPFInfo.a lib/libLLVMHexagonInfo.a lib/libLLVMLanaiInfo.a lib/libLLVMMipsInfo.a lib/libLLVMMSP430Info.a lib/libLLVMNVPTXInfo.a lib/libLLVMPowerPCInfo.a lib/libLLVMRISCVInfo.a lib/libLLVMSparcInfo.a lib/libLLVMSystemZInfo.a lib/libLLVMWebAssemblyInfo.a lib/libLLVMX86Info.a lib/libLLVMXCoreInfo.a lib/libLLVMBinaryFormat.a lib/libLLVMCore.a lib/libLLVMDlltoolDriver.a lib/libLLVMLibDriver.a lib/libLLVMObject.a lib/libLLVMSupport.a -lpthread lib/libLLVMAArch64Utils.a lib/libLLVMAMDGPUUtils.a lib/libLLVMARMUtils.a lib/libLLVMMCDisassembler.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMTextAPI.a lib/libLLVMOption.a lib/libLLVMBitReader.a lib/libLLVMCore.a lib/libLLVMBinaryFormat.a lib/libLLVMRemarks.a lib/libLLVMBitstreamReader.a lib/libLLVMSupport.a -lrt -ldl -lpthread -lm /usr/lib/x86_64-linux-gnu/libz.so /usr/lib/x86_64-linux-gnu/libtinfo.so lib/libLLVMDemangle.a && : This shows, for example, that the libz.so my build uses is located in /usr/lib/x86_64-linux-gnu. By having your equivalent path, we can hopefully confirm that the issue is caused by a different set of system libraries being used. Comment Actions
Hi, @jhenderson [1/2] D:\WORK\Utilities\MSYS2\mingw64\bin\c++.exe -DGTEST_HAS_RTTI=0 -D_DEBUG -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/llvm-ar -ID:/WORK/LLVM/llvm-project/llvm/tools/llvm-ar -Iinclude -ID:/WORK/LLVM/llvm-project/llvm/include -Wa,-mbig-obj -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -O2 -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -MF tools\llvm-ar\CMakeFiles\llvm-ar.dir\llvm-ar.cpp.obj.d -o tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -c D:/WORK/LLVM/llvm-project/llvm/tools/llvm-ar/llvm-ar.cpp [2/2] cmd.exe /C "cd . && D:\WORK\Utilities\MSYS2\mingw64\bin\c++.exe -Wa,-mbig-obj -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -O2 -Wl,--stack,16777216 tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -o bin\llvm-ar.exe -Wl,--out-implib,lib\libllvm-ar.dll.a -Wl,--major-image-version,0,--minor-image-version,0 lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMBinaryFormat.a lib/libLLVMCore.a lib/libLLVMDlltoolDriver.a lib/libLLVMLibDriver.a lib/libLLVMObject.a lib/libLLVMSupport.a lib/libLLVMMCDisassembler.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMTextAPI.a lib/libLLVMOption.a lib/libLLVMBitReader.a lib/libLLVMCore.a lib/libLLVMBinaryFormat.a lib/libLLVMRemarks.a lib/libLLVMBitstreamReader.a lib/libLLVMSupport.a -lpsapi -lshell32 -lole32 -luuid -ladvapi32 D:/WORK/Utilities/MSYS2/mingw64/lib/libz.dll.a lib/libLLVMDemangle.a -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ." As you can libz is here D:/WORK/Utilities/MSYS2/mingw64/lib/libz.dll.a Comment Actions Thanks (for clarity, libz isn't what I'm looking for, it was just an example of how this information might be useful). As it is, I think we might need more information still - that commandline hides which files are actually used slightly. Could you try explicitly running the c++ command that is being requested, but with the -Wl,--trace option added too. That will tell the linker to print what files it opens when it runs the link, and should give us an hint as to which system library is different for your usage than ours. Comment Actions
I added -W --trace to the end of cmdline. The output is in the file: Comment Actions Sorry, but -W --trace is not the same as -Wl,--trace. The former is a pair of options used by the compiler (one of which describes the files used by the compiler). The latter is an option passed to the linker, and is what we need. Please try again! Comment Actions
I used -W, because my c++ doesn't know -Wl option. C:\Users\Denis>c++ -Wl c++: error: unrecognized command-line option '-Wl'; did you mean '-W'? c++: fatal error: no input files compilation terminated. Could you provide exact cmd line me to use if I'm doing smth wrong? Comment Actions -Wl on its own isn't an option. It is used as a prefix to pass an option to the linker, so the exact string you pass to the compiler is -Wl,--trace (no spaces, comma required). The compiler sees the -Wl, prefix, removes it and then passes the remainder straight to the linker. You can see examples of this in your existing commandline with things like -Wl,--stack,16777216. The cmdline should be: D:\WORK\Utilities\MSYS2\mingw64\bin\c++.exe -DGTEST_HAS_RTTI=0 -D_DEBUG -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/llvm-ar -ID:/WORK/LLVM/llvm-project/llvm/tools/llvm-ar -Iinclude -ID:/WORK/LLVM/llvm-project/llvm/include -Wa,-mbig-obj -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -O2 -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -MF tools\llvm-ar\CMakeFiles\llvm-ar.dir\llvm-ar.cpp.obj.d -o tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -c D:/WORK/LLVM/llvm-project/llvm/tools/llvm-ar/llvm-ar.cpp [2/2] cmd.exe /C "cd . && D:\WORK\Utilities\MSYS2\mingw64\bin\c++.exe -Wa,-mbig-obj -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -O2 -Wl,--stack,16777216 tools/llvm-ar/CMakeFiles/llvm-ar.dir/llvm-ar.cpp.obj -o bin\llvm-ar.exe -Wl,--out-implib,lib\libllvm-ar.dll.a -Wl,--major-image-version,0,--minor-image-version,0 lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Info.a lib/libLLVMBinaryFormat.a lib/libLLVMCore.a lib/libLLVMDlltoolDriver.a lib/libLLVMLibDriver.a lib/libLLVMObject.a lib/libLLVMSupport.a lib/libLLVMMCDisassembler.a lib/libLLVMMCParser.a lib/libLLVMMC.a lib/libLLVMDebugInfoCodeView.a lib/libLLVMDebugInfoMSF.a lib/libLLVMTextAPI.a lib/libLLVMOption.a lib/libLLVMBitReader.a lib/libLLVMCore.a lib/libLLVMBinaryFormat.a lib/libLLVMRemarks.a lib/libLLVMBitstreamReader.a lib/libLLVMSupport.a -lpsapi -lshell32 -lole32 -luuid -ladvapi32 D:/WORK/Utilities/MSYS2/mingw64/lib/libz.dll.a lib/libLLVMDemangle.a -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 -Wl,--trace Comment Actions Thanks, yes that does the trick. As I expected, you can see that all the libraries that are used are contained withing the msys2/mingw subdirectory, and therefore are not the standard Windows SDK files that you would typically use for a Windows build using Visual Studio. One of those will contain the definitions we are talking about. @abhina.sreeskantharajan - I think you could this issue in one of two ways. One would be to detect when the tools/libraries used are within the msys2/mingw installation. Another way might be to actually build and run a miniature program at CMake time that prints out the messages, in such a way that CMake can inspect the output and use it to populate the lit configuration. Note: I am not a CMake expert, so don't know if this is strictly possible. The advantage of this approach is that you can remove the reliance on the system.platform call in python, and just always use the output of this simple program. Comment Actions Thanks, I'm also not sure if the second way is possible. I had another idea but I will need @ASDenysPetrov to test it out. import errno import os print ( os.strerror(errno.ENOENT)) If so, we can use this as the error message instead of hardcoding it per platform. Comment Actions I created the following review https://reviews.llvm.org/D97472 which I think should fix the error on all platforms. Let me know if this fixes the issue on MSYS2. Comment Actions This issue has been fixed in https://reviews.llvm.org/D97472. |
Add one more space - it helps the indentation stand out more and therefore make it clearer this line is a continuation.