All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class
@ 2019-06-21  8:47 Atharva Lele
  2019-06-21  8:47 ` [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to " Atharva Lele
                   ` (18 more replies)
  0 siblings, 19 replies; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Various functions in the autobuild-run script use a lot of common data.
To make it easier to work with, create a Builder class.

For ease of review, this commit only introduces the Builder class but
does not actually use it for anything. Subsequent patches will do that.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 scripts/autobuild-run | 858 +++++++++++++++++++++---------------------
 1 file changed, 430 insertions(+), 428 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 601fb31..254d1b6 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -270,473 +270,474 @@ class SystemInfo:
 
         return not missing_requirements
 
-def prepare_build(**kwargs):
-    """Prepare for the next build of the specified instance
+class Builder:
+    def prepare_build(self, **kwargs):
+        """Prepare for the next build of the specified instance
 
-    This function prepares the build by making sure all the needed
-    directories are created, cloning or updating the Buildroot source
-    code, and cleaning up remaining stuff from previous builds.
-    """
+        This function prepares the build by making sure all the needed
+        directories are created, cloning or updating the Buildroot source
+        code, and cleaning up remaining stuff from previous builds.
+        """
 
-    idir = "instance-%d" % kwargs['instance']
-    log = kwargs['log']
-
-    log_write(log, "INFO: preparing a new build")
-
-    # Create the download directory if it doesn't exist
-    dldir = os.path.join(idir, "dl")
-    if not os.path.exists(dldir):
-        os.mkdir(dldir)
-
-    # recursively find files under root
-    def find_files(root):
-        for r, d, f in os.walk(root):
-            # do not remove individual files from git caches. 'git' can
-            # be either dl/<package>/git or dl/git and we want to
-            # eventually remove tarballs for the git package, so check
-            # for '.git' instead to match only dl/<package>/git/.git .
-            if '.git' in d:
-                del d[:]
-                continue
-            for i in f:
-                yield os.path.join(r, i)
-
-    # Remove 5 random files from the download directory. Removing
-    # random files from the download directory allows to ensure we
-    # regularly re-download files to check that their upstream
-    # location is still correct.
-    for i in range(0, 5):
-        flist = list(find_files(dldir))
-        if not flist:
-            break
-        f = flist[randint(0, len(flist) - 1)]
-        log_write(log, "INFO: removing %s from downloads" %
-                  os.path.relpath(f, dldir))
-        os.remove(f)
-
-    branch = get_branch()
-    log_write(log, "INFO: testing branch '%s'" % branch)
-
-    # Clone Buildroot. This only happens if the source directory
-    # didn't exist already.
-    srcdir = os.path.join(idir, "buildroot")
-    if not os.path.exists(srcdir):
-        ret = subprocess.call(["git", "clone", kwargs['repo'], srcdir],
-                              stdout=log, stderr=log)
+        idir = "instance-%d" % kwargs['instance']
+        log = kwargs['log']
+
+        log_write(log, "INFO: preparing a new build")
+
+        # Create the download directory if it doesn't exist
+        dldir = os.path.join(idir, "dl")
+        if not os.path.exists(dldir):
+            os.mkdir(dldir)
+
+        # recursively find files under root
+        def find_files(root):
+            for r, d, f in os.walk(root):
+                # do not remove individual files from git caches. 'git' can
+                # be either dl/<package>/git or dl/git and we want to
+                # eventually remove tarballs for the git package, so check
+                # for '.git' instead to match only dl/<package>/git/.git .
+                if '.git' in d:
+                    del d[:]
+                    continue
+                for i in f:
+                    yield os.path.join(r, i)
+
+        # Remove 5 random files from the download directory. Removing
+        # random files from the download directory allows to ensure we
+        # regularly re-download files to check that their upstream
+        # location is still correct.
+        for i in range(0, 5):
+            flist = list(find_files(dldir))
+            if not flist:
+                break
+            f = flist[randint(0, len(flist) - 1)]
+            log_write(log, "INFO: removing %s from downloads" %
+                    os.path.relpath(f, dldir))
+            os.remove(f)
+
+        branch = get_branch()
+        log_write(log, "INFO: testing branch '%s'" % branch)
+
+        # Clone Buildroot. This only happens if the source directory
+        # didn't exist already.
+        srcdir = os.path.join(idir, "buildroot")
+        if not os.path.exists(srcdir):
+            ret = subprocess.call(["git", "clone", kwargs['repo'], srcdir],
+                                stdout=log, stderr=log)
+            if ret != 0:
+                log_write(log, "ERROR: could not clone Buildroot sources")
+                return -1
+
+        # Update the Buildroot sources.
+        abssrcdir = os.path.abspath(srcdir)
+        ret = subprocess.call(["git", "fetch", "origin"], cwd=abssrcdir, stdout=log, stderr=log)
         if ret != 0:
-            log_write(log, "ERROR: could not clone Buildroot sources")
+            log_write(log, "ERROR: could not fetch Buildroot sources")
             return -1
 
-    # Update the Buildroot sources.
-    abssrcdir = os.path.abspath(srcdir)
-    ret = subprocess.call(["git", "fetch", "origin"], cwd=abssrcdir, stdout=log, stderr=log)
-    if ret != 0:
-        log_write(log, "ERROR: could not fetch Buildroot sources")
-        return -1
-
-    ret = subprocess.call(["git", "checkout", "--detach", "origin/%s" % branch], cwd=abssrcdir, stdout=log, stderr=log)
-    if ret != 0:
-        log_write(log, "ERROR: could not check out Buildroot sources")
-        return -1
-
-    # Create an empty output directory. We remove it first, in case a previous build was aborted.
-    outputdir = os.path.join(idir, "output")
-    if os.path.exists(outputdir):
-        # shutil.rmtree doesn't remove write-protected files
-        subprocess.call(["rm", "-rf", outputdir])
-    os.mkdir(outputdir)
-    with open(os.path.join(outputdir, "branch"), "w") as branchf:
-        branchf.write(branch)
-
-    return 0
-
-def gen_config(**kwargs):
-    """Generate a new random configuration."""
-    idir = "instance-%d" % kwargs['instance']
-    log = kwargs['log']
-    outputdir = os.path.abspath(os.path.join(idir, "output"))
-    srcdir = os.path.join(idir, "buildroot")
-
-    log_write(log, "INFO: generate the configuration")
-
-    if kwargs['debug']:
-        devnull = log
-    else:
-        devnull = open(os.devnull, "w")
-
-    args = [os.path.join(srcdir, "utils/genrandconfig"),
-            "-o", outputdir, "-b", srcdir]
-
-    toolchains_csv = kwargs['toolchains_csv']
-    if toolchains_csv:
-        if not os.path.isabs(toolchains_csv):
-            toolchains_csv = os.path.join(srcdir, toolchains_csv)
-        args.extend(["--toolchains-csv", toolchains_csv])
-
-    ret = subprocess.call(args, stdout=devnull, stderr=log)
-    return ret
-
-def stop_on_build_hang(monitor_thread_hung_build_flag,
-                       monitor_thread_stop_flag,
-                       sub_proc, outputdir, log):
-    build_time_logfile = os.path.join(outputdir, "build/build-time.log")
-    while True:
-        if monitor_thread_stop_flag.is_set():
-            return
-        if os.path.exists(build_time_logfile):
-            mtime = datetime.datetime.fromtimestamp(os.stat(build_time_logfile).st_mtime)
-
-            if mtime < datetime.datetime.now() - datetime.timedelta(minutes=HUNG_BUILD_TIMEOUT):
-                if sub_proc.poll() is None:
-                    monitor_thread_hung_build_flag.set() # Used by do_build() to determine build hang
-                    log_write(log, "INFO: build hung")
-                    sub_proc.kill()
-                break
-        monitor_thread_stop_flag.wait(30)
+        ret = subprocess.call(["git", "checkout", "--detach", "origin/%s" % branch], cwd=abssrcdir, stdout=log, stderr=log)
+        if ret != 0:
+            log_write(log, "ERROR: could not check out Buildroot sources")
+            return -1
 
-def check_reproducibility(**kwargs):
-    """Check reproducibility of builds
+        # Create an empty output directory. We remove it first, in case a previous build was aborted.
+        outputdir = os.path.join(idir, "output")
+        if os.path.exists(outputdir):
+            # shutil.rmtree doesn't remove write-protected files
+            subprocess.call(["rm", "-rf", outputdir])
+        os.mkdir(outputdir)
+        with open(os.path.join(outputdir, "branch"), "w") as branchf:
+            branchf.write(branch)
+
+        return 0
+
+    def gen_config(self, **kwargs):
+        """Generate a new random configuration."""
+        idir = "instance-%d" % kwargs['instance']
+        log = kwargs['log']
+        outputdir = os.path.abspath(os.path.join(idir, "output"))
+        srcdir = os.path.join(idir, "buildroot")
 
-    Use diffoscope on the built images, if diffoscope is not
-    installed, fallback to cmp
-    """
+        log_write(log, "INFO: generate the configuration")
 
-    log = kwargs['log']
-    idir = "instance-%d" % kwargs['instance']
-    outputdir = os.path.join(idir, "output")
-    srcdir = os.path.join(idir, "buildroot")
-    reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
-    # Using only tar images for now
-    build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
-    build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
-
-    with open(reproducible_results, 'w') as diff:
-        if kwargs['sysinfo'].has("diffoscope"):
-            # Prefix to point diffoscope towards cross-tools
-            prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
-            # Remove TARGET_CROSS= and \n from the string
-            prefix = prefix[13:-1]
-            log_write(log, "INFO: running diffoscope on images")
-            subprocess.call(["diffoscope", build_1_image, build_2_image,
-                                "--tool-prefix-binutils", prefix], stdout=diff, stderr=log)
+        if kwargs['debug']:
+            devnull = log
         else:
-            log_write(log, "INFO: diffoscope not installed, falling back to cmp")
-            subprocess.call(["cmp", "-b", build_1_image, build_2_image], stdout=diff, stderr=log)
-
-    if os.stat(reproducible_results).st_size > 0:
-        log_write(log, "INFO: Build is non-reproducible.")
-        return -1
-
-    # rootfs images match byte-for-byte -> reproducible image
-    log_write(log, "INFO: Build is reproducible!")
-    return 0
-
-def do_build(**kwargs):
-    """Run the build itself"""
-
-    idir = "instance-%d" % kwargs['instance']
-    log = kwargs['log']
-    nice = kwargs['nice']
-
-    # We need the absolute path to use with O=, because the relative
-    # path to the output directory here is not relative to the
-    # Buildroot sources, but to the location of the autobuilder
-    # script.
-    dldir = os.path.abspath(os.path.join(idir, "dl"))
-    outputdir = os.path.abspath(os.path.join(idir, "output"))
-    srcdir = os.path.join(idir, "buildroot")
-    f = open(os.path.join(outputdir, "logfile"), "w+")
-    log_write(log, "INFO: build started")
-
-    cmd = ["nice", "-n", str(nice),
-            "make", "O=%s" % outputdir,
-            "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
-            "BR2_JLEVEL=%s" % kwargs['njobs']] \
-          + kwargs['make_opts'].split()
-    sub = subprocess.Popen(cmd, stdout=f, stderr=f)
-
-    # Setup hung build monitoring thread
-    monitor_thread_hung_build_flag = Event()
-    monitor_thread_stop_flag = Event()
-    build_monitor = Thread(target=stop_on_build_hang,
-                           args=(monitor_thread_hung_build_flag,
-                                 monitor_thread_stop_flag,
-                                 sub, outputdir, log))
-    build_monitor.daemon = True
-    build_monitor.start()
-
-    kwargs['buildpid'][kwargs['instance']] = sub.pid
-    ret = sub.wait()
-    kwargs['buildpid'][kwargs['instance']] = 0
-
-    # If build failed, monitor thread would have exited at this point
-    if monitor_thread_hung_build_flag.is_set():
-        log_write(log, "INFO: build timed out [%d]" % ret)
-        return -2
-    else:
-        # Stop monitor thread as this build didn't timeout
-        monitor_thread_stop_flag.set()
-    # Monitor thread should be exiting around this point
-
-    if ret != 0:
-        log_write(log, "INFO: build failed [%d]" % ret)
-        return -1
-
-    cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
-            "BR2_DL_DIR=%s" % dldir, "legal-info"] \
-          + kwargs['make_opts'].split()
-    ret = subprocess.call(cmd, stdout=f, stderr=f)
-    if ret != 0:
-        log_write(log, "INFO: build failed during legal-info")
-        return -1
-    log_write(log, "INFO: build successful")
-    return 0
-
-def do_reproducible_build(**kwargs):
-    """Run the builds for reproducibility testing
-
-    Build twice with the same configuration. Calls do_build() to
-    perform the actual build.
-    """
+            devnull = open(os.devnull, "w")
+
+        args = [os.path.join(srcdir, "utils/genrandconfig"),
+                "-o", outputdir, "-b", srcdir]
 
-    idir = "instance-%d" % kwargs['instance']
-    outputdir = os.path.abspath(os.path.join(idir, "output"))
-    srcdir = os.path.join(idir, "buildroot")
-    log = kwargs['log']
+        toolchains_csv = kwargs['toolchains_csv']
+        if toolchains_csv:
+            if not os.path.isabs(toolchains_csv):
+                toolchains_csv = os.path.join(srcdir, toolchains_csv)
+            args.extend(["--toolchains-csv", toolchains_csv])
 
-    # Start the first build
-    log_write(log, "INFO: Reproducible Build Test, starting build 1")
-    ret = do_build(**kwargs)
-    if ret != 0:
-        log_write(log, "INFO: build 1 failed, skipping build 2")
+        ret = subprocess.call(args, stdout=devnull, stderr=log)
         return ret
 
-    # First build has been built, move files and start build 2
-    os.rename(os.path.join(outputdir, "images"), os.path.join(outputdir, "images-1"))
+    def stop_on_build_hang(self, monitor_thread_hung_build_flag,
+                        monitor_thread_stop_flag,
+                        sub_proc, outputdir, log):
+        build_time_logfile = os.path.join(outputdir, "build/build-time.log")
+        while True:
+            if monitor_thread_stop_flag.is_set():
+                return
+            if os.path.exists(build_time_logfile):
+                mtime = datetime.datetime.fromtimestamp(os.stat(build_time_logfile).st_mtime)
+
+                if mtime < datetime.datetime.now() - datetime.timedelta(minutes=HUNG_BUILD_TIMEOUT):
+                    if sub_proc.poll() is None:
+                        monitor_thread_hung_build_flag.set() # Used by do_build() to determine build hang
+                        log_write(log, "INFO: build hung")
+                        sub_proc.kill()
+                    break
+            monitor_thread_stop_flag.wait(30)
+
+    def check_reproducibility(self, **kwargs):
+        """Check reproducibility of builds
+
+        Use diffoscope on the built images, if diffoscope is not
+        installed, fallback to cmp
+        """
 
