Commit fafbdc8
Address review feedback about config lookup
Review feedback: Git treats core.hooksPath case-insensitively even though GitConfigParser preserves option casing, empty values should safely fall back, and the default-path test can inherit a host-level hooksPath.
Find the configured option with a case-insensitive scan of the effective core section, treat missing and empty values as the legacy .git/hooks setting, and spell the regression setting with mixed case. Scope the legacy fallback test to repository configuration and exercise both missing and empty values so system and user settings cannot affect it.1 parent f7aa470 commit fafbdc8
2 files changed
Lines changed: 19 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -67,11 +67,13 @@ def hook_path(name: str, git_dir: PathLike) -> str: | |||
| 67 | 67 | def _commit_hook_path(name: str, index: "IndexFile") -> str: | |
| 68 | 68 | """:return: path to the named commit hook, respecting Git's core.hooksPath.""" | |
| 69 | 69 | with index.repo.config_reader() as config: | |
| 70 | - if not config.has_option("core", "hooksPath"): | ||
| 71 | - return hook_path(name, index.repo.git_dir) | ||
| 72 | - hooks_dir = osp.expanduser(config.get("core", "hooksPath")) | ||
| 70 | + core_items = config.items("core") if config.has_section("core") else [] | ||
| 71 | + hooks_dir = next((value for option, value in reversed(core_items) if option.lower() == "hookspath"), None) | ||
| 73 | 72 | ||
| 74 | - return osp.abspath(osp.join(index.repo.working_dir, hooks_dir, name)) | ||
| 73 | + if not hooks_dir: | ||
| 74 | + return hook_path(name, index.repo.git_dir) | ||
| 75 | + | ||
| 76 | + return osp.abspath(osp.join(index.repo.working_dir, osp.expanduser(hooks_dir), name)) | ||
| 75 | 77 | ||
| 76 | 78 | ||
| 77 | 79 | def _has_file_extension(path: str) -> str: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1099,10 +1099,18 @@ class Mocked: | |||
| 1099 | 1099 | def test_run_commit_hook(self, rw_repo): | |
| 1100 | 1100 | index = rw_repo.index | |
| 1101 | 1101 | _make_hook(index.repo.git_dir, "fake-hook", "echo 'ran fake hook' >output.txt") | |
| 1102 | - with mock.patch.object(Git, "execute", side_effect=AssertionError("hook lookup must not run git")): | ||
| 1103 | - run_commit_hook("fake-hook", index) | ||
| 1104 | - output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8") | ||
| 1105 | - self.assertEqual(output, "ran fake hook\n") | ||
| 1102 | + output = Path(rw_repo.git_dir, "output.txt") | ||
| 1103 | + with mock.patch.object(Repo, "config_level", ("repository",)): | ||
| 1104 | + with mock.patch.object(Git, "execute", side_effect=AssertionError("hook lookup must not run git")): | ||
| 1105 | + run_commit_hook("fake-hook", index) | ||
| 1106 | + self.assertEqual(output.read_text(encoding="utf-8"), "ran fake hook\n") | ||
| 1107 | + | ||
| 1108 | + output.unlink() | ||
| 1109 | + with index.repo.config_writer() as writer: | ||
| 1110 | + writer.set_value("core", "hooksPath", "") | ||
| 1111 | + run_commit_hook("fake-hook", index) | ||
| 1112 | + | ||
| 1113 | + self.assertEqual(output.read_text(encoding="utf-8"), "ran fake hook\n") | ||
| 1106 | 1114 | ||
| 1107 | 1115 | @with_rw_directory | |
| 1108 | 1116 | def test_run_commit_hook_outside_worktree_on_windows(self, rw_dir): | |
@@ -1198,7 +1206,7 @@ def test_pre_commit_hook_respects_core_hooks_path(self, rw_repo): | |||
| 1198 | 1206 | os.chmod(hp, 0o744) | |
| 1199 | 1207 | ||
| 1200 | 1208 | with index.repo.config_writer() as writer: | |
| 1201 | - writer.set_value("core", "hooksPath", "custom-hooks") | ||
| 1209 | + writer.set_value("core", "HoOkSpAtH", "custom-hooks") | ||
| 1202 | 1210 | ||
| 1203 | 1211 | index.commit("This should run the custom hook") | |
| 1204 | 1212 | output = Path(rw_repo.working_dir, "custom-hook-output.txt").read_text(encoding="utf-8") | |
0 commit comments