From 7182166ec00ad3645821435d72c5424b4629165f Mon Sep 17 00:00:00 2001 From: Pierre Riteau Date: Wed, 17 Nov 2021 12:29:06 +0100 Subject: [PATCH] Fix use of removed --preserve-merges option The --preserve-merges (-p) option was replaced by --rebase-merges (-r). This fixes the following error when using git version 2.34.0: Errors running git rebase -p -i remotes/gerrit/stable/xena fatal: --preserve-merges was replaced by --rebase-merges In order to keep compatibility with git < 2.18.0 we detect the git version and use the old --preserve-merges flag when the version is older than 2.18.0. Co-Authored-By: Clark Boylan Change-Id: I04de3d0f20aa6bafcf746b7706d61dd9b9af296c --- git_review/cmd.py | 23 +++++++++++++++++++---- git_review/tests/test_git_review.py | 29 +++++++++++++++++++---------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/git_review/cmd.py b/git_review/cmd.py index 16b613ac..9215b5d1 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -909,10 +909,25 @@ def rebase_changes(branch, remote, interactive=True): "re-run with the '-R' option enabled." % (branch, remote)) sys.exit(1) - if interactive: - cmd = "git rebase -p -i %s" % remote_branch - else: - cmd = "git rebase -p %s" % remote_branch + # Determine git version to set rebase flags below. + output = run_command("git version") + rebase_flag = "--rebase-merges" + if "git version" in output: + try: + v = output.rsplit(None, 1)[1] + gitv = tuple(map(int, v.split('.')[:3])) + if gitv < (2, 18, 0): + rebase_flag = "--preserve-merges" + except Exception: + # We tried to determine the version and failed. Use current git + # flag as fallback. + warn("Could not determine git version. " + "Using modern git rebase flags.") + + interactive_flag = interactive and '-i' or '' + + cmd = "git rebase %s %s %s" % \ + (rebase_flag, interactive_flag, remote_branch) (status, output) = run_command_status(cmd, GIT_EDITOR='true') if status != 0: diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index 4717bdde..dcb92323 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -323,9 +323,11 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): 'create conflict with master') exc = self.assertRaises(Exception, self._run_git_review) - self.assertIn( - "Errors running git rebase -p -i remotes/%s/master" % self._remote, - exc.args[0]) + rebased = ("Errors running git rebase --rebase-merges " + "-i remotes/%s/master" % self._remote in exc.args[0] or + "Errors running git rebase --preserve-merges " + "-i remotes/%s/master" % self._remote in exc.args[0]) + self.assertTrue(rebased) self.assertIn("It is likely that your change has a merge conflict.", exc.args[0]) @@ -341,9 +343,11 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self._dir('test', 'new_test_file.txt')) review_res = self._run_git_review('-v') - self.assertIn( - "Running: git rebase -p -i remotes/%s/master" % self._remote, - review_res) + rebased = ("Running: git rebase --rebase-merges " + "-i remotes/%s/master" % self._remote in review_res or + "Running: git rebase --preserve-merges " + "-i remotes/%s/master" % self._remote in review_res) + self.assertTrue(rebased) self.assertEqual(self._run_git('rev-parse', 'HEAD^1'), head_1) def test_uploads_with_nondefault_rebase(self): @@ -383,8 +387,10 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): review_res = self._run_git_review('-v') # no rebase needed; if it breaks it would try to rebase to master - self.assertNotIn("Running: git rebase -p -i remotes/origin/master", - review_res) + self.assertNotIn("Running: git rebase --rebase-merges " + "-i remotes/origin/master", review_res) + self.assertNotIn("Running: git rebase --preserve-merges " + "-i remotes/origin/master", review_res) # Don't need to query gerrit for the branch as the second half # of this test will work only if the branch was correctly # stored in gerrit @@ -401,8 +407,11 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase): self.assertEqual(change_id, new_change_id) review_res = self._run_git_review('-v') # caused the right thing to happen - self.assertIn("Running: git rebase -p -i remotes/origin/maint", - review_res) + rebased = ("Running: git rebase --rebase-merges " + "-i remotes/origin/maint" in review_res or + "Running: git rebase --preserve-merges " + "-i remotes/origin/maint" in review_res) + self.assertTrue(rebased) # track different branch than expected in changeset branch = self._run_git('rev-parse', '--abbrev-ref', 'HEAD')