-    # Clean up build 1
-    f = open(os.path.join(outputdir, "logfile"), "w+")
-    subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "clean"], stdout=f, stderr=f)
+        log = kwargs['log']
+        idir = "instance-%d" % kwargs['instance']
+        outputdir = os.path.join(idir, "output")
+        srcdir = os.path.join(idir, "buildroot")
+        reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
+        # Using only tar images for now
+        build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
+        build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
+
+        with open(reproducible_results, 'w') as diff:
+            if kwargs['sysinfo'].has("diffoscope"):
+                # Prefix to point diffoscope towards cross-tools
+                prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
+                # Remove TARGET_CROSS= and \n from the string
+                prefix = prefix[13:-1]
+                log_write(log, "INFO: running diffoscope on images")
+                subprocess.call(["diffoscope", build_1_image, build_2_image,
+                                    "--tool-prefix-binutils", prefix], stdout=diff, stderr=log)
+            else:
+                log_write(log, "INFO: diffoscope not installed, falling back to cmp")
+                subprocess.call(["cmp", "-b", build_1_image, build_2_image], stdout=diff, stderr=log)
 
-    # Start the second build
-    log_write(log, "INFO: Reproducible Build Test, starting build 2")
-    ret = do_build(**kwargs)
-    if ret != 0:
-        log_write(log, "INFO: build 2 failed")
-        return ret
+        if os.stat(reproducible_results).st_size > 0:
+            log_write(log, "INFO: Build is non-reproducible.")
+            return -1
 
-    # Assuming both have built successfully
-    ret = check_reproducibility(**kwargs)
-    return ret
+        # rootfs images match byte-for-byte -> reproducible image
+        log_write(log, "INFO: Build is reproducible!")
+        return 0
 
-def send_results(result, **kwargs):
-    """Prepare and store/send tarball with results
+    def do_build(self, **kwargs):
+        """Run the build itself"""
 
-    This function prepares the tarball with the results, and either
-    submits them to the official server (if the appropriate credentials
-    are available) or stores them locally as tarballs.
-    """
+        idir = "instance-%d" % kwargs['instance']
+        log = kwargs['log']
+        nice = kwargs['nice']
 
-    idir = "instance-%d" % kwargs['instance']
-    log = kwargs['log']
-
-    outputdir = os.path.abspath(os.path.join(idir, "output"))
-    srcdir = os.path.join(idir, "buildroot")
-    resultdir = os.path.join(outputdir, "results")
-
-    shutil.copyfile(os.path.join(outputdir, ".config"),
-                    os.path.join(resultdir, "config"))
-    shutil.copyfile(os.path.join(outputdir, "defconfig"),
-                    os.path.join(resultdir, "defconfig"))
-    shutil.copyfile(os.path.join(outputdir, "branch"),
-                    os.path.join(resultdir, "branch"))
-
-    def copy_if_exists(directory, src, dst=None):
-        if os.path.exists(os.path.join(outputdir, directory, src)):
-            shutil.copyfile(os.path.join(outputdir, directory, src),
-                            os.path.join(resultdir, src if dst is None else dst))
-
-    copy_if_exists("build", "build-time.log")
-    copy_if_exists("build", "packages-file-list.txt")
-    copy_if_exists("build", "packages-file-list-host.txt")
-    copy_if_exists("build", "packages-file-list-staging.txt")
-    copy_if_exists("legal-info", "manifest.csv", "licenses-manifest.csv")
-
-    subprocess.call(["git log -n 1 --pretty=format:%%H > %s" % \
-                     os.path.join(resultdir, "gitid")],
-                    shell=True, cwd=srcdir)
-
-    # Return True if the result should be rejected, False otherwise
-    def reject_results():
-        lastlines = decode_bytes(subprocess.Popen(
-            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
-            stdout=subprocess.PIPE).communicate()[0]).splitlines()
-
-	# Reject results where qemu-user refused to build
-	regexp = re.compile(r'^package/qemu/qemu.mk:.*Refusing to build qemu-user')
-	for line in lastlines:
-	    if regexp.match(line):
-		return True
-
-	return False
-
-    if reject_results():
-        return
-
-    def get_failure_reason():
-        # Output is a tuple (package, version), or None.
-        lastlines = decode_bytes(subprocess.Popen(
-            ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
-            stdout=subprocess.PIPE).communicate()[0]).splitlines()
-
-        regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
-        for line in lastlines:
-            m = regexp.search(line)
-            if m:
-                return m.group(1).rsplit('-', 1)
-
-        # not found
-        return None
+        # We need the absolute path to use with O=, because the relative
+        # path to the output directory here is not relative to the
+        # Buildroot sources, but to the location of the autobuilder
+        # script.
+        dldir = os.path.abspath(os.path.join(idir, "dl"))
+        outputdir = os.path.abspath(os.path.join(idir, "output"))
+        srcdir = os.path.join(idir, "buildroot")
+        f = open(os.path.join(outputdir, "logfile"), "w+")
+        log_write(log, "INFO: build started")
+
+        cmd = ["nice", "-n", str(nice),
+                "make", "O=%s" % outputdir,
+                "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
+                "BR2_JLEVEL=%s" % kwargs['njobs']] \
+            + kwargs['make_opts'].split()
+        sub = subprocess.Popen(cmd, stdout=f, stderr=f)
+
+        # Setup hung build monitoring thread
+        monitor_thread_hung_build_flag = Event()
+        monitor_thread_stop_flag = Event()
+        build_monitor = Thread(target=self.stop_on_build_hang,
+                            args=(monitor_thread_hung_build_flag,
+                                    monitor_thread_stop_flag,
+                                    sub, outputdir, log))
+        build_monitor.daemon = True
+        build_monitor.start()
+
+        kwargs['buildpid'][kwargs['instance']] = sub.pid
+        ret = sub.wait()
+        kwargs['buildpid'][kwargs['instance']] = 0
+
+        # If build failed, monitor thread would have exited at this point
+        if monitor_thread_hung_build_flag.is_set():
+            log_write(log, "INFO: build timed out [%d]" % ret)
+            return -2
+        else:
+            # Stop monitor thread as this build didn't timeout
+            monitor_thread_stop_flag.set()
+        # Monitor thread should be exiting around this point
 
-    def extract_end_log(resultfile):
-        """Save the last part of the build log, starting from the failed package"""
+        if ret != 0:
+            log_write(log, "INFO: build failed [%d]" % ret)
+            return -1
 
-        def extract_last_500_lines():
-            subprocess.call(["tail -500 %s > %s" % \
-                             (os.path.join(outputdir, "logfile"), resultfile)],
-                            shell=True)
+        cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
+                "BR2_DL_DIR=%s" % dldir, "legal-info"] \
+            + kwargs['make_opts'].split()
+        ret = subprocess.call(cmd, stdout=f, stderr=f)
+        if ret != 0:
+            log_write(log, "INFO: build failed during legal-info")
+            return -1
+        log_write(log, "INFO: build successful")
+        return 0
 
-        reason = get_failure_reason()
-        if not reason:
-            extract_last_500_lines()
-        else:
-            f = open(os.path.join(outputdir, "logfile"), 'r')
-            mf = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
-            mf.seek(0)
-            # Search for first action on the failed package
-            offset = mf.find(encode_str('>>> %s' % ' '.join(reason)))
-            if offset != -1:
-                with open(resultfile, "w") as endlog:
-                    endlog.write(decode_bytes(mf[offset:]))
-            else:
-                # not found, use last 500 lines as fallback
-                extract_last_500_lines()
+    def do_reproducible_build(self, **kwargs):
+        """Run the builds for reproducibility testing
 
-            mf.close()
-            f.close()
+        Build twice with the same configuration. Calls do_build() to
+        perform the actual build.
+        """
 
-    extract_end_log(os.path.join(resultdir, "build-end.log"))
+        idir = "instance-%d" % kwargs['instance']
+        outputdir = os.path.abspath(os.path.join(idir, "output"))
+        srcdir = os.path.join(idir, "buildroot")
+        log = kwargs['log']
 
-    def copy_config_log_files():
-        """Recursively copy any config.log files from the failing package"""
+        # Start the first build
+        log_write(log, "INFO: Reproducible Build Test, starting build 1")
+        ret = self.do_build(**kwargs)
+        if ret != 0:
+            log_write(log, "INFO: build 1 failed, skipping build 2")
+            return ret
 
-        reason = get_failure_reason()
-        if not reason:
-            return
+        # First build has been built, move files and start build 2
+        os.rename(os.path.join(outputdir, "images"), os.path.join(outputdir, "images-1"))
 
-        srcroot = os.path.join(outputdir, "build", '-'.join(reason))
-        destroot = os.path.join(resultdir, '-'.join(reason))
-        config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
-            'CMakeOutput.log')
-
-        for root, dirs, files in os.walk(srcroot):
-            dest = os.path.join(destroot, os.path.relpath(root, srcroot))
-
-            for fname in files:
-                if fname in config_files:
-                    if not os.path.exists(dest):
-                        os.makedirs(dest)
-                    shutil.copy(os.path.join(root, fname), os.path.join(dest, fname))
-
-    copy_config_log_files()
-
-    resultf = open(os.path.join(resultdir, "status"), "w+")
-    if result == 0:
-        resultf.write("OK")
-    elif result == -1:
-        resultf.write("NOK")
-    elif result == -2:
-        resultf.write("TIMEOUT")
-    resultf.close()
-
-    with open(os.path.join(resultdir, "submitter"), "w+") as submitterf:
-        submitterf.write(kwargs['submitter'])
-
-    # Yes, shutil.make_archive() would be nice, but it doesn't exist
-    # in Python 2.6.
-    ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
-                          cwd=outputdir, stdout=log, stderr=log)
-    if ret != 0:
-        log_write(log, "ERROR: could not make results tarball")
-        sys.exit(1)
+        # Clean up build 1
+        f = open(os.path.join(outputdir, "logfile"), "w+")
+        subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "clean"], stdout=f, stderr=f)
 
-    if kwargs['upload']:
-        # Submit results. Yes, Python has some HTTP libraries, but
-        # none of the ones that are part of the standard library can
-        # upload a file without writing dozens of lines of code.
-        ret = subprocess.call(["curl", "-u",
-                               "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
-                               "-H", "Expect:",
-                               "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
-                               "-F", "uploadsubmit=1",
-                               kwargs['http_url']],
-                              stdout=log, stderr=log)
+        # Start the second build
+        log_write(log, "INFO: Reproducible Build Test, starting build 2")
+        ret = self.do_build(**kwargs)
         if ret != 0:
-            log_write(log, "INFO: results could not be submitted, %d" % ret)
-        else:
-            log_write(log, "INFO: results were submitted successfully")
-    else:
-        # No http login/password, keep tarballs locally
-        with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
-            sha1 = hashlib.sha1(f.read()).hexdigest()
-        resultfilename = "instance-%d-%s.tar.bz2" % (kwargs['instance'], sha1)
-        os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
-        log_write(log, "INFO: results saved as %s" % resultfilename)
-
-def run_instance(**kwargs):
-    """Main per-instance loop
-
-    Prepare the build, generate a configuration, run the build, and submit the
-    results.
-    """
+            log_write(log, "INFO: build 2 failed")
+            return ret
 
-    idir = "instance-%d" % kwargs['instance']
+        # Assuming both have built successfully
+        ret = self.check_reproducibility(**kwargs)
+        return ret
 
-    # If it doesn't exist, create the instance directory
-    if not os.path.exists(idir):
-        os.mkdir(idir)
+    def send_results(self, result, **kwargs):
+        """Prepare and store/send tarball with results
 
-    if kwargs['debug']:
-        kwargs['log'] = sys.stdout
-    else:
-        kwargs['log'] = open(os.path.join(idir, "instance.log"), "a+")
-    log_write(kwargs['log'], "INFO: instance started")
+        This function prepares the tarball with the results, and either
+        submits them to the official server (if the appropriate credentials
+        are available) or stores them locally as tarballs.
+        """
 
-    while True:
-        check_version()
+        idir = "instance-%d" % kwargs['instance']
+        log = kwargs['log']
 
-        ret = prepare_build(**kwargs)
-        if ret != 0:
-            continue
+        outputdir = os.path.abspath(os.path.join(idir, "output"))
+        srcdir = os.path.join(idir, "buildroot")
+        resultdir = os.path.join(outputdir, "results")
+
+        shutil.copyfile(os.path.join(outputdir, ".config"),
+                        os.path.join(resultdir, "config"))
+        shutil.copyfile(os.path.join(outputdir, "defconfig"),
+                        os.path.join(resultdir, "defconfig"))
+        shutil.copyfile(os.path.join(outputdir, "branch"),
+                        os.path.join(resultdir, "branch"))
+
+        def copy_if_exists(directory, src, dst=None):
+            if os.path.exists(os.path.join(outputdir, directory, src)):
+                shutil.copyfile(os.path.join(outputdir, directory, src),
+                                os.path.join(resultdir, src if dst is None else dst))
+
+        copy_if_exists("build", "build-time.log")
+        copy_if_exists("build", "packages-file-list.txt")
+        copy_if_exists("build", "packages-file-list-host.txt")
+        copy_if_exists("build", "packages-file-list-staging.txt")
+        copy_if_exists("legal-info", "manifest.csv", "licenses-manifest.csv")
+
+        subprocess.call(["git log -n 1 --pretty=format:%%H > %s" % \
+                        os.path.join(resultdir, "gitid")],
+                        shell=True, cwd=srcdir)
+
+        # Return True if the result should be rejected, False otherwise
+        def reject_results():
+            lastlines = decode_bytes(subprocess.Popen(
+                ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
+                stdout=subprocess.PIPE).communicate()[0]).splitlines()
+
+            # Reject results where qemu-user refused to build
+            regexp = re.compile(r'^package/qemu/qemu.mk:.*Refusing to build qemu-user')
+            for line in lastlines:
+                if regexp.match(line):
+                    return True
+
+            return False
+
+        if reject_results():
+            return
+
+        def get_failure_reason():
+            # Output is a tuple (package, version), or None.
+            lastlines = decode_bytes(subprocess.Popen(
+                ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
+                stdout=subprocess.PIPE).communicate()[0]).splitlines()
 
-        resultdir = os.path.join(idir, "output", "results")
-        os.mkdir(resultdir)
+            regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
+            for line in lastlines:
+                m = regexp.search(line)
+                if m:
+                    return m.group(1).rsplit('-', 1)
 
