[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