[PATCH] [zorg] Support Windows/Linux Clang CMake builds with unified builder.

[Galina]

REPOSITORY
  rL LLVM

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:513
@@ +512,3 @@
+'build_cmd': ['ninja'], Build source code command.
+build_jobs: '-j%(jobs)s -l%(loadaverage)', job control if supported.
+'build_dir': 'stage1.build',
----------------
It looks like you have changed your mind and have a separate flag for the loadaverage. Could you update the description to address this, please?


================
Comment at: zorg/buildbot/builders/ClangBuilder.py:540
@@ +539,3 @@
+            ],
+            update=[] # Update each stage [{},...]
+            ):
----------------
You do not really need an empty array here, do you? None would do.


================
Comment at: zorg/buildbot/builders/ClangBuilder.py:611
@@ +610,3 @@
+        if 'usePreviousStageResults' in stage:
+            built_clang_dir = stage.get('usePreviousStageResults','stage1.install')
+            # CMake escapes these variables. Windows \ is fatal.
----------------
'stage1.install' is not always a previous stage.
We either

1. require the previous stage to do the install and use that as the path (in this case usePreviousStageResults is just a flag), or

2. always require a valid path in usePreviousStageResults value without providing any default value.

I'm in favor of # 1, but either would do.


================
Comment at: zorg/buildbot/builders/ClangBuilder.py:636
@@ +635,3 @@
+        if 'build_loadaverage' in stage:
+            build_cmd.append(WithProperties(stage['build_loadaverage']))
+        f.addStep(WarningCountingShellCommand(name=stageName+': build',
----------------
This looks inconsistent with the usage of -l option in build_jobs, etc. as described in the doc strings.
Did you change your mind in favor of having a separate 'build_loadaverage' parameter and forgot to change the description above?


================
Comment at: zorg/buildbot/builders/ClangBuilder.py:660
@@ +659,3 @@
+                        flunkOnFailure=ignoreTestFail,
+                        env=env))
+
----------------
Setting all 3 flags (haltOnFailure, warnOnFailure, and flunkOnFailure) to the same value doesn't look right.


================
Comment at: zorg/buildbot/builders/ClangBuilder.py:663
@@ +662,3 @@
+        ############# STAGE 2
+        if 'usePreviousStageResults' or 'install' in stage:
+            install_cmd = stage.get('install_cmd',['ninja', 'install'])
----------------
It doesn't seem the usePreviousStageResults for this stage should require installation step.
I can see if you would check the usePreviousStageResults of the next stage for this purpose, though.

http://reviews.llvm.org/D6866

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