-        ret = gen_config(**kwargs)
+            # not found
+            return None
+
+        def extract_end_log(resultfile):
+            """Save the last part of the build log, starting from the failed package"""
+
+            def extract_last_500_lines():
+                subprocess.call(["tail -500 %s > %s" % \
+                                (os.path.join(outputdir, "logfile"), resultfile)],
+                                shell=True)
+
+            reason = get_failure_reason()
+            if not reason:
+                extract_last_500_lines()
+            else:
+                f = open(os.path.join(outputdir, "logfile"), 'r')
+                mf = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
+                mf.seek(0)
+                # Search for first action on the failed package
+                offset = mf.find(encode_str('>>> %s' % ' '.join(reason)))
+                if offset != -1:
+                    with open(resultfile, "w") as endlog:
+                        endlog.write(decode_bytes(mf[offset:]))
+                else:
+                    # not found, use last 500 lines as fallback
+                    extract_last_500_lines()
+
+                mf.close()
+                f.close()
+
+        extract_end_log(os.path.join(resultdir, "build-end.log"))
+
+        def copy_config_log_files():
+            """Recursively copy any config.log files from the failing package"""
+
+            reason = get_failure_reason()
+            if not reason:
+                return
+
+            srcroot = os.path.join(outputdir, "build", '-'.join(reason))
+            destroot = os.path.join(resultdir, '-'.join(reason))
+            config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
+                'CMakeOutput.log')
+
+            for root, dirs, files in os.walk(srcroot):
+                dest = os.path.join(destroot, os.path.relpath(root, srcroot))
+
+                for fname in files:
+                    if fname in config_files:
+                        if not os.path.exists(dest):
+                            os.makedirs(dest)
+                        shutil.copy(os.path.join(root, fname), os.path.join(dest, fname))
+
+        copy_config_log_files()
+
+        resultf = open(os.path.join(resultdir, "status"), "w+")
+        if result == 0:
+            resultf.write("OK")
+        elif result == -1:
+            resultf.write("NOK")
+        elif result == -2:
+            resultf.write("TIMEOUT")
+        resultf.close()
+
+        with open(os.path.join(resultdir, "submitter"), "w+") as submitterf:
+            submitterf.write(kwargs['submitter'])
+
+        # Yes, shutil.make_archive() would be nice, but it doesn't exist
+        # in Python 2.6.
+        ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
+                            cwd=outputdir, stdout=log, stderr=log)
         if ret != 0:
-            log_write(kwargs['log'], "WARN: failed to generate configuration")
-            continue
+            log_write(log, "ERROR: could not make results tarball")
+            sys.exit(1)
+
+        if kwargs['upload']:
+            # Submit results. Yes, Python has some HTTP libraries, but
+            # none of the ones that are part of the standard library can
+            # upload a file without writing dozens of lines of code.
+            ret = subprocess.call(["curl", "-u",
+                                "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
+                                "-H", "Expect:",
+                                "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
+                                "-F", "uploadsubmit=1",
+                                kwargs['http_url']],
+                                stdout=log, stderr=log)
+            if ret != 0:
+                log_write(log, "INFO: results could not be submitted, %d" % ret)
+            else:
+                log_write(log, "INFO: results were submitted successfully")
+        else:
+            # No http login/password, keep tarballs locally
+            with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
+                sha1 = hashlib.sha1(f.read()).hexdigest()
+            resultfilename = "instance-%d-%s.tar.bz2" % (kwargs['instance'], sha1)
+            os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
+            log_write(log, "INFO: results saved as %s" % resultfilename)
+
+    def run_instance(self, **kwargs):
+        """Main per-instance loop
+
+        Prepare the build, generate a configuration, run the build, and submit the
+        results.
+        """
 
-        # Check if the build test is supposed to be a reproducible test
-        outputdir = os.path.abspath(os.path.join(idir, "output"))
-        with open(os.path.join(outputdir, ".config"), "r") as fconf:
-            reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
-        if reproducible:
-            ret = do_reproducible_build(**kwargs)
+        idir = "instance-%d" % kwargs['instance']
+
+        # If it doesn't exist, create the instance directory
+        if not os.path.exists(idir):
+            os.mkdir(idir)
+
+        if kwargs['debug']:
+            kwargs['log'] = sys.stdout
         else:
-            ret = do_build(**kwargs)
+            kwargs['log'] = open(os.path.join(idir, "instance.log"), "a+")
+        log_write(kwargs['log'], "INFO: instance started")
+
+        while True:
+            check_version()
+
+            ret = self.prepare_build(**kwargs)
+            if ret != 0:
+                continue
+
+            resultdir = os.path.join(idir, "output", "results")
+            os.mkdir(resultdir)
+
+            ret = self.gen_config(**kwargs)
+            if ret != 0:
+                log_write(kwargs['log'], "WARN: failed to generate configuration")
+                continue
+
+            # Check if the build test is supposed to be a reproducible test
+            outputdir = os.path.abspath(os.path.join(idir, "output"))
+            with open(os.path.join(outputdir, ".config"), "r") as fconf:
+                reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
+            if reproducible:
+                ret = self.do_reproducible_build(**kwargs)
+            else:
+                ret = self.do_build(**kwargs)
 
-        send_results(ret, **kwargs)
+            self.send_results(ret, **kwargs)
 
 # args / config file merging inspired by:
 # https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
@@ -839,7 +840,8 @@ def main():
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        p = multiprocessing.Process(target=run_instance, kwargs=dict(
+        builder = Builder()
+        p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
                 instance = i,
                 njobs = args['--njobs'],
                 sysinfo = sysinfo,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 20:31   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 03/19] autobuild-run: move njobs " Atharva Lele
                   ` (17 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

As discussed in the previous patch, these common variables are needed
in many functions, and it'll be better to have them as part of the class.
This will make the code cleaner and make it easier to introduce variability
for reproducibility testing. Succeeding patches will move all variables from
kwargs to the Builder constructor.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 254d1b6..291cd9f 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -271,6 +271,9 @@ class SystemInfo:
         return not missing_requirements
 
 class Builder:
+    def __init__(self, instance):
+        self.instance = instance
+
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
 
@@ -279,7 +282,7 @@ class Builder:
         code, and cleaning up remaining stuff from previous builds.
         """
 
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         log = kwargs['log']
 
         log_write(log, "INFO: preparing a new build")
@@ -353,7 +356,7 @@ class Builder:
 
     def gen_config(self, **kwargs):
         """Generate a new random configuration."""
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         log = kwargs['log']
         outputdir = os.path.abspath(os.path.join(idir, "output"))
         srcdir = os.path.join(idir, "buildroot")
@@ -403,7 +406,7 @@ class Builder:
         """
 
         log = kwargs['log']
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         outputdir = os.path.join(idir, "output")
         srcdir = os.path.join(idir, "buildroot")
         reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
@@ -435,7 +438,7 @@ class Builder:
     def do_build(self, **kwargs):
         """Run the build itself"""
 
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         log = kwargs['log']
         nice = kwargs['nice']
 
@@ -466,9 +469,9 @@ class Builder:
         build_monitor.daemon = True
         build_monitor.start()
 
-        kwargs['buildpid'][kwargs['instance']] = sub.pid
+        kwargs['buildpid'][self.instance] = sub.pid
         ret = sub.wait()
-        kwargs['buildpid'][kwargs['instance']] = 0
+        kwargs['buildpid'][self.instance] = 0
 
         # If build failed, monitor thread would have exited at this point
         if monitor_thread_hung_build_flag.is_set():
@@ -500,7 +503,7 @@ class Builder:
         perform the actual build.
         """
 
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         outputdir = os.path.abspath(os.path.join(idir, "output"))
         srcdir = os.path.join(idir, "buildroot")
         log = kwargs['log']
@@ -538,7 +541,7 @@ class Builder:
         are available) or stores them locally as tarballs.
         """
 
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
         log = kwargs['log']
 
         outputdir = os.path.abspath(os.path.join(idir, "output"))
@@ -690,7 +693,7 @@ class Builder:
             # No http login/password, keep tarballs locally
             with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
                 sha1 = hashlib.sha1(f.read()).hexdigest()
-            resultfilename = "instance-%d-%s.tar.bz2" % (kwargs['instance'], sha1)
+            resultfilename = "instance-%d-%s.tar.bz2" % (self.instance, sha1)
             os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
             log_write(log, "INFO: results saved as %s" % resultfilename)
 
@@ -701,7 +704,7 @@ class Builder:
         results.
         """
 
-        idir = "instance-%d" % kwargs['instance']
+        idir = "instance-%d" % self.instance
 
         # If it doesn't exist, create the instance directory
         if not os.path.exists(idir):
@@ -840,9 +843,8 @@ def main():
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        builder = Builder()
+        builder = Builder(i)
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                instance = i,
                 njobs = args['--njobs'],
                 sysinfo = sysinfo,
                 http_url = args['--http-url'],
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 03/19] autobuild-run: move njobs from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
  2019-06-21  8:47 ` [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:34   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo " Atharva Lele
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 291cd9f..82b8f45 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -271,8 +271,9 @@ class SystemInfo:
         return not missing_requirements
 
 class Builder:
-    def __init__(self, instance):
+    def __init__(self, instance, njobs):
         self.instance = instance
+        self.njobs = njobs
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -455,7 +456,7 @@ class Builder:
         cmd = ["nice", "-n", str(nice),
                 "make", "O=%s" % outputdir,
                 "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
-                "BR2_JLEVEL=%s" % kwargs['njobs']] \
+                "BR2_JLEVEL=%s" % self.njobs] \
             + kwargs['make_opts'].split()
         sub = subprocess.Popen(cmd, stdout=f, stderr=f)
 
@@ -843,9 +844,8 @@ def main():
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        builder = Builder(i)
+        builder = Builder(i, args['--njobs'])
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                njobs = args['--njobs'],
                 sysinfo = sysinfo,
                 http_url = args['--http-url'],
                 http_login = args['--http-login'],
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
  2019-06-21  8:47 ` [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to " Atharva Lele
  2019-06-21  8:47 ` [Buildroot] [PATCH 03/19] autobuild-run: move njobs " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:35   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 05/19] autobuild-run: move http variables " Atharva Lele
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 82b8f45..22452cf 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -271,9 +271,10 @@ class SystemInfo:
         return not missing_requirements
 
 class Builder:
-    def __init__(self, instance, njobs):
+    def __init__(self, instance, njobs, sysinfo):
         self.instance = instance
         self.njobs = njobs
+        self.sysinfo = sysinfo
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -416,7 +417,7 @@ class Builder:
         build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
 
         with open(reproducible_results, 'w') as diff:
-            if kwargs['sysinfo'].has("diffoscope"):
+            if self.sysinfo.has("diffoscope"):
                 # Prefix to point diffoscope towards cross-tools
                 prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
                 # Remove TARGET_CROSS= and \n from the string
@@ -844,9 +845,8 @@ def main():
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        builder = Builder(i, args['--njobs'])
+        builder = Builder(i, args['--njobs'], sysinfo)
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                sysinfo = sysinfo,
                 http_url = args['--http-url'],
                 http_login = args['--http-login'],
                 http_password = args['--http-password'],
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 05/19] autobuild-run: move http variables from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (2 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:36   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 06/19] autobuild-run: move submitter " Atharva Lele
                   ` (14 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 22452cf..03c54a9 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -271,10 +271,14 @@ class SystemInfo:
         return not missing_requirements
 
 class Builder:
-    def __init__(self, instance, njobs, sysinfo):
+    def __init__(self, instance, njobs, sysinfo,
+                 http_url, http_login, http_password):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
+        self.http_url = http_url
+        self.http_login = http_login
+        self.http_password = http_password
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -681,11 +685,11 @@ class Builder:
             # none of the ones that are part of the standard library can
             # upload a file without writing dozens of lines of code.
             ret = subprocess.call(["curl", "-u",
-                                "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
+                                "%s:%s" % (self.http_login, self.http_password),
                                 "-H", "Expect:",
                                 "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
                                 "-F", "uploadsubmit=1",
-                                kwargs['http_url']],
+                                self.http_url],
                                 stdout=log, stderr=log)
             if ret != 0:
                 log_write(log, "INFO: results could not be submitted, %d" % ret)
@@ -845,11 +849,9 @@ def main():
     buildpid = multiprocessing.Array('i', int(args['--ninstances']))
     processes = []
     for i in range(0, int(args['--ninstances'])):
-        builder = Builder(i, args['--njobs'], sysinfo)
+        builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
+                          args['--http-login'], args['--http-password'])
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                http_url = args['--http-url'],
-                http_login = args['--http-login'],
-                http_password = args['--http-password'],
                 submitter = args['--submitter'],
                 make_opts = (args['--make-opts'] or ''),
                 nice = (args['--nice'] or 0),
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 06/19] autobuild-run: move submitter from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (3 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 05/19] autobuild-run: move http variables " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:38   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 07/19] autobuild-run: move niceness " Atharva Lele
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 03c54a9..20b453e 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -272,13 +272,15 @@ class SystemInfo:
 
 class Builder:
     def __init__(self, instance, njobs, sysinfo,
-                 http_url, http_login, http_password):
+                 http_url, http_login, http_password,
+                 submitter):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
         self.http_url = http_url
         self.http_login = http_login
         self.http_password = http_password
+        self.submitter = submitter
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -670,7 +672,7 @@ class Builder:
         resultf.close()
 
         with open(os.path.join(resultdir, "submitter"), "w+") as submitterf:
-            submitterf.write(kwargs['submitter'])
+            submitterf.write(self.submitter)
 
         # Yes, shutil.make_archive() would be nice, but it doesn't exist
         # in Python 2.6.
@@ -850,9 +852,9 @@ def main():
     processes = []
     for i in range(0, int(args['--ninstances'])):
         builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
-                          args['--http-login'], args['--http-password'])
+                          args['--http-login'], args['--http-password'],
+                          args['--submitter'])
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                submitter = args['--submitter'],
                 make_opts = (args['--make-opts'] or ''),
                 nice = (args['--nice'] or 0),
                 toolchains_csv = args['--toolchains-csv'],
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 07/19] autobuild-run: move niceness from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (4 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 06/19] autobuild-run: move submitter " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:38   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv " Atharva Lele
                   ` (12 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 20b453e..a435570 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -273,7 +273,7 @@ class SystemInfo:
 class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
-                 submitter):
+                 submitter, make_opts, nice):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -281,6 +281,8 @@ class Builder:
         self.http_login = http_login
         self.http_password = http_password
         self.submitter = submitter
+        self.make_opts = make_opts
+        self.nice = nice
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -448,7 +450,6 @@ class Builder:
 
         idir = "instance-%d" % self.instance
         log = kwargs['log']
-        nice = kwargs['nice']
 
         # We need the absolute path to use with O=, because the relative
         # path to the output directory here is not relative to the
@@ -460,11 +461,11 @@ class Builder:
         f = open(os.path.join(outputdir, "logfile"), "w+")
         log_write(log, "INFO: build started")
 
-        cmd = ["nice", "-n", str(nice),
+        cmd = ["nice", "-n", str(self.nice),
                 "make", "O=%s" % outputdir,
                 "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
                 "BR2_JLEVEL=%s" % self.njobs] \
-            + kwargs['make_opts'].split()
+            + self.make_opts.split()
         sub = subprocess.Popen(cmd, stdout=f, stderr=f)
 
         # Setup hung build monitoring thread
@@ -496,7 +497,7 @@ class Builder:
 
         cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
                 "BR2_DL_DIR=%s" % dldir, "legal-info"] \
-            + kwargs['make_opts'].split()
+            + self.make_opts.split()
         ret = subprocess.call(cmd, stdout=f, stderr=f)
         if ret != 0:
             log_write(log, "INFO: build failed during legal-info")
@@ -853,10 +854,9 @@ def main():
     for i in range(0, int(args['--ninstances'])):
         builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
                           args['--http-login'], args['--http-password'],
