[PATCH] D41957: Utility for checking out llvm, clang, and associated tools and configuring a build folder

[Joerg Sonnenberger via Phabricator via llvm-commits]

joerg added inline comments.


================
Comment at: utils/checkout_llvm.sh:1
+if [ $# -gt 1 ]
+  then
----------------
Add an explicit #!/bin/sh


================
Comment at: utils/checkout_llvm.sh:13
+
+pushd llvm/tools
+svn co http://llvm.org/svn/llvm-project/lld/$1 lld
----------------
Please just use `cd llvm/tools` and `cd .././` below. Same for the rest.


================
Comment at: utils/checkout_llvm.sh:40
+pushd llvm-build
+CXX=$COMPILER cmake -G "Ninja" ../llvm -DCMAKE_BUILD_TYPE:STRING=Release -DCMAKE_INSTALL_PREFIX:PATH=$2 -DCOMPILER_RT_INCLUDE_TESTS:BOOL=OFF -DLLVM_ENABLE_ASSERTIONS:BOOL=OFF -DBUILD_SHARED_LIBS:BOOL=ON -DLLVM_OPTIMIZED_TABLEGEN:BOOL=ON
+popd
----------------
I'd suggest putting $2 into a variable, shifting twice and then appending any remaining arguments. Makes the script a bit more useful and allows overriding undesirable settings.


Repository:
  rL LLVM

https://reviews.llvm.org/D41957