BUILDBOT: Rework packaging step

- Don't copy files directly from worker but upload them to the master
- Make destination directory configurable
- Setup a URL to serve the freshly built package

buildbot-config/builds.py

@@ -1,6 +1,7 @@
 import os
 import shutil
 import sys
+import urllib.parse as urlp
 
 from buildbot.plugins import util
 from buildbot.plugins import changes
@@ -18,12 +19,11 @@ lock_build = util.WorkerLock("worker", maxCount = 1)
 
 # builds contains all build trees
 # ccache is the cache for compiled objects used by ccache
-# packages contains all final packages generated
 # src contains the source trees
 # triggers is some working directory needed by triggers
 # bshomes is used for various build systems (like Gradle) to avoid downloading things at each run
 # pollers is used by poll modules to maintain their state
-for data_dir in ["builds", "ccache", "packages", "src", "triggers", "bshomes", "pollers" ]:
+for data_dir in ["builds", "ccache", "src", "triggers", "bshomes", "pollers" ]:
     os.makedirs(os.path.join(config.data_dir, data_dir), exist_ok=True)
 shutil.copyfile(os.path.join(config.configuration_dir, "ccache.conf"),
     os.path.join(config.data_dir, "ccache", "ccache.conf"))
@@ -250,7 +250,11 @@ class ScummVMBuild(StandardBuild):
         src_path = "{0}/src/{1}".format("/data", self.name)
         configure_path = src_path + "/configure"
         build_path = "{0}/builds/{1}/{2}".format("/data", platform.name, self.name)
-        packages_path = "{0}/packages/snapshots/{1}".format("/data", self.name)
+
+        # snapshots_path is used in Package step on master side
+        snapshots_path = os.path.join(config.snapshots_dir, self.name)
+        # Ensure last path component doesn't get removed here and in packaging step
+        snapshots_url = urlp.urljoin(config.snapshots_url + '/', self.name + '/')
 
         env = platform.getEnv(self)
 
@@ -308,15 +312,17 @@ class ScummVMBuild(StandardBuild):
                     env = env))
 
         if platform.canPackage(self):
-            f.addStep(scummsteps.Package(disttarget = packaging_cmd,
-                                    srcpath = src_path,
-                                    dstpath = packages_path,
-                                    data_files = self.data_files,
-                                    buildname = "{0}-{1}".format(platform.name, self.name),
-                                    platform_built_files = platform.getBuiltFiles(self),
-                                    platform_data_files = platform.getDataFiles(self),
-                                    archive_format = platform.archiveext,
-                                    env = env))
+            f.addSteps(scummsteps.get_package_steps(
+                buildname = "{0}-{1}".format(platform.name, self.name),
+                srcpath = src_path,
+                dstpath = snapshots_path,
+                dsturl = snapshots_url,
+                archive_format = platform.archiveext,
+                disttarget = packaging_cmd,
+                build_data_files = self.data_files,
+                platform_data_files = platform.getDataFiles(self),
+                platform_built_files = platform.getBuiltFiles(self),
+                env = env))
 
         return [util.BuilderConfig(
             name = "{0}-{1}".format(self.name, platform.name),
@@ -405,7 +411,11 @@ class ScummVMToolsBuild(StandardBuild):
         src_path = "{0}/src/{1}".format("/data", self.name)
         configure_path = src_path + "/configure"
         build_path = "{0}/builds/{1}/{2}".format("/data", platform.name, self.name)
-        packages_path = "{0}/packages/snapshots/{1}".format("/data", self.name)
+
+        # snapshots_path is used in Package step on master side
+        snapshots_path = os.path.join(config.snapshots_dir, self.name)
+        # Ensure last path component doesn't get removed here and in packaging step
+        snapshots_url = urlp.urljoin(config.snapshots_url + '/', self.name + '/')
 
         env = platform.getEnv(self)
 
@@ -453,15 +463,17 @@ class ScummVMToolsBuild(StandardBuild):
                     env = env))
 
         if platform.canPackage(self):