-                          args['--submitter'])
+                          args['--submitter'], (args['--make-opts'] or ''),
+                          (args['--nice'] or 0))
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                make_opts = (args['--make-opts'] or ''),
-                nice = (args['--nice'] or 0),
                 toolchains_csv = args['--toolchains-csv'],
                 repo = args['--repo'],
                 upload = upload,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (5 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 07/19] autobuild-run: move niceness " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:37   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 09/19] autobuild-run: move repo " Atharva Lele
                   ` (11 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a435570..f737b27 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -273,7 +273,7 @@ class SystemInfo:
 class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
-                 submitter, make_opts, nice):
+                 submitter, make_opts, nice, toolchains_csv):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -283,6 +283,7 @@ class Builder:
         self.submitter = submitter
         self.make_opts = make_opts
         self.nice = nice
+        self.toolchains_csv = toolchains_csv
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -381,7 +382,7 @@ class Builder:
         args = [os.path.join(srcdir, "utils/genrandconfig"),
                 "-o", outputdir, "-b", srcdir]
 
-        toolchains_csv = kwargs['toolchains_csv']
+        toolchains_csv = self.toolchains_csv
         if toolchains_csv:
             if not os.path.isabs(toolchains_csv):
                 toolchains_csv = os.path.join(srcdir, toolchains_csv)
@@ -855,9 +856,8 @@ def main():
         builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
                           args['--http-login'], args['--http-password'],
                           args['--submitter'], (args['--make-opts'] or ''),
-                          (args['--nice'] or 0))
+                          (args['--nice'] or 0), args['--toolchains-csv'])
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                toolchains_csv = args['--toolchains-csv'],
                 repo = args['--repo'],
                 upload = upload,
                 buildpid = buildpid,
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 09/19] autobuild-run: move repo from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (6 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:39   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 10/19] autobuild-run: move upload variable " Atharva Lele
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index f737b27..a883ca8 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -273,7 +273,8 @@ class SystemInfo:
 class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
-                 submitter, make_opts, nice, toolchains_csv):
+                 submitter, make_opts, nice, toolchains_csv,
+                 repo):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -284,6 +285,7 @@ class Builder:
         self.make_opts = make_opts
         self.nice = nice
         self.toolchains_csv = toolchains_csv
+        self.repo = repo
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -336,7 +338,7 @@ class Builder:
         # didn't exist already.
         srcdir = os.path.join(idir, "buildroot")
         if not os.path.exists(srcdir):
-            ret = subprocess.call(["git", "clone", kwargs['repo'], srcdir],
+            ret = subprocess.call(["git", "clone", self.repo, srcdir],
                                 stdout=log, stderr=log)
             if ret != 0:
                 log_write(log, "ERROR: could not clone Buildroot sources")
@@ -856,9 +858,9 @@ def main():
         builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
                           args['--http-login'], args['--http-password'],
                           args['--submitter'], (args['--make-opts'] or ''),
-                          (args['--nice'] or 0), args['--toolchains-csv'])
+                          (args['--nice'] or 0), args['--toolchains-csv'],
+                          args['--repo'])
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                repo = args['--repo'],
                 upload = upload,
                 buildpid = buildpid,
                 debug = args['--debug']
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 10/19] autobuild-run: move upload variable from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (7 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 09/19] autobuild-run: move repo " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:40   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 11/19] autobuild-run: move buildpid " Atharva Lele
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a883ca8..7695211 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -274,7 +274,7 @@ class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
                  submitter, make_opts, nice, toolchains_csv,
-                 repo):
+                 repo, upload):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -286,6 +286,7 @@ class Builder:
         self.nice = nice
         self.toolchains_csv = toolchains_csv
         self.repo = repo
+        self.upload = upload
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -686,7 +687,7 @@ class Builder:
             log_write(log, "ERROR: could not make results tarball")
             sys.exit(1)
 
-        if kwargs['upload']:
+        if self.upload:
             # Submit results. Yes, Python has some HTTP libraries, but
             # none of the ones that are part of the standard library can
             # upload a file without writing dozens of lines of code.
@@ -859,9 +860,8 @@ def main():
                           args['--http-login'], args['--http-password'],
                           args['--submitter'], (args['--make-opts'] or ''),
                           (args['--nice'] or 0), args['--toolchains-csv'],
-                          args['--repo'])
+                          args['--repo'], upload)
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                upload = upload,
                 buildpid = buildpid,
                 debug = args['--debug']
             ))
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 11/19] autobuild-run: move buildpid from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (8 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 10/19] autobuild-run: move upload variable " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:41   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 12/19] autobuild-run: move debug " Atharva Lele
                   ` (8 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 7695211..7c3e891 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -274,7 +274,7 @@ class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
                  submitter, make_opts, nice, toolchains_csv,
-                 repo, upload):
+                 repo, upload, buildpid):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -287,6 +287,7 @@ class Builder:
         self.toolchains_csv = toolchains_csv
         self.repo = repo
         self.upload = upload
+        self.buildpid = buildpid
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -482,9 +483,9 @@ class Builder:
         build_monitor.daemon = True
         build_monitor.start()
 
-        kwargs['buildpid'][self.instance] = sub.pid
+        self.buildpid[self.instance] = sub.pid
         ret = sub.wait()
-        kwargs['buildpid'][self.instance] = 0
+        self.buildpid[self.instance] = 0
 
         # If build failed, monitor thread would have exited at this point
         if monitor_thread_hung_build_flag.is_set():
@@ -860,9 +861,8 @@ def main():
                           args['--http-login'], args['--http-password'],
                           args['--submitter'], (args['--make-opts'] or ''),
                           (args['--nice'] or 0), args['--toolchains-csv'],
-                          args['--repo'], upload)
+                          args['--repo'], upload, buildpid)
         p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                buildpid = buildpid,
                 debug = args['--debug']
             ))
         p.start()
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 12/19] autobuild-run: move debug from kwargs to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (9 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 11/19] autobuild-run: move buildpid " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:43   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of " Atharva Lele
                   ` (7 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 7c3e891..0db2995 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -274,7 +274,7 @@ class Builder:
     def __init__(self, instance, njobs, sysinfo,
                  http_url, http_login, http_password,
                  submitter, make_opts, nice, toolchains_csv,
-                 repo, upload, buildpid):
+                 repo, upload, buildpid, debug):
         self.instance = instance
         self.njobs = njobs
         self.sysinfo = sysinfo
@@ -288,6 +288,7 @@ class Builder:
         self.repo = repo
         self.upload = upload
         self.buildpid = buildpid
+        self.debug = debug
 
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
@@ -378,7 +379,7 @@ class Builder:
 
         log_write(log, "INFO: generate the configuration")
 
-        if kwargs['debug']:
+        if self.debug:
             devnull = log
         else:
             devnull = open(os.devnull, "w")
@@ -724,7 +725,7 @@ class Builder:
         if not os.path.exists(idir):
             os.mkdir(idir)
 
-        if kwargs['debug']:
+        if self.debug:
             kwargs['log'] = sys.stdout
         else:
             kwargs['log'] = open(os.path.join(idir, "instance.log"), "a+")
@@ -861,10 +862,8 @@ def main():
                           args['--http-login'], args['--http-password'],
                           args['--submitter'], (args['--make-opts'] or ''),
                           (args['--nice'] or 0), args['--toolchains-csv'],
-                          args['--repo'], upload, buildpid)
-        p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
-                debug = args['--debug']
-            ))
+                          args['--repo'], upload, buildpid, args['--debug'])
+        p = multiprocessing.Process(target=builder.run_instance)
         p.start()
         processes.append(p)
 
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (10 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 12/19] autobuild-run: move debug " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-24 21:44   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 14/19] autobuild-run: move log variable to " Atharva Lele
                   ` (6 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

We need idir as a class member before moving log out of kwargs.

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 49 +++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 0db2995..e69020c 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -290,6 +290,9 @@ class Builder:
         self.buildpid = buildpid
         self.debug = debug
 
+        # frequently needed directories
+        self.idir = "instance-%d" % self.instance
+
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
 
@@ -298,13 +301,12 @@ class Builder:
         code, and cleaning up remaining stuff from previous builds.
         """
 
-        idir = "instance-%d" % self.instance
         log = kwargs['log']
 
         log_write(log, "INFO: preparing a new build")
 
         # Create the download directory if it doesn't exist
-        dldir = os.path.join(idir, "dl")
+        dldir = os.path.join(self.idir, "dl")
         if not os.path.exists(dldir):
             os.mkdir(dldir)
 
@@ -339,7 +341,7 @@ class Builder:
 
         # Clone Buildroot. This only happens if the source directory
         # didn't exist already.
-        srcdir = os.path.join(idir, "buildroot")
+        srcdir = os.path.join(self.idir, "buildroot")
         if not os.path.exists(srcdir):
             ret = subprocess.call(["git", "clone", self.repo, srcdir],
                                 stdout=log, stderr=log)
@@ -360,7 +362,7 @@ class Builder:
             return -1
 
         # Create an empty output directory. We remove it first, in case a previous build was aborted.
-        outputdir = os.path.join(idir, "output")
+        outputdir = os.path.join(self.idir, "output")
         if os.path.exists(outputdir):
             # shutil.rmtree doesn't remove write-protected files
             subprocess.call(["rm", "-rf", outputdir])
@@ -372,10 +374,9 @@ class Builder:
 
     def gen_config(self, **kwargs):
         """Generate a new random configuration."""
-        idir = "instance-%d" % self.instance
         log = kwargs['log']
-        outputdir = os.path.abspath(os.path.join(idir, "output"))
-        srcdir = os.path.join(idir, "buildroot")
+        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
+        srcdir = os.path.join(self.idir, "buildroot")
 
         log_write(log, "INFO: generate the configuration")
 
@@ -422,9 +423,8 @@ class Builder:
         """
 
         log = kwargs['log']
-        idir = "instance-%d" % self.instance
-        outputdir = os.path.join(idir, "output")
-        srcdir = os.path.join(idir, "buildroot")
+        outputdir = os.path.join(self.idir, "output")
+        srcdir = os.path.join(self.idir, "buildroot")
         reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
         # Using only tar images for now
         build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
@@ -454,16 +454,15 @@ class Builder:
     def do_build(self, **kwargs):
         """Run the build itself"""
 
-        idir = "instance-%d" % self.instance
         log = kwargs['log']
 
         # We need the absolute path to use with O=, because the relative
         # path to the output directory here is not relative to the
         # Buildroot sources, but to the location of the autobuilder
         # script.
-        dldir = os.path.abspath(os.path.join(idir, "dl"))
-        outputdir = os.path.abspath(os.path.join(idir, "output"))
-        srcdir = os.path.join(idir, "buildroot")
+        dldir = os.path.abspath(os.path.join(self.idir, "dl"))
+        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
+        srcdir = os.path.join(self.idir, "buildroot")
         f = open(os.path.join(outputdir, "logfile"), "w+")
         log_write(log, "INFO: build started")
 
@@ -518,9 +517,8 @@ class Builder:
         perform the actual build.
         """
 
-        idir = "instance-%d" % self.instance
-        outputdir = os.path.abspath(os.path.join(idir, "output"))
-        srcdir = os.path.join(idir, "buildroot")
+        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
+        srcdir = os.path.join(self.idir, "buildroot")
         log = kwargs['log']
 
         # Start the first build
@@ -556,11 +554,10 @@ class Builder:
         are available) or stores them locally as tarballs.
         """
 
-        idir = "instance-%d" % self.instance
         log = kwargs['log']
 
-        outputdir = os.path.abspath(os.path.join(idir, "output"))
-        srcdir = os.path.join(idir, "buildroot")
+        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
+        srcdir = os.path.join(self.idir, "buildroot")
         resultdir = os.path.join(outputdir, "results")
 
         shutil.copyfile(os.path.join(outputdir, ".config"),
@@ -719,16 +716,14 @@ class Builder:
         results.
         """
 
-        idir = "instance-%d" % self.instance
-
         # If it doesn't exist, create the instance directory
-        if not os.path.exists(idir):
-            os.mkdir(idir)
+        if not os.path.exists(self.idir):
+            os.mkdir(self.idir)
 
         if self.debug:
             kwargs['log'] = sys.stdout
         else:
-            kwargs['log'] = open(os.path.join(idir, "instance.log"), "a+")
+            kwargs['log'] = open(os.path.join(self.idir, "instance.log"), "a+")
         log_write(kwargs['log'], "INFO: instance started")
 
         while True:
@@ -738,7 +733,7 @@ class Builder:
             if ret != 0:
                 continue
 
-            resultdir = os.path.join(idir, "output", "results")
+            resultdir = os.path.join(self.idir, "output", "results")
             os.mkdir(resultdir)
 
             ret = self.gen_config(**kwargs)
@@ -747,7 +742,7 @@ class Builder:
                 continue
 
             # Check if the build test is supposed to be a reproducible test
-            outputdir = os.path.abspath(os.path.join(idir, "output"))
+            outputdir = os.path.abspath(os.path.join(self.idir, "output"))
             with open(os.path.join(outputdir, ".config"), "r") as fconf:
                 reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
             if reproducible:
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 14/19] autobuild-run: move log variable to Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (11 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 14:20   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions Atharva Lele
                   ` (5 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 94 ++++++++++++++++++++-----------------------
 1 file changed, 43 insertions(+), 51 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index e69020c..ff8ef0a 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -293,6 +293,11 @@ class Builder:
         # frequently needed directories
         self.idir = "instance-%d" % self.instance
 
+        if self.debug:
+            self.log = sys.stdout
+        else:
+            self.log = open(os.path.join(self.idir, "instance.log"), "a+")
+
     def prepare_build(self, **kwargs):
         """Prepare for the next build of the specified instance
 
@@ -301,9 +306,7 @@ class Builder:
         code, and cleaning up remaining stuff from previous builds.
         """
 
-        log = kwargs['log']
-
-        log_write(log, "INFO: preparing a new build")
+        log_write(self.log, "INFO: preparing a new build")
 
         # Create the download directory if it doesn't exist
         dldir = os.path.join(self.idir, "dl")
@@ -332,33 +335,33 @@ class Builder:
             if not flist:
                 break
             f = flist[randint(0, len(flist) - 1)]
-            log_write(log, "INFO: removing %s from downloads" %
+            log_write(self.log, "INFO: removing %s from downloads" %
                     os.path.relpath(f, dldir))
             os.remove(f)
 
         branch = get_branch()
-        log_write(log, "INFO: testing branch '%s'" % branch)
+        log_write(self.log, "INFO: testing branch '%s'" % branch)
 
         # Clone Buildroot. This only happens if the source directory
         # didn't exist already.
         srcdir = os.path.join(self.idir, "buildroot")
         if not os.path.exists(srcdir):
             ret = subprocess.call(["git", "clone", self.repo, srcdir],
-                                stdout=log, stderr=log)
+                                stdout=self.log, stderr=self.log)
             if ret != 0:
-                log_write(log, "ERROR: could not clone Buildroot sources")
+                log_write(self.log, "ERROR: could not clone Buildroot sources")
                 return -1
 
         # Update the Buildroot sources.
         abssrcdir = os.path.abspath(srcdir)
-        ret = subprocess.call(["git", "fetch", "origin"], cwd=abssrcdir, stdout=log, stderr=log)
+        ret = subprocess.call(["git", "fetch", "origin"], cwd=abssrcdir, stdout=self.log, stderr=self.log)
         if ret != 0:
-            log_write(log, "ERROR: could not fetch Buildroot sources")
+            log_write(self.log, "ERROR: could not fetch Buildroot sources")
             return -1
 
-        ret = subprocess.call(["git", "checkout", "--detach", "origin/%s" % branch], cwd=abssrcdir, stdout=log, stderr=log)
+        ret = subprocess.call(["git", "checkout", "--detach", "origin/%s" % branch], cwd=abssrcdir, stdout=self.log, stderr=self.log)
         if ret != 0:
-            log_write(log, "ERROR: could not check out Buildroot sources")
+            log_write(self.log, "ERROR: could not check out Buildroot sources")
             return -1
 
         # Create an empty output directory. We remove it first, in case a previous build was aborted.
@@ -374,14 +377,13 @@ class Builder:
 
     def gen_config(self, **kwargs):
         """Generate a new random configuration."""
-        log = kwargs['log']
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         srcdir = os.path.join(self.idir, "buildroot")
 
-        log_write(log, "INFO: generate the configuration")
+        log_write(self.log, "INFO: generate the configuration")
 
         if self.debug:
-            devnull = log
+            devnull = self.log
         else:
             devnull = open(os.devnull, "w")
 
@@ -394,12 +396,12 @@ class Builder:
                 toolchains_csv = os.path.join(srcdir, toolchains_csv)
             args.extend(["--toolchains-csv", toolchains_csv])
 
-        ret = subprocess.call(args, stdout=devnull, stderr=log)
+        ret = subprocess.call(args, stdout=devnull, stderr=self.log)
         return ret
 
     def stop_on_build_hang(self, monitor_thread_hung_build_flag,
                         monitor_thread_stop_flag,
-                        sub_proc, outputdir, log):
+                        sub_proc, outputdir):
         build_time_logfile = os.path.join(outputdir, "build/build-time.log")
         while True:
             if monitor_thread_stop_flag.is_set():
@@ -410,7 +412,7 @@ class Builder:
                 if mtime < datetime.datetime.now() - datetime.timedelta(minutes=HUNG_BUILD_TIMEOUT):
                     if sub_proc.poll() is None:
                         monitor_thread_hung_build_flag.set() # Used by do_build() to determine build hang
-                        log_write(log, "INFO: build hung")
+                        log_write(self.log, "INFO: build hung")
                         sub_proc.kill()
                     break
             monitor_thread_stop_flag.wait(30)
@@ -422,7 +424,6 @@ class Builder:
         installed, fallback to cmp
         """
 
-        log = kwargs['log']
         outputdir = os.path.join(self.idir, "output")
         srcdir = os.path.join(self.idir, "buildroot")
         reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
@@ -436,26 +437,24 @@ class Builder:
                 prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
                 # Remove TARGET_CROSS= and \n from the string
                 prefix = prefix[13:-1]
-                log_write(log, "INFO: running diffoscope on images")
+                log_write(self.log, "INFO: running diffoscope on images")
                 subprocess.call(["diffoscope", build_1_image, build_2_image,
-                                    "--tool-prefix-binutils", prefix], stdout=diff, stderr=log)
+                                    "--tool-prefix-binutils", prefix], stdout=diff, stderr=self.log)
             else:
