[zorg] Rev1: Fix get slave environment in LLDB Windows builder.

[rfoos]

Summary

Fixed RemoveDirectory, regenerated patch.

[zturner]

Thanks a million times for working on this. I never would have figured out the part of about the slave environment on my own :)

zorg/buildbot/builders/LLDBBuilder.py

37

I'm not crazy about this %VS120COMNTOOLS% usage. This is going to break when we switch to a different version of Visual Studio. In getLLDBWindowsCMakeBuildFactory(), we're already specifying a path to the Visual Studio installation folder, so I think we should just pass that into this function. This way, one can easily make two builders with different versions of Visual Studio on the same slave by just specifying a different value for vs_install_dir when creating the factory.

53

While it will probably work either way, any reason why this was necessary?

74

Can you move the comment inside the function?

127–132

Is the install necessary? What happens if we don't install? I've never done it before, so it seems like an unnecessary step unless I'm missing something.

134–140

Can getLLDBWindowsCMAKEBuildFactory take a boolean that says whether to run tests?

[rfoos]

The changes are reasonably small. I'll get this back up, and would like to go live soon.

zorg/buildbot/builders/LLDBBuilder.py

37

Yes, this is temporary. I was just minimizing changes to your builder.

In the generic builder, slave_envCmd lets the caller specify a complete command that is run on the slave. This will handle any case.
http://reviews.llvm.org/D6866

r""""%VS120COMNTOOLS%\..\..\VC\vcvarsall.bat" %PROCESSOR_ARCHITECTURE% & set"""

I picked VS120COMNTOOLS since it handles multiple installations of VS. For example on my buildlsave, three versions of MSVC are installed. All can access vcvarsall.bat in the same way by changing the VSxxxCOMNTOOLS variable.

In this way, it will work on your slave or my slave as a default. Since the entire command can be specified by the caller, any special cases can be easily addressed.

VS100COMNTOOLS=c:\Program Files (x86)\Microsoft Visual Studio 10.0\Common7\Tools
VS110COMNTOOLS=C:\Program Files (x86)\Microsoft Visual Studio 11.0\Common7\Tools
VS120COMNTOOLS=C:\Program Files (x86)\Microsoft Visual Studio 12.0\Common7\Tools

53

Yes both ways work. All of the additions to python_source_dir were forward slash paths, so the result was a mixed path. CMake can handle this. Make can most of the time.

Basically all forward slashes was just easier on my eyes when reading the log file :)

"-DPYTHON_LIBRARY=C:\src\python/PCbuild/python27_d.lib" 
"-DPYTHON_INCLUDE_DIR=C:\src\python/Include"
"-DPYTHON_EXECUTABLE=C:\src\python/PCbuild/python_d.exe"

74

Yes the comments can move. In the proposed new getLLVMSource, you would just add LLD as an additional entry to the project list

[svn-llvm, svn-clang, svn-lldb, svn-lld]

127–132

The current CMake install step doesn't copy over the python DLL, or site-packages, and there is no info inside the exe on where to find them.

Doing the install step exposes these problems. But you need to get the installed version from the buildslave manually to see there is something wrong.

In the future, adding the ability to run the installed version of LLDB through LIT would be useful.

134–140

Yes, but one of the goals of this build factory was to put in place additions so that LLDB could run tests on Windows.

[zturner]

zorg/buildbot/builders/LLDBBuilder.py

37

Ultimately I guess I just want one way of specifying the VS location. Currently it's specified twice (once when you create this factory, and once when this function is run), and it would be be possible for them to get out of sync. As long as the person creating the factory can point to a different VS location, and as long as whatever they specify gets plumbed all the way through, then it's fine.

134–140

Yes but right now most of the tests still fail and haven't yet been marked XFAIL. So it would be nice if we could leave the tests off until we get that sorted out, so that we can at least verify the tree remains green in the meantime. Otherwise the builder will just be red all the time. Also eventually we will have multiple bots, and we may want some bots that only test compilation with different settings, and then have one master configuration that runs tests. To save CPU cycles on the machine for example.

[rfoos]

Moved comments to getLLDBSource.

Replaced vs_install_dir with vs_common, and allow caller to change the value. Compromise until entire slave env command passed by caller. Caller can select VS version.

Added test, and install step flags so caller can select the additional steps. Both are default true.

If test has a value of 'ignoreFail', a test step failure will not fail the build (current default behavior).

[gkistanova]

Thanks for working on this, Rick!

If I get it right, this will be refactored once D6866 is in place. With this in mind, the patch looks good to me, assuming you will make the tests optional to reduce the immediate noise of the tests false failures.

