From 6321b26685ff9bdf5858e574dc4967761990ce5b Mon Sep 17 00:00:00 2001 From: Gavin Mak Date: Tue, 25 Aug 2026 08:58:04 -0700 Subject: [PATCH] project: make fast-forward merges explicit Every caller expects _FastForward to reject a merge commit. Always pass --ff-only and remove the optional parameter so a violated ancestry assumption fails instead of silently creating a merge. Bug: 553599402 Change-Id: I04690b5ca675cb7d3062df08d5e20cd82f5b9e82 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/634003 Tested-by: Gavin Mak Reviewed-by: Brian Gan Commit-Queue: Gavin Mak --- project.py | 11 ++++++----- subcmds/download.py | 2 +- tests/test_project.py | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/project.py b/project.py index 26e7845ca..a10de9904 100644 --- a/project.py +++ b/project.py @@ -3963,14 +3963,15 @@ class Project: if GitCommand(self, cmd).Wait() != 0: raise GitError(f"{self.name} rebase {upstream} ", project=self.name) - def _FastForward(self, head, ffonly=False, quiet=True): - cmd = ["merge", "--no-stat", head] - if ffonly: - cmd.append("--ff-only") + def _FastForward(self, head: str, quiet: bool = True) -> None: + cmd = ["merge", "--no-stat", "--ff-only"] if quiet: cmd.append("-q") + cmd.append(head) if GitCommand(self, cmd).Wait() != 0: - raise GitError(f"{self.name} merge {head} ", project=self.name) + raise GitError( + f"{self.name} merge --ff-only {head}", project=self.name + ) def _ReprojectCheckout( self, revid: str, head: Optional[str], verbose: bool = False diff --git a/subcmds/download.py b/subcmds/download.py index 1c0bf5ced..edeadefb1 100644 --- a/subcmds/download.py +++ b/subcmds/download.py @@ -195,7 +195,7 @@ If no project is specified try to use current directory as a project. elif opt.revert: project._Revert(dl.commit) elif opt.ffonly: - project._FastForward(dl.commit, ffonly=True) + project._FastForward(dl.commit) else: if opt.branch: project.StartBranch(opt.branch, revision=dl.commit) diff --git a/tests/test_project.py b/tests/test_project.py index e7a3bec60..ab20105fd 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -775,6 +775,29 @@ class ProjectTests(unittest.TestCase): ) self.assertEqual(2, proj.work_git.DiffZ.call_count) + def test_fast_forward_always_rejects_merge_commits(self) -> None: + """The fast-forward helper always passes --ff-only to Git.""" + with utils_for_test.TempGitTree() as tempdir: + proj = _create_mock_project(tempdir) + with mock.patch.object( + project, "GitCommand", autospec=True + ) as run_git: + run_git.return_value.Wait.return_value = 0 + proj._FastForward("revision") + run_git.assert_called_once_with( + proj, ["merge", "--no-stat", "--ff-only", "-q", "revision"] + ) + + run_git.reset_mock() + proj._FastForward("revision", quiet=False) + run_git.assert_called_once_with( + proj, ["merge", "--no-stat", "--ff-only", "revision"] + ) + + run_git.return_value.Wait.return_value = 1 + with self.assertRaises(error.GitError): + proj._FastForward("revision") + @unittest.skipUnless( utils_for_test.supports_reftable(), "git reftable support is required for this test",