-                log_write(log, "INFO: diffoscope not installed, falling back to cmp")
-                subprocess.call(["cmp", "-b", build_1_image, build_2_image], stdout=diff, stderr=log)
+                log_write(self.log, "INFO: diffoscope not installed, falling back to cmp")
+                subprocess.call(["cmp", "-b", build_1_image, build_2_image], stdout=diff, stderr=self.log)
 
         if os.stat(reproducible_results).st_size > 0:
-            log_write(log, "INFO: Build is non-reproducible.")
+            log_write(self.log, "INFO: Build is non-reproducible.")
             return -1
 
         # rootfs images match byte-for-byte -> reproducible image
-        log_write(log, "INFO: Build is reproducible!")
+        log_write(self.log, "INFO: Build is reproducible!")
         return 0
 
     def do_build(self, **kwargs):
         """Run the build itself"""
 
-        log = kwargs['log']
-
         # We need the absolute path to use with O=, because the relative
         # path to the output directory here is not relative to the
         # Buildroot sources, but to the location of the autobuilder
@@ -464,7 +463,7 @@ class Builder:
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         srcdir = os.path.join(self.idir, "buildroot")
         f = open(os.path.join(outputdir, "logfile"), "w+")
-        log_write(log, "INFO: build started")
+        log_write(self.log, "INFO: build started")
 
         cmd = ["nice", "-n", str(self.nice),
                 "make", "O=%s" % outputdir,
@@ -479,7 +478,7 @@ class Builder:
         build_monitor = Thread(target=self.stop_on_build_hang,
                             args=(monitor_thread_hung_build_flag,
                                     monitor_thread_stop_flag,
-                                    sub, outputdir, log))
+                                    sub, outputdir))
         build_monitor.daemon = True
         build_monitor.start()
 
@@ -489,7 +488,7 @@ class Builder:
 
         # If build failed, monitor thread would have exited at this point
         if monitor_thread_hung_build_flag.is_set():
-            log_write(log, "INFO: build timed out [%d]" % ret)
+            log_write(self.log, "INFO: build timed out [%d]" % ret)
             return -2
         else:
             # Stop monitor thread as this build didn't timeout
@@ -497,7 +496,7 @@ class Builder:
         # Monitor thread should be exiting around this point
 
         if ret != 0:
-            log_write(log, "INFO: build failed [%d]" % ret)
+            log_write(self.log, "INFO: build failed [%d]" % ret)
             return -1
 
         cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
@@ -505,9 +504,9 @@ class Builder:
             + self.make_opts.split()
         ret = subprocess.call(cmd, stdout=f, stderr=f)
         if ret != 0:
-            log_write(log, "INFO: build failed during legal-info")
+            log_write(self.log, "INFO: build failed during legal-info")
             return -1
-        log_write(log, "INFO: build successful")
+        log_write(self.log, "INFO: build successful")
         return 0
 
     def do_reproducible_build(self, **kwargs):
@@ -519,13 +518,12 @@ class Builder:
 
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         srcdir = os.path.join(self.idir, "buildroot")
-        log = kwargs['log']
 
         # Start the first build
-        log_write(log, "INFO: Reproducible Build Test, starting build 1")
+        log_write(self.log, "INFO: Reproducible Build Test, starting build 1")
         ret = self.do_build(**kwargs)
         if ret != 0:
-            log_write(log, "INFO: build 1 failed, skipping build 2")
+            log_write(self.log, "INFO: build 1 failed, skipping build 2")
             return ret
 
         # First build has been built, move files and start build 2
@@ -536,10 +534,10 @@ class Builder:
         subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "clean"], stdout=f, stderr=f)
 
         # Start the second build
-        log_write(log, "INFO: Reproducible Build Test, starting build 2")
+        log_write(self.log, "INFO: Reproducible Build Test, starting build 2")
         ret = self.do_build(**kwargs)
         if ret != 0:
-            log_write(log, "INFO: build 2 failed")
+            log_write(self.log, "INFO: build 2 failed")
             return ret
 
         # Assuming both have built successfully
@@ -554,8 +552,6 @@ class Builder:
         are available) or stores them locally as tarballs.
         """
 
-        log = kwargs['log']
-
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         srcdir = os.path.join(self.idir, "buildroot")
         resultdir = os.path.join(outputdir, "results")
@@ -681,9 +677,9 @@ class Builder:
         # Yes, shutil.make_archive() would be nice, but it doesn't exist
         # in Python 2.6.
         ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
-                            cwd=outputdir, stdout=log, stderr=log)
+                            cwd=outputdir, stdout=self.log, stderr=self.log)
         if ret != 0:
-            log_write(log, "ERROR: could not make results tarball")
+            log_write(self.log, "ERROR: could not make results tarball")
             sys.exit(1)
 
         if self.upload:
@@ -696,18 +692,18 @@ class Builder:
                                 "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
                                 "-F", "uploadsubmit=1",
                                 self.http_url],
-                                stdout=log, stderr=log)
+                                stdout=self.log, stderr=self.log)
             if ret != 0:
-                log_write(log, "INFO: results could not be submitted, %d" % ret)
+                log_write(self.log, "INFO: results could not be submitted, %d" % ret)
             else:
-                log_write(log, "INFO: results were submitted successfully")
+                log_write(self.log, "INFO: results were submitted successfully")
         else:
             # No http login/password, keep tarballs locally
             with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
                 sha1 = hashlib.sha1(f.read()).hexdigest()
             resultfilename = "instance-%d-%s.tar.bz2" % (self.instance, sha1)
             os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
-            log_write(log, "INFO: results saved as %s" % resultfilename)
+            log_write(self.log, "INFO: results saved as %s" % resultfilename)
 
     def run_instance(self, **kwargs):
         """Main per-instance loop
@@ -720,11 +716,7 @@ class Builder:
         if not os.path.exists(self.idir):
             os.mkdir(self.idir)
 
-        if self.debug:
-            kwargs['log'] = sys.stdout
-        else:
-            kwargs['log'] = open(os.path.join(self.idir, "instance.log"), "a+")
-        log_write(kwargs['log'], "INFO: instance started")
+        log_write(self.log, "INFO: instance started")
 
         while True:
             check_version()
@@ -738,7 +730,7 @@ class Builder:
 
             ret = self.gen_config(**kwargs)
             if ret != 0:
-                log_write(kwargs['log'], "WARN: failed to generate configuration")
+                log_write(self.log, "WARN: failed to generate configuration")
                 continue
 
             # Check if the build test is supposed to be a reproducible test
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (12 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 14/19] autobuild-run: move log variable to " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 20:03   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class Atharva Lele
                   ` (4 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index ff8ef0a..7c390c1 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -298,7 +298,7 @@ class Builder:
         else:
             self.log = open(os.path.join(self.idir, "instance.log"), "a+")
 
-    def prepare_build(self, **kwargs):
+    def prepare_build(self):
         """Prepare for the next build of the specified instance
 
         This function prepares the build by making sure all the needed
@@ -375,7 +375,7 @@ class Builder:
 
         return 0
 
-    def gen_config(self, **kwargs):
+    def gen_config(self):
         """Generate a new random configuration."""
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         srcdir = os.path.join(self.idir, "buildroot")
@@ -417,7 +417,7 @@ class Builder:
                     break
             monitor_thread_stop_flag.wait(30)
 
-    def check_reproducibility(self, **kwargs):
+    def check_reproducibility(self):
         """Check reproducibility of builds
 
         Use diffoscope on the built images, if diffoscope is not
@@ -452,7 +452,7 @@ class Builder:
         log_write(self.log, "INFO: Build is reproducible!")
         return 0
 
-    def do_build(self, **kwargs):
+    def do_build(self):
         """Run the build itself"""
 
         # We need the absolute path to use with O=, because the relative
@@ -509,7 +509,7 @@ class Builder:
         log_write(self.log, "INFO: build successful")
         return 0
 
-    def do_reproducible_build(self, **kwargs):
+    def do_reproducible_build(self):
         """Run the builds for reproducibility testing
 
         Build twice with the same configuration. Calls do_build() to
@@ -521,7 +521,7 @@ class Builder:
 
         # Start the first build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 1")
-        ret = self.do_build(**kwargs)
+        ret = self.do_build()
         if ret != 0:
             log_write(self.log, "INFO: build 1 failed, skipping build 2")
             return ret
@@ -535,16 +535,16 @@ class Builder:
 
         # Start the second build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 2")
-        ret = self.do_build(**kwargs)
+        ret = self.do_build()
         if ret != 0:
             log_write(self.log, "INFO: build 2 failed")
             return ret
 
         # Assuming both have built successfully
-        ret = self.check_reproducibility(**kwargs)
+        ret = self.check_reproducibility()
         return ret
 
-    def send_results(self, result, **kwargs):
+    def send_results(self, result):
         """Prepare and store/send tarball with results
 
         This function prepares the tarball with the results, and either
@@ -705,7 +705,7 @@ class Builder:
             os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
             log_write(self.log, "INFO: results saved as %s" % resultfilename)
 
-    def run_instance(self, **kwargs):
+    def run_instance(self):
         """Main per-instance loop
 
         Prepare the build, generate a configuration, run the build, and submit the
@@ -721,14 +721,14 @@ class Builder:
         while True:
             check_version()
 
-            ret = self.prepare_build(**kwargs)
+            ret = self.prepare_build()
             if ret != 0:
                 continue
 
             resultdir = os.path.join(self.idir, "output", "results")
             os.mkdir(resultdir)
 
-            ret = self.gen_config(**kwargs)
+            ret = self.gen_config()
             if ret != 0:
                 log_write(self.log, "WARN: failed to generate configuration")
                 continue
@@ -738,11 +738,11 @@ class Builder:
             with open(os.path.join(outputdir, ".config"), "r") as fconf:
                 reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
             if reproducible:
-                ret = self.do_reproducible_build(**kwargs)
+                ret = self.do_reproducible_build()
             else:
-                ret = self.do_build(**kwargs)
+                ret = self.do_build()
 
-            self.send_results(ret, **kwargs)
+            self.send_results(ret)
 
 # args / config file merging inspired by:
 # https://github.com/docopt/docopt/blob/master/examples/config_file_example.py
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (13 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 20:07   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 17/19] autobuild-run: define download " Atharva Lele
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 29 ++++++++++++-----------------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 7c390c1..a237dfd 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -292,6 +292,7 @@ class Builder:
 
         # frequently needed directories
         self.idir = "instance-%d" % self.instance
+        self.srcdir = os.path.join(self.idir, "buildroot")
 
         if self.debug:
             self.log = sys.stdout
@@ -344,16 +345,15 @@ class Builder:
 
         # Clone Buildroot. This only happens if the source directory
         # didn't exist already.
-        srcdir = os.path.join(self.idir, "buildroot")
-        if not os.path.exists(srcdir):
-            ret = subprocess.call(["git", "clone", self.repo, srcdir],
+        if not os.path.exists(self.srcdir):
+            ret = subprocess.call(["git", "clone", self.repo, self.srcdir],
                                 stdout=self.log, stderr=self.log)
             if ret != 0:
                 log_write(self.log, "ERROR: could not clone Buildroot sources")
                 return -1
 
         # Update the Buildroot sources.
-        abssrcdir = os.path.abspath(srcdir)
+        abssrcdir = os.path.abspath(self.srcdir)
         ret = subprocess.call(["git", "fetch", "origin"], cwd=abssrcdir, stdout=self.log, stderr=self.log)
         if ret != 0:
             log_write(self.log, "ERROR: could not fetch Buildroot sources")
@@ -378,7 +378,6 @@ class Builder:
     def gen_config(self):
         """Generate a new random configuration."""
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        srcdir = os.path.join(self.idir, "buildroot")
 
         log_write(self.log, "INFO: generate the configuration")
 
@@ -387,13 +386,13 @@ class Builder:
         else:
             devnull = open(os.devnull, "w")
 
-        args = [os.path.join(srcdir, "utils/genrandconfig"),
-                "-o", outputdir, "-b", srcdir]
+        args = [os.path.join(self.srcdir, "utils/genrandconfig"),
+                "-o", outputdir, "-b", self.srcdir]
 
         toolchains_csv = self.toolchains_csv
         if toolchains_csv:
             if not os.path.isabs(toolchains_csv):
-                toolchains_csv = os.path.join(srcdir, toolchains_csv)
+                toolchains_csv = os.path.join(self.srcdir, toolchains_csv)
             args.extend(["--toolchains-csv", toolchains_csv])
 
         ret = subprocess.call(args, stdout=devnull, stderr=self.log)
@@ -425,7 +424,6 @@ class Builder:
         """
 
         outputdir = os.path.join(self.idir, "output")
-        srcdir = os.path.join(self.idir, "buildroot")
         reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
         # Using only tar images for now
         build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
@@ -434,7 +432,7 @@ class Builder:
         with open(reproducible_results, 'w') as diff:
             if self.sysinfo.has("diffoscope"):
                 # Prefix to point diffoscope towards cross-tools
-                prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
+                prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", self.srcdir, "printvars", "VARS=TARGET_CROSS"])
                 # Remove TARGET_CROSS= and \n from the string
                 prefix = prefix[13:-1]
                 log_write(self.log, "INFO: running diffoscope on images")
@@ -461,13 +459,12 @@ class Builder:
         # script.
         dldir = os.path.abspath(os.path.join(self.idir, "dl"))
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        srcdir = os.path.join(self.idir, "buildroot")
         f = open(os.path.join(outputdir, "logfile"), "w+")
         log_write(self.log, "INFO: build started")
 
         cmd = ["nice", "-n", str(self.nice),
                 "make", "O=%s" % outputdir,
-                "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
+                "-C", self.srcdir, "BR2_DL_DIR=%s" % dldir,
                 "BR2_JLEVEL=%s" % self.njobs] \
             + self.make_opts.split()
         sub = subprocess.Popen(cmd, stdout=f, stderr=f)
@@ -499,7 +496,7 @@ class Builder:
             log_write(self.log, "INFO: build failed [%d]" % ret)
             return -1
 
-        cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
+        cmd = ["make", "O=%s" % outputdir, "-C", self.srcdir,
                 "BR2_DL_DIR=%s" % dldir, "legal-info"] \
             + self.make_opts.split()
         ret = subprocess.call(cmd, stdout=f, stderr=f)
@@ -517,7 +514,6 @@ class Builder:
         """
 
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        srcdir = os.path.join(self.idir, "buildroot")
 
         # Start the first build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 1")
@@ -531,7 +527,7 @@ class Builder:
 
         # Clean up build 1
         f = open(os.path.join(outputdir, "logfile"), "w+")
-        subprocess.call(["make", "O=%s" % outputdir, "-C", srcdir, "clean"], stdout=f, stderr=f)
+        subprocess.call(["make", "O=%s" % outputdir, "-C", self.srcdir, "clean"], stdout=f, stderr=f)
 
         # Start the second build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 2")
@@ -553,7 +549,6 @@ class Builder:
         """
 
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        srcdir = os.path.join(self.idir, "buildroot")
         resultdir = os.path.join(outputdir, "results")
 
         shutil.copyfile(os.path.join(outputdir, ".config"),
@@ -576,7 +571,7 @@ class Builder:
 
         subprocess.call(["git log -n 1 --pretty=format:%%H > %s" % \
                         os.path.join(resultdir, "gitid")],
-                        shell=True, cwd=srcdir)
+                        shell=True, cwd=self.srcdir)
 
         # Return True if the result should be rejected, False otherwise
         def reject_results():
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 17/19] autobuild-run: define download directory as part of Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (14 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 20:08   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 18/19] autobuild-run: define output " Atharva Lele
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index a237dfd..f9e2414 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -293,6 +293,7 @@ class Builder:
         # frequently needed directories
         self.idir = "instance-%d" % self.instance
         self.srcdir = os.path.join(self.idir, "buildroot")
+        self.dldir = os.path.join(self.idir, "dl")
 
         if self.debug:
             self.log = sys.stdout
@@ -310,9 +311,8 @@ class Builder:
         log_write(self.log, "INFO: preparing a new build")
 
         # Create the download directory if it doesn't exist
-        dldir = os.path.join(self.idir, "dl")
-        if not os.path.exists(dldir):
-            os.mkdir(dldir)
+        if not os.path.exists(self.dldir):
+            os.mkdir(self.dldir)
 
         # recursively find files under root
         def find_files(root):
@@ -332,12 +332,12 @@ class Builder:
         # regularly re-download files to check that their upstream
         # location is still correct.
         for i in range(0, 5):
-            flist = list(find_files(dldir))
+            flist = list(find_files(self.dldir))
             if not flist:
                 break
             f = flist[randint(0, len(flist) - 1)]
             log_write(self.log, "INFO: removing %s from downloads" %
-                    os.path.relpath(f, dldir))
+                    os.path.relpath(f, self.dldir))
             os.remove(f)
 
         branch = get_branch()
@@ -457,14 +457,13 @@ class Builder:
         # path to the output directory here is not relative to the
         # Buildroot sources, but to the location of the autobuilder
         # script.
-        dldir = os.path.abspath(os.path.join(self.idir, "dl"))
         outputdir = os.path.abspath(os.path.join(self.idir, "output"))
         f = open(os.path.join(outputdir, "logfile"), "w+")
         log_write(self.log, "INFO: build started")
 
         cmd = ["nice", "-n", str(self.nice),
                 "make", "O=%s" % outputdir,
-                "-C", self.srcdir, "BR2_DL_DIR=%s" % dldir,
+                "-C", self.srcdir, "BR2_DL_DIR=%s" % self.dldir,
                 "BR2_JLEVEL=%s" % self.njobs] \
             + self.make_opts.split()
         sub = subprocess.Popen(cmd, stdout=f, stderr=f)
@@ -497,7 +496,7 @@ class Builder:
             return -1
 
         cmd = ["make", "O=%s" % outputdir, "-C", self.srcdir,
-                "BR2_DL_DIR=%s" % dldir, "legal-info"] \
+                "BR2_DL_DIR=%s" % self.dldir, "legal-info"] \
             + self.make_opts.split()
         ret = subprocess.call(cmd, stdout=f, stderr=f)
         if ret != 0:
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 18/19] autobuild-run: define output directory as part of Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (15 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 17/19] autobuild-run: define download " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 20:10   ` Arnout Vandecappelle
  2019-06-21  8:47 ` [Buildroot] [PATCH 19/19] autobuild-run: define results " Atharva Lele
  2019-06-24 21:22 ` [Buildroot] [PATCH 01/19] autobuild-run: introduce " Arnout Vandecappelle
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 86 ++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 47 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index f9e2414..5f0fd0a 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -294,6 +294,11 @@ class Builder:
         self.idir = "instance-%d" % self.instance
         self.srcdir = os.path.join(self.idir, "buildroot")
         self.dldir = os.path.join(self.idir, "dl")
+        # We need the absolute path to use with O=, because the relative
+        # path to the output directory here is not relative to the
+        # Buildroot sources, but to the location of the autobuilder
+        # script.
+        self.outputdir = os.path.abspath(os.path.join(self.idir, "output"))
 
         if self.debug:
             self.log = sys.stdout
@@ -365,19 +370,17 @@ class Builder:
             return -1
 
         # Create an empty output directory. We remove it first, in case a previous build was aborted.
-        outputdir = os.path.join(self.idir, "output")
-        if os.path.exists(outputdir):
+        if os.path.exists(self.outputdir):
             # shutil.rmtree doesn't remove write-protected files
-            subprocess.call(["rm", "-rf", outputdir])
-        os.mkdir(outputdir)
-        with open(os.path.join(outputdir, "branch"), "w") as branchf:
+            subprocess.call(["rm", "-rf", self.outputdir])
+        os.mkdir(self.outputdir)
+        with open(os.path.join(self.outputdir, "branch"), "w") as branchf:
             branchf.write(branch)
 
         return 0
 
     def gen_config(self):
         """Generate a new random configuration."""
-        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
 
         log_write(self.log, "INFO: generate the configuration")
 
@@ -387,7 +390,7 @@ class Builder:
             devnull = open(os.devnull, "w")
 
         args = [os.path.join(self.srcdir, "utils/genrandconfig"),
-                "-o", outputdir, "-b", self.srcdir]
+                "-o", self.outputdir, "-b", self.srcdir]
 
         toolchains_csv = self.toolchains_csv
         if toolchains_csv:
@@ -400,8 +403,8 @@ class Builder:
 
     def stop_on_build_hang(self, monitor_thread_hung_build_flag,
                         monitor_thread_stop_flag,
-                        sub_proc, outputdir):
-        build_time_logfile = os.path.join(outputdir, "build/build-time.log")
+                        sub_proc):
+        build_time_logfile = os.path.join(self.outputdir, "build/build-time.log")
         while True:
             if monitor_thread_stop_flag.is_set():
                 return
@@ -423,16 +426,15 @@ class Builder:
         installed, fallback to cmp
         """
 
-        outputdir = os.path.join(self.idir, "output")
-        reproducible_results = os.path.join(outputdir, "results", "reproducible_results")
+        reproducible_results = os.path.join(self.outputdir, "results", "reproducible_results")
         # Using only tar images for now
-        build_1_image = os.path.join(outputdir, "images-1", "rootfs.tar")
-        build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
+        build_1_image = os.path.join(self.outputdir, "images-1", "rootfs.tar")
+        build_2_image = os.path.join(self.outputdir, "images", "rootfs.tar")
 
         with open(reproducible_results, 'w') as diff:
             if self.sysinfo.has("diffoscope"):
                 # Prefix to point diffoscope towards cross-tools
-                prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", self.srcdir, "printvars", "VARS=TARGET_CROSS"])
+                prefix = subprocess.check_output(["make", "O=%s" % self.outputdir, "-C", self.srcdir, "printvars", "VARS=TARGET_CROSS"])
                 # Remove TARGET_CROSS= and \n from the string
                 prefix = prefix[13:-1]
                 log_write(self.log, "INFO: running diffoscope on images")
@@ -453,16 +455,11 @@ class Builder:
     def do_build(self):
         """Run the build itself"""
 
-        # We need the absolute path to use with O=, because the relative
-        # path to the output directory here is not relative to the
-        # Buildroot sources, but to the location of the autobuilder
-        # script.
-        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        f = open(os.path.join(outputdir, "logfile"), "w+")
+        f = open(os.path.join(self.outputdir, "logfile"), "w+")
         log_write(self.log, "INFO: build started")
 
         cmd = ["nice", "-n", str(self.nice),
-                "make", "O=%s" % outputdir,
+                "make", "O=%s" % self.outputdir,
                 "-C", self.srcdir, "BR2_DL_DIR=%s" % self.dldir,
                 "BR2_JLEVEL=%s" % self.njobs] \
             + self.make_opts.split()
@@ -473,8 +470,7 @@ class Builder:
         monitor_thread_stop_flag = Event()
         build_monitor = Thread(target=self.stop_on_build_hang,
                             args=(monitor_thread_hung_build_flag,
-                                    monitor_thread_stop_flag,
-                                    sub, outputdir))
+                                  monitor_thread_stop_flag, sub))
         build_monitor.daemon = True
         build_monitor.start()
 
@@ -495,7 +491,7 @@ class Builder:
             log_write(self.log, "INFO: build failed [%d]" % ret)
             return -1
 
-        cmd = ["make", "O=%s" % outputdir, "-C", self.srcdir,
+        cmd = ["make", "O=%s" % self.outputdir, "-C", self.srcdir,
                 "BR2_DL_DIR=%s" % self.dldir, "legal-info"] \
             + self.make_opts.split()
         ret = subprocess.call(cmd, stdout=f, stderr=f)
@@ -512,8 +508,6 @@ class Builder:
         perform the actual build.
         """
 
-        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-
         # Start the first build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 1")
         ret = self.do_build()
@@ -522,11 +516,11 @@ class Builder:
             return ret
 
         # First build has been built, move files and start build 2
-        os.rename(os.path.join(outputdir, "images"), os.path.join(outputdir, "images-1"))
+        os.rename(os.path.join(self.outputdir, "images"), os.path.join(self.outputdir, "images-1"))
 
         # Clean up build 1
-        f = open(os.path.join(outputdir, "logfile"), "w+")
-        subprocess.call(["make", "O=%s" % outputdir, "-C", self.srcdir, "clean"], stdout=f, stderr=f)
+        f = open(os.path.join(self.outputdir, "logfile"), "w+")
+        subprocess.call(["make", "O=%s" % self.outputdir, "-C", self.srcdir, "clean"], stdout=f, stderr=f)
 
         # Start the second build
         log_write(self.log, "INFO: Reproducible Build Test, starting build 2")
