[Zorg] Adding test-suite to CMake ClangBuilder

[rengolin]

Summary

This patch allows running the test-suite on the CMake version of the
Clang builder. Stage2 builds will be used built. This should not change
any of the current builders' logic.

[rnk]

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.

zorg/buildbot/builders/ClangBuilder.py

449–451

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.

686–688

These and other assignments should probably have space separation, since they aren't kwargs.

693–694

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.

708–714

This file appears to use four-space indentation, so let's stick with that.

709

isinstance(submitURL, list) would probably be more readable.

[gkistanova]

Hello Renato,

It seems you just need to build the bootstrapped clang and use it to run the test-suite, right?
May I suggest subclassing the current ClangCMakeBuildFactory instead of adding parameters and more complicated logic?

You can move the getClangCMakeBuildFactory build steps creation to a separate method (similar to addSVNUpdateSteps), call it with the request for 2 stages, and then add whatever it takes to build and run the test-suite in your ClangCMakeAndTestsuiteBuildFactory(?).

Should be more clear code after all.

What do you think?

[rengolin]

Hi Reid,

Thanks for the comments, I'll make the changes.

Galina,

I'll try to split the methods, but I'm very deficient in Python. Also, there is a parallel move to make that work better on Windows, and I don't want to move it too much away from what that merge request is coming from.

cheers,
--renato

[rengolin]

Galina,

About splitting the code, there are some factors to consider:

1. The test-suite needs some internal knowledge of what was the last build (stage1/2) and what is the compiler used. I can refactor the compiler name out, but I'm not sure how to do the same about what stage directory to use without leaking internal information through (ie. install directory name).
2. This builder will do more than just test. The goal is to have it building with and without all variations of libraries and tools (libgcc vs. compiler-rt, libstdc++ vs. libc++, ld vs. lld, stage1 vs. stage2, etc), and separating them apart may create a lot of interdependency between them (see item 1 above).

So, unless we do a big refactoring to the code, I'm not sure any effort in splitting it would do justice, or make it simpler. Mostly because I'm not great at Python to do it right...

If someone could help with the refactoring, I'd be glad to follow the pattern, but I'm afraid I can't create the pattern myself... :(

zorg/buildbot/builders/ClangBuilder.py

693–694

So, this cc/cxx depends on which was the last stage (1 or 2), while the previous usage is constant on stage1 build.

If I sink this before runTestSuite, I'll have to sink the other set, and I'll still have to reassign it here for the stage2 if necessary, duplicating the "if useTwoStage" conditional.

I'm not sure how keeping them separate could hurt checking for the correct compiler more than it already was. Ie. I don't really understand what the problem is, and I don't want to try to fix a problem I myself don't understand. By keeping it similar, it would be simpler for someone that understands the problem better to fix in the future.

[rengolin]

Addressing some cosmetic issues from reviews. Still in one Factory.

[rengolin]

Using isinstance instead of comparing the type.

Reid, Galina, what do you think about keeping this one simple and re-factoring into multiple functions later?

[rnk]

I'm OK leaving this as one function.

zorg/buildbot/builders/ClangBuilder.py

686–688

These assignments and others are still not using spaces. I think it bothers me because when you do it like this I assume it's part of a sequence of kwargs and I go looking for a function call to match it up with.

[rengolin]

Ok, I'll leave it as it is and check with Rick what he prefers for the Windows part. We can refactor that out later. Thanks!

zorg/buildbot/builders/ClangBuilder.py

686–688

Now I get what you mean by this "not being kwargs"... :)

I'll change those assignments and check for others in a subsequent commit, to make sure I'm not adding a lot of cosmetic unrelated changes.

[gkistanova]

If you call this simple. :)

Fine with later re-factoring.
LGTM, assuming you will fix the cosmetic things, Reid pointed to.

[rengolin]

Thanks! r239928

I agree splitting it is the best option, I just need time to do it properly. :)

zorg/buildbot/builders/ClangBuilder.py

if package_dst:
                                workdir=".",
                                warnOnFailure=True,
                                flunkOnFailure=False,
                                haltOnFailure=False,
                                env=merged_env))
 
     return f
 
-def addSVNUpdateSteps(f, checkout_clang_tools_extra=True, checkout_compiler_rt=True):
+def addSVNUpdateSteps(f,
+                      checkout_clang_tools_extra,
+                      checkout_compiler_rt,
+                      checkout_test_suite):
     # We *must* checkout at least Clang+LLVM
     f.addStep(SVN(name='svn-llvm',
                   mode='update', baseURL='http://llvm.org/svn/llvm-project/llvm/',
                   defaultBranch='trunk',
                   workdir='llvm'))
     f.addStep(SVN(name='svn-clang',
                   mode='update', baseURL='http://llvm.org/svn/llvm-project/cfe/',
                   defaultBranch='trunk',
                   workdir='llvm/tools/clang'))
 
     # Extra stuff that will be built/tested
     if checkout_clang_tools_extra:
         f.addStep(SVN(name='svn-clang-tools-extra',
                       mode='update', baseURL='http://llvm.org/svn/llvm-project/clang-tools-extra/',
                       defaultBranch='trunk',
                       workdir='llvm/tools/clang/tools/extra'))
     if checkout_compiler_rt:
         f.addStep(SVN(name='svn-compiler-rt',
                       mode='update', baseURL='http://llvm.org/svn/llvm-project/compiler-rt/',
                       defaultBranch='trunk',
                       workdir='llvm/projects/compiler-rt'))
