diff --git a/forwardport/tests/test_simple.py b/forwardport/tests/test_simple.py index 8bdb7779..5f5e21b8 100644 --- a/forwardport/tests/test_simple.py +++ b/forwardport/tests/test_simple.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta import pytest -from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, part_of2 +from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of FMT = '%Y-%m-%d %H:%M:%S' FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT) @@ -206,7 +206,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port old_b = prod.read_tree(b_head) head_b = prod.commit('b') assert head_b.message == message_template % pr1.number - assert prod.commit(head_b.parents[0]).message == part_of2(f'p_0\n\nX-original-commit: {p_0_merged}', pr1, separator='\n') + assert prod.commit(head_b.parents[0]).message == part_of(f'p_0\n\nX-original-commit: {p_0_merged}', pr1, separator='\n') b_tree = prod.read_tree(head_b) assert b_tree == { **old_b, @@ -215,7 +215,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port old_c = prod.read_tree(c_head) head_c = prod.commit('c') assert head_c.message == message_template % pr2.number - assert prod.commit(head_c.parents[0]).message == part_of2(f'p_0\n\nX-original-commit: {p_0_merged}', pr2, separator='\n') + assert prod.commit(head_c.parents[0]).message == part_of(f'p_0\n\nX-original-commit: {p_0_merged}', pr2, separator='\n') c_tree = prod.read_tree(head_c) assert c_tree == { **old_c, diff --git a/mergebot_test_utils/utils.py b/mergebot_test_utils/utils.py index 42b20a9d..fae0d208 100644 --- a/mergebot_test_utils/utils.py +++ b/mergebot_test_utils/utils.py @@ -137,9 +137,4 @@ def to_pr(env, pr): return pr def part_of(label, pr_id, *, separator='\n\n'): - """ Adds the "part-of" pseudo-header in the footer. - """ - return f'{label}{separator}Part-of: {pr_id.display_name}' - -def part_of2(label, pr_id, *, separator='\n\n'): - return part_of(label, pr_id, separator=separator) + '\n' + return f'{label}{separator}Part-of: {pr_id.display_name}\n' diff --git a/runbot_merge/git.py b/runbot_merge/git.py index a06a293d..e75d3384 100644 --- a/runbot_merge/git.py +++ b/runbot_merge/git.py @@ -172,6 +172,28 @@ class Repo(Generic[T]): return dest, mapping + def merge(self, c1: str, c2: str, msg: str, *, author: Tuple[str, str]) -> str: + repo = self.stdout().with_config(text=True, check=False) + + t = repo.merge_tree(c1, c2) + if t.returncode: + raise MergeError(t.stderr) + + c = repo.with_config(env={ + **os.environ, + 'GIT_AUTHOR_NAME': author[0], + 'GIT_AUTHOR_EMAIL': author[1], + 'TZ': 'UTC', + }).commit_tree( + '-p', c1, + '-p', c2, + '-m', msg, + t.stdout.strip() + ) + if c.returncode: + raise MergeError(c.stderr) + return c.stdout.strip() + def check(p: subprocess.CompletedProcess) -> subprocess.CompletedProcess: if not p.returncode: return p diff --git a/runbot_merge/models/stagings_create.py b/runbot_merge/models/stagings_create.py index 59ff6ef5..9e474362 100644 --- a/runbot_merge/models/stagings_create.py +++ b/runbot_merge/models/stagings_create.py @@ -8,7 +8,7 @@ import os import re import tempfile from difflib import Differ -from itertools import count, takewhile +from itertools import count, takewhile, chain, repeat from pathlib import Path from subprocess import CompletedProcess from typing import Dict, Union, Optional, Literal, Callable, Iterator, Tuple, List, TypeAlias @@ -246,7 +246,7 @@ def staging_setup( *(pr.head for pr in all_prs if pr.repository == repo) ) original_heads[repo] = head - staging_state[repo] = StagingSlice(gh=gh, head=head, repo=source) + staging_state[repo] = StagingSlice(gh=gh, head=head, repo=source.stdout().with_config(text=True, check=False)) return original_heads, staging_state @@ -501,14 +501,12 @@ def stage_squash(pr: PullRequests, info: StagingSlice, target: str, commits: Lis committer_name, committer_email = committers.pop() # should committers also be added to co-authors? - r = info.repo.stdout().with_config(check=False, text=True).merge_tree(info.head, pr.head) + r = info.repo.merge_tree(info.head, pr.head) if r.returncode: raise exceptions.MergeError(pr, r.stderr) merge_tree = r.stdout.strip() - r = info.repo.stdout().with_config( - check=False, - text=True, + r = info.repo.with_config( env={ **os.environ, 'GIT_AUTHOR_NAME': author_name, @@ -530,10 +528,7 @@ def stage_squash(pr: PullRequests, info: StagingSlice, target: str, commits: Lis head = r.stdout.strip() # TODO: remove when we stop using tmp. - r = info.repo.with_config(text=True).check(False).push( - url, - f'{head}:{target}' - ) + r = info.repo.push(url, f'{head}:{target}') if r.returncode: raise exceptions.MergeError(pr, r.stderr) @@ -552,10 +547,7 @@ def stage_rebase_ff(pr: PullRequests, info: StagingSlice, target: str, commits: head, mapping = info.repo.rebase(info.head, commits=commits) # TODO: remove when we stop using tmp. - r = info.repo.with_config(text=True).check(False).push( - git.source_url(pr.repository, 'github'), - f'{head}:{target}' - ) + r = info.repo.push(git.source_url(pr.repository, 'github'), f'{head}:{target}') if r.returncode: raise exceptions.MergeError(pr, r.stderr) @@ -564,9 +556,20 @@ def stage_rebase_ff(pr: PullRequests, info: StagingSlice, target: str, commits: def stage_rebase_merge(pr: PullRequests, info: StagingSlice, target: str, commits: List[github.PrCommit], related_prs: PullRequests) -> str : add_self_references(pr, commits) - h, mapping = info.gh.rebase(pr.number, target, reset=True, commits=commits) + h, mapping = info.repo.rebase(info.head, commits=commits) msg = pr._build_merge_message(pr, related_prs=related_prs) - merge_head = info.gh.merge(h, target, str(msg))['sha'] + + project = pr.repository.project_id + merge_head= info.repo.merge( + info.head, h, str(msg), + author=(project.github_name, project.github_email), + ) + + # TODO: remove when we stop using tmp. + r = info.repo.push(git.source_url(pr.repository, 'github'), f'{merge_head}:{target}') + if r.returncode: + raise exceptions.MergeError(pr, r.stderr) + pr.commits_map = json.dumps({**mapping, '': merge_head}) return merge_head @@ -593,25 +596,57 @@ def stage_merge(pr: PullRequests, info: StagingSlice, target: str, commits: List if base_commit: # replicate pr_head with base_commit replaced by # the current head - merge_tree = info.gh.merge(pr_head['sha'], target, 'temp merge')['tree']['sha'] + t = info.repo.merge_tree(info.head, pr_head['sha']) + if t.returncode: + raise exceptions.MergeError(pr, t.stderr) + merge_tree = t.stdout.strip() new_parents = [info.head] + list(head_parents - {base_commit}) msg = pr._build_merge_message(pr_head['commit']['message'], related_prs=related_prs) - copy = info.gh('post', 'git/commits', json={ - 'message': str(msg), - 'tree': merge_tree, - 'author': pr_head['commit']['author'], - 'committer': pr_head['commit']['committer'], - 'parents': new_parents, - }).json() - info.gh.set_ref(target, copy['sha']) + + author = pr_head['commit']['author'] + committer = pr_head['commit']['committer'] + c = info.repo.with_config(env={ + **os.environ, + 'GIT_AUTHOR_NAME': author['name'], + 'GIT_AUTHOR_EMAIL': author['email'], + 'GIT_AUTHOR_DATE': author['date'], + 'GIT_COMMITTER_NAME': committer['name'], + 'GIT_COMMITTER_EMAIL': committer['email'], + 'GIT_COMMITTER_DATE': committer['date'], + }).commit_tree( + *(chain.from_iterable(zip( + repeat('-p'), + new_parents, + ))), + '-m', str(msg), + merge_tree, + ) + if c.returncode: + raise exceptions.MergeError(pr, c.stderr) + copy = c.stdout.strip() + + # TODO: remove when we stop using tmp. + r = info.repo.push(git.source_url(pr.repository, 'github'), f'{copy}:{target}') + if r.returncode: + raise exceptions.MergeError(pr, r.stderr) + # merge commit *and old PR head* map to the pr head replica - commits_map[''] = commits_map[pr_head['sha']] = copy['sha'] + commits_map[''] = commits_map[pr_head['sha']] = copy pr.commits_map = json.dumps(commits_map) - return copy['sha'] + return copy else: # otherwise do a regular merge msg = pr._build_merge_message(pr) - merge_head = info.gh.merge(pr.head, target, str(msg))['sha'] + project = pr.repository.project_id + merge_head = info.repo.merge( + info.head, pr.head, str(msg), + author=(project.github_name, project.github_email), + ) + # TODO: remove when we stop using tmp. + r = info.repo.push(git.source_url(pr.repository, 'github'), f'{merge_head}:{target}') + if r.returncode: + raise exceptions.MergeError(pr, r.stderr) + # and the merge commit is the normal merge head commits_map[''] = merge_head pr.commits_map = json.dumps(commits_map) diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py index f7378f49..815e6c8b 100644 --- a/runbot_merge/tests/test_basic.py +++ b/runbot_merge/tests/test_basic.py @@ -1,7 +1,6 @@ import datetime import itertools import json -import textwrap import time from unittest import mock @@ -10,7 +9,7 @@ import requests from lxml import html import odoo -from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr, part_of, part_of2 +from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr, part_of @pytest.fixture @@ -124,7 +123,7 @@ def test_trivial_flow(env, repo, page, users, config): 'b': 'a second file', } assert master.message == "gibberish\n\nblahblah\n\ncloses {repo.name}#1"\ - "\n\nSigned-off-by: {reviewer.formatted_email}"\ + "\n\nSigned-off-by: {reviewer.formatted_email}\n"\ .format(repo=repo, reviewer=get_partner(env, users['reviewer'])) class TestCommitMessage: @@ -702,9 +701,9 @@ def test_ff_failure_batch(env, repo, users, config): reviewer = get_partner(env, users["reviewer"]).formatted_email assert messages == { 'initial', 'NO!', - part_of('a1', pr_a), part_of('a2', pr_a), f'A\n\ncloses {pr_a.display_name}\n\nSigned-off-by: {reviewer}', - part_of('b1', pr_b), part_of('b2', pr_b), f'B\n\ncloses {pr_b.display_name}\n\nSigned-off-by: {reviewer}', - part_of('c1', pr_c), part_of('c2', pr_c), f'C\n\ncloses {pr_c.display_name}\n\nSigned-off-by: {reviewer}', + part_of('a1', pr_a), part_of('a2', pr_a), f'A\n\ncloses {pr_a.display_name}\n\nSigned-off-by: {reviewer}\n', + part_of('b1', pr_b), part_of('b2', pr_b), f'B\n\ncloses {pr_b.display_name}\n\nSigned-off-by: {reviewer}\n', + part_of('c1', pr_c), part_of('c2', pr_c), f'C\n\ncloses {pr_c.display_name}\n\nSigned-off-by: {reviewer}\n', } class TestPREdition: @@ -1533,7 +1532,7 @@ commits, I need to know how to merge it: nb1 = node(part_of('B1', pr_id), node(part_of('B0', pr_id), nm2)) reviewer = get_partner(env, users["reviewer"]).formatted_email merge_head = ( - f'title\n\nbody\n\ncloses {pr_id.display_name}\n\nSigned-off-by: {reviewer}', + f'title\n\nbody\n\ncloses {pr_id.display_name}\n\nSigned-off-by: {reviewer}\n', frozenset([nm2, nb1]) ) assert staging == merge_head @@ -1630,7 +1629,7 @@ commits, I need to know how to merge it: nm2 = node('M2', node('M1', node('M0'))) reviewer = get_partner(env, users["reviewer"]).formatted_email nb1 = node(f'B1\n\ncloses {pr_id.display_name}\n\nSigned-off-by: {reviewer}\n', - node(part_of2('B0', pr_id), nm2)) + node(part_of('B0', pr_id), nm2)) assert staging == nb1 with repo: @@ -1798,7 +1797,8 @@ first closes {repo.name}#{pr.number} -Signed-off-by: {reviewer}""", "should not contain the content which follows the thematic break" +Signed-off-by: {reviewer} +""", "should not contain the content which follows the thematic break" def test_pr_message_setex_title(self, repo, env, users, config): """ should not break on a proper SETEX-style title """ @@ -1843,7 +1843,8 @@ This is more text closes {repo.name}#{pr.number} -Signed-off-by: {reviewer}""", "should not break the SETEX titles" +Signed-off-by: {reviewer} +""", "should not break the SETEX titles" def test_rebase_no_edit(self, repo, env, users, config): """ Only the merge messages should be de-breaked @@ -2001,7 +2002,7 @@ Part-of: {pr_id.display_name} m1 = node('M1') reviewer = get_partner(env, users["reviewer"]).formatted_email expected = node( - 'T\n\nTT\n\ncloses {}#{}\n\nSigned-off-by: {}'.format(repo.name, prx.number, reviewer), + 'T\n\nTT\n\ncloses {}#{}\n\nSigned-off-by: {}\n'.format(repo.name, prx.number, reviewer), node('M2', m1), node('C1', node('C0', m1), node('B0', m1)) ) @@ -2668,12 +2669,12 @@ class TestBatching(object): staging = log_to_node(log) reviewer = get_partner(env, users["reviewer"]).formatted_email p1 = node( - 'title PR1\n\nbody PR1\n\ncloses {}\n\nSigned-off-by: {}'.format(pr1.display_name, reviewer), + 'title PR1\n\nbody PR1\n\ncloses {}\n\nSigned-off-by: {}\n'.format(pr1.display_name, reviewer), node('initial'), node(part_of('commit_PR1_01', pr1), node(part_of('commit_PR1_00', pr1), node('initial'))) ) p2 = node( - 'title PR2\n\nbody PR2\n\ncloses {}\n\nSigned-off-by: {}'.format(pr2.display_name, reviewer), + 'title PR2\n\nbody PR2\n\ncloses {}\n\nSigned-off-by: {}\n'.format(pr2.display_name, reviewer), p1, node(part_of('commit_PR2_01', pr2), node(part_of('commit_PR2_00', pr2), p1)) ) @@ -2708,12 +2709,12 @@ class TestBatching(object): reviewer = get_partner(env, users["reviewer"]).formatted_email p1 = node( - 'title PR1\n\nbody PR1\n\ncloses {}#{}\n\nSigned-off-by: {}'.format(repo.name, pr1.number, reviewer), + 'title PR1\n\nbody PR1\n\ncloses {}#{}\n\nSigned-off-by: {}\n'.format(repo.name, pr1.number, reviewer), node('initial'), node('commit_PR1_01', node('commit_PR1_00', node('initial'))) ) p2 = node( - 'title PR2\n\nbody PR2\n\ncloses {}#{}\n\nSigned-off-by: {}'.format(repo.name, pr2.number, reviewer), + 'title PR2\n\nbody PR2\n\ncloses {}#{}\n\nSigned-off-by: {}\n'.format(repo.name, pr2.number, reviewer), p1, node('commit_PR2_01', node('commit_PR2_00', node('initial'))) )