Allow users to force clean builds from the web

[rnk]

Summary

From time to time, buildbots may go red because the build directory
contains some bad intermediate state. For example, the CMakeCache.txt
file may be stale. Before this change, the only way to fix such a bot
was to contact the admin and wait for them to fix it. Now, users can
manually request clean builds without the help of a buildbot admin.

Also, clean the installation directories if we're doing a clean build.
Otherwise our packages uploaded to GCS may contain stale resource
directories or old executables that have been renamed.

[gkistanova]

Hello Reid,

Thanks for refactoring the ClangBuilder.

Unless I'm missing something, there are 3 changes in this patch:

1. Clean property usage (in the title of this review item),
2. Refactoring condition when a stage 1 build should install the results,
3. Cleaning installed files and refactoring a bit the build files clean step on stage 2.

What do you think about splitting this to 3 patches?

zorg/buildbot/builders/ClangBuilder.py

633

Maybe using RemoveDirectory is more compatible? This command wouldn't work well on Windows targets.

654

You can skip this step and use FileDoesNotExist from zorg.buildbot.conditions.FileConditions in doStepIf of that cmake command instead.

zorg/buildbot/builders/ClangBuilder.py

if vs:
                            " else (echo Missing & exit 1)"]
 
     # Global configurations
     stage1_build = 'stage1'
     stage1_install = 'stage1.install'
     stage2_build = 'stage2'
     stage2_install = 'stage2.install'
 
+    # The installation step can be skipped if we aren't doing a multi-stage
+    # build, an upload, or an LNT test.
+    should_install_stage1 = (useTwoStage or runTestSuite or
+                             stage1_upload_directory)
+
     # 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')
 
+    # Allow users to force clean builds from the builder page by setting the
+    # "clean" property.
+    clean_build_requested = lambda step: step.build.getProperty("clean") or clean
+
 
     ############# CLEANING
-    if clean:
-        f.addStep(ShellCommand(name='clean stage 1',
-                               command=['rm','-rf',stage1_build],
-                               warnOnFailure=True,
-                               description='cleaning stage 1',
-                               descriptionDone='clean',
-                               workdir='.',
-                               env=env))
-    else:
+                           doStepIf=clean_build_requested))
+
+    if should_install_stage1:
+        # Clean the installation directory as well to remove stale build
+        # artifacts that no longer exist, such as old clang resource
+        # directories.
+        f.addStep(ShellCommand(name='clean stage 1 install',
+                               command=['rm','-rf',stage1_install],
+                               warnOnFailure=True,
+                               description='cleaning stage 1',
+                               descriptionDone='clean',
+                               workdir='.',
+                               doStepIf=clean_build_requested))
+
+    if not clean:
+        # On builders that are normally incremental, test if build.ninja already
+        # exists to see if we can avoid an extra cmake run.
         f.addStep(SetProperty(name="check ninja files 1",
                               workdir=stage1_build,
                               command=check_build_cmd,
                               flunkOnFailure=False,
                               property="exists_ninja_1"))
 
 
     ############# STAGE 1
     f.addStep(ShellCommand(name='cmake stage 1',
if test and testStage1:
         f.addStep(lit_test_command.LitTestCommand(name='ninja check 1',
                                    command=ninja_check_cmd,
                                    haltOnFailure=haltOnStage1Check,
                                    description=["checking stage 1"],
                                    descriptionDone=["stage 1 checked"],
                                    workdir=stage1_build,
                                    env=env))
 
-    if useTwoStage or runTestSuite or stage1_upload_directory:
+    if should_install_stage1:
         f.addStep(ShellCommand(name='install stage 1',
                                command=ninja_install_cmd,
                                description='ninja install',
                                workdir=stage1_build,
                                env=env))
 
     if stage1_upload_directory:
         addGCSUploadSteps(f, 'stage 1', stage1_install, stage1_upload_directory,
                           env, gcs_url_property='stage1_package_gcs_url')
 
     # 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:
-        # We always cleanly build the stage 2. If the compiler has been
-        # changed on the stage 1, we cannot trust any of the intermediate file
-        # from the old compiler. And if the stage 1 compiler is the same, we
-        # should not build in the first place.
+        # We always cleanly build the stage 2. If the compiler has been changed
+        # on the stage 1, we cannot trust any of the intermediate files from the
+        # old compiler. And if the stage 1 compiler is the same, we should not
+        # build in the first place.
         f.addStep(ShellCommand(name='clean stage 2',
                                command=['rm','-rf',stage2_build],
                                warnOnFailure=True,
                                description='cleaning stage 2',
                                descriptionDone='clean',
-                               workdir='.',
-                               env=env))
+                               workdir='.'))
+
+        f.addStep(ShellCommand(name='clean stage 2 install',
+                               command=['rm','-rf',stage2_install],
+                               warnOnFailure=True,
+                               description='cleaning stage 2',
+                               descriptionDone='clean',
+                               workdir='.'))
+
 
         # 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,
                       "-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))