+    if checkout_test_suite:
+        f.addStep(SVN(name='svn-lnt',
+                      mode='update', baseURL='http://llvm.org/svn/llvm-project/lnt/',
+                      defaultBranch='trunk',
+                      workdir='test/lnt'))
+        f.addStep(SVN(name='svn-test-suite',
+                      mode='update', baseURL='http://llvm.org/svn/llvm-project/test-suite/',
+                      defaultBranch='trunk',
+                      workdir='test/test-suite'))
 
 
 def getClangCMakeBuildFactory(
             clean=True,
             test=True,
             cmake='cmake',
             jobs=None,
 
             # VS tools environment variable if using MSVC. For example,
             # %VS120COMNTOOLS% selects the 2013 toolchain.
             vs=None,
             vs_target_arch='x86',
 
             # Multi-stage compilation
             useTwoStage=False,
             testStage1=True,
             stage1_config='Release',
             stage2_config='Release',
 
+            # Test-suite
+            runTestSuite=False,
+            nt_flags=[],
+            submitURL=None,
+            testerName=None,
+
             # Environmental variables for all steps.
             env={},
             extra_cmake_args=[],
 
             # Extra repositories
             checkout_clang_tools_extra=True,
             checkout_compiler_rt=True):
 
     ############# PREPARING
     f = buildbot.process.factory.BuildFactory()
 
-    addSVNUpdateSteps(f, checkout_clang_tools_extra=checkout_clang_tools_extra,
-                      checkout_compiler_rt=checkout_compiler_rt)
+    addSVNUpdateSteps(f,
+                      checkout_clang_tools_extra=checkout_clang_tools_extra,
+                      checkout_compiler_rt=checkout_compiler_rt,
+                      checkout_test_suite=runTestSuite)
 
     # If jobs not defined, Ninja will choose a suitable value
     jobs_cmd=[]
     lit_args="'-v"
     if jobs is not None:
         jobs_cmd=["-j"+str(jobs)]
         lit_args+=" -j"+str(jobs)+"'"
     else:
         lit_args+="'"
     ninja_cmd=['ninja'] + jobs_cmd
     ninja_install_cmd=['ninja', 'install'] + jobs_cmd
     ninja_check_cmd=['ninja', 'check-all'] + jobs_cmd
     check_build_cmd = ["sh", "-c",
                        "test -e build.ninja && echo OK || echo Missing"]
     if vs:
         check_build_cmd = ["cmd", "/c", "if exist build.ninja (echo OK) " +
                            " else (echo Missing & exit 1)"]
 
     # Global configurations
     stage1_build='stage1'
     stage1_install='stage1.install'
     stage2_build='stage2'
+    stage2_install='stage2.install'
 
     # Set up VS environment, if appropriate.
     if vs:
         f.addStep(SetProperty(
             command=builders_util.getVisualStudioEnvironment(vs, vs_target_arch),
             extract_fn=builders_util.extractSlaveEnvironment))
         assert not env, "Can't have custom builder env vars with VS"
         env = Property('slave_env')
f.addStep(WarningCountingShellCommand(name='build stage 1',
                                           haltOnFailure=True,
                                           description='ninja all',
                                           workdir=stage1_build,
                                           env=env))
 
     if test and testStage1:
         f.addStep(lit_test_command.LitTestCommand(name='ninja check 1',
                                    command=ninja_check_cmd,
+                                   haltOnFailure=not runTestSuite,
                                    description=["checking stage 1"],
                                    descriptionDone=["stage 1 checked"],
                                    workdir=stage1_build,
                                    env=env))
 
-    if not useTwoStage:
-        return f
-
-    ############# STAGE 2
+    if useTwoStage or runTestSuite:
         f.addStep(ShellCommand(name='install stage 1',
                                command=ninja_install_cmd,
                                description='ninja install',
                                workdir=stage1_build,
                                env=env))
 
+    # Compute the cmake define flag to set the C and C++ compiler to clang. Use
+    # clang-cl if we used MSVC for stage1.
+    if not vs:
+        cc = 'clang'
+        cxx = 'clang++'
+    else:
+        cc = 'clang-cl.exe'
+        cxx = 'clang-cl.exe'
+
+
+    ############# STAGE 2
+    if useTwoStage:
         if clean:
             f.addStep(ShellCommand(name='clean stage 2',
                                    command=['rm','-rf',stage2_build],
                                    warnOnFailure=True,
                                    description='cleaning stage 2',
                                    descriptionDone='clean',
                                    workdir='.',
                                    env=env))
         else:
             f.addStep(SetProperty(name="check ninja files 2",
                                   workdir=stage2_build,
                                   command=check_build_cmd,
                                   flunkOnFailure=False,
                                   property="exists_ninja_2"))
 
