[PATCH] Create an experimental Windows LLDB Builder

Rick Foos rfoos at codeaurora.org
Wed Jan 7 13:21:23 PST 2015


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:14
@@ +13,3 @@
+def generateVisualStudioEnvironment(vs_install_dir, target_arch):
+    arch_arg = {'x86': 'x86', 'x64': 'amd64', 'amd64': 'amd64'}.get(target_arch, None)
+    if arch_arg is None:
----------------
I think these are intended to define a 32 or 64 bit host executable for X86 or ARM.

They not to define an LLVM cross compiler target arch (i.e. AARCH64, Hexagon...)

To match the build host, vcvarsall %PROCESSOR_ARCHITECTURE%

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:21
@@ +20,3 @@
+    process = subprocess.Popen(vcvars_command, shell = True, stdout=subprocess.PIPE, stderr = None)
+    (stdout, _) = process.communicate()
+    vars = stdout.splitlines()
----------------
Why not use buildbot's SetProperty? It has a facility for returning stdout. This seems like more work to support.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:28
@@ +27,3 @@
+        value = keyval_pair[1]
+        env[key] = value
+
----------------
gkistanova wrote:
> zturner wrote:
> > gkistanova wrote:
> > > 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.
> > I thought about that, but it's difficult to know exactly which ones we need.  There are some that are obvious, like include paths, library paths, etc, but some that are not so obvious.  In theory we just need to be able find cl.exe, link.exe, and include / library paths, but the use of many of the other variables is kind of mysterious and not well documented by Microsoft, so it's not clear if we will trigger their access somehow.  
> > 
> > In other words, I can come up with a whitelist but I would not be 100% confident that it would be accurate.
> Agree. This could be time consuming. Adding a FIXME: with the description of the issue would do for now. 
I'm not sure the env is so much longer using this method. The existing slave environment on Windows is quite long, and many of the variables are needed for other purposes.

Once you have the slave environment into a dictionary you can define merge and strip functions. 

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:39
@@ +38,3 @@
+            # Source directory containing a built python
+            python_source_dir='C:\\src\\python\\',
+
----------------
zturner wrote:
> gkistanova wrote:
> > 2. Could you use raw strings here and everywhere else for Windows paths with dir separators, please? Like r'C:\src\python\'.
> Unfortunately it seems like a raw string cannot end with a single backslash.  This might be a bug in python, but I tested it out.  r'C:\src\python\' is an error, and r'C:\src\python\\' actually puts two backslashes at the end.
> 
> I can probably change it to not include any backslashes at the end though, and then just make sure that the case of 0 backslashes and 1 backslash are both correctly handled in the function.
I second the motion for 0 trailing backslashes. I've not had a case where this has been a problem to date.

>>> x=r"""c:\asdf\\"""
>>> print x
c:\asdf\\

I think it's a bug too, and that I would have no luck in getting it accepted by the python community :)

Since you have to quote all the paths in windows, this appears to work
>>> x=r""""c:\asdf\ fdas \""""
>>> print x
"c:\asdf\ fdas \"

But I think something else that escapes this string will mess it up later.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:45
@@ +44,3 @@
+            target_arch='x86',
+
+            extra_cmake_args=[]):
----------------
gkistanova wrote:
> 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.
I'm suggesting a generic way to invoke VCVARSALL.BAT that works on any buildslave. 

This is for a 64 bit MSVC build, using a 64 bit version of Python.
slavenvCmd=r"""PATH=C:\Python2764;C:\Python2764\Scripts;%PATH% & "%VS120COMNTOOLS%\..\..\VC\vcvarsall.bat" amd64 & set""",

(As part of this, I have to admit that I don't have a generic method for identifying a 64 bit python needed for 64 bit builds of LLDB)


If you have multiple MSVC installations, these variables provide a reliable index to vcvarsall.bat. The locations are installation dependant, and the values can be different for each buildslave.

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
\
ATM, MSVC 2013 is all that works. As it was in the past, MSVC 2014/2015 will become an option soon.



side note, VSINSTALLDIR isn't defined until after vcvarsall is run and a specific version selected.



================
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
----------------
gkistanova wrote:
> 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.
I'm having the same bug, but can we add loadaverage?

I have fast servers, and they overload themselves, keep going and plow through builds with 140+ loadaverages (32 being 100% utilization)

http://reviews.llvm.org/D6745

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






More information about the llvm-commits mailing list