[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