-    # Compute the cmake define flag to set the C and C++ compiler to clang. Use
-    # clang-cl if we used MSVC for stage1.
-    if not vs:
-        cc = 'clang'
-        cxx = 'clang++'
-    else:
-        cc = 'clang-cl.exe'
-        cxx = 'clang-cl.exe'
-
         # Set the compiler using the CC and CXX environment variables to work around
         # backslash string escaping bugs somewhere between buildbot and cmake. The
         # env.exe helper is required to run the tests, so hopefully it's already on
         # PATH.
         cmake_cmd2 = ['env',
                       WithProperties('CC=%(workdir)s/'+stage1_install+'/bin/'+cc),
                       WithProperties('CXX=%(workdir)s/'+stage1_install+'/bin/'+cxx),
                       cmake, "-G", "Ninja", "../llvm",
                       "-DCMAKE_BUILD_TYPE="+stage2_config,
                       "-DLLVM_ENABLE_ASSERTIONS=True",
-                  "-DLLVM_LIT_ARGS="+lit_args] + extra_cmake_args
+                      "-DLLVM_LIT_ARGS="+lit_args,
+                      "-DCMAKE_INSTALL_PREFIX=../"+stage2_install] + extra_cmake_args
 
         f.addStep(ShellCommand(name='cmake stage 2',
                                command=cmake_cmd2,
                                haltOnFailure=True,
                                description='cmake stage 2',
                                workdir=stage2_build,
                                doStepIf=lambda step: step.build.getProperty("exists_ninja_2") != "OK",
                                env=env))
 
         f.addStep(WarningCountingShellCommand(name='build stage 2',
                                               command=ninja_cmd,
                                               haltOnFailure=True,
                                               description='ninja all',
                                               workdir=stage2_build,
                                               env=env))
 
         if test:
             f.addStep(lit_test_command.LitTestCommand(name='ninja check 2',
                                        command=ninja_check_cmd,
+                                       haltOnFailure=not runTestSuite,
                                        description=["checking stage 2"],
                                        descriptionDone=["stage 2 checked"],
                                        workdir=stage2_build,
                                        env=env))
 
+    ############# TEST SUITE
+    ## Test-Suite (stage 2 if built, stage 1 otherwise)
+    if runTestSuite:
+        compiler_path=stage1_install
+        if useTwoStage:
+            compiler_path=stage2_install
+            f.addStep(ShellCommand(name='install stage 2',
+                                   command=ninja_install_cmd,
+                                   description='ninja install 2',
+                                   workdir=stage2_build,
+                                   env=env))
+
+        # 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
+        sandbox=WithProperties('%(workdir)s/test/sandbox')
+        test_suite_dir=WithProperties('%(workdir)s/test/test-suite')
+
+        # 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
+        if jobs is None:
+            jobs=1
+        test_suite_cmd = [python, lnt, 'runtest', 'nt',
+                          '-j'+str(jobs),
+                          '--no-timestamp',
+                          '--sandbox', sandbox,
+                          '--test-suite', test_suite_dir,
+                          '--cc', cc,
+                          '--cxx', cxx]
+        # Append any option provided by the user
+        test_suite_cmd.extend(nt_flags)
+        # Only submit if a URL has been specified
+        if submitURL is not None:
+            if not isinstance(submitURL, list):
+                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
+        test_suite_env=env
+        test_suite_env['CC'] = cc
+        test_suite_env['CXX'] = cxx
+
+        # Steps to prepare, build and run LNT
+        f.addStep(ShellCommand(name='clean sandbox',
+                               command=['rm', '-rf', 'sandbox'],
+                               haltOnFailure=True,
+                               description='removing sandbox directory',
+                               workdir='test',
+                               env=env))
+        f.addStep(ShellCommand(name='recreate sandbox',
+                               command=['virtualenv', 'sandbox'],
+                               haltOnFailure=True,
+                               description='recreating sandbox',
+                               workdir='test',
+                               env=env))
+        f.addStep(ShellCommand(name='setup lit',
+                               command=[python, lnt_setup, 'develop'],
+                               haltOnFailure=True,
+                               description='setting up LNT in sandbox',
+                               workdir='test/sandbox',
+                               env=env))
+        f.addStep(commands.LitTestCommand.LitTestCommand(
+                               name='test-suite',
+                               command=test_suite_cmd,
+                               haltOnFailure=True,
+                               description=['running the test suite'],
+                               workdir='test/sandbox',
+                               logfiles={'configure.log'   : 'build/configure.log',
+                                         'build-tools.log' : 'build/build-tools.log',
+                                         'test.log'        : 'build/test.log',
+                                         'report.json'     : 'build/report.json'},
+                               env=test_suite_env))
+
     return f
 
 def getClangMSVCBuildFactory(update=True, clean=True, vcDrive='c', jobs=1, cmake=r"cmake"):
     f = buildbot.process.factory.BuildFactory()
 
     if update:
         f.addStep(SVN(name='svn-llvm',
                       mode='update', baseURL='http://llvm.org/svn/llvm-project/llvm/',