[PATCH] Create an experimental Windows LLDB Builder

Galina gkistanova at gmail.com
Tue Jan 6 15:59:44 PST 2015


Hello Zachary,

Thanks for working on this!

It is almost there. Please see my comments inline above.
Feel free to commit with the fixed ## 1 and 4.

Don't worry about having a local master. I'll stage the changes and will let you know if anything would go wrong. I feel much more comfortable with the current patch.


================
Comment at: buildbot/osuosl/master/config/slaves.py:174
@@ -172,1 +173,3 @@
+        # Windows Server 2008 R2, Quad 2.6GHz Intel Xeon(R) 4GB RAM
+        create_slave("zturner-win2008", properties={'jobs': 4}, max_builds=1),
         # Defunct.
----------------
The 'jobs' property is not used by the factory. See the note # 4 below.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:28
@@ +27,3 @@
+        value = keyval_pair[1]
+        env[key] = value
+
----------------
1. Having all the env vars from a slave could be too much and noisy in the logs. How about white list only what you really need? This is not a show stopper, though.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:39
@@ +38,3 @@
+            # Source directory containing a built python
+            python_source_dir='C:\\src\\python\\',
+
----------------
2. Could you use raw strings here and everywhere else for Windows paths with dir separators, please? Like r'C:\src\python\'.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:45
@@ +44,3 @@
+            target_arch='x86',
+
+            extra_cmake_args=[]):
----------------
3. Maybe add an optional env param here and support optional env property propagated from a slave and builder definitions to merge with the envs you are requesting from the slave?

In some cases it might be needed to define something in addition.

Not a how stopper and could wait till someone would really need the feature. Just be nice to have it.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:72
@@ +71,3 @@
+    if jobs is not None:
+        jobs_cmd=["-j"+str(jobs)]
+    ninja_cmd=['ninja'] + jobs_cmd
----------------
4. Here you are missing the 'jobs' property defined with the slave. You need to reconcile the 'jobs' param with the 'jobs' property before building the cmd string.

http://reviews.llvm.org/D6745

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






More information about the llvm-commits mailing list