build-script-impl: Generate the test commands from cmake and ninja in build-script-impl #5576
Conversation
separate steps so that if the target does not exist the script will fail instead of continuing. `set -e` is supposed to fix this, but since the command is empty if the target doesn't exist, the subshell returns 0 and the script continues. Also, print out the cmake and ninja commands so that we don't hide so much of the machinery. Fixes rdar://problem/29003970
|
@swift-ci Please smoke test and merge |
| echo "Generating dry run test command from: ${build_cmd[@]} -n -v ${target}" | ||
| dry_run_command_output="$(${build_cmd[@]} -n -v ${target} | sed -e 's/[^]]*] //')" | ||
| echo "Test command: ${dry_run_command_output}" | ||
| sh -e -x -c "${dry_run_command_output}" |
There was a problem hiding this comment.
Does this still do the right thing for paths with spaces in them?
There was a problem hiding this comment.
Checking now. Actually, I'm seeing some possible problems when compiling llvm with a space in the directory. There's an error printed, yet the build keeps going...
[582/2251] Linking CXX shared module lib/LLVMHello.dylib
FAILED: lib/LLVMHello.dylib
: && /Applications/Badger/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -g -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Werror=date-time -std=c++11 -fmodules -fmodules-cache-path=module.cache -fcxx-modules -gmodules -fcolor-diagnostics -g -isysroot /Applications/Badger/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9 -bundle -Wl,-headerpad_max_install_names -stdlib=libc++ -Wl,-flat_namespace -Wl,-undefined -Wl,suppress -Wl,-exported_symbols_list,/Users/erg/src 1/build/Ninja-DebugAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports -o lib/LLVMHello.dylib lib/Transforms/Hello/CMakeFiles/LLVMHello.dir/Hello.cpp.o -Wl,-rpath,@executable_path/../lib && :
clang: error: no such file or directory: '1/build/Ninja-DebugAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports'
There was a problem hiding this comment.
It's the -Wl,-exported_symbols_list,/Users/erg/src 1/build/Ninja-DebugAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports line.
There was a problem hiding this comment.
That's, um, also a problem, and something we should fix, although we don't need LLVMHello.dylib (it's an example transform). But for now I just don't want to regress the Swift part.
There was a problem hiding this comment.
We have to fix this or I can't easily test swift.
There was a problem hiding this comment.
? It's clearly been working fine up until now. I thought our problem was that we weren't catching failures properly, not that we had blocked successes. What changed?
There was a problem hiding this comment.
llvm is failing to build when I have a space in a directory. I don't know how long this has been broken for. Maybe we should have jenkins build in /home/jenkins/work space to catch these errors.
This patch fixes a specific hack where we invoke sh in a way where the command substitution in its body fails and returns an empty string, but then the sh command executes and returns 0, and the script keeps going.
There was a problem hiding this comment.
$(false | sed 's///') && echo $? && echo I wanted the first command to fail.
0
I wanted the first command to fail.There was a problem hiding this comment.
Ah, I see what you mean. But we should fix it, then. I don't want to regress here.
There was a problem hiding this comment.
Compiling llvm and swift is completely broken if your path has spaces. I don't know if it's ever worked.
I have some changes that make it work up to a point in swift so far.
Fix build-script-impl for test targets that do not exist.