Skip to content

build-script-impl: Generate the test commands from cmake and ninja in build-script-impl #5576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 1, 2016
Merged

build-script-impl: Generate the test commands from cmake and ninja in build-script-impl #5576

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion utils/build-script-impl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2792,7 +2792,12 @@ for host in "${ALL_HOSTS[@]}"; do
# executing ninja directly, have it dump the commands it would
# run, strip Ninja's progress prefix with sed, and tell the
# shell to execute that.
sh -e -x -c "$("${build_cmd[@]}" -n -v ${target} | sed -e 's/[^]]*] //')"
# However, if we do this in a subshell in the ``sh -e -x -c`` line,
# errors in the command will not stop the script as they should.
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}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still do the right thing for paths with spaces in them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the -Wl,-exported_symbols_list,/Users/erg/src 1/build/Ninja-DebugAssert/llvm-macosx-x86_64/lib/Transforms/Hello/LLVMHello.exports line.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to fix this or I can't easily test swift.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

 $(false | sed 's///') && echo $? && echo I wanted the first command to fail.
0
I wanted the first command to fail.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see what you mean. But we should fix it, then. I don't want to regress here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

else
call "${build_cmd[@]}" ${BUILD_TARGET_FLAG} ${target}
fi
Expand Down