[CHG] runbot_merge: convert rebase-merge and merge to working locally

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,

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}\n'
-    """
-    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'

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

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(
+    r = info.repo.with_config(
-        check=False,
-        text=True,
         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(
+    r = info.repo.push(url, f'{head}:{target}')
-        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(
+    r = info.repo.push(git.source_url(pr.repository, 'github'), f'{head}:{target}')
-        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),
+        author = pr_head['commit']['author']
-            'tree': merge_tree,
+        committer = pr_head['commit']['committer']
-            'author': pr_head['commit']['author'],
+        c = info.repo.with_config(env={
-            'committer': pr_head['commit']['committer'],
+            **os.environ,
-            'parents': new_parents,
+            'GIT_AUTHOR_NAME': author['name'],
-        }).json()
+            'GIT_AUTHOR_EMAIL': author['email'],
-        info.gh.set_ref(target, copy['sha'])
+            '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)

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('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}',
+        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}',
+        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')))
         )