From 8fdfea94563cb7e1df511fff946340804245ecc6 Mon Sep 17 00:00:00 2001 From: Ralf Stephan Date: Thu, 8 May 2014 15:43:14 +0200 Subject: [PATCH 1/3] refactor fetch/merge/sandboxing; implement safe-only --- src/patchbot.py | 16 ++++++++++++-- src/trac.py | 55 +++++++++++++++++++++++++++++-------------------- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/src/patchbot.py b/src/patchbot.py index 460d9c1..c0b7666 100755 --- a/src/patchbot.py +++ b/src/patchbot.py @@ -28,7 +28,7 @@ from http_post_file import post_multipart -from trac import scrape, pull_from_trac +from trac import scrape, fetch_from_trac, merge_ticket, inplace_safe, clone_sandbox from util import (now_str as datetime, prune_pending, do_or_die, get_version, current_reports, git_commit, ConfigException) import version as patchbot_version @@ -254,6 +254,7 @@ def get_config(self): "max_behind_commits": 10, "max_behind_days": 2.0, "use_ccache": True, + "safe_only": False, } default_bonus = { "needs_review": 1000, @@ -432,7 +433,17 @@ def test_a_ticket(self, ticket=None): os.environ['GIT_AUTHOR_NAME'] = os.environ['GIT_COMMITTER_NAME'] = 'patchbot' os.environ['GIT_AUTHOR_EMAIL'] = os.environ['GIT_COMMITTER_EMAIL'] = 'patchbot@localhost' os.environ['GIT_AUTHOR_DATE'] = os.environ['GIT_COMMITTER_DATE'] = '1970-01-01T00:00:00' - pull_from_trac(self.sage_root, ticket['id'], force=True, use_ccache=self.config['use_ccache']) + ticket_id = ticket['id'] + fetch_from_trac(ticket_id) + ticket_is_safe = inplace_safe() + if self.config['safe_only'] and not ticket_is_safe: + print "Unsafe ticket and --safe-only set. Bailing out..." + return + if ticket_is_safe: + os.chdir(self.sage_root) + merge_ticket(ticket_id) + else: + clone_sandbox(ticket_id, use_ccache=self.config['use_ccache']) t.finish("Apply") state = 'applied' if not self.plugin_only: @@ -700,6 +711,7 @@ def main(args): parser.add_option("--skip-base", action="store_true", dest="skip_base", default=False) parser.add_option("--dry-run", action="store_true", dest="dry_run", default=False) parser.add_option("--plugin-only", action="store_true", dest="plugin_only", default=False) + parser.add_option("--safe-only", action="store_true", dest="safe_only", default=False) (options, args) = parser.parse_args(args) conf_path = options.config and os.path.abspath(options.config) diff --git a/src/trac.py b/src/trac.py index 80b3cd8..614b43f 100644 --- a/src/trac.py +++ b/src/trac.py @@ -268,37 +268,43 @@ def inplace_safe(): """ safe = True # TODO: Are removed files sufficiently cleaned up? - for file in subprocess.check_output(["git", "diff", "--name-only", "patchbot/base..patchbot/ticket_merged"]).split('\n'): + commit = subprocess.check_output(['git', 'merge-base', 'patchbot/base', 'patchbot/ticket_upstream']).split('\n')[0] + for file in subprocess.check_output(["git", "diff", "--name-only", "%s..patchbot/ticket_upstream" % commit]).split('\n'): if not file: continue - if file.startswith("src/sage") or file in ("src/setup.py", "src/module_list.py", "README.txt", ".gitignore"): + if file.startswith("src/sage") or file.startswith("src/doc") or file in ("src/setup.py", "src/module_list.py", "README.txt", ".gitignore"): continue else: print "Unsafe file:", file safe = False return safe -def pull_from_trac(sage_root, ticket, branch=None, force=None, interactive=None, inplace=None, use_ccache=False): +def fetch_from_trac(ticket_id): # There are four branches at play here: # patchbot/base -- the latest release that all tickets are merged into for testing # patchbot/base_upstream -- temporary staging area for patchbot/base # patchbot/ticket_upstream -- pristine clone of the ticket on trac # patchbot/ticket_merged -- merge of patchbot/ticket_upstream into patchbot/base - merge_failure = False try: - ticket_id = ticket - info = scrape(ticket_id) - os.chdir(sage_root) + info = scrape(ticket_id) do_or_die("git checkout patchbot/base") if ticket_id == 0: do_or_die("git branch -f patchbot/ticket_upstream patchbot/base") - do_or_die("git branch -f patchbot/ticket_merged patchbot/base") return branch = info['git_branch'] repo = info['git_repo'] do_or_die("git fetch %s +%s:patchbot/ticket_upstream" % (repo, branch)) - do_or_die("git rev-list --left-right --count patchbot/base..patchbot/ticket_upstream") - do_or_die("git branch -f patchbot/ticket_merged patchbot/base") + except Exception, exn: + raise ConfigException, exn.message + + +def merge_ticket(ticket_id): + merge_failure = False + try: + if ticket_id == 0: + do_or_die("git branch -f patchbot/ticket_merged patchbot/base") + return + do_or_die("git branch -f patchbot/ticket_merged patchbot/base") do_or_die("git checkout patchbot/ticket_merged") try: do_or_die("git merge -X patience patchbot/ticket_upstream") @@ -306,24 +312,29 @@ def pull_from_trac(sage_root, ticket, branch=None, force=None, interactive=None, do_or_die("git merge --abort") merge_failure = True raise - if not inplace_safe(): - tmp_dir = tempfile.mkdtemp("-sage-git-temp-%s" % ticket_id) - do_or_die("git clone . '%s'" % tmp_dir) - os.chdir(tmp_dir) - os.symlink(os.path.join(sage_root, "upstream"), "upstream") - os.environ['SAGE_ROOT'] = tmp_dir - do_or_die("git branch -f patchbot/base remotes/origin/patchbot/base") - do_or_die("git branch -f patchbot/ticket_upstream remotes/origin/patchbot/ticket_upstream") - if use_ccache: - if not os.path.exists('logs'): - os.mkdir('logs') - do_or_die("./sage -i ccache") except Exception, exn: if merge_failure: raise else: raise ConfigException, exn.message +def clone_sandbox(ticket_id, use_ccache=False): + try: + tmp_dir = tempfile.mkdtemp("-sage-git-temp-%s" % ticket_id) + do_or_die("git clone . '%s'" % tmp_dir) + os.chdir(tmp_dir) + merge_ticket(ticket_id) + os.symlink(os.path.join(sage_root, "upstream"), "upstream") + os.environ['SAGE_ROOT'] = tmp_dir + do_or_die("git branch -f patchbot/base remotes/origin/patchbot/base") + do_or_die("git branch -f patchbot/ticket_upstream remotes/origin/patchbot/ticket_upstream") + if use_ccache: + if not os.path.exists('logs'): + os.mkdir('logs') + do_or_die("./sage -i ccache") + except Exception, exn: + raise Exception, exn.message + def push_from_trac(sage_root, ticket, branch=None, force=None, interactive=None): raise NotImplementedError From 590947f571af2e5300d3a239e4fa26376646a3ee Mon Sep 17 00:00:00 2001 From: Ralf Stephan Date: Thu, 8 May 2014 18:47:20 +0200 Subject: [PATCH 2/3] fix problems with previous commit --- src/patchbot.py | 18 ++++++++++-------- src/trac.py | 3 ++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/patchbot.py b/src/patchbot.py index c0b7666..2de899b 100755 --- a/src/patchbot.py +++ b/src/patchbot.py @@ -189,13 +189,14 @@ def sha1file(path, blocksize=2**16): class Patchbot: - def __init__(self, sage_root, server, config_path, dry_run=False, plugin_only=False): + def __init__(self, sage_root, server, config_path, dry_run=False, plugin_only=False, safe_only=False): self.sage_root = sage_root self.server = server self.base = get_version(sage_root) self.behind_base = {} self.dry_run = dry_run self.plugin_only = plugin_only + self.safe_only = safe_only self.config_path = config_path self.reload_config() self.last_pull = 0 @@ -254,7 +255,6 @@ def get_config(self): "max_behind_commits": 10, "max_behind_days": 2.0, "use_ccache": True, - "safe_only": False, } default_bonus = { "needs_review": 1000, @@ -404,6 +404,13 @@ def test_a_ticket(self, ticket=None): history = open("%s/history.txt" % self.log_dir, "a") history.write("%s %s\n" % (datetime(), ticket['id'])) history.close() + if not ticket['spkgs']: + ticket_id = ticket['id'] + fetch_from_trac(ticket_id) + ticket_is_safe = inplace_safe() + if self.safe_only and not ticket_is_safe: + print "Unsafe ticket and --safe-only set. Bailing out..." + return if not self.plugin_only: self.report_ticket(ticket, status='Pending', log=log) plugins_results = [] @@ -434,11 +441,6 @@ def test_a_ticket(self, ticket=None): os.environ['GIT_AUTHOR_EMAIL'] = os.environ['GIT_COMMITTER_EMAIL'] = 'patchbot@localhost' os.environ['GIT_AUTHOR_DATE'] = os.environ['GIT_COMMITTER_DATE'] = '1970-01-01T00:00:00' ticket_id = ticket['id'] - fetch_from_trac(ticket_id) - ticket_is_safe = inplace_safe() - if self.config['safe_only'] and not ticket_is_safe: - print "Unsafe ticket and --safe-only set. Bailing out..." - return if ticket_is_safe: os.chdir(self.sage_root) merge_ticket(ticket_id) @@ -722,7 +724,7 @@ def main(args): tickets = None count = int(options.count) - patchbot = Patchbot(os.path.abspath(options.sage_root), options.server, conf_path, dry_run=options.dry_run, plugin_only=options.plugin_only) + patchbot = Patchbot(os.path.abspath(options.sage_root), options.server, conf_path, dry_run=options.dry_run, plugin_only=options.plugin_only, safe_only=options.safe_only) conf = patchbot.get_config() if options.list: diff --git a/src/trac.py b/src/trac.py index 614b43f..ddec6ae 100644 --- a/src/trac.py +++ b/src/trac.py @@ -323,8 +323,9 @@ def clone_sandbox(ticket_id, use_ccache=False): tmp_dir = tempfile.mkdtemp("-sage-git-temp-%s" % ticket_id) do_or_die("git clone . '%s'" % tmp_dir) os.chdir(tmp_dir) + fetch_from_trac(ticket_id) merge_ticket(ticket_id) - os.symlink(os.path.join(sage_root, "upstream"), "upstream") + os.symlink(os.path.join(os.environ['SAGE_ROOT'], "upstream"), "upstream") os.environ['SAGE_ROOT'] = tmp_dir do_or_die("git branch -f patchbot/base remotes/origin/patchbot/base") do_or_die("git branch -f patchbot/ticket_upstream remotes/origin/patchbot/ticket_upstream") From 14c4ad7220d4853dff73901e8b3546ee0bbe7d57 Mon Sep 17 00:00:00 2001 From: Ralf Stephan Date: Sun, 11 May 2014 18:38:28 +0200 Subject: [PATCH 3/3] sleep a bit after bailing out --- src/patchbot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/patchbot.py b/src/patchbot.py index 2de899b..6237d56 100755 --- a/src/patchbot.py +++ b/src/patchbot.py @@ -410,6 +410,7 @@ def test_a_ticket(self, ticket=None): ticket_is_safe = inplace_safe() if self.safe_only and not ticket_is_safe: print "Unsafe ticket and --safe-only set. Bailing out..." + time.sleep(10) return if not self.plugin_only: self.report_ticket(ticket, status='Pending', log=log)