From 16e0b4e206ec722d4cc9b5c53ca02534278f50f5 Mon Sep 17 00:00:00 2001
From: tandrii <tandrii@chromium.org>
Date: Tue, 7 Jun 2016 10:34:28 -0700
Subject: [PATCH] Gerrit git cl upload: warn and offer to remove Gerrit
 commit-msg hook.

R=andybons@chromium.org,sergiyb@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2045973002
---
 git_cl.py            | 22 ++++++++++++++++++++
 tests/git_cl_test.py | 48 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/git_cl.py b/git_cl.py
index 29331d693..05fb354d1 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -2314,6 +2314,27 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
           hostname=parsed_url.netloc)
     return None
 
+  def _GerritCommitMsgHookCheck(self, offer_removal):
+    hook = os.path.join(settings.GetRoot(), '.git', 'hooks', 'commit-msg')
+    if not os.path.exists(hook):
+      return
+    # Crude attempt to distinguish Gerrit Codereview hook from potentially
+    # custom developer made one.
+    data = gclient_utils.FileRead(hook)
+    if not('From Gerrit Code Review' in data and 'add_ChangeId()' in data):
+      return
+    print('Warning: you have Gerrit commit-msg hook installed.\n'
+          'It is not neccessary for uploading with git cl in squash mode, '
+          'and may interfere with it in subtle ways.\n'
+          'We recommend you remove the commit-msg hook.')
+    if offer_removal:
+      reply = ask_for_data('Do you want to remove it now? [Yes/No]')
+      if reply.lower().startswith('y'):
+        gclient_utils.rm_file_or_tree(hook)
+        print('Gerrit commit-msg hook removed.')
+      else:
+        print('OK, will keep Gerrit commit-msg hook in place.')
+
   def CMDUploadChange(self, options, args, change):
     """Upload the current branch to Gerrit."""
     if options.squash and options.no_squash:
@@ -2329,6 +2350,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
                           pending_prefix='')
 
     if options.squash:
+      self._GerritCommitMsgHookCheck(offer_removal=not options.force)
       if not self.GetIssue():
         # TODO(tandrii): deperecate this after 2016Q2.  Backwards compatibility
         # with shadow branch, which used to contain change-id for a given
diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py
index eed3fe56e..3561edc2e 100755
--- a/tests/git_cl_test.py
+++ b/tests/git_cl_test.py
@@ -876,8 +876,10 @@ class TestGitCl(TestCase):
     """Generic gerrit upload test framework."""
     reviewers = reviewers or []
     self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
-    self.mock(git_cl.gerrit_util, "CookiesAuthenticator",
+    self.mock(git_cl.gerrit_util, 'CookiesAuthenticator',
               CookiesAuthenticatorMockFactory(same_cookie='same_cred'))
+    self.mock(git_cl._GerritChangelistImpl, '_GerritCommitMsgHookCheck',
+              lambda _, offer_removal: None)
     self.calls = self._gerrit_base_calls(issue=issue)
     self.calls += self._gerrit_upload_calls(
         description, reviewers, squash,
@@ -1581,6 +1583,50 @@ class TestGitCl(TestCase):
     ]
     self.assertEqual(0, git_cl.main(['issue', '0']))
 
+  def _common_GerritCommitMsgHookCheck(self):
+    self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
+    self.mock(git_cl.os.path, 'abspath',
+              lambda path: self._mocked_call(['abspath', path]))
+    self.mock(git_cl.os.path, 'exists',
+              lambda path: self._mocked_call(['exists', path]))
+    self.mock(git_cl.gclient_utils, 'FileRead',
+              lambda path: self._mocked_call(['FileRead', path]))
+    self.mock(git_cl.gclient_utils, 'rm_file_or_tree',
+              lambda path: self._mocked_call(['rm_file_or_tree', path]))
+    self.calls = [
+        ((['git', 'rev-parse', '--show-cdup'],), '../'),
+        ((['abspath', '../'],), '/abs/git_repo_root'),
+    ]
+    return git_cl.Changelist(codereview='gerrit', issue=123)
+
+  def test_GerritCommitMsgHookCheck_custom_hook(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True),
+        ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         '#!/bin/sh\necho "custom hook"')
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
+  def test_GerritCommitMsgHookCheck_not_exists(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), False),
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
+  def test_GerritCommitMsgHookCheck(self):
+    cl = self._common_GerritCommitMsgHookCheck()
+    self.calls += [
+        ((['exists', '/abs/git_repo_root/.git/hooks/commit-msg'],), True),
+        ((['FileRead', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         '...\n# From Gerrit Code Review\n...\nadd_ChangeId()\n'),
+        (('Do you want to remove it now? [Yes/No]',), 'Yes'),
+        ((['rm_file_or_tree', '/abs/git_repo_root/.git/hooks/commit-msg'],),
+         ''),
+    ]
+    cl._codereview_impl._GerritCommitMsgHookCheck(offer_removal=True)
+
 
 if __name__ == '__main__':
   git_cl.logging.basicConfig(