[PATCH] Add support for running lldb-test on android targets

[Siva Chandra]

Hi,

This change is fairly large that I need time to digest all of it. I have few preliminary nitty comments. Addressing them might help me digest faster.

A general comment is that commands being specified as strings and not lists. We should do this only if it is unavoidable, for example on LLDBBuilder.py:368. I should have pointed this out during the review of http://reviews.llvm.org/D8711.


================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:416
@@ -359,4 +415,3 @@
                                    test_compilers=None,
-                                   remote_test_archs=None,
-                                   remote_test_compilers=None,
+                                   remote_configs=None,
                                    jobs='%(jobs)s',
----------------
I think time has come to add doc strings describing what these args are and how they should be formatted.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:475
@@ +474,3 @@
+# for cmake and compile
+def getLLDBCmakeCompileSteps(f,
+                             build_compiler,
----------------
I think this function can be named getLLDBAndroidCmakeStep and decoupled with compilation step. Non-Android cmake step can be as before. See my comments below.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:504
@@ +503,3 @@
+            cmake_args.append("-DCMAKE_CXX_COMPILER=g++")
+
+    elif target_platform is 'android':
----------------
I know this is copied over code, but I think it is better done this way:

    cmake_args.append("-DCMAKE_C_COMPILER=%s" % getCCompilerCmd(build_compiler))
    cmake_args.append("-DCMAKE_CXX_COMPILER=%s" % getCxxCompilerCmd(build_compiler))

The get functions as follows:

  def getCCompilerCmd(compiler):
    if compiler == "clang":
      return "clang"
    elif compiler == "gcc":
      return "gcc"

  def getCxxCompilerCmd(compiler):
    if compiler == "clang":
      return "clang++"
    elif compiler == "gcc":
      return "gcc++"

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:528
@@ -431,1 +527,3 @@
+                                          haltOnFailure=True,
+                                          workdir=llvm_builddir))
     return f
----------------
Looks like the only reason config step and compile step are combined in this function is because of |extra_ninja_cmd|. If you treat |extra_ninja_cmd| as |ninja_targets|, then this will not be required. As in, linux builds take no targets (implying all targets), while android builds take one target named "lldb-server".

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:531
@@ -432,2 +530,3 @@
 
+
 def getLLDBxcodebuildFactory(use_cc=None):
----------------
+1 wrt PEP8, but I think it is not following the style in this file.

http://reviews.llvm.org/D9354

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