zorg/buildbot/builders/LLDBBuilder.py

127–132

I'm second on this. We should do the formal install and use the installed result for testing.
This would help exposing missing dependencies.
With few exceptions when the install takes too long or requires too much resources on a particular slave. Though, this is not a subject of this particular patch.

134–140

Not sure about boolean, but agree in general. It should be a graceful way to skip tests for a particular builder.

[rfoos]

Ready to go.

zorg/buildbot/builders/LLDBBuilder.py

56

This allows VS versions to be selected by env variable or a hardcoded path to a place where ..\..\VC\vcvarsall.bat can be found.

I'd prefer when we refactor to allow the entire slave command to be specified here. This would replace all the function of generateVisualStudioEnvironment.

I've used the command to set a path to a 64 bit python in addition to the VS dev environment for a specific builder. A full command also leaves it compatible with linux as printenv works with this routine as well.

62

Both test and install can be disabled here.

Additionally, the test step can be marked ignoreFail'ures. This executes the tests, but ignores the failures.

All the builds will be green whether tests pass or fail. This is what we do here until experimental tests are working.

The defaults for test and install are on for now, and the builder can select if something different is needed.

[rfoos]

r226700, and r226731

URL: http://llvm.org/viewvc/llvm-project?rev=226700&view=rev
Log: [zorg] Rev1: Fix get slave environment in LLDB Windows builder.

URL: http://llvm.org/viewvc/llvm-project?rev=226731&view=rev
Log: [zorg] Fix ignorefail in r226700

zorg/buildbot/builders/LLDBBuilder.py

 import os
-import subprocess
 
 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.process.properties import WithProperties
+from buildbot.steps.slave import RemoveDirectory
+from buildbot.process.properties import WithProperties, Property
 import zorg.buildbot.commands.BatchFileDownload as batch_file_download
 from zorg.buildbot.commands.LitTestCommand import LitTestCommand
 
-def generateVisualStudioEnvironment(vs_install_dir, target_arch):
-    arch_arg = {'x86': 'x86', 'x64': 'amd64', 'amd64': 'amd64'}.get(target_arch, None)
-    if arch_arg is None:
-        return None
+def slave_env_glob2list(rc, stdout, stderr):
+    '''Extract function for SetPropertyCommand. Loads Slave Environment
+    into a dictionary, and returns slave_env property for ShellCommands.'''
+    if not rc:
+        slave_env_dict = dict(l.strip().split('=',1) 
+            for l in stdout.split('\n') if len(l.split('=',1))==2)
+    return {'slave_env': slave_env_dict}
+
+# We *must* checkout at least Clang, LLVM, and LLDB.  Once we add a step to run
+# tests (e.g. ninja check-lldb), we will also need to add a step for LLD, since
+# MSVC LD.EXE cannot link executables with DWARF debug info.
+def getLLDBSource(f,llvmTopDir='llvm'):
+    f.addStep(SVN(name='svn-llvm',
+                  mode='update', baseURL='http://llvm.org/svn/llvm-project/llvm/',
+                  defaultBranch='trunk',
+                  workdir=llvmTopDir))
+    f.addStep(SVN(name='svn-clang',
+                  mode='update', baseURL='http://llvm.org/svn/llvm-project/cfe/',
+                  defaultBranch='trunk',
+                  workdir='%s/tools/clang' % llvmTopDir))
+    f.addStep(SVN(name='svn-lldb',
+                  mode='update', baseURL='http://llvm.org/svn/llvm-project/lldb/',
+                  defaultBranch='trunk',
+                  workdir='%s/tools/lldb' % llvmTopDir))
+    return f
+
+def generateVisualStudioEnvironment(vs_common=r"%VS120COMNTOOLS%", target_arch=None):
+    arch_arg = {'x86': 'x86', 'x64': 'amd64', 'amd64': 'amd64'}.get(target_arch, '%PROCESSOR_ARCHITECTURE%')
     
-    vcvars_command = "\"" + os.path.join(vs_install_dir, 'VC', 'vcvarsall.bat') + "\""
+    vcvars_command = "\"" + os.path.join(vs_common, '..','..','VC', 'vcvarsall.bat') + "\""
     vcvars_command = "%s %s && set" % (vcvars_command, arch_arg)
