[CHG] runbot_merge: add signoff & related to all commits

if rebased. Untouched commits (straight merge) remain unalterated, but
all rebased or squashed commits now get signoff and `Related` headers
added on top of the already previously added `part-of`.

Implement by generalising `_build_merge_message` to `_build_message`
and having `add_self_references` delegate to it, removes some of the
redundancy / differential handling.

Update the `part_of` helper to also add the S-O-B header to the PR,
although it currently does not reference the entire forward port
chain.

Fixes #876

forwardport/models/project.py

@@ -519,28 +519,6 @@ stderr:
             prev = configured.stdout().rev_parse('HEAD').stdout.decode()
             logger.info('%s: success -> %s', commit_sha, prev)
 
-    def _build_merge_message(self, message, related_prs=()):
-        msg = super()._build_merge_message(message, related_prs=related_prs)
-
-        # ensures all reviewers in the review path are on the PR in order:
-        # original reviewer, then last conflict reviewer, then current PR
-        reviewers = (self | self.root_id | self.source_id)\
-            .mapped('reviewed_by.formatted_email')
-
-        sobs = msg.headers.getlist('signed-off-by')
-        msg.headers.remove('signed-off-by')
-        msg.headers.extend(
-            ('signed-off-by', signer)
-            for signer in sobs
-            if signer not in reviewers
-        )
-        msg.headers.extend(
-            ('signed-off-by', reviewer)
-            for reviewer in reversed(reviewers)
-        )
-
-        return msg
-
     def _make_fp_message(self, commit):
         cmap = json.loads(self.commits_map)
         msg = Message.from_message(commit['commit']['message'])

mergebot_test_utils/utils.py

@@ -182,7 +182,10 @@ def to_pr(env, 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}'
+    return f"""\
+{label}{separator}\
+Part-of: {pr_id.display_name}
+Signed-off-by: {pr_id.reviewed_by.formatted_email}"""
 
 def ensure_one(records):
     assert len(records) == 1

runbot_merge/models/pull_requests.py

@@ -1339,19 +1339,36 @@ class PullRequests(models.Model):
             if commit:
                 self.env.cr.commit()
 
-    def _build_merge_message(self, message: Union['PullRequests', str], related_prs=()) -> 'Message':
+    def _build_message(self, message: Union['PullRequests', str], related_prs: 'PullRequests' = (), merge: bool = True) -> 'Message':
         # handle co-authored commits (https://help.github.com/articles/creating-a-commit-with-multiple-authors/)
         m = Message.from_message(message)
         if not is_mentioned(message, self):
-            m.body += f'\n\ncloses {self.display_name}'
+            if merge:
+                m.body += f'\n\ncloses {self.display_name}'
+            else:
+                m.headers.pop('Part-Of', None)
+                m.headers.add('Part-Of', self.display_name)
 
         for r in related_prs:
             if not is_mentioned(message, r, full_reference=True):
                 m.headers.add('Related', r.display_name)
 
-        if self.reviewed_by:
-            m.headers.add('signed-off-by', self.reviewed_by.formatted_email)
+        # ensures all reviewers in the review path are on the PR in order:
+        # original reviewer, then last conflict reviewer, then current PR
+        reviewers = (self | self.root_id | self.source_id)\
+            .mapped('reviewed_by.formatted_email')
 
+        sobs = m.headers.getlist('signed-off-by')
+        m.headers.remove('signed-off-by')
+        m.headers.extend(
+            ('signed-off-by', signer)
+            for signer in sobs
+            if signer not in reviewers
+        )
+        m.headers.extend(
+            ('signed-off-by', reviewer)
+            for reviewer in reversed(reviewers)
+        )
         return m
 
     def unstage(self, reason, *args):

runbot_merge/models/stagings_create.py

@@ -454,7 +454,7 @@ def stage(pr: PullRequests, info: StagingSlice, related_prs: PullRequests) -> Tu
     return method, new_head
 
 def stage_squash(pr: PullRequests, info: StagingSlice, commits: List[github.PrCommit], related_prs: PullRequests) -> str:
