[PATCH] Build procedure for lldb project with cmake/ninja
Siva Chandra
sivachandra at google.com
Tue Feb 10 22:21:44 PST 2015
There are a bunch of style nits that I pointed out. They are optional, but I think definitely good to have. Even if the existing code is haphazardly formatted, it is good to set a precedent for future changes.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:254
@@ +253,3 @@
+def getLLDBUbuntuCMakeBuildFactory(
+ triple,
+ outOfDir=True,
----------------
I am not sure if this is relevant for CMake builds.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:298
@@ +297,3 @@
+ '%s/tools/clang' % llvm_srcdir]
+
+ f.addStep(ShellCommand(name='svn-llvm',
----------------
nit: line 287-297 can be better formatted according to PEP8.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:313
@@ +312,3 @@
+ always_purge=True,
+ workdir='%s/tools/lldb' % llvm_srcdir))
+
----------------
I do not think that the linux builders (debian, freebsd and ubuntu) are doing it right wrt getting the source. I think the windows builders are dong it right. Look at the function getLLDBSource above. The function getLLDBxcodebuildFactory below is also doing the same as getLLDBSource.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:321
@@ +320,3 @@
+ "-DCMAKE_BUILD_TYPE=Debug",
+ WithProperties("../%s" %llvm_srcdir),
+ ]
----------------
nit: Fix indentation of above lines to follow PEP8 rules.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:324
@@ +323,3 @@
+ if triple:
+ config_args += ['--build=%s' % triple]
+ config_args += extra_configure_args
----------------
As I mentioned above, I do not think this is relevant for CMake builds.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:332
@@ +331,3 @@
+ env=env,
+ workdir='%s' % llvm_builddir))
+
----------------
nit: Fix indentation of above 4 lines.
================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:338
@@ +337,3 @@
+ env=env,
+ workdir=llvm_builddir))
+ # Compile
----------------
nit: Fix indentation of above 3 lines.
http://reviews.llvm.org/D7521
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list