-    process = subprocess.Popen(vcvars_command, shell = True, stdout=subprocess.PIPE, stderr = None)
-    (stdout, _) = process.communicate()
-    vars = stdout.splitlines()
-    env = {}
-    # At some point it may be worth trying to see if we can create an trimmed down whitelist of only
-    # those environment variables which are necessary in order for the parts of the toolchain that
-    # we need, such as the compiler and linker.
-    for var in vars:
-        keyval_pair = var.split('=')
-        key = keyval_pair[0]
-        value = keyval_pair[1]
-        env[key] = value
+    return vcvars_command
 
-    return env
-
 # CMake Windows builds
 def getLLDBWindowsCMakeBuildFactory(
-            clean=True,
+            clean=False,
             cmake='cmake',
             jobs="%(jobs)s",
 
             # Source directory containing a built python
-            python_source_dir=r'C:\src\python',
+            python_source_dir=r'C:/src/python',
 
-            # Default values for VS version and build configuration
-            vs_install_dir=r'C:\Program Files (x86)\Microsoft Visual Studio 12.0',
+            # Default values for VS devenv and build configuration
+            vs_common=r"%VS120COMNTOOLS%",
             config='Release',
             target_arch='x86',
 
-            extra_cmake_args=[]):
+            extra_cmake_args=[],
+            test='ignoreFail',
+            install=True):
 
     ############# PREPARING
     f = buildbot.process.factory.BuildFactory()
 
-    env = generateVisualStudioEnvironment(vs_install_dir, target_arch)
+    vcvars_command = generateVisualStudioEnvironment(vs_common,target_arch)
+    # Determine Slave Environment and Set MSVC environment.
+    f.addStep(SetProperty(
+        command=vcvars_command,
+        extract_fn=slave_env_glob2list))
 
-    # We *must* checkout at least Clang, LLVM, and LLDB.  Once we add a step to run
-    # tests (e.g. ninja check-lldb), we will also need to add a step for LLD, since
-    # MSVC LD.EXE cannot link executables with DWARF debug info.
-    f.addStep(SVN(name='svn-llvm',
-                  mode='update', baseURL='http://llvm.org/svn/llvm-project/llvm/',
-                  defaultBranch='trunk',
-                  workdir='llvm'))
-    f.addStep(SVN(name='svn-clang',
-                  mode='update', baseURL='http://llvm.org/svn/llvm-project/cfe/',
-                  defaultBranch='trunk',
-                  workdir='llvm/tools/clang'))
-    f.addStep(SVN(name='svn-lldb',
-                  mode='update', baseURL='http://llvm.org/svn/llvm-project/lldb/',
-                  defaultBranch='trunk',
-                  workdir='llvm/tools/lldb'))
+    f = getLLDBSource(f,'llvm')
 
     ninja_cmd=['ninja', WithProperties("-j%s" % jobs)]
 

     build_dir='build'
 
     ############# CLEANING
-    if clean:
-        f.addStep(ShellCommand(name='clean',
-                               command=['rmdir', '/S/Q', build_dir],
-                               warnOnFailure=True,
-                               description='Cleaning',
-                               descriptionDone='clean',
-                               workdir='.',
-                               env=env))
+    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
+                ))
 
     if config.lower() == 'release':
         python_lib = 'python27.lib'

                                          # Need to use our custom built version of python
                                          '-DPYTHON_LIBRARY=' + python_lib,
                                          '-DPYTHON_INCLUDE_DIR=' + python_include,
-                                         '-DPYTHON_EXECUTABLE=' + python_exe]
+                                         '-DPYTHON_EXECUTABLE=' + python_exe,
+                                         "-DCMAKE_INSTALL_PREFIX=../install"]
                                          + extra_cmake_args,
                                 workdir=build_dir))
 

                            haltOnFailure=True,
                            description='cmake gen',
                            workdir=build_dir,
-                           env=env))
+                           env=Property('slave_env')))
 
     f.addStep(WarningCountingShellCommand(name='build',
-                                          command=ninja_cmd,
-                                          haltOnFailure=True,
-                                          description='ninja build',
-                                          workdir=build_dir,
-                                          env=env))
+                          env=Property('slave_env')))
 
+    f.addStep(ShellCommand(name='install',
+                          command=[ninja_cmd,'install'],
+                          haltOnFailure=False,
+                          description='ninja install',
+                          workdir=build_dir,
+                          doStepIf=install,
+                          env=Property('slave_env')))
+
+    f.addStep(ShellCommand(name='test',
+                          command=[ninja_cmd,'check-lldb'],
+                          haltOnFailure=False,
+                          flunkOnFailure=(test == 'ignoreFail'),
+                          description='ninja test',
+                          workdir=build_dir,
+                          doStepIf=bool(test),
+                          env=Property('slave_env')))
+
     return f
 
 def getLLDBBuildFactory(