-    msg = pr._build_merge_message(pr, related_prs=related_prs)
+    msg = pr._build_message(pr, related_prs=related_prs)
 
     authors = {
         (c['commit']['author']['name'], c['commit']['author']['email'])
@@ -499,11 +499,7 @@ def stage_squash(pr: PullRequests, info: StagingSlice, commits: List[github.PrCo
     return head
 
 def stage_rebase_ff(pr: PullRequests, info: StagingSlice, commits: List[github.PrCommit], related_prs: PullRequests) -> str:
-    # updates head commit with PR number (if necessary) then rebases
-    # on top of target
-    msg = pr._build_merge_message(commits[-1]['commit']['message'], related_prs=related_prs)
-    commits[-1]['commit']['message'] = str(msg)
-    add_self_references(pr, commits[:-1])
+    add_self_references(pr, commits, related_prs=related_prs, merge=commits[-1])
 
     _logger.debug("rebasing %s on %s (commits=%s)",
                   pr.display_name, info.head, len(commits))
@@ -512,11 +508,11 @@ def stage_rebase_ff(pr: PullRequests, info: StagingSlice, commits: List[github.P
     return head
 
 def stage_rebase_merge(pr: PullRequests, info: StagingSlice, commits: List[github.PrCommit], related_prs: PullRequests) -> str :
-    add_self_references(pr, commits)
+    add_self_references(pr, commits, related_prs=related_prs)
     _logger.debug("rebasing %s on %s (commits=%s)",
                   pr.display_name, info.head, len(commits))
     h, mapping = info.repo.rebase(info.head, commits=commits)
-    msg = pr._build_merge_message(pr, related_prs=related_prs)
+    msg = pr._build_message(pr, related_prs=related_prs)
 
     project = pr.repository.project_id
     merge_head= info.repo.merge(
@@ -554,7 +550,7 @@ def stage_merge(pr: PullRequests, info: StagingSlice, commits: List[github.PrCom
             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)
+        msg = pr._build_message(pr_head['commit']['message'], related_prs=related_prs)
 
         d2t = itemgetter('name', 'email', 'date')
         c = info.repo.commit_tree(
@@ -574,7 +570,7 @@ def stage_merge(pr: PullRequests, info: StagingSlice, commits: List[github.PrCom
         return copy
     else:
         # otherwise do a regular merge
-        msg = pr._build_merge_message(pr)
+        msg = pr._build_message(pr)
         project = pr.repository.project_id
         merge_head = info.repo.merge(
             info.head, pr.head, str(msg),
@@ -595,17 +591,21 @@ def is_mentioned(message: Union[PullRequests, str], pr: PullRequests, *, full_re
         pattern = fr'( |\b{repository})#{pr.number}\b'
     return bool(re.search(pattern, message if isinstance(message, str) else message.message))
 
-def add_self_references(pr: PullRequests, commits: List[github.PrCommit]):
+def add_self_references(
+        pr: PullRequests,
+        commits: List[github.PrCommit],
+        related_prs: PullRequests,
+        merge: Optional[github.PrCommit] = None,
+):
     """Adds a footer reference to ``self`` to all ``commits`` if they don't
     already refer to the PR.
     """
     for c in (c['commit'] for c in commits):
-        if not is_mentioned(c['message'], pr):
-            message = c['message']
-            m = Message.from_message(message)
-            m.headers.pop('Part-Of', None)
-            m.headers.add('Part-Of', pr.display_name)
-            c['message'] = str(m)
+        c['message'] = str(pr._build_message(
+            c['message'],
+            related_prs=related_prs,
+            merge=merge and c['url'] == merge['commit']['url'],
+        ))
 
 BREAK = re.compile(r'''
     [ ]{0,3} # 0-3 spaces of indentation

runbot_merge/tests/test_basic.py

@@ -1965,7 +1965,8 @@ Some: thing
 
 is odd
 
-Part-of: {pr_id.display_name}"""
+Part-of: {pr_id.display_name}
+Signed-off-by: {reviewer}"""
 
     def test_pr_mergehead(self, repo, env, config):
         """ if the head of the PR is a merge commit and one of the parents is