-            f.addStep(scummsteps.Package(disttarget = packaging_cmd,
-                                    srcpath = src_path,
-                                    dstpath = packages_path,
-                                    data_files = self.data_files,
-                                    buildname = "{0}-{1}".format(platform.name, self.name),
-                                    platform_built_files = platform.getBuiltFiles(self),
-                                    platform_data_files = platform.getDataFiles(self),
-                                    archive_format = platform.archiveext,
-                                    env = env))
+            f.addSteps(scummsteps.get_package_steps(
+                buildname = "{0}-{1}".format(platform.name, self.name),
+                srcpath = src_path,
+                dstpath = snapshots_path,
+                dsturl = snapshots_url,
+                archive_format = platform.archiveext,
+                disttarget = packaging_cmd,
+                build_data_files = self.data_files,
+                platform_data_files = platform.getDataFiles(self),
+                platform_built_files = platform.getBuiltFiles(self),
+                env = env))
 
         return [util.BuilderConfig(
             name = "{0}-{1}".format(self.name, platform.name),

buildbot-config/config.py.example

@@ -4,6 +4,8 @@ import os
 configuration_dir = os.path.dirname(__file__)
 # data_dir is where all builds data is stored: intermediate files, ccache, ...
 data_dir = os.path.normpath(os.path.join(configuration_dir, "..", "buildbot-data"))
+# snapshots_dir is where all packages are moved and get served
+snapshots_dir = os.path.join(data_dir, 'packages', 'snapshots')
 
 # If a relative path is used here, it will be relative to the buildbot base directory, not configuration one
 db = {
@@ -55,6 +57,9 @@ www_port = ("127.0.0.1", 8010)
 # URL used to reach Buildbot instance
 buildbotURL = "https://buildbot.scummvm.org/"
 
+# URL used to download snapshots
+snapshots_url = "https://buildbot.scummvm.org/snapshots/"
+
 # IRC configuration
 #irc = {
 #    'server': 'chat.freenode.net',

buildbot-config/scummsteps.py

@@ -1,5 +1,6 @@
 import os
 from datetime import datetime
+import urllib.parse as urlp
 
 from twisted.internet import defer
 
@@ -143,31 +144,38 @@ PACKAGE_FORMAT_COMMANDS = {
     "zip": ["zip", "-r"],
 }
 
-def Package(disttarget, srcpath, dstpath, data_files,
-        buildname, platform_built_files, platform_data_files, archive_format,
+# Helper function which generates a list of steps that build the package on the worker,
+# upload it to master and create the symlink for the latest
+def get_package_steps(buildname, srcpath, dstpath, dsturl,
+        archive_format, disttarget,
+        build_data_files, platform_data_files,
+        platform_built_files,
         **kwargs):
+    if archive_format not in PACKAGE_FORMAT_COMMANDS:
+        archive_format = "tar.bz2"
+    archive_base_command = PACKAGE_FORMAT_COMMANDS.get(archive_format)
+
     files = []
-    # dont pack up the default files if the port has its own dist target
-    if not disttarget:
-        files += [ os.path.join(srcpath, f) for f in data_files ]
+
     files += platform_built_files
     # If file is absolute or begins with a $ (environment variable) don't prepend srcpath
     if platform_data_files:
         files += [ f if (os.path.isabs(f) or f[0:1] == '$') else os.path.join(srcpath, f)
                 for f in platform_data_files ]
+    # dont pack up the default files if the port has its own dist target
+    if not disttarget:
+        files += [ os.path.join(srcpath, f) for f in build_data_files ]
+
+    def createNames(props):
+        name = "{0}-{1}".format(buildname, props["revision"][:8])
+        archive = "{0}.{1}".format(name, archive_format)
+        symlink = "{0}-latest.{1}".format(buildname, archive_format)
+        return name, archive, symlink
 
     @util.renderer
     def generateCommands(props):
-        # Create a mutable variable from the outer one
-        archive_format_ = archive_format
-        if archive_format_ not in PACKAGE_FORMAT_COMMANDS:
-            archive_format_ = "tar.bz2"
-
-        name = "{0}-{1}".format(buildname, props["revision"][:8])
-        archive = "{0}.{1}".format(name, archive_format_)
-        symlink = "{0}-latest.{1}".format(buildname, archive_format_)
-
-        archive_command = PACKAGE_FORMAT_COMMANDS.get(archive_format_) + [archive, name+"/"]
+        name, archive, _ = createNames(props)
+        archive_full_command = archive_base_command + [archive, name+"/"]
 
         commands = []
 
@@ -181,22 +189,14 @@ def Package(disttarget, srcpath, dstpath, data_files,
         # WARNING: files aren't surrounded with quotes to let it happen
         commands.append(util.ShellArg('cp -r ' + ' '.join(files) + ' "{0}/"'.format(name),
             logname="archive", haltOnFailure=True))
-        commands.append(util.ShellArg(archive_command,
+        commands.append(util.ShellArg(archive_full_command,
             logname="archive", haltOnFailure=True))
-        commands.append(util.ShellArg(["chmod", "644", archive],
-            logname="publish", haltOnFailure=True))
-        commands.append(util.ShellArg(["mkdir", "-p", dstpath+"/"],
-            logname="publish", haltOnFailure=True))
-        commands.append(util.ShellArg(["mv", archive, dstpath+"/"],
-            logname="publish", haltOnFailure=True))
-        commands.append(util.ShellArg(["ln", "-sf", archive, os.path.join(dstpath, symlink)],
-            logname="publish", haltOnFailure=True))
 
         return commands
 
     @util.renderer
     def generateCleanup(props):
-        name = "{0}-{1}".format(buildname, props["revision"][:8])
+        name, _, _ = createNames(props)
 
         commands = []
         commands.append(util.ShellArg(["rm", "-rf", name],
@@ -210,7 +210,27 @@ def Package(disttarget, srcpath, dstpath, data_files,
                 props["revision"] is not None and
                 bool(props["package"]))
 
-    return CleanShellSequence(
+    @util.renderer
+    def getWorkerSrc(props):
+        _, archive, _ = createNames(props)
+        return archive
+
+    @util.renderer
+    def getMasterDest(props):
+        _, archive, _ = createNames(props)
+        return os.path.join(dstpath, archive)
+
+    @util.renderer
+    def getArchiveURL(props):
+        _, archive, _ = createNames(props)
+        return urlp.urljoin(dsturl, archive)
+
+    @util.renderer
+    def getLinkCommand(props):
+        _, archive, symlink = createNames(props)
+        return "ln", "-sf", archive, os.path.join(dstpath, symlink)
+
+    build_package = CleanShellSequence(
         name = "package",
         description = "packaging",
         descriptionDone = "package",
@@ -222,6 +242,28 @@ def Package(disttarget, srcpath, dstpath, data_files,
         **kwargs
     )
 
+    # dstpath will get created by FileUpload
+    upload_package = steps.FileUpload(
+        name = "upload package",
+        description = "uploading",
+        descriptionDone = "uploaded",
+        haltOnFailure = True,
+        flunkOnFailure = True,
+        workersrc = getWorkerSrc,
+        masterdest = getMasterDest,
+        mode = 0o0644,
+        url = getArchiveURL if dsturl else None)
+    link = steps.MasterShellCommand(
+        name = "link latest snapshot",
+        description = "linking",
+        descriptionDone = "linked",
+        haltOnFailure = True,
+        flunkOnFailure = True,
+        command = getLinkCommand,
+        env = {})
+
+    return build_package, upload_package, link
+
 # buildstep class to wipe all build folders (eg "trunk-*")
 def Clean(**kwargs):
     return steps.RemoveDirectory(

buildbot-config/workers.py

@@ -29,7 +29,6 @@ def StandardBuilderWorker(name, **kwargs):
             '{0}/ccache:/data/ccache'.format(config.data_dir),
             '{0}/src:/data/src:ro'.format(config.data_dir),
             '{0}/builds:/data/builds'.format(config.data_dir),
-            '{0}/packages:/data/packages'.format(config.data_dir),
             '{0}/bshomes:/data/bshomes'.format(config.data_dir),
         ],
         hostconfig={

workers/m4/debian-builder-base.m4

@@ -18,9 +18,9 @@ RUN apt-get update && \
                 && \
         rm -rf /var/lib/apt/lists/*
 
-RUN mkdir -p /data/ccache /data/src /data/builds /data/packages /data/bshomes && \
-	chown buildbot:buildbot /data/ccache /data/src /data/builds /data/packages /data/bshomes
-VOLUME /data/ccache /data/src /data/builds /data/packages /data/bshomes
+RUN mkdir -p /data/ccache /data/src /data/builds /data/bshomes && \
+	chown buildbot:buildbot /data/ccache /data/src /data/builds /data/bshomes
+VOLUME /data/ccache /data/src /data/builds /data/bshomes
 
 ENV CCACHE_DIR=/data/ccache