[zorg] Allow SanitizerBuilder.getSanitizerBuildFactory to accept extra args

[Conanap]

Summary

There is a need to allow SanitizerBuilder to accept extra args so cmake args can be passed in, in turn allowing us to specify -j with cmake.

[Conanap]

gentle ping

[gkistanova]

There are few comments for the buildbot part.

I didn’t look closely at the scripts part. At a quick glance adding quotation marks in multiple places wouldn’t work smoothly. You might end up with something you didn’t expect.

zorg/buildbot/builders/SanitizerBuilder.py

12

This is cosmetic. Could you move this new parameter up, right after depends_on_projects, please? So we would have env and timeout at the end of the arguments.

37

Did you want to add quotation marks around extra_configure_args value and missed it on the right hand side?

If I get the idea right, you are trying to make sure the extra_configure_args you want to pass through would go as a single argument on each stage. If so, this might be tricky with respect to possible different shells and variations of quotation marks users could specify in extra_configure_args. The same stands for buildbot_selector.py.

How about using something like -- to separate these args from other options which are to be handled in the calling script? Then it should be relatively easy to pass everything down and be transparent to quotation marks stripping by a shell and such. What do you think?

Not a requirement for this path, just some loud thinking.

61

If you make command an array, you would not need to bother with quotation marks. Buildbot will do that for you. Assuming something like -- is used for the pass through params.

[Conanap]

zorg/buildbot/builders/SanitizerBuilder.py

37

that's actually a really good idea, thank you! I'll go ahead and do that.

[Conanap]

Changed from using strings to -- to improve portability; changed order of arguments. I'll update the dependent patch to properly reflect the changes made here.

[Conanap]

Updated to align with changes on the patch this depends on.

[gkistanova]

Thanks, Albion!
Looks much better now.

This approve is for the buildbot part.

[gkistanova]

Hm. This is weird.
I see a wrong patch assigned to this review now.,

The patch I see now adds "-CMAKE_ARGS=-DLLVM_LIT_ARGS=-v -j256" to extra_configure_args. Which is supposedly https://reviews.llvm.org/D98647.

But the patch I have seen before was good. :)

Not sure is this a Phabricator glitch.

[Conanap]

Not sure if there was a glitch but this should be the correct patch again

[Conanap]

Oh that is very weird. Let me reupload the differential for records sake. Would you mind taking a quick peak again @gkistanova, just to be on the safe side? Thank you!

zorg/buildbot/builders/SanitizerBuilder.py

 # TODO: Change WithProperties to Interpolate
 from buildbot.process.properties import WithProperties
 from zorg.buildbot.commands.AnnotatedCommand import AnnotatedCommand
 from zorg.buildbot.process.factory import LLVMBuildFactory
 
 def getSanitizerBuildFactory(
     clean=False,
     depends_on_projects=None,
+    extra_configure_args=None,
     env=None,
     timeout=1200):
 
     # Prepare environmental variables. Set here all env we want everywhere.
     merged_env = {
         "TERM" : "dumb", # Make sure Clang doesn't use color escape sequences.
                  }
     # Clobber bot if we need a clean build.
     if clean:
         merged_env["BUILDBOT_CLOBBER"] = "1"
     # Overwrite pre-set items with the given ones, so user can set anything.
     if env is not None:
         merged_env.update(env)
 
     if depends_on_projects is None:
         depends_on_projects = [
             "llvm",
             "clang",
             "compiler-rt",
             "libcxx",
             "libcxxabi",
             "libunwind",
             "lld"]
 
