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

Zachary Turner zturner at google.com
Wed Jan 21 09:36:48 PST 2015


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)
----------------
rfoos wrote:
> 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
> 
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.

================
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')))
+
----------------
rfoos wrote:
> 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.
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.

http://reviews.llvm.org/D7077

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






More information about the llvm-commits mailing list