[PATCH] [Zorg] Adding test-suite to CMake ClangBuilder

Reid Kleckner rnk at google.com
Thu Jun 4 10:11:06 PDT 2015


This seems pretty reasonable. I've never tried to run test-suite on Windows, so I wouldn't worry too much about trying to make it work here.


REPOSITORY
  rL LLVM

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:449-451
@@ -449,1 +448,5 @@
+def addSVNUpdateSteps(f,
+                      checkout_clang_tools_extra=True,
+                      checkout_compiler_rt=True,
+                      checkout_test_suite=True):
     # We *must* checkout at least Clang+LLVM
----------------
Let's just drop the =True defaults, given that the only caller is here in this file. It gives the wrong impression that most builders are going to run test-suite, when the true default is below in getClangCMakeBuildFactory.

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:686-688
@@ +685,5 @@
+        # Get generated python, lnt
+        python=WithProperties('%(workdir)s/test/sandbox/bin/python')
+        lnt=WithProperties('%(workdir)s/test/sandbox/bin/lnt')
+        lnt_setup=WithProperties('%(workdir)s/test/lnt/setup.py')
+        # Paths
----------------
These and other assignments should probably have space separation, since they aren't kwargs.

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:693-694
@@ +692,4 @@
+        # Get latest built Clang (stage1 or stage2)
+        cc=WithProperties('%(workdir)s/'+compiler_path+'/bin/'+cc)
+        cxx=WithProperties('%(workdir)s/'+compiler_path+'/bin/'+cxx)
+        # LNT Command line
----------------
I think ultimately the test suite will want the path to the gcc-compatible clang driver and not clang-cl. If and when we port it to Windows, we can figure out how to get clang-cl if we need it then. I'd suggest sinking the cc and cxx assignments above into the `if useTwoStage:` block and just using clang and clang++ here.

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:708-714
@@ +707,9 @@
+        # Only submit if a URL has been specified
+        if submitURL is not None:
+          if type(submitURL) != type([]):
+            submitURL = [submitURL]
+          for url in submitURL:
+            test_suite_cmd.extend(['--submit', url])
+          if testerName:
+            test_suite_cmd.extend(['--no-machdep-info', testerName])
+        # CC and CXX are needed as env for build-tools
----------------
This file appears to use four-space indentation, so let's stick with that.

================
Comment at: zorg/buildbot/builders/ClangBuilder.py:709
@@ +708,3 @@
+        if submitURL is not None:
+          if type(submitURL) != type([]):
+            submitURL = [submitURL]
----------------
`isinstance(submitURL, list)` would probably be more readable.

http://reviews.llvm.org/D10244

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






More information about the llvm-commits mailing list