Merge "Don't make hook script read-only"

git_review/cmd.py

@@ -24,6 +24,7 @@ import json
 import os
 import re
 import shlex
+import stat
 import subprocess
 import sys
 import textwrap
@@ -439,8 +440,17 @@ def set_hooks_commit_msg(remote, target_file):
             "git", "submodule", "foreach",
             'cp -p %s "$(git rev-parse --git-dir)/hooks/"' % target_file)
 
-    if not os.access(target_file, os.X_OK):
-        os.chmod(target_file, os.path.stat.S_IREAD | os.path.stat.S_IEXEC)
+    old_mask = stat.S_IMODE(os.stat(target_file).st_mode)
+    # make sure the owner always has read+write+exec perms
+    mask = old_mask | stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR
+    # add executable permission for everyone else who has read access
+    if mask | stat.S_IRGRP == mask:
+        mask |= stat.S_IXGRP
+    if mask | stat.S_IROTH == mask:
+        mask |= stat.S_IXOTH
+    # only call chmod if we need to change the mask
+    if mask != old_mask:
+        os.chmod(target_file, mask)
 
 
 def test_remote_url(remote_url):

git_review/tests/test_git_review.py

@@ -110,6 +110,24 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
         self._run_git_review(
             '-s', chdir=os.path.join(self.test_dir, 'subdirectory'))
 
+    def test_install_remote_hook(self):
+        """Test whether git-review -s correctly creates the commit-msg hook
+        from the Gerrit server with appropriate permissions and content.
+        """
+        hooks_subdir = ".git/hooks"
+        hook_file = os.path.join(self.test_dir, hooks_subdir, 'commit-msg')
+        self.reset_remote()
+
+        self._run_git_review('-s')
+        self.assertTrue(os.path.exists(hook_file))
+        self.assertTrue(os.access(hook_file, os.W_OK))
+        self.assertTrue(os.access(hook_file, os.X_OK))
+        with open(hook_file) as f:
+            content = f.read()
+            self.assertTrue(content.startswith('#!/'))
+            # This line is injected by the Gerrit server in its version
+            self.assertIn('# From Gerrit Code Review', content)
+
     def test_git_review_s_without_core_hooks_path_option(self):
         """Test whether git-review -s correctly creates the commit-msg hook,
         with the Git core.hooksPath option unset.