Allow adding files to repository in pre-commit hook. Fixes #916

NEWS

@@ -1,5 +1,8 @@
 0.20.27	UNRELEASED
 
+ * Allow adding files to repository in pre-commit hook.
+   (Jelmer Vernooij, #916)
+
 0.20.26	2021-10-29
 
  * Support os.PathLike arguments to Repo.stage().

dulwich/hooks.py

@@ -117,10 +117,10 @@ class ShellHook(Hook):
 class PreCommitShellHook(ShellHook):
     """pre-commit shell hook"""
 
-    def __init__(self, controldir):
+    def __init__(self, cwd, controldir):
         filepath = os.path.join(controldir, "hooks", "pre-commit")
 
-        ShellHook.__init__(self, "pre-commit", filepath, 0, cwd=controldir)
+        ShellHook.__init__(self, "pre-commit", filepath, 0, cwd=cwd)
 
 
 class PostCommitShellHook(ShellHook):

dulwich/repo.py

@@ -895,6 +895,14 @@ class BaseRepo(object):
           New commit SHA1
         """
 
+        try:
+            if not no_verify:
+                self.hooks["pre-commit"].execute()
+        except HookError as e:
+            raise CommitError(e)
+        except KeyError:  # no hook defined, silent fallthrough
+            pass
+
         c = Commit()
         if tree is None:
             index = self.open_index()
@@ -904,14 +912,6 @@ class BaseRepo(object):
                 raise ValueError("tree must be a 40-byte hex sha string")
             c.tree = tree
 
-        try:
-            if not no_verify:
-                self.hooks["pre-commit"].execute()
-        except HookError as e:
-            raise CommitError(e)
-        except KeyError:  # no hook defined, silent fallthrough
-            pass
-
         config = self.get_config_stack()
         if merge_heads is None:
             merge_heads = self._read_heads("MERGE_HEAD")
@@ -1057,7 +1057,6 @@ class Repo(BaseRepo):
             if os.path.isfile(hidden_path):
                 with open(hidden_path, "r") as f:
                     path = read_gitfile(f)
-                self.bare = False
                 self._controldir = os.path.join(root, path)
             else:
                 self._controldir = hidden_path
@@ -1101,7 +1100,7 @@ class Repo(BaseRepo):
             with graft_file:
                 self._graftpoints.update(parse_graftpoints(graft_file))
 
-        self.hooks["pre-commit"] = PreCommitShellHook(self.controldir())
+        self.hooks["pre-commit"] = PreCommitShellHook(self.path, self.controldir())
         self.hooks["commit-msg"] = CommitMsgShellHook(self.controldir())
         self.hooks["post-commit"] = PostCommitShellHook(self.controldir())
         self.hooks["post-receive"] = PostReceiveShellHook(self.controldir())

dulwich/tests/test_hooks.py

@@ -71,7 +71,7 @@ exit 0
         )
 
         pre_commit = os.path.join(repo_dir, "hooks", "pre-commit")
-        hook = PreCommitShellHook(repo_dir)
+        hook = PreCommitShellHook(repo_dir, repo_dir)
 
         with open(pre_commit, "w") as f:
             f.write(pre_commit_fail)

dulwich/tests/test_repository.py

@@ -568,9 +568,9 @@ exit 0
         self.assertRaises(
             errors.CommitError,
             r.do_commit,
-            "failed commit",
-            committer="Test Committer <test@nodomain.com>",
-            author="Test Author <test@nodomain.com>",
+            b"failed commit",
+            committer=b"Test Committer <test@nodomain.com>",
+            author=b"Test Author <test@nodomain.com>",
             commit_timestamp=12345,
             commit_timezone=0,
             author_timestamp=12345,
@@ -642,6 +642,51 @@ exit 0
         )
         self.assertEqual([], r[commit_sha].parents)
 
+    def test_shell_hook_pre_commit_add_files(self):
+        if os.name != "posix":
+            self.skipTest("shell hook tests requires POSIX shell")
+
+        pre_commit_contents = """#!%(executable)s
+import os
+from dulwich.repo import Repo
+
+with open('foo', 'w') as f:
+    f.write('newfile')
+
+r = Repo('.')
+r.stage(['foo'])
+""" % {'executable': sys.executable}
+
+        repo_dir = os.path.join(self.mkdtemp())
+        self.addCleanup(shutil.rmtree, repo_dir)
+        r = Repo.init(repo_dir)
+        self.addCleanup(r.close)
+
+        with open(os.path.join(repo_dir, 'blah'), 'w') as f:
+            f.write('blah')
+
+        r.stage(['blah'])
+
+        pre_commit = os.path.join(r.controldir(), "hooks", "pre-commit")
+
+        with open(pre_commit, "w") as f:
+            f.write(pre_commit_contents)
+        os.chmod(pre_commit, stat.S_IREAD | stat.S_IWRITE | stat.S_IEXEC)
+
+        commit_sha = r.do_commit(
+            b"new commit",
+            committer=b"Test Committer <test@nodomain.com>",
+            author=b"Test Author <test@nodomain.com>",
+            commit_timestamp=12395,
+            commit_timezone=0,
+            author_timestamp=12395,
+            author_timezone=0,
+        )
+        self.assertEqual([], r[commit_sha].parents)
+
+        tree = r[r[commit_sha].tree]
+        self.assertEqual(set([b'blah', b'foo']), set(tree))
+
     def test_shell_hook_post_commit(self):
         if os.name != "posix":
             self.skipTest("shell hook tests requires POSIX shell")