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

[rfoos]

Summary

Fixes for lldb-x86-windows-msvc

Change vcvars_command to use '\\' to join paths.
Change vs_common default to r"""...""" string.

Fixes for lldb-x86-win7-msvc

Change ninja_cmd to unique build, install, and check commands.
Use a list append to attach job control property variable.
Add ignoreInstallFail
Add warnings and halt to ignore fail actions.

[gkistanova]

zorg/buildbot/builders/LLDBBuilder.py

42

os.path.join runs in the context of the buildmaster, which is MacOS for the LLVM buildbot.
Having the directory separator defined explicitly for Windows is the right fix for this case.

56

Maybe usage of ' vs. " would make it more readable?

139

Setting all 3 mutually exclusive flags (haltOnFailure, warnOnFailure, and flunkOnFailure) to the same value doesn't look right.

You want only one of these be true, right?

[zturner]

This looks good to me after Galina's suggestion about the 3 mutually exclusive values. Is this ready to go otherwise?

[rfoos]

Yes, the three comments are good to implement.

The _jobs variables will be renamed jobcontrol, default for ninja, -j%(jobs)s.
The caller, knowing the buildslave properties, can add loadaverage, or the different syntax for msbuild (/maxcpucount:%(jobs)s).

in the original self hosted builder, the first step of stage2 was an install in stage1. I did this by specifying an install step, so the caller had to remember to stitch this together in both stages.

By adding an iterator on the stage list, it becomes possible to look in the next stage to see if you need to do the install step in the first stage. There are probably other use cases for lookahead, or look previous stages.

The before and after stage operations will be done by functions so the command itself can just return a flag in an if statement.

Then I think its ready.

[rfoos]

Testing now.

zorg/buildbot/builders/LLDBBuilder.py

56

changing vs_common to vs.

In the common builder, this will become an entire command executed on the slave to set up whatever environment you want.

83

Since this is windows, no loadaverage, I'll leave jobs alone here.

139

Yes, dropping to flunkOnFailure is correct.

When I had haltOnFailure in as well, then warnOnFailure triggered, so then I added all three.

The right answer is flunk on failure only.

[rfoos]

vs_common -> vs
ignoreFail: remove halt and warn OnFailure.
default is no install or test steps.

This should detect compile problems, and be a platform for finishing the lit tests for windows.

[gkistanova]

Looks good to me.

I'd prefer using NinjaCommand instead of building a shell command for ninja, but this is not critical for this patch.

[rfoos]

Yes, NinjaCommand looks interesting.

It looks like a wrapper for WarningCountingShell. If it could handle make and msbuild, it would cover all current cases.

I'm not quite sure how an override to env gets passed to NinjaCommand.

Thanks for looking at this! Time to make some builders green.

zorg/buildbot/builders/LLDBBuilder.py

                   workdir='%s/tools/lldb' % llvmTopDir))
     return f
 
-def generateVisualStudioEnvironment(vs_common=r"%VS120COMNTOOLS%", target_arch=None):
+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_common, '..','..','VC', 'vcvarsall.bat') + "\""
-    vcvars_command = "%s %s && set" % (vcvars_command, arch_arg)
+    vcvars_command = "\"" + r"\\".join((vs_common, '..','..','VC', 'vcvarsall.bat')) + "\""
+    vcvars_command = r"%s %s && set" % (vcvars_command, arch_arg)
     return vcvars_command
 
 # CMake Windows builds

             python_source_dir=r'C:/src/python',
 
             # Default values for VS devenv and build configuration
-            vs_common=r"%VS120COMNTOOLS%",
+            vs_common=r"""%VS120COMNTOOLS%""",
             config='Release',
             target_arch='x86',
 
             extra_cmake_args=[],
             test='ignoreFail',
-            install=True):
+            install='ignoreFail'):
 
     ############# PREPARING
     f = buildbot.process.factory.BuildFactory()

 
     f = getLLDBSource(f,'llvm')
 
-    ninja_cmd=['ninja', WithProperties("-j%s" % jobs)]
+    build_cmd=['ninja']
+    install_cmd = ['ninja','install']
+    test_cmd = ['ninja','check-lldb']
 
+    if jobs:
+        build_cmd.append(WithProperties("-j%s" % jobs))
+        install_cmd.append(WithProperties("-j%s" % jobs))
+        test_cmd.append(WithProperties("-j%s" % jobs))
+
     # Global configurations
     build_dir='build'
 

                            env=Property('slave_env')))
 
     f.addStep(WarningCountingShellCommand(name='build',
-                          command=ninja_cmd,
+                          command=build_cmd,
                           haltOnFailure=True,
                           description='ninja build',
                           workdir=build_dir,
                           env=Property('slave_env')))
 
+    ignoreInstallFail = bool(install != 'ignoreFail')
     f.addStep(ShellCommand(name='install',
-                          command=[ninja_cmd,'install'],
-                          haltOnFailure=False,
+                          command=install_cmd,
+                          haltOnFailure=ignoreInstallFail,
+                          warnOnFailure=ignoreInstallFail,
+                          flunkOnFailure=ignoreInstallFail,
                           description='ninja install',
                           workdir=build_dir,
-                          doStepIf=install,
+                          doStepIf=bool(install),
                           env=Property('slave_env')))
 
+    ignoreTestFail = bool(test != 'ignoreFail')
     f.addStep(ShellCommand(name='test',
-                          command=[ninja_cmd,'check-lldb'],
-                          haltOnFailure=False,
-                          flunkOnFailure=bool(test != 'ignoreFail'),
+                          command=test_cmd,
+                          haltOnFailure=ignoreTestFail,
+                          warnOnFailure=ignoreTestFail,
+                          flunkOnFailure=ignoreTestFail,
                           description='ninja test',
                           workdir=build_dir,
                           doStepIf=bool(test),