From c0d69d8ea6595934a8267af295bb089be8ff896b Mon Sep 17 00:00:00 2001 From: Sanne Raymaekers Date: Tue, 23 Jul 2019 16:12:45 +0200 Subject: [PATCH] bots: Let make-checkout produce a standalone, temporary directory Closes #12426 --- .gitignore | 1 + bots/issue-scan | 15 +++++++----- bots/make-checkout | 59 ++++++++++++++++------------------------------ bots/tests-invoke | 18 +++++++------- bots/tests-scan | 11 ++++----- 5 files changed, 43 insertions(+), 61 deletions(-) diff --git a/.gitignore b/.gitignore index afba60842..ac6ef4edc 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,7 @@ depcomp /bots/images/*.partial /bots/images/*.xz /bots/images/*.img +/bots/make-checkout-workdir /test/images/* /test/verify/naughty-*/* /test/container-probe* diff --git a/bots/issue-scan b/bots/issue-scan index 12ff075c5..4c2727c0f 100755 --- a/bots/issue-scan +++ b/bots/issue-scan @@ -163,11 +163,11 @@ def tasks_for_issues(issues_data): checklist = github.Checklist(issue["body"]) for item, checked in checklist.items.items(): if not checked: - results.append((item, issue)) + results.append((item, issue, api.repo)) break return results -def output_task(command, issue, verbose): +def output_task(command, issue, repo, verbose): name, unused, context = command.partition(" ") if name not in NAMES: return None @@ -182,7 +182,9 @@ def output_task(command, issue, verbose): # `--issues-data` should also be able to receive pull_request events, in that # case pull_request won't be present in the object, but commits will be if "pull_request" in issue or "commits" in issue: - checkout += "bots/make-checkout --verbose pull/{issue}/head && " + checkout += "bots/make-checkout --verbose --repo {repo} pull/{issue}/head && " + else: + checkout += "bots/make-checkout --verbose --repo {repo} master && " if verbose: return "issue-{issue} {name} {context} {priority}".format( @@ -194,11 +196,12 @@ def output_task(command, issue, verbose): else: if context: context = pipes.quote(context) - return (checkout + cmd).format( + return (checkout + "cd bots/make-checkout-workdir && " + cmd + " ; cd ../..").format( issue=int(number), priority=distributed_queue.MAX_PRIORITY, name=name, context=context, + repo=repo, ) def queue_task(channel, result): @@ -216,8 +219,8 @@ def scan(issues_data, verbose): results = [ ] # Now go through each fixture - for (command, issue) in tasks_for_issues(issues_data): - result = output_task(command, issue, verbose) + for (command, issue, repo) in tasks_for_issues(issues_data): + result = output_task(command, issue, repo, verbose) if result is not None: results.append(result) diff --git a/bots/make-checkout b/bots/make-checkout index 8eea605ab..49c9d3a34 100755 --- a/bots/make-checkout +++ b/bots/make-checkout @@ -19,6 +19,7 @@ import argparse import os +import shutil import subprocess import sys from task import github @@ -30,6 +31,7 @@ sys.dont_write_bytecode = True BOTS = os.path.dirname(__file__) BASE = os.path.normpath(os.path.join(BOTS, "..")) +TARGET_DIR = os.path.join(BOTS, "make-checkout-workdir") def main(): parser = argparse.ArgumentParser(description="Fetch and checkout specific revision") @@ -46,54 +48,33 @@ def main(): api = github.GitHub(repo=opts.repo) - def execute(*args): + def execute(*args, cwd=BASE, error_on_fail=True): + output = None if opts.verbose: sys.stderr.write("+ " + " ".join(args) + "\n") try: - output = subprocess.check_output(args, cwd=BASE, universal_newlines=True) + output = subprocess.check_output(args, cwd=cwd, universal_newlines=True) except subprocess.CalledProcessError as ex: - sys.exit(ex.returncode) - if opts.verbose and output: - sys.stderr.write("> " + output + "\n") + sys.exit(ex.returncode if error_on_fail else 0) + finally: + if opts.verbose and output: + sys.stderr.write("> " + output + "\n") return output - # do this before fetching the actual test repo, so that FETCH_HEAD always points to that at the end - if opts.bots_ref: - execute("git", "fetch", "origin", opts.bots_ref) - bots_ref = execute("git", "rev-parse", "FETCH_HEAD").strip() - else: - bots_ref = "origin/master" + if os.path.exists(TARGET_DIR): + shutil.rmtree(TARGET_DIR) - # Testing external project, firstly we needs to add remote - if api.repo != "cockpit-project/cockpit": - if "test\n" in subprocess.check_output([ "git", "remote" ], universal_newlines=True): - execute("git", "remote", "remove", "test") - execute("git", "remote", "add", "test", "https://github.com/" + api.repo) + cache = os.getenv('XDG_CACHE_HOME', ".") + execute("git", "clone", "--reference-if-able", "{}/{}".format(cache, api.repo), "https://github.com/{}".format(api.repo), TARGET_DIR) + execute("git", "fetch", "origin", opts.ref, cwd=TARGET_DIR, error_on_fail=False) + execute("git", "checkout", "--detach", opts.revision, cwd=TARGET_DIR, error_on_fail=False) - # if the pr was updated while the command was in the queue, the revision - # won't be tied to the ref anymore, and it won't exist locally, so exit - # silently - output = None - try: - args = ["git", "fetch", "test" if api.repo != "cockpit-project/cockpit" else "origin", opts.ref] - if opts.verbose: - sys.stderr.write("+ {0}\n".format(" ".join(args))) - output = subprocess.check_output(args, cwd=BASE, universal_newlines=True) - args = ["git", "checkout", "--detach", opts.revision] - if opts.verbose: - sys.stderr.write("+ {0}\n".format(" ".join(args))) - output = subprocess.check_output(args, cwd=BASE, universal_newlines=True) - except subprocess.CalledProcessError: - return 0 - finally: - if opts.verbose and output: - sys.stderr.write("> " + output + "\n") - - # If the bots directory doesn't exist in this branch or repo, check it out from master + # get bots/ if api.repo != "cockpit-project/cockpit" or (opts.base and opts.base != "master"): - sys.stderr.write("Checking out bots directory from Cockpit %s ...\n" % (opts.bots_ref or bots_ref)) - execute("git", "checkout", "--force", bots_ref, "--", "bots/") - + execute("git", "remote", "add", "bots_origin", "https://github.com/cockpit-project/cockpit", cwd=TARGET_DIR) + execute("git", "fetch", "--depth=1", "bots_origin", opts.bots_ref or "master", cwd=TARGET_DIR) + bots_ref = execute("git", "rev-parse", "FETCH_HEAD", cwd=TARGET_DIR).strip() + execute("git", "checkout", "--force", bots_ref or "bots_origin/master", "--", "bots/", cwd=TARGET_DIR, error_on_fail=False) if __name__ == '__main__': sys.exit(main()) diff --git a/bots/tests-invoke b/bots/tests-invoke index 18bd6b9cb..dff8ccb3f 100755 --- a/bots/tests-invoke +++ b/bots/tests-invoke @@ -37,8 +37,6 @@ DEVNULL = open("/dev/null", "r+") def main(): parser = argparse.ArgumentParser(description='Run integration tests') parser.add_argument('--rebase', help="Rebase onto the specific branch before testing") - parser.add_argument('--remote', help="The Git remote to use (instead of 'origin')", - default="origin") parser.add_argument('-o', "--offline", action='store_true', help="Work offline, don''t fetch new data from origin for rebase") parser.add_argument('--publish', dest='publish', default=os.environ.get("TEST_PUBLISH", ""), @@ -58,8 +56,9 @@ def main(): test_branch = os.environ.get("TEST_BRANCH") try: - task = PullTask(name, revision, opts.context, opts.rebase, opts.remote, - test_project=test_project, test_branch=test_branch, html_logs=opts.html_logs) + task = PullTask(name, revision, opts.context, opts.rebase, + test_project=test_project, test_branch=test_branch, + html_logs=opts.html_logs) ret = task.run(opts) except RuntimeError as ex: ret = str(ex) @@ -70,12 +69,11 @@ def main(): return 0 class PullTask(object): - def __init__(self, name, revision, context, base, remote, test_project, test_branch, html_logs): + def __init__(self, name, revision, context, base, test_project, test_branch, html_logs): self.name = name self.revision = revision self.context = context self.base = base - self.remote = remote self.test_project = test_project self.test_branch = test_branch self.html_logs = html_logs @@ -193,8 +191,8 @@ class PullTask(object): # Include information about which base we're testing against if self.base: - subprocess.check_call([ "git", "fetch", self.remote, self.base ]) - commit = subprocess.check_output([ "git", "rev-parse", self.remote + "/" + self.base ], + subprocess.check_call([ "git", "fetch", "origin", self.base ]) + commit = subprocess.check_output([ "git", "rev-parse", "origin/" + self.base ], universal_newlines=True).strip() status["base"] = commit @@ -206,7 +204,7 @@ class PullTask(object): self.sink = sink.Sink(host, identifier, status) def rebase(self): - remote_base = self.remote + "/" + self.base + remote_base = "origin" + "/" + self.base # Rebase this branch onto the base, but only if it's not already an ancestor try: @@ -271,7 +269,7 @@ class PullTask(object): # Retrieve information about our base branch and master (for bots/) if self.base and not opts.offline: - subprocess.check_call([ "git", "fetch", self.remote, self.base, "master" ]) + subprocess.check_call([ "git", "fetch", "origin", self.base, "master" ]) # Clean out the test directory subprocess.check_call([ "git", "clean", "-d", "--force", "--quiet", "-x", "--", "test/" ]) diff --git a/bots/tests-scan b/bots/tests-scan index 4ff748af2..6f818b3a2 100755 --- a/bots/tests-scan +++ b/bots/tests-scan @@ -157,12 +157,10 @@ def tests_invoke(priority, name, number, revision, ref, context, base, original_ if bots_ref: checkout += " --bots-ref={bots_ref}" - else: - # we are checking the external repo on a PR of that external repo - cmd = "GITHUB_BASE={repo} " + cmd - if repo != "cockpit-project/cockpit": - cmd = cmd + " --remote=test" + # The repo of this test differs from the PR's repo + if options.repo != repo: + cmd = "GITHUB_BASE={github_base} " + cmd if repo in REPO_EXTRA_INVOKE_OPTIONS: cmd += " " + " ".join(REPO_EXTRA_INVOKE_OPTIONS[repo]) @@ -179,7 +177,7 @@ def tests_invoke(priority, name, number, revision, ref, context, base, original_ # we are testing the repo itself, checkout revision from the PR checkout += " {ref} {revision} && " - return (checkout + cmd).format( + return (checkout + "cd bots/make-checkout-workdir && " + cmd + " ; cd ../..").format( priority=priority, name=pipes.quote(name), revision=pipes.quote(revision), @@ -190,6 +188,7 @@ def tests_invoke(priority, name, number, revision, ref, context, base, original_ current=current, pull_number=number, repo=pipes.quote(repo), + github_base=pipes.quote(options.repo), ) def queue_test(priority, name, number, revision, ref, context, base, original_base, repo, bots_ref, channel, options):