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

[Rick Foos]

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


REPOSITORY
  rL LLVM

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:37
@@ -14,1 +36,3 @@
+
+def generateVisualStudioEnvironment(vs_common=r"%VS120COMNTOOLS%", target_arch=None):
     arch_arg = {'x86': 'x86', 'x64': 'amd64', 'amd64': 'amd64'}.get(target_arch, None)
----------------
zturner wrote:
> 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.
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


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:53
@@ -41,3 +52,3 @@
             # Source directory containing a built python
-            python_source_dir=r'C:\src\python',
+            python_source_dir=r'C:/src/python',
 
----------------
zturner wrote:
> While it will probably work either way, any reason why this was necessary?
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" 


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:74
@@ -58,14 +73,3 @@
     # 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')
 
----------------
zturner wrote:
> Can you move the comment inside the function?
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]

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:127-132
@@ -123,1 +126,8 @@
 
+    f.addStep(ShellCommand(name='install',
+                          command=[ninja_cmd,'install'],
+                          haltOnFailure=False,
+                          description='ninja install',
+                          workdir=build_dir,
+                          env=Property('slave_env')))
+
----------------
zturner wrote:
> 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.
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.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:134-140
@@ +133,9 @@
+
+    f.addStep(ShellCommand(name='test',
+                          command=[ninja_cmd,'check-lldb'],
+                          haltOnFailure=False,
+                          flunkOnFailure=False,
+                          description='ninja test',
+                          workdir=build_dir,
+                          env=Property('slave_env')))
+
----------------
zturner wrote:
> Can getLLDBWindowsCMAKEBuildFactory take a boolean that says whether to run tests?
Yes, but one of the goals of this build factory was to put in place additions so that LLDB could run tests on Windows.

http://reviews.llvm.org/D7077

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/