[zorg] Addition of extraLitArgs variable to ClangAndLLDBuilder.py

[sqlbyme]

Summary

Adding a extraLitArgs variable to be able to pass a list of extra lit args in from the builder definition. Also, did a bit of cleanup and removed an old FIXME which has been hanging out for a while.

[gkistanova]

zorg/buildbot/builders/ClangAndLLDBuilder.py

111

Maybe just let user define -DLLVM_LIT_ARGS with extraCmakeOptions param instead? With a meaningful default, let's say "-v".
This way it would be clear of how a particular builder is configured.

With the current approach, you may end up having multiple conflicting -DLLVM_LIT_ARGS, one coming from extraCmakeOptions, and the other one coming from extraLitArgs.

The other option is to assert on having -DLLVM_LIT_ARGS in extraCmakeOptions. This is up to you which option to choose.

[gkistanova]

Looks good with one note about the extraLitArgs usage.

Thanks

Galina

[gkistanova]

zorg/buildbot/builders/ClangAndLLDBuilder.py

104

Did you mean checking extraCmakeOptions instead? extraCompilerOptions are C/C++ flags...

However, this assert would not work as you think it is.
You want something similar to this:

assert any(a.startswith('-DLLVM_LIT_ARGS=') for a in extraCmakeOptions, "Please use extraLitArgs for LIT arguments instead of defining them in extraCmakeOptions."

[gkistanova]

Committed as r261378 with few small changes.

[sqlbyme]

zorg/buildbot/builders/ClangAndLLDBuilder.py

104

Yep. I'll fix this and re-submit. Thanks.

111

As "-DLLVM_LIT_ARGS=\"-v\"" was previously defined and may still be relied upon I'll add an assertion.

zorg/buildbot/builders/ClangAndLLDBuilder.py

            buildWithSanitizerOptions=None,
            triple=None,
            isMSVC=False,
-           prefixCommand=["nice", "-n", "10"] # For backward compatibility.
+           prefixCommand=["nice", "-n", "10"], # For backward compatibility.
+           extraLitArgs=None
     ):
 
     llvm_srcdir = "llvm.src"

     if isMSVC:
         options = []
     if extraCompilerOptions:
+        assert any(a.startswith('-DLLVM_LIT_ARGS=') for a in extraCmakeOptions, "Please use extraLitArgs for LIT arguments instead of defining them in extraCmakeOptions."       
         options += extraCompilerOptions
 
     if buildWithSanitizerOptions:
         options += buildWithSanitizerOptions
 
-    # FIXME: This is a temporary workaround till all the slaves will
-    # be configured to have C_COMPILER and CXX_COMPILER set locally.
-    # Will be rolled back later.
-    # cmakeCommand = [
-        # "cmake",
-        # "-DCMAKE_BUILD_TYPE=Release",
-        # "-DLLVM_ENABLE_ASSERTIONS=ON",
-        # "-DCMAKE_CXX_FLAGS=\"%s\"" % (" ".join(options)),
-        # "-DLLVM_LIT_ARGS=\"-v\"",
-        # "-G", "Ninja",
-        # "../%s" % llvm_srcdir]
+    lit_args = ["-v"]
+    if extraLitArgs:
+        lit_args += extraLitArgs
+    lit_args_str = "-DLLVM_LIT_ARGS=\"%s\"" % (" ".join(lit_args))
 
     cmakeCommand = [
         "cmake",
         "-DCMAKE_BUILD_TYPE=Release",
-        "-DLLVM_ENABLE_ASSERTIONS=ON"]
+        "-DLLVM_ENABLE_ASSERTIONS=ON"
+    ]
     if buildWithSanitizerOptions:
         cmakeCommand += [
             "-DCMAKE_C_COMPILER=clang",
-            "-DCMAKE_CXX_COMPILER=clang++"]
+            "-DCMAKE_CXX_COMPILER=clang++"
+        ]
     if triple:
         cmakeCommand += [
-            "-DLLVM_DEFAULT_TARGET_TRIPLE=%s" % triple]
-
+            "-DLLVM_DEFAULT_TARGET_TRIPLE=%s" % triple
+        ]
     if extraCmakeOptions:
         cmakeCommand += extraCmakeOptions
-
     if not isMSVC:
         cmakeCommand += [
             "-DCMAKE_C_FLAGS=\"%s\"" % (" ".join(options)),
             "-DCMAKE_CXX_FLAGS=\"-std=c++11 %s\"" % (" ".join(options)),
-            "-DLLVM_LIT_ARGS=\"-v\"",
         ]
+    cmakeCommand += lit_args_str
     cmakeCommand += [
        "-G", "Ninja",
         "../%s" % llvm_srcdir