From 2898c7edd418e348084336d5c01c2cced658f768 Mon Sep 17 00:00:00 2001
From: Xavier Morel <xmo@odoo.com>
Date: Mon, 7 Feb 2022 12:00:31 +0100
Subject: [PATCH] [FIX] runbot_merge: updating of commit date on rebase

On #509, the rebasing process was changed to forcefully update the
commit date of the commits, in order to force trigger builds.

However when squashing was re-enabled for #539 for some fool reason it
implemented its own bespoke rebasing (despite that not actually saving
any API call that I can see), meaning it did *not* update the commit
date. As such, an old commit being squashed would not get picked up by
the runbot, which is what happened to odoo/documentation#1226 (which
ultimately had to be hand-rebased after some confusion as to why it
did not work).

Update `_stage_squash` to go through `rebase` the normal way, also
update `rebase` to pop the commit date entirely instead of setting it
manually, and update the squashing test to check that the commit date
gets properly updated.

Fixes #579, closes #582
---
 runbot_merge/github.py               |  7 +++----
 runbot_merge/models/pull_requests.py | 18 +++++-------------
 runbot_merge/tests/test_basic.py     | 21 +++++++++++++++++----
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/runbot_merge/github.py b/runbot_merge/github.py
index 89a873e4..8a368ed2 100644
--- a/runbot_merge/github.py
+++ b/runbot_merge/github.py
@@ -308,15 +308,14 @@ class GH(object):
         prev = original_head
         mapping = {}
         for c in commits:
+            committer = c['commit']['committer']
+            committer.pop('date')
             copy = self('post', 'git/commits', json={
                 'message': c['commit']['message'],
                 'tree': c['new_tree'],
                 'parents': [prev],
                 'author': c['commit']['author'],
-                'committer': dict(
-                    c['commit']['committer'],
-                    date=datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%SZ")
-                ),
+                'committer': committer,
             }, check={409: MergeError}).json()
             logger.debug('copied %s to %s (parent: %s)', c['sha'], copy['sha'], prev)
             prev = mapping[c['sha']] = copy['sha']
diff --git a/runbot_merge/models/pull_requests.py b/runbot_merge/models/pull_requests.py
index c003d85d..429636e6 100644
--- a/runbot_merge/models/pull_requests.py
+++ b/runbot_merge/models/pull_requests.py
@@ -1260,20 +1260,12 @@ class PullRequests(models.Model):
             gh, target, pr_commits, related_prs=related_prs)
 
     def _stage_squash(self, gh, target, commits, related_prs=()):
-        original_head = gh.head(target)
+        assert len(commits) == 1, "can only squash a single commit"
         msg = self._build_merge_message(self, related_prs=related_prs)
-        [commit] = commits
-        merge_tree = gh.merge(commit['sha'], target, 'temp')['tree']['sha']
-        squashed = gh('post', 'git/commits', json={
-            'message': str(msg),
-            'tree': merge_tree,
-            'author': commit['commit']['author'],
-            'committer': commit['commit']['committer'],
-            'parents': [original_head],
-        }).json()['sha']
-        gh.set_ref(target, squashed)
-        self.commits_map = json.dumps({commit['sha']: squashed, '': squashed})
-        return squashed
+        commits[0]['commit']['message'] = str(msg)
+        head, mapping = gh.rebase(self.number, target, commits=commits)
+        self.commits_map = json.dumps({**mapping, '': head})
+        return head
 
     def _stage_rebase_ff(self, gh, target, commits, related_prs=()):
         # updates head commit with PR number (if necessary) then rebases
diff --git a/runbot_merge/tests/test_basic.py b/runbot_merge/tests/test_basic.py
index 3c05421b..99358859 100644
--- a/runbot_merge/tests/test_basic.py
+++ b/runbot_merge/tests/test_basic.py
@@ -1,12 +1,12 @@
 import datetime
 import itertools
 import json
-import re
 import textwrap
+import time
 from unittest import mock
 
 import pytest
-from lxml import html, etree
+from lxml import html
 
 import odoo
 from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr, part_of
@@ -1958,7 +1958,11 @@ Part-of: {pr_id.display_name}"""
         with repo:
             repo.make_commits(None, Commit('initial', tree={'a': '0'}), ref='heads/master')
 
-            repo.make_commits('master', Commit('sub', tree={'b': '0'}), ref='heads/other')
+            repo.make_commits(
+                'master',
+                Commit('sub', tree={'b': '0'}, committer={'name': 'bob', 'email': 'builder@example.org', 'date': '1999-04-12T08:19:30Z'}),
+                ref='heads/other'
+            )
             pr1 = repo.make_pr(title='first pr', target='master', head='other')
             repo.post_status('other', 'success', 'legal/cla')
             repo.post_status('other', 'success', 'ci/runbot')
@@ -1988,12 +1992,21 @@ Part-of: {pr_id.display_name}"""
             (users['reviewer'], 'hansen r+ squash'),
             (users['user'], 'Merge method set to squash')
         ]
-        assert repo.commit('master').message == f"""first pr
+        merged_head = repo.commit('master')
+        assert merged_head.message == f"""first pr
 
 closes {pr1_id.display_name}
 
 Signed-off-by: {get_partner(env, users["reviewer"]).formatted_email}\
 """
+        assert merged_head.committer['name'] == 'bob'
+        assert merged_head.committer['email'] == 'builder@example.org'
+        commit_date = datetime.datetime.strptime(merged_head.committer['date'], '%Y-%m-%dT%H:%M:%SZ')
+        # using timestamp (and working in seconds) because `pytest.approx`
+        # silently fails on datetimes (#8395)
+        assert commit_date.timestamp() == pytest.approx(time.time(), abs=5*60), \
+            "the commit date of the merged commit should be about now, despite" \
+            " the source commit being >20 years old"
 
         pr2_id = to_pr(env, pr2)
         assert pr2_id.state == 'ready'