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 <gavinmak@google.com>
Reviewed-by: Brian Gan <brgan@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

project.py

diff --git a/project.py b/project.py
index 26e7845..a10de99 100644
--- a/project.py
+++ b/project.py
@@ -3963,14 +3963,15 @@
         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

subcmds/download.py

diff --git a/subcmds/download.py b/subcmds/download.py
index 1c0bf5c..edeadef 100644
--- a/subcmds/download.py
+++ b/subcmds/download.py
@@ -195,7 +195,7 @@
                 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)

tests/test_project.py

diff --git a/tests/test_project.py b/tests/test_project.py
index e7a3bec..ab20105 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -775,6 +775,29 @@
             )
             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",