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

Ying Chen chying at google.com
Wed Apr 29 16:53:53 PDT 2015


> 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. 
Using lists makes it slightly more complicated when there're single/double quotes in the command. 
I will try to convert strings to lists in the next revision.


================
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',
----------------
sivachandra wrote:
> I think time has come to add doc strings describing what these args are and how they should be formatted.
Will do this in next revision.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:475
@@ +474,3 @@
+# for cmake and compile
+def getLLDBCmakeCompileSteps(f,
+                             build_compiler,
----------------
sivachandra wrote:
> 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.
This function contains both steps, because cmake and compilation step will always be called together for local configuration first, and then once for each target platform. If compilation step is decoupled, then we need to have the same code for compilation step in multiple places.

================
Comment at: zorg/buildbot/builders/LLDBBuilder.py:528
@@ -431,1 +527,3 @@
+                                          haltOnFailure=True,
+                                          workdir=llvm_builddir))
     return f
----------------
sivachandra wrote:
> 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".
Same as above, these two steps are combined because they will be called together.
It's good to add ninja_targets.

http://reviews.llvm.org/D9354

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






More information about the llvm-commits mailing list