+    if extra_configure_args:
+        extra_configure_args = ['--'] + list(extra_configure_args)
+    else:
+        extra_configure_args = []
+
     # Explicitly use '/' as separator, because it works on *nix and Windows.
     sanitizer_script_dir = "sanitizer_buildbot/sanitizers"
     sanitizer_script = "../%s/zorg/buildbot/builders/sanitizers/%s" % (sanitizer_script_dir, "buildbot_selector.py")
 
     f = LLVMBuildFactory(
         clean=clean,
         depends_on_projects=depends_on_projects,
         llvm_srcdir=sanitizer_script_dir)
 
     # Get sanitizer buildbot scripts.
     f.addGetSourcecodeForProject(
         name='update-annotate-scripts',
         project='zorg',
         src_dir=sanitizer_script_dir,
         alwaysUseLatest=True)
 
     # Run annotated command for sanitizer.
+    command = ['python', sanitizer_script] + extra_configure_args
     f.addStep(AnnotatedCommand(name="annotate",
                                description="annotate",
                                timeout=timeout,
                                haltOnFailure=True,
-                               command="python " + sanitizer_script,
+                               command=command,
                                env=merged_env))
     return f

zorg/buildbot/builders/sanitizers/buildbot_cmake.sh

 #!/usr/bin/env bash
 
 set -x
 set -e
 set -u
 
 HERE="$(dirname $0)"
 . ${HERE}/buildbot_functions.sh
 
 ROOT=`pwd`
 PLATFORM=`uname`
 ARCH=`uname -m`
 export PATH="/usr/local/bin:$PATH"
 export ANDROID_SDK_HOME=$ROOT/../../..
 
+for arg in "$@"
+do
+    case $arg in
+        --CMAKE_ARGS=*)
+        CMAKE_ARGS="${arg#*=}"
+    esac
+done
+
 # Always clobber bootstrap build trees.
 rm -rf compiler_rt_build llvm_build* symbolizer_build*
 
 USE_CCACHE=
 if ccache -s ; then
   USE_CCACHE="-DLLVM_CCACHE_BUILD=ON"
   rm -rf clang_build
 fi
 
 SUPPORTS_32_BITS=${SUPPORTS_32_BITS:-1}
 MAKE_JOBS=${MAX_MAKE_JOBS:-$(nproc)}
 LLVM=$ROOT/llvm
 ZLIB=$ROOT/zlib
 
-CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS:-} -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_PARALLEL_LINK_JOBS=10"
+CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS:-} -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_PARALLEL_LINK_JOBS=10 ${CMAKE_ARGS}"
 ENABLE_LIBCXX_FLAG=
 if [ "$PLATFORM" == "Darwin" ]; then
   CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS} -DPYTHON_EXECUTABLE=/usr/bin/python"
   ENABLE_LIBCXX_FLAG="-DLLVM_ENABLE_LIBCXX=ON"
 fi
 
 if [ -e /usr/include/plugin-api.h ]; then
   CMAKE_COMMON_OPTIONS="${CMAKE_COMMON_OPTIONS} -DLLVM_BINUTILS_INCDIR=/usr/include"

zorg/buildbot/builders/sanitizers/buildbot_selector.py

 #!/usr/bin/python
 
 import os
 import subprocess
 import sys
 
 THIS_DIR=os.path.dirname(sys.argv[0])
+extra_args = sys.argv[1:]
 
 
 def bash(path):
     return 'bash ' + os.path.join(THIS_DIR, path)
 
 def cmd_call(path):
     return 'call ' + os.path.join(THIS_DIR, path)
 
BOT_ADDITIONAL_ENV = {
     'sanitizer-x86_64-linux-bootstrap-msan': {},
     'sanitizer-x86_64-linux-bootstrap-ubsan': {},
     'sanitizer-x86_64-linux-gn': {},
 }
 
 def Main():
   builder = os.environ.get('BUILDBOT_BUILDERNAME')
   print "builder name: %s" % (builder)
-  cmd = BOT_ASSIGNMENT.get(builder)
+  cmd = BOT_ASSIGNMENT.get(builder) + ' '.join(extra_args)
   if not cmd:
     sys.stderr.write('ERROR - unset/invalid builder name\n')
     sys.exit(1)
 
   print "%s runs: %s\n" % (builder, cmd)
   sys.stdout.flush()
 
   bot_env = os.environ