@@ -547,19 +541,18 @@ class Builder:
         are available) or stores them locally as tarballs.
         """
 
-        outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-        resultdir = os.path.join(outputdir, "results")
+        resultdir = os.path.join(self.outputdir, "results")
 
-        shutil.copyfile(os.path.join(outputdir, ".config"),
+        shutil.copyfile(os.path.join(self.outputdir, ".config"),
                         os.path.join(resultdir, "config"))
-        shutil.copyfile(os.path.join(outputdir, "defconfig"),
+        shutil.copyfile(os.path.join(self.outputdir, "defconfig"),
                         os.path.join(resultdir, "defconfig"))
-        shutil.copyfile(os.path.join(outputdir, "branch"),
+        shutil.copyfile(os.path.join(self.outputdir, "branch"),
                         os.path.join(resultdir, "branch"))
 
         def copy_if_exists(directory, src, dst=None):
-            if os.path.exists(os.path.join(outputdir, directory, src)):
-                shutil.copyfile(os.path.join(outputdir, directory, src),
+            if os.path.exists(os.path.join(self.outputdir, directory, src)):
+                shutil.copyfile(os.path.join(self.outputdir, directory, src),
                                 os.path.join(resultdir, src if dst is None else dst))
 
         copy_if_exists("build", "build-time.log")
@@ -575,7 +568,7 @@ class Builder:
         # Return True if the result should be rejected, False otherwise
         def reject_results():
             lastlines = decode_bytes(subprocess.Popen(
-                ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
+                ["tail", "-n", "3", os.path.join(self.outputdir, "logfile")],
                 stdout=subprocess.PIPE).communicate()[0]).splitlines()
 
             # Reject results where qemu-user refused to build
@@ -592,7 +585,7 @@ class Builder:
         def get_failure_reason():
             # Output is a tuple (package, version), or None.
             lastlines = decode_bytes(subprocess.Popen(
-                ["tail", "-n", "3", os.path.join(outputdir, "logfile")],
+                ["tail", "-n", "3", os.path.join(self.outputdir, "logfile")],
                 stdout=subprocess.PIPE).communicate()[0]).splitlines()
 
             regexp = re.compile(r'make: \*\*\* .*/(?:build|toolchain)/([^/]*)/')
@@ -609,14 +602,14 @@ class Builder:
 
             def extract_last_500_lines():
                 subprocess.call(["tail -500 %s > %s" % \
-                                (os.path.join(outputdir, "logfile"), resultfile)],
+                                (os.path.join(self.outputdir, "logfile"), resultfile)],
                                 shell=True)
 
             reason = get_failure_reason()
             if not reason:
                 extract_last_500_lines()
             else:
-                f = open(os.path.join(outputdir, "logfile"), 'r')
+                f = open(os.path.join(self.outputdir, "logfile"), 'r')
                 mf = mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ)
                 mf.seek(0)
                 # Search for first action on the failed package
@@ -640,7 +633,7 @@ class Builder:
             if not reason:
                 return
 
-            srcroot = os.path.join(outputdir, "build", '-'.join(reason))
+            srcroot = os.path.join(self.outputdir, "build", '-'.join(reason))
             destroot = os.path.join(resultdir, '-'.join(reason))
             config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
                 'CMakeOutput.log')
@@ -671,7 +664,7 @@ class Builder:
         # Yes, shutil.make_archive() would be nice, but it doesn't exist
         # in Python 2.6.
         ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
-                            cwd=outputdir, stdout=self.log, stderr=self.log)
+                            cwd=self.outputdir, stdout=self.log, stderr=self.log)
         if ret != 0:
             log_write(self.log, "ERROR: could not make results tarball")
             sys.exit(1)
@@ -683,7 +676,7 @@ class Builder:
             ret = subprocess.call(["curl", "-u",
                                 "%s:%s" % (self.http_login, self.http_password),
                                 "-H", "Expect:",
-                                "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
+                                "-F", "uploadedfile=@%s" % os.path.join(self.outputdir, "results.tar.bz2"),
                                 "-F", "uploadsubmit=1",
                                 self.http_url],
                                 stdout=self.log, stderr=self.log)
@@ -693,10 +686,10 @@ class Builder:
                 log_write(self.log, "INFO: results were submitted successfully")
         else:
             # No http login/password, keep tarballs locally
-            with open(os.path.join(outputdir, "results.tar.bz2"), 'rb') as f:
+            with open(os.path.join(self.outputdir, "results.tar.bz2"), 'rb') as f:
                 sha1 = hashlib.sha1(f.read()).hexdigest()
             resultfilename = "instance-%d-%s.tar.bz2" % (self.instance, sha1)
-            os.rename(os.path.join(outputdir, "results.tar.bz2"), resultfilename)
+            os.rename(os.path.join(self.outputdir, "results.tar.bz2"), resultfilename)
             log_write(self.log, "INFO: results saved as %s" % resultfilename)
 
     def run_instance(self):
@@ -728,8 +721,7 @@ class Builder:
                 continue
 
             # Check if the build test is supposed to be a reproducible test
-            outputdir = os.path.abspath(os.path.join(self.idir, "output"))
-            with open(os.path.join(outputdir, ".config"), "r") as fconf:
+            with open(os.path.join(self.outputdir, ".config"), "r") as fconf:
                 reproducible = "BR2_REPRODUCIBLE=y\n" in fconf.read()
             if reproducible:
                 ret = self.do_reproducible_build()
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 19/19] autobuild-run: define results directory as part of Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (16 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 18/19] autobuild-run: define output " Atharva Lele
@ 2019-06-21  8:47 ` Atharva Lele
  2019-06-25 20:10   ` Arnout Vandecappelle
  2019-06-24 21:22 ` [Buildroot] [PATCH 01/19] autobuild-run: introduce " Arnout Vandecappelle
  18 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-21  8:47 UTC (permalink / raw)
  To: buildroot

Signed-off-by: Atharva Lele <itsatharva@gmail.com>
---
 scripts/autobuild-run | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/scripts/autobuild-run b/scripts/autobuild-run
index 5f0fd0a..92afb26 100755
--- a/scripts/autobuild-run
+++ b/scripts/autobuild-run
@@ -299,6 +299,7 @@ class Builder:
         # Buildroot sources, but to the location of the autobuilder
         # script.
         self.outputdir = os.path.abspath(os.path.join(self.idir, "output"))
+        self.resultdir = os.path.join(self.outputdir, "results")
 
         if self.debug:
             self.log = sys.stdout
@@ -541,19 +542,17 @@ class Builder:
         are available) or stores them locally as tarballs.
         """
 
-        resultdir = os.path.join(self.outputdir, "results")
-
         shutil.copyfile(os.path.join(self.outputdir, ".config"),
-                        os.path.join(resultdir, "config"))
+                        os.path.join(self.resultdir, "config"))
         shutil.copyfile(os.path.join(self.outputdir, "defconfig"),
-                        os.path.join(resultdir, "defconfig"))
+                        os.path.join(self.resultdir, "defconfig"))
         shutil.copyfile(os.path.join(self.outputdir, "branch"),
-                        os.path.join(resultdir, "branch"))
+                        os.path.join(self.resultdir, "branch"))
 
         def copy_if_exists(directory, src, dst=None):
             if os.path.exists(os.path.join(self.outputdir, directory, src)):
                 shutil.copyfile(os.path.join(self.outputdir, directory, src),
-                                os.path.join(resultdir, src if dst is None else dst))
+                                os.path.join(self.resultdir, src if dst is None else dst))
 
         copy_if_exists("build", "build-time.log")
         copy_if_exists("build", "packages-file-list.txt")
@@ -562,7 +561,7 @@ class Builder:
         copy_if_exists("legal-info", "manifest.csv", "licenses-manifest.csv")
 
         subprocess.call(["git log -n 1 --pretty=format:%%H > %s" % \
-                        os.path.join(resultdir, "gitid")],
+                        os.path.join(self.resultdir, "gitid")],
                         shell=True, cwd=self.srcdir)
 
         # Return True if the result should be rejected, False otherwise
@@ -624,7 +623,7 @@ class Builder:
                 mf.close()
                 f.close()
 
-        extract_end_log(os.path.join(resultdir, "build-end.log"))
+        extract_end_log(os.path.join(self.resultdir, "build-end.log"))
 
         def copy_config_log_files():
             """Recursively copy any config.log files from the failing package"""
@@ -634,7 +633,7 @@ class Builder:
                 return
 
             srcroot = os.path.join(self.outputdir, "build", '-'.join(reason))
-            destroot = os.path.join(resultdir, '-'.join(reason))
+            destroot = os.path.join(self.resultdir, '-'.join(reason))
             config_files = ('config.log', 'CMakeCache.txt', 'CMakeError.log',
                 'CMakeOutput.log')
 
@@ -649,7 +648,7 @@ class Builder:
 
         copy_config_log_files()
 
-        resultf = open(os.path.join(resultdir, "status"), "w+")
+        resultf = open(os.path.join(self.resultdir, "status"), "w+")
         if result == 0:
             resultf.write("OK")
         elif result == -1:
@@ -658,7 +657,7 @@ class Builder:
             resultf.write("TIMEOUT")
         resultf.close()
 
-        with open(os.path.join(resultdir, "submitter"), "w+") as submitterf:
+        with open(os.path.join(self.resultdir, "submitter"), "w+") as submitterf:
             submitterf.write(self.submitter)
 
         # Yes, shutil.make_archive() would be nice, but it doesn't exist
@@ -712,8 +711,7 @@ class Builder:
             if ret != 0:
                 continue
 
-            resultdir = os.path.join(self.idir, "output", "results")
-            os.mkdir(resultdir)
+            os.mkdir(self.resultdir)
 
             ret = self.gen_config()
             if ret != 0:
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to " Atharva Lele
@ 2019-06-24 20:31   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 20:31 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> As discussed in the previous patch, these common variables are needed
> in many functions, and it'll be better to have them as part of the class.
> This will make the code cleaner and make it easier to introduce variability
> for reproducibility testing. Succeeding patches will move all variables from
> kwargs to the Builder constructor.
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class
  2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
                   ` (17 preceding siblings ...)
  2019-06-21  8:47 ` [Buildroot] [PATCH 19/19] autobuild-run: define results " Atharva Lele
@ 2019-06-24 21:22 ` Arnout Vandecappelle
  2019-06-25  4:50   ` Atharva Lele
  18 siblings, 1 reply; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:22 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Various functions in the autobuild-run script use a lot of common data.
> To make it easier to work with, create a Builder class.
> 
> For ease of review, this commit only introduces the Builder class but
> does not actually use it for anything. Subsequent patches will do that.
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Still a small whitespace nit...


> +            log_write(log, "INFO: removing %s from downloads" %
> +                    os.path.relpath(f, dldir))

 Continuation line is no longer properly indented.

> +            os.remove(f)
> +
> +        branch = get_branch()
> +        log_write(log, "INFO: testing branch '%s'" % branch)
> +
> +        # Clone Buildroot. This only happens if the source directory
> +        # didn't exist already.
> +        srcdir = os.path.join(idir, "buildroot")
> +        if not os.path.exists(srcdir):
> +            ret = subprocess.call(["git", "clone", kwargs['repo'], srcdir],
> +                                stdout=log, stderr=log)

 Same here.

[snip]
> +        args = [os.path.join(srcdir, "utils/genrandconfig"),
> +                "-o", outputdir, "-b", srcdir]

 And here.

[snip]
> +    def stop_on_build_hang(self, monitor_thread_hung_build_flag,
> +                        monitor_thread_stop_flag,
> +                        sub_proc, outputdir, log):

 And here.

[snip]
> +        cmd = ["nice", "-n", str(nice),
> +                "make", "O=%s" % outputdir,
> +                "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
> +                "BR2_JLEVEL=%s" % kwargs['njobs']] \

 And here it's indented too much.

> +            + kwargs['make_opts'].split()
> +        sub = subprocess.Popen(cmd, stdout=f, stderr=f)
> +
> +        # Setup hung build monitoring thread
> +        monitor_thread_hung_build_flag = Event()
> +        monitor_thread_stop_flag = Event()
> +        build_monitor = Thread(target=self.stop_on_build_hang,
> +                            args=(monitor_thread_hung_build_flag,

 Not enough again.

> +                                    monitor_thread_stop_flag,
> +                                    sub, outputdir, log))
[snip]
> +        cmd = ["make", "O=%s" % outputdir, "-C", srcdir,
> +                "BR2_DL_DIR=%s" % dldir, "legal-info"] \

 Too much.

> +            + kwargs['make_opts'].split()
> +        ret = subprocess.call(cmd, stdout=f, stderr=f)
> +        if ret != 0:
> +            log_write(log, "INFO: build failed during legal-info")
> +            return -1
> +        log_write(log, "INFO: build successful")
> +        return 0
[snip]
> +            def extract_last_500_lines():
> +                subprocess.call(["tail -500 %s > %s" % \
> +                                (os.path.join(outputdir, "logfile"), resultfile)],

 Here too.

> +                                shell=True)
[snip]
> +        ret = subprocess.call(["tar", "cjf", "results.tar.bz2", "results"],
> +                            cwd=outputdir, stdout=log, stderr=log)

 And here.


 There may be more that I missed...

 Just make sure four spaces are added everywhere.

 Regards,
 Arnout

[snip]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 03/19] autobuild-run: move njobs from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 03/19] autobuild-run: move njobs " Atharva Lele
@ 2019-06-24 21:34   ` Arnout Vandecappelle
  2019-06-25  4:53     ` Atharva Lele
  0 siblings, 1 reply; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:34 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 291cd9f..82b8f45 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -271,8 +271,9 @@ class SystemInfo:
>          return not missing_requirements
>  
>  class Builder:
> -    def __init__(self, instance):
> +    def __init__(self, instance, njobs):
>          self.instance = instance
> +        self.njobs = njobs
>  
>      def prepare_build(self, **kwargs):
>          """Prepare for the next build of the specified instance
> @@ -455,7 +456,7 @@ class Builder:
>          cmd = ["nice", "-n", str(nice),
>                  "make", "O=%s" % outputdir,
>                  "-C", srcdir, "BR2_DL_DIR=%s" % dldir,
> -                "BR2_JLEVEL=%s" % kwargs['njobs']] \
> +                "BR2_JLEVEL=%s" % self.njobs] \
>              + kwargs['make_opts'].split()
>          sub = subprocess.Popen(cmd, stdout=f, stderr=f)
>  
> @@ -843,9 +844,8 @@ def main():
>      buildpid = multiprocessing.Array('i', int(args['--ninstances']))
>      processes = []
>      for i in range(0, int(args['--ninstances'])):
> -        builder = Builder(i)
> +        builder = Builder(i, args['--njobs'])

 Since there will be a lot of arguments in the end, it would be better to make
it more explicit, similar like it was in kwargs, i.e.

     builder = Builder(i
         njobs = args['--njobs'])

 Still, not essential, so

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

>          p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
> -                njobs = args['--njobs'],
>                  sysinfo = sysinfo,
>                  http_url = args['--http-url'],
>                  http_login = args['--http-login'],
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo " Atharva Lele
@ 2019-06-24 21:35   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:35 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  scripts/autobuild-run | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 82b8f45..22452cf 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -271,9 +271,10 @@ class SystemInfo:
>          return not missing_requirements
>  
>  class Builder:
> -    def __init__(self, instance, njobs):
> +    def __init__(self, instance, njobs, sysinfo):
>          self.instance = instance
>          self.njobs = njobs
> +        self.sysinfo = sysinfo
>  
>      def prepare_build(self, **kwargs):
>          """Prepare for the next build of the specified instance
> @@ -416,7 +417,7 @@ class Builder:
>          build_2_image = os.path.join(outputdir, "images", "rootfs.tar")
>  
>          with open(reproducible_results, 'w') as diff:
> -            if kwargs['sysinfo'].has("diffoscope"):
> +            if self.sysinfo.has("diffoscope"):
>                  # Prefix to point diffoscope towards cross-tools
>                  prefix = subprocess.check_output(["make", "O=%s" % outputdir, "-C", srcdir, "printvars", "VARS=TARGET_CROSS"])
>                  # Remove TARGET_CROSS= and \n from the string
> @@ -844,9 +845,8 @@ def main():
>      buildpid = multiprocessing.Array('i', int(args['--ninstances']))
>      processes = []
>      for i in range(0, int(args['--ninstances'])):
> -        builder = Builder(i, args['--njobs'])
> +        builder = Builder(i, args['--njobs'], sysinfo)
>          p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
> -                sysinfo = sysinfo,
>                  http_url = args['--http-url'],
>                  http_login = args['--http-login'],
>                  http_password = args['--http-password'],
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 05/19] autobuild-run: move http variables from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 05/19] autobuild-run: move http variables " Atharva Lele
@ 2019-06-24 21:36   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:36 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  scripts/autobuild-run | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index 22452cf..03c54a9 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -271,10 +271,14 @@ class SystemInfo:
>          return not missing_requirements
>  
>  class Builder:
> -    def __init__(self, instance, njobs, sysinfo):
> +    def __init__(self, instance, njobs, sysinfo,
> +                 http_url, http_login, http_password):
>          self.instance = instance
>          self.njobs = njobs
>          self.sysinfo = sysinfo
> +        self.http_url = http_url
> +        self.http_login = http_login
> +        self.http_password = http_password
>  
>      def prepare_build(self, **kwargs):
>          """Prepare for the next build of the specified instance
> @@ -681,11 +685,11 @@ class Builder:
>              # none of the ones that are part of the standard library can
>              # upload a file without writing dozens of lines of code.
>              ret = subprocess.call(["curl", "-u",
> -                                "%s:%s" % (kwargs['http_login'], kwargs['http_password']),
> +                                "%s:%s" % (self.http_login, self.http_password),
>                                  "-H", "Expect:",
>                                  "-F", "uploadedfile=@%s" % os.path.join(outputdir, "results.tar.bz2"),
>                                  "-F", "uploadsubmit=1",
> -                                kwargs['http_url']],
> +                                self.http_url],
>                                  stdout=log, stderr=log)
>              if ret != 0:
>                  log_write(log, "INFO: results could not be submitted, %d" % ret)
> @@ -845,11 +849,9 @@ def main():
>      buildpid = multiprocessing.Array('i', int(args['--ninstances']))
>      processes = []
>      for i in range(0, int(args['--ninstances'])):
> -        builder = Builder(i, args['--njobs'], sysinfo)
> +        builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
> +                          args['--http-login'], args['--http-password'])
>          p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
> -                http_url = args['--http-url'],
> -                http_login = args['--http-login'],
> -                http_password = args['--http-password'],
>                  submitter = args['--submitter'],
>                  make_opts = (args['--make-opts'] or ''),
>                  nice = (args['--nice'] or 0),
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv " Atharva Lele
@ 2019-06-24 21:37   ` Arnout Vandecappelle
  2019-06-25  5:01     ` Atharva Lele
  0 siblings, 1 reply; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:37 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index a435570..f737b27 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -273,7 +273,7 @@ class SystemInfo:
>  class Builder:
>      def __init__(self, instance, njobs, sysinfo,
>                   http_url, http_login, http_password,
> -                 submitter, make_opts, nice):
> +                 submitter, make_opts, nice, toolchains_csv):
>          self.instance = instance
>          self.njobs = njobs
>          self.sysinfo = sysinfo
> @@ -283,6 +283,7 @@ class Builder:
>          self.submitter = submitter
>          self.make_opts = make_opts
>          self.nice = nice
> +        self.toolchains_csv = toolchains_csv
>  
>      def prepare_build(self, **kwargs):
>          """Prepare for the next build of the specified instance
> @@ -381,7 +382,7 @@ class Builder:
>          args = [os.path.join(srcdir, "utils/genrandconfig"),
>                  "-o", outputdir, "-b", srcdir]
>  
> -        toolchains_csv = kwargs['toolchains_csv']
> +        toolchains_csv = self.toolchains_csv

 Creating a local variable here is not very useful anymore. Better use
