Skip to content

Commit

Permalink
Add batched merge (rollup) feature
Browse files Browse the repository at this point in the history
Currently it works as follows:

1. Mark the commits to be merged together as rollup. (e.g. r+ rollup)
   These commits will have an implicit priority of -1 to postpone the
   individual merge.
2. If one of the marked commits reaches the top of the queue, all the
   marked commits will be merged together and tested.
3. While merging commits, those commits that fail to merge are ignored.
4. You can prioritize the rollup by setting p=.

Typical usage:

1. We have three commits to be tested together.
2. Set r+ rollup to the first commit.
3. Set r+ rollup to the second commit.
4. Set r+ rollup p=10 to the last commit, which will trigger the rollup
   process soon.

Fixes graydon#34.
  • Loading branch information
barosl committed Dec 12, 2014
1 parent 5d20f5b commit f57c6cc
Showing 1 changed file with 105 additions and 11 deletions.
116 changes: 105 additions & 11 deletions bors.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ def __init__(self, cfg, gh, j):
else:
self.test_ref = '%s-integration-%s-%s' % (self.user, self.num, self.ref)

self.batch_ref = 'batch'

self.title=ustr(j["title"])
self.body=ustr(j["body"])
self.merge_sha = None
Expand Down Expand Up @@ -342,12 +344,21 @@ def approval_list(self):

def priority(self):
p = 0

if self.batched(): p = -1

for (d, u, c) in self.head_comments:
m = re.search(r"\bp=(-?\d+)\b", c)
if m is not None:
p = max(p, int(m.group(1)))
return p

def batched(self):
for date, user, comment in self.head_comments:
if re.search(r'\brollup\b', comment):
return True
return False

def prioritized_state(self):
return (self.current_state(),
self.priority(),
Expand Down Expand Up @@ -456,7 +467,7 @@ def get_merge_sha(self):
if s["creator"]["login"].encode("utf8") == self.user and s["state"].encode("utf8") == "pending"]
if len(statusdescs) > 0:
# parse it
m = re.match(r"running tests for candidate ([a-z0-9]+)", statusdescs[0])
m = re.match(r"running tests for.*?candidate ([a-z0-9]+)", statusdescs[0])
if m:
self.merge_sha = m.group(1)

Expand Down Expand Up @@ -540,6 +551,84 @@ def merge_pull_head_to_test_ref(self):
self.add_comment(self.sha, s)
self.set_error(s)

def merge_batched_pull_reqs_to_test_ref(self, pulls, cfg):
if cfg.get('max_pulls_per_run') != 1:
msg = 'max_pulls_per_run must be 1 to merge batched pull requests'
self.log.info(msg)
self.add_comment(self.sha, msg)
self.set_error(msg)
return

batch_msg = 'batch merging {} pull requests into {}'.format(
len([x for x in pulls if x.current_state() == STATE_APPROVED]),
self.batch_ref,
)
self.log.info(batch_msg)
self.add_comment(self.sha, batch_msg)

info = self.dst().git().refs().heads(self.target_ref).get()
target_sha = info['object']['sha'].encode('utf-8')
try:
self.dst().git().refs().heads(self.batch_ref).get()
self.dst().git().refs().heads(self.batch_ref).patch(sha=target_sha, force=True)
except github.ApiError:
self.dst().git().refs().post(sha=target_sha, ref='refs/heads/' + self.batch_ref)

successes = []
failures = []

batch_sha = ''

for pull in pulls:
if pull.current_state() == STATE_APPROVED:
self.log.info('merging {} into {}'.format(pull.short(), self.batch_ref))

msg = 'Merge pull request #{} from {}/{}\n\n{}\n\nReviewed-by: {}'.format(
pull.num,
pull.src_owner, pull.ref,
pull.title,
', '.join(pull.approval_list())
)
pull_repr = '- {}/{} = {}: {}'.format(pull.src_owner, pull.ref, pull.sha, pull.title)

try:
info = self.dst().merges().post(base=self.batch_ref, head=pull.sha, commit_message=msg)
batch_sha = info['sha'].encode('utf-8')
except github.ApiError:
failures.append(pull_repr)
else:
successes.append(pull_repr)

if batch_sha:
try:
self.dst().git().refs().heads(self.test_ref).get()
self.dst().git().refs().heads(self.test_ref).patch(sha=batch_sha)
except github.ApiError as e:
self.dst().git().refs().post(sha=batch_sha, ref='refs/heads/' + self.test_ref)

