diff --git a/git_review/cmd.py b/git_review/cmd.py index 0b3104bd..fc600dbe 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -24,6 +24,7 @@ import json import os import re import shlex +import stat import subprocess import sys import textwrap @@ -429,8 +430,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): diff --git a/git_review/tests/test_git_review.py b/git_review/tests/test_git_review.py index ba5b2f61..b7965dbf 100644 --- a/git_review/tests/test_git_review.py +++ b/git_review/tests/test_git_review.py @@ -105,6 +105,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.