self.toolchains_csv everywhere below.

 That can be done in the same patch.

 Either way:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

In other words: you can respin with self.toolchains_csv everywhere, or Thomas
can apply without it. Either way is fine.

 Regards,
 Arnout


>          if toolchains_csv:
>              if not os.path.isabs(toolchains_csv):
>                  toolchains_csv = os.path.join(srcdir, toolchains_csv)
> @@ -855,9 +856,8 @@ def main():
>          builder = Builder(i, args['--njobs'], sysinfo, args['--http-url'],
>                            args['--http-login'], args['--http-password'],
>                            args['--submitter'], (args['--make-opts'] or ''),
> -                          (args['--nice'] or 0))
> +                          (args['--nice'] or 0), args['--toolchains-csv'])
>          p = multiprocessing.Process(target=builder.run_instance, kwargs=dict(
> -                toolchains_csv = args['--toolchains-csv'],
>                  repo = args['--repo'],
>                  upload = upload,
>                  buildpid = buildpid,
> 

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 06/19] autobuild-run: move submitter from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 06/19] autobuild-run: move submitter " Atharva Lele
@ 2019-06-24 21:38   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:38 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 07/19] autobuild-run: move niceness from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 07/19] autobuild-run: move niceness " Atharva Lele
@ 2019-06-24 21:38   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:38 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 09/19] autobuild-run: move repo from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 09/19] autobuild-run: move repo " Atharva Lele
@ 2019-06-24 21:39   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:39 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 10/19] autobuild-run: move upload variable from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 10/19] autobuild-run: move upload variable " Atharva Lele
@ 2019-06-24 21:40   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:40 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 11/19] autobuild-run: move buildpid from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 11/19] autobuild-run: move buildpid " Atharva Lele
@ 2019-06-24 21:41   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:41 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 12/19] autobuild-run: move debug from kwargs to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 12/19] autobuild-run: move debug " Atharva Lele
@ 2019-06-24 21:43   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:43 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of " Atharva Lele
@ 2019-06-24 21:44   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-24 21:44 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> We need idir as a class member before moving log out of kwargs.
> 
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class
  2019-06-24 21:22 ` [Buildroot] [PATCH 01/19] autobuild-run: introduce " Arnout Vandecappelle
@ 2019-06-25  4:50   ` Atharva Lele
  0 siblings, 0 replies; 42+ messages in thread
From: Atharva Lele @ 2019-06-25  4:50 UTC (permalink / raw)
  To: buildroot

On Tue, Jun 25, 2019 at 2:52 AM Arnout Vandecappelle <arnout@mind.be> wrote:
> Still a small whitespace nit...
>  There may be more that I missed...
>
>  Just make sure four spaces are added everywhere.
>
>  Regards,
>  Arnout

I'll review all the whitespace issues and send a v2 later today. Thank
you for reviewing!

Regards,
Atharva Lele

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 03/19] autobuild-run: move njobs from kwargs to Builder class
  2019-06-24 21:34   ` Arnout Vandecappelle
@ 2019-06-25  4:53     ` Atharva Lele
  0 siblings, 0 replies; 42+ messages in thread
From: Atharva Lele @ 2019-06-25  4:53 UTC (permalink / raw)
  To: buildroot

On Tue, Jun 25, 2019 at 3:04 AM Arnout Vandecappelle <arnout@mind.be> wrote:
>
> On 21/06/2019 10:47, Atharva Lele wrote:
> > Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> >      for i in range(0, int(args['--ninstances'])):
> > -        builder = Builder(i)
> > +        builder = Builder(i, args['--njobs'])
>
>  Since there will be a lot of arguments in the end, it would be better to make
> it more explicit, similar like it was in kwargs, i.e.
>
>      builder = Builder(i
>          njobs = args['--njobs'])
>
>  Still, not essential, so
>  Regards,
>  Arnout

It makes sense to have it more explicit. I'll have it in the v2.

Regards,
Atharva Lele

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv from kwargs to Builder class
  2019-06-24 21:37   ` Arnout Vandecappelle
@ 2019-06-25  5:01     ` Atharva Lele
  2019-06-25  8:08       ` Arnout Vandecappelle
  0 siblings, 1 reply; 42+ messages in thread
From: Atharva Lele @ 2019-06-25  5:01 UTC (permalink / raw)
  To: buildroot

On Tue, Jun 25, 2019 at 3:07 AM Arnout Vandecappelle <arnout@mind.be> wrote:
>
> On 21/06/2019 10:47, Atharva Lele wrote:
> > Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> > ---
> >  scripts/autobuild-run | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > -        toolchains_csv = kwargs['toolchains_csv']
> > +        toolchains_csv = self.toolchains_csv
>
>  Creating a local variable here is not very useful anymore. Better use
> self.toolchains_csv everywhere below.
>
>  That can be done in the same patch.
>
>  Either way:
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> In other words: you can respin with self.toolchains_csv everywhere, or Thomas
> can apply without it. Either way is fine.
>
>  Regards,
>  Arnout

I tried replacing toolchains_csv with self.toolchains_csv everywhere.
However it presented a weird issue where after the first build was
over, it would not be able to find toolchains_csv. It would then go
into instance-0/buildroot/instance-0/buildroot.. and so on for every
time it was unable to find toolchains_csv. I'll try it later again
today and contact on IRC if i come across the same error.

Regards,
Atharva Lele

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv from kwargs to Builder class
  2019-06-25  5:01     ` Atharva Lele
@ 2019-06-25  8:08       ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25  8:08 UTC (permalink / raw)
  To: buildroot



On 25/06/2019 07:01, Atharva Lele wrote:
> On Tue, Jun 25, 2019 at 3:07 AM Arnout Vandecappelle <arnout@mind.be> wrote:
>>
>> On 21/06/2019 10:47, Atharva Lele wrote:
>>> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
>>> ---
>>>  scripts/autobuild-run | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>> -        toolchains_csv = kwargs['toolchains_csv']
>>> +        toolchains_csv = self.toolchains_csv
>>
>>  Creating a local variable here is not very useful anymore. Better use
>> self.toolchains_csv everywhere below.
>>
>>  That can be done in the same patch.
>>
>>  Either way:
>>
>> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>> In other words: you can respin with self.toolchains_csv everywhere, or Thomas
>> can apply without it. Either way is fine.
>>
>>  Regards,
>>  Arnout
> 
> I tried replacing toolchains_csv with self.toolchains_csv everywhere.
> However it presented a weird issue where after the first build was
> over, it would not be able to find toolchains_csv. It would then go
> into instance-0/buildroot/instance-0/buildroot.. and so on for every
> time it was unable to find toolchains_csv. I'll try it later again
> today and contact on IRC if i come across the same error.

 Ah, of course:

        toolchains_csv = self.toolchains_csv
        if toolchains_csv:
            if not os.path.isabs(toolchains_csv):
                toolchains_csv = os.path.join(self.srcdir, toolchains_csv)


If self.srcdir is not an absolute path, this will keep on appending over and
over again.

 The easiest solution is to move the appending of srcdir to __init__.

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 14/19] autobuild-run: move log variable to Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 14/19] autobuild-run: move log variable to " Atharva Lele
@ 2019-06-25 14:20   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 14:20 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

[snip]
> +        log_write(self.log, "INFO: preparing a new build")

 Since log_write is now only used within the Builder class, it would be nice to
convert it into a member of that class, so the calls become

        self.log_write("INFO: preparing for a new build")

 But that's really a nice-to-have so don't put it on your priority list :-)

 Regards,
 Arnout

[snip]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions
  2019-06-21  8:47 ` [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions Atharva Lele
@ 2019-06-25 20:03   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 20:03 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class Atharva Lele
@ 2019-06-25 20:07   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 20:07 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

[snip]
> -        abssrcdir = os.path.abspath(srcdir)
> +        abssrcdir = os.path.abspath(self.srcdir)

 A further improvement could be to make srcdir an absolute path in __init__. Or
actually, even idir could be made an absolute path, so the rest automatically is
as well. Note that abspath simply prepends the current directory if the path is
not absolute, so it will work even if idir doesn't exist yet.

 But obviously, that's for a separate patch and not at all a priority.

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 17/19] autobuild-run: define download directory as part of Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 17/19] autobuild-run: define download " Atharva Lele
@ 2019-06-25 20:08   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 20:08 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 18/19] autobuild-run: define output directory as part of Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 18/19] autobuild-run: define output " Atharva Lele
@ 2019-06-25 20:10   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 20:10 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>
> ---
>  scripts/autobuild-run | 86 ++++++++++++++++++++-----------------------
>  1 file changed, 39 insertions(+), 47 deletions(-)
> 
> diff --git a/scripts/autobuild-run b/scripts/autobuild-run
> index f9e2414..5f0fd0a 100755
> --- a/scripts/autobuild-run
> +++ b/scripts/autobuild-run
> @@ -294,6 +294,11 @@ class Builder:
>          self.idir = "instance-%d" % self.instance
>          self.srcdir = os.path.join(self.idir, "buildroot")
>          self.dldir = os.path.join(self.idir, "dl")
> +        # We need the absolute path to use with O=, because the relative
> +        # path to the output directory here is not relative to the
> +        # Buildroot sources, but to the location of the autobuilder
> +        # script.
> +        self.outputdir = os.path.abspath(os.path.join(self.idir, "output"))

 Unlike the previous patches, this one does not simply make outputdir part of
the Builder class. Indeed, some instances of outputdir were made absolute and
some were not.

 So the commit message should explain this difference, why it is necessary, and
why it is OK for the cases where the abspath was absent.

 Regards,
 Arnout

[snip]

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [Buildroot] [PATCH 19/19] autobuild-run: define results directory as part of Builder class
  2019-06-21  8:47 ` [Buildroot] [PATCH 19/19] autobuild-run: define results " Atharva Lele
@ 2019-06-25 20:10   ` Arnout Vandecappelle
  0 siblings, 0 replies; 42+ messages in thread
From: Arnout Vandecappelle @ 2019-06-25 20:10 UTC (permalink / raw)
  To: buildroot



On 21/06/2019 10:47, Atharva Lele wrote:
> Signed-off-by: Atharva Lele <itsatharva@gmail.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2019-06-25 20:10 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21  8:47 [Buildroot] [PATCH 01/19] autobuild-run: introduce Builder class Atharva Lele
2019-06-21  8:47 ` [Buildroot] [PATCH 02/19] autobuild-run: move instance variable from kwargs to " Atharva Lele
2019-06-24 20:31   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 03/19] autobuild-run: move njobs " Atharva Lele
2019-06-24 21:34   ` Arnout Vandecappelle
2019-06-25  4:53     ` Atharva Lele
2019-06-21  8:47 ` [Buildroot] [PATCH 04/19] autobuild-run: move sysinfo " Atharva Lele
2019-06-24 21:35   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 05/19] autobuild-run: move http variables " Atharva Lele
2019-06-24 21:36   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 06/19] autobuild-run: move submitter " Atharva Lele
2019-06-24 21:38   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 07/19] autobuild-run: move niceness " Atharva Lele
2019-06-24 21:38   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 08/19] autobuild-run: move toolchains_csv " Atharva Lele
2019-06-24 21:37   ` Arnout Vandecappelle
2019-06-25  5:01     ` Atharva Lele
2019-06-25  8:08       ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 09/19] autobuild-run: move repo " Atharva Lele
2019-06-24 21:39   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 10/19] autobuild-run: move upload variable " Atharva Lele
2019-06-24 21:40   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 11/19] autobuild-run: move buildpid " Atharva Lele
2019-06-24 21:41   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 12/19] autobuild-run: move debug " Atharva Lele
2019-06-24 21:43   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 13/19] autobuild-run: define instance directory as a part of " Atharva Lele
2019-06-24 21:44   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 14/19] autobuild-run: move log variable to " Atharva Lele
2019-06-25 14:20   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 15/19] autobuild-run: remove kwargs argument from function calls and definitions Atharva Lele
2019-06-25 20:03   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 16/19] autobuild-run: define source directory as part of Builder class Atharva Lele
2019-06-25 20:07   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 17/19] autobuild-run: define download " Atharva Lele
2019-06-25 20:08   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 18/19] autobuild-run: define output " Atharva Lele
2019-06-25 20:10   ` Arnout Vandecappelle
2019-06-21  8:47 ` [Buildroot] [PATCH 19/19] autobuild-run: define results " Atharva Lele
2019-06-25 20:10   ` Arnout Vandecappelle
2019-06-24 21:22 ` [Buildroot] [PATCH 01/19] autobuild-run: introduce " Arnout Vandecappelle
2019-06-25  4:50   ` Atharva Lele

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.