Clean old uploads by commit, not by timestamp

master/custom_steps.py

@@ -11,7 +11,7 @@
 from buildbot.steps.worker import CompositeStepMixin
 from twisted.internet import defer
 
-__all__ = ['CleanOldFiles', 'CTest', 'FileUploadIfNotExist', 'SetPropertiesFromCMakeCache']
+__all__ = ['CleanOldFiles', 'CTest', 'DeleteMatchingFilesInDir', 'FileUploadIfNotExist', 'SetPropertiesFromCMakeCache']
 
 
 class SetPropertiesFromCMakeCache(CompositeStepMixin, BuildStep):
@@ -131,6 +131,43 @@ def run(self):
         return status
 
 
+# Delete all files in workdir that match must_match_re but do not match must_not_match_re.
+# If must_not_match_re is None, delete all files that match must_match_re.
+# Note that the regexps are compared against the (leaf) name, not the full pathname.
+class DeleteMatchingFilesInDir(BuildStep):
+    name = 'delete-matching-files-in-dir'
+
+    renderables = ['must_match_re', 'must_not_match_re']
+
+    def __init__(self, *, workdir, must_match_re, must_not_match_re=None, **kwargs):
+        super().__init__(**kwargs)
+        self.workdir = workdir
+        self.must_match_re = must_match_re
+        self.must_not_match_re = must_not_match_re
+
+    @defer.inlineCallbacks
+    def run(self):
+        stdio = yield self.addLog('stdio')
+        status = SUCCESS
+
+        for entry in Path(self.workdir).iterdir():
+            if not entry.is_file():
+                continue
+            if not re.match(self.must_match_re, entry.name):
+                continue
+            if self.must_not_match_re and re.match(self.must_not_match_re, entry.name):
+                continue
+            try:
+                entry.unlink()
+                stdio.addStdout(f'Removed: {entry.resolve()}\n')
+            except (FileNotFoundError, OSError) as e:
+                stdio.addStderr(f'Could not delete {entry.resolve()}: {e}\n')
+                status = FAILURE
+
+        yield stdio.finish()
+        return status
+
+
 # Like FileUpload, but if the dest file already exists,
 # just log that to stdio and do nothing. Useful when the
 # filename contains (eg) a git commit or SHA that uniquely

master/master.cfg

@@ -35,7 +35,8 @@ from buildbot.www.authz.roles import RolesFromUsername
 from buildbot.www.hooks.github import GitHubEventHandler
 from twisted.internet import defer
 
-from custom_steps import CTest, CleanOldFiles, FileUploadIfNotExist, SetPropertiesFromCMakeCache
+from custom_steps import (CTest, CleanOldFiles, DeleteMatchingFilesInDir,
+                          FileUploadIfNotExist, SetPropertiesFromCMakeCache)
 
 # This is the dictionary that the buildmaster pays attention to. We also use
 # a shorter alias to save typing.
@@ -722,8 +723,7 @@ def get_cmake_build_command(builder_type, build_dir, targets=None):
     return cmd
 
 
-# options in LLVM that we always want OFF because they aren't needed
-# for Halide buildbot usage.
+# options in LLVM that we always want OFF because they aren't needed for Halide.
 _LLVM_OFF_OPTS = [
     "CLANG_ENABLE_ARCMT"
     "CLANG_ENABLE_CLANGD"
@@ -1119,10 +1119,13 @@ def add_llvm_steps(factory, builder_type, clean_rebuild):
                      env=Property('env'),
                      command=get_llvm_latest_commit_cmd(builder_type)))
 
+    def get_packaged_version(builder_type):
+        v = LLVM_BRANCHES[builder_type.llvm_branch].version
+        return "%d.%d.%d" % (v.major, v.minor, v.patch)
+
     # Note that this must only be called by a Renderer
     def get_packaged_name(props, builder_type):
-        v = LLVM_BRANCHES[builder_type.llvm_branch].version
-        version = "%d.%d.%d" % (v.major, v.minor, v.patch)
+        version = get_packaged_version(builder_type)
         # 'halide_target()' is correct here because we want the halide-style
         # triple that this llvm was build for
         target = builder_type.halide_target()
@@ -1198,17 +1201,31 @@ def add_llvm_steps(factory, builder_type, clean_rebuild):
                              mode=0o644,
                              masterdest=get_upload_dest_path(builder_type)))
 
-    def pkg_version_and_target(path: Path):
-        # Archives names are formatted like: llvm-[version]-[arch]-[commit].[ext]
-        # This grabs "llvm-[version]-[arch]".
-        match = re.match(r'^(.*)-[a-f0-9]+\.zst', path.name)
-        return match.group(1) if match else None
+    def matches_version_and_target():
+        @renderer
+        def render(props):
+            version = get_packaged_version(builder_type)
+            target = builder_type.halide_target()
+            return rf'^llvm-{version}-{target}-([a-f0-9]+)\.zst$'
 
-    factory.addStep(CleanOldFiles(
+        return render
+
+    def matches_version_and_target_and_commit():
+        @renderer
+        def render(props):
+            version = get_packaged_version(builder_type)
+            target = builder_type.halide_target()
+            commit = props.getProperty('got_revision')['llvm']
+            return rf'^llvm-{version}-{target}-{commit}\.zst$'
+
+        return render
+
+    factory.addStep(DeleteMatchingFilesInDir(
         name='Clean old LLVM packages',
         workdir=ARTIFACTS_DIR,
         locks=[performance_lock.access('counting')],
-        groupfn=pkg_version_and_target))
+        must_match_re=matches_version_and_target(),
+        must_not_match_re=matches_version_and_target_and_commit()))
 
 
 def add_halide_cmake_build_steps(factory, builder_type):