[zorg] Fix LLDBCmakeBuildFactory

[labath]

Summary

The attempt in D69341 was not complete, as in the monorepo world, we
also need to pass LLVM_ENABLE_PROJECTS in order to build lldb (and
clang and lld).

This also updates the code to use the CmakeCommand class instead of the
generic ShellCommand.

[jankratochvil]

2019-10-29 11:38:04+0100 [-] error while parsing config file
2019-10-29 11:38:04+0100 [-] Unhandled Error
	Traceback (most recent call last):
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 197, in loadTheConfigFile
	    d = self.loadConfig(f)
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 579, in loadConfig
	    d.addCallback(do_load)
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 322, in addCallback
	    callbackKeywords=kw)
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 311, in addCallbacks
	    self._runCallbacks()
	--- <exception caught here> ---
	  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 654, in _runCallbacks
	    current.result = callback(current.result, *args, **kw)
	  File "/home/buildbot/.local/lib/python2.7/site-packages/buildbot-latest-py2.7.egg/buildbot/master.py", line 226, in do_load
	    exec f in localDict
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in <module>
	    c['builders'] = builders = list(config.builders.get_builders())
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 1456, in get_builders
	    for b in _get_lldb_builders():
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 829, in _get_lldb_builders
	    '-DLLVM_LIT_ARGS="-v"'])},
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py", line 129, in getLLDBCMakeBuildFactory
	    doStepIf=FileDoesNotExist(
	exceptions.NameError: global name 'FileDoesNotExist' is not defined
	
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/master.cfg", line 83, in <module>
	    c['builders'] = builders = list(config.builders.get_builders())
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 1456, in get_builders
	    for b in _get_lldb_builders():
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/config/builders.py", line 829, in _get_lldb_builders
	    '-DLLVM_LIT_ARGS="-v"'])},
	  File "/quad/home/buildbot/zorg-git/buildbot/osuosl/master/zorg/buildbot/builders/LLDBBuilder.py", line 129, in getLLDBCMakeBuildFactory
	    doStepIf=FileDoesNotExist(
	exceptions.NameError: global name 'FileDoesNotExist' is not defined

[labath]

import FileDoesNotExist

[jankratochvil]

It does run now, I did not test it with a slave.

[stella.stamenova]

Thanks @labath

[stella.stamenova]

Sadly, I think this broke the windows bot now (assuming it got applied to staging, otherwise I have to figure out what did):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/46/steps/cmake-configure/logs/stdio

[gkistanova]

The cmake command redefinition effectively is not supported any more, but the 'cmake' arg is left behind.
+ inline comments.

These have been addressed by https://reviews.llvm.org/rZORG425eeb1bf21b.

zorg/buildbot/builders/LLDBBuilder.py

110–112

This would build a path per notations on master. Which could be different than what a builder expects. Think of master running on Windows, for example.

Better use a "/" path separator as these days it seems supported by all platforms we care about.

130

It is better to always run the cmake configure step. It doesn't take long but handles some dependencies.

[labath]

In D69555#1727882, @stella.stamenova wrote:

Sadly, I think this broke the windows bot now (assuming it got applied to staging, otherwise I have to figure out what did):

http://lab.llvm.org:8014/builders/lldb-x64-windows-ninja/builds/46/steps/cmake-configure/logs/stdio

I am afraid that was due to 425eeb1b (or its combination with this patch, if you will). I am afraid we have too many quotes now :(

In an separate email, @gkistanova wrote:

In D69555 the problem of quotations around -DLLVM_ENABLE_PROJECTS remains.

"-DLLVM_ENABLE_PROJECTS=" + ";".join(f.depends_on_projects),

Actually, that's not true. :( The way that this patch resolved the quoting problem is by passing the command as an array. You'll notice that originally the command was being passed as a string, while now it's an array of strings. One does not need to add extra quotes when passing commands as arrays, as the array already provides the information about argument boundaries. Adding the extra quotes is not necessary and it seems to break stuff in some situations. The linux bot does not seem to be affected by this (which actually surprises me a lot), but the windows bot gets utterly confused by that.

I think that just removing the extra quotes should make everything happy.

zorg/buildbot/builders/LLDBBuilder.py

110–112

Thanks. I didn't know about the LLVMBuildFactory.pathRelativeToBuild stuff.

[labath]

In D69555#1728291, @labath wrote:

(which actually surprises me a lot)

Actually, nevermind. The linux bot fails now in the same way as the windows one (http://lab.llvm.org:8014/builders/lldb-x86_64-debian/builds/6262). Galina, will you remove the extra quotes, or should I?

zorg/buildbot/builders/LLDBBuilder.py

 import os
 import json
 import collections
 import buildbot
 import buildbot.process.factory
 from buildbot.steps.source import SVN
 from buildbot.steps.shell import Configure, SetProperty
 from buildbot.steps.shell import ShellCommand, WarningCountingShellCommand
 from buildbot.steps.slave import RemoveDirectory
 from buildbot.process.properties import WithProperties, Property
 from buildbot.steps import trigger
 import zorg.buildbot.commands.BatchFileDownload as batch_file_download
 from zorg.buildbot.commands.LitTestCommand import LitTestCommand
+from zorg.buildbot.commands.CmakeCommand import CmakeCommand
 from zorg.buildbot.builders.Util import getVisualStudioEnvironment
 from zorg.buildbot.builders.Util import extractSlaveEnvironment
 from zorg.buildbot.process.factory import LLVMBuildFactory
 
 # We *must* checkout at least Clang, LLVM, and LLDB.  Also check out LLD since
 # it is needed to run the LLDB test suite.
 def getLLDBSource(f,llvmTopDir='llvm'):
     f.addStep(SVN(name='svn-llvm',
def getLLDBCMakeBuildFactory(
     cleanBuildRequested = lambda step: step.build.getProperty("clean") or clean
     f.addStep(RemoveDirectory(name='clean '+build_dir,
                 dir=build_dir,
                 haltOnFailure=False,
                 flunkOnFailure=False,
                 doStepIf=cleanBuildRequested
                 ))
 
-    cmake_cmd = [
-        "cmake", "-G", "Ninja", os.path.join(os.pardir, f.monorepo_dir, "llvm"),
+    path = os.path.join(os.pardir, f.monorepo_dir, "llvm")
+    cmake_options = [
+        "-G", "Ninja",
         "-DCMAKE_BUILD_TYPE=" + config,
-        "-DCMAKE_INSTALL_PREFIX=../install"
+        "-DCMAKE_INSTALL_PREFIX=../install",
+        "-DLLVM_ENABLE_PROJECTS=" + ";".join(f.depends_on_projects),
         ]
     if python_source_dir:
-        cmake_cmd.append("-DPYTHON_HOME=" + python_source_dir)
+        cmake_options.append("-DPYTHON_HOME=" + python_source_dir)
     if extra_cmake_args:
-        cmake_cmd += extra_cmake_args
-    # Note: ShellCommand does not pass the params with special symbols right.
-    # The " ".join is a workaround for this bug.
-    f.addStep(ShellCommand(name="cmake-configure",
+        cmake_options += extra_cmake_args
+
+    f.addStep(CmakeCommand(name="cmake-configure",
                            description=["cmake configure"],
-                           command=WithProperties(" ".join(cmake_cmd)),
                            haltOnFailure=True,
-                           warnOnWarnings=True,
+                           options=cmake_options,
+                           path="../%s" % f.llvm_srcdir,
+                           env=Property('slave_env'),
                            workdir=build_dir,
-                           env=Property('slave_env')))
+                           doStepIf=FileDoesNotExist(
+                                        "./%s/CMakeCache.txt" % build_dir)))
 
     f.addStep(WarningCountingShellCommand(name='build',
                           command=build_cmd,
                           haltOnFailure=True,
                           description='ninja build',
                           workdir=build_dir,
                           env=Property('slave_env')))