url = 'https://{}/{}/{}/commit/{}'.format(self.gh_host, self.dst_owner, self.dst_repo, batch_sha)
short_msg = 'running tests for rollup candidate {} ({} successes, {} failures)'.format(batch_sha, len(successes), len(failures))
msg = 'Testing rollup candidate = {:.8}'.format(batch_sha)
if successes: msg += '\n\n**Successful merges:**\n\n{}'.format('\n'.join(successes))
if failures: msg += '\n\n**Failed merges:**\n\n{}'.format('\n'.join(failures))

self.log.info(short_msg)
self.add_comment(self.sha, msg)
self.set_pending(short_msg, url)
else:
batch_msg += ' failed'

self.log.info(batch_msg)
self.add_comment(self.sha, batch_msg)
self.set_error(batch_msg)

def merge_or_batch(self, pulls, cfg):
self.reset_test_ref_to_target()
if self.batched():
self.merge_batched_pull_reqs_to_test_ref(pulls, cfg)
else:
self.merge_pull_head_to_test_ref()

def advance_target_ref_to_test(self):
assert self.merge_sha is not None
s = ("fast-forwarding %s to %s = %.8s" %
Expand All @@ -556,6 +645,11 @@ def advance_target_ref_to_test(self):
except github.ApiError:
self.log.info("deleting integration branch %s failed" % self.test_ref)

try:
self.dst().git().refs().heads(self.batch_ref).delete()
except github.ApiError:
self.log.info('deleting batch branch {} failed'.format(self.batch_ref))

self.maybe_delete_source_branch()

except github.ApiError:
Expand All @@ -573,6 +667,7 @@ def fresh(self):
# parents of the merge sha are
# the tip of the merge-target and the
# feature branch
assert self.merge_sha
owner = self.cfg["owner"].encode("utf8")
repo = self.cfg["repo"].encode("utf8")
target_head = self.gh.repos(owner)(repo).git().refs().heads(self.target_ref).get()
Expand All @@ -583,7 +678,7 @@ def fresh(self):
target_sha in test_parents and
self.sha in test_parents)

def try_advance(self):
def try_advance(self, pulls, cfg):
s = self.current_state()

self.log.info("considering %s", self.desc())
Expand All @@ -601,8 +696,7 @@ def try_advance(self):
self.src_repo,
self.sha))))

self.reset_test_ref_to_target()
self.merge_pull_head_to_test_ref()
self.merge_or_batch(pulls, cfg)

elif s == STATE_PENDING:
# Make sure the optional merge sha is loaded
Expand All @@ -612,13 +706,12 @@ def try_advance(self):
test_sha = test_head["object"]["sha"].encode("utf8")
self.merge_sha = test_sha

if not self.fresh():
if not self.fresh() and not self.batched():
c = ("Merge sha %.8s is stale."
% (self.merge_sha,))
self.log.info(c)
self.add_comment(self.sha, c)
self.reset_test_ref_to_target()
self.merge_pull_head_to_test_ref()
self.merge_or_batch(pulls, cfg)
return
self.log.info("%s - found pending state, checking tests", self.short())
assert self.merge_sha is not None
Expand Down Expand Up @@ -676,16 +769,15 @@ def try_advance(self):
self.log.info("%s - tests successful, waiting for merge approval",
self.short())
return
if self.fresh():
if self.fresh() or self.batched():
self.log.info("%s - tests successful, attempting landing", self.short())
self.advance_target_ref_to_test()
else:
c = ("Merge sha %.8s is stale."
% (self.merge_sha,))
self.log.info(c)
self.add_comment(self.sha, c)
self.reset_test_ref_to_target()
self.merge_pull_head_to_test_ref()
self.merge_or_batch(pulls, cfg)



Expand Down Expand Up @@ -850,12 +942,14 @@ def main():
pull.priority(),
pull.desc())

all_pulls = pulls

max_pulls_per_run = cfg.get('max_pulls_per_run')
if max_pulls_per_run:
logging.info("Only considering %d pull-requests this run", max_pulls_per_run)
pulls = pulls[-max_pulls_per_run:]

[p.try_advance() for p in reversed(pulls)]
[p.try_advance(list(reversed(all_pulls)), cfg) for p in reversed(pulls)]

if __name__ == "__main__":
try:
Expand Down

0 comments on commit f57c6cc

Please sign in to comment.