diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 60e34a651..76f276323 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,25 +20,16 @@ A contribution that works only narrowly but lowers the quality of the codebase may be declined. The maintainers may not always be able to provide detailed feedback. -## AI-assisted contributions +## Prevent agent impersonation -If AI edits files for you, disclose it in the pull request description and commit -metadata. Prefer making the agent identity part of the commit, for example by using -an AI author such as `$agent $version ` or a co-author via -a `Co-authored-by: ` trailer. +AI agents communicating through a person's account must identify themselves, for +example in issue or PR descriptions and comments. AI assistance that does not replace +the person as the speaker, such as proofreading or wording polish, does not require +identification. -Agents operating through a person's GitHub account must identify themselves. For -example, comments posted by an agent should say so directly with phrases like -`AI agent on behalf of : ...`. - -Fully AI-generated comments on pull requests or issues must also be disclosed. -Undisclosed AI-generated comments may lead to the pull request or issue being closed. - -AI-assisted proofreading or wording polish does not need disclosure, but it is still -courteous to mention it when the AI materially influenced the final text. - -Automated or "full-auto" AI contributions without a human responsible for reviewing -and standing behind the work may be closed. +Attributing AI assistance in commit metadata, for example with a `Co-authored-by` +trailer, is welcome but not required. Code is reviewed the same way regardless of its +origin. ## Fuzzing Test Specific Documentation diff --git a/VERSION b/VERSION index 2c5a9fd4a..406837cb8 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.52 +3.1.53 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 2bfe6dd17..138826a3c 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,18 @@ Changelog ========= +3.1.53 +====== + +A security fix for +https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-3rp5-jjmw-4wv2 + +If you can, also try and provide feedback on the upcoming v4 branch +https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.53 + 3.1.52 ====== diff --git a/git/cmd.py b/git/cmd.py index 150c9d16f..1868862d3 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1358,7 +1358,7 @@ def execute( # Allow the user to have the command executed in their working dir. try: - cwd = self._working_dir or os.getcwd() # type: Union[None, str] + cwd = self._working_dir or os.getcwd() # type: Optional[PathLike] if not os.access(str(cwd), os.X_OK): cwd = None except FileNotFoundError: diff --git a/git/config.py b/git/config.py index 64f501424..821710ea3 100644 --- a/git/config.py +++ b/git/config.py @@ -897,6 +897,22 @@ def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> s def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None: if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name): raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label) + if label == "section" and isinstance(name, str): + in_quotes = False + escaped = False + for index, char in enumerate(name): + if escaped: + escaped = False + elif in_quotes and char == "\\": + escaped = True + elif char == '"': + if not in_quotes and (index == 0 or name[index - 1] not in " \t"): + raise ValueError("Git config quoted subsection names must begin after whitespace") + in_quotes = not in_quotes + elif char == "]" and not in_quotes: + raise ValueError("Git config section names must not contain an unquoted closing bracket") + if in_quotes: + raise ValueError("Git config section names must not contain an unterminated quote") @needs_values @set_dirty_and_flush_changes diff --git a/git/index/fun.py b/git/index/fun.py index 629c19b1e..5d52486f9 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -64,6 +64,17 @@ def hook_path(name: str, git_dir: PathLike) -> str: return osp.join(git_dir, "hooks", name) +def _commit_hook_path(name: str, index: "IndexFile") -> str: + """:return: path to the named commit hook, respecting Git's core.hooksPath.""" + with index.repo.config_reader() as config: + hooks_dir = config.get("core", "hooksPath", fallback="") + + if not hooks_dir: + return hook_path(name, index.repo.git_dir) + + return osp.abspath(osp.join(index.repo.working_dir, osp.expanduser(hooks_dir), name)) + + def _has_file_extension(path: str) -> str: return osp.splitext(path)[1] @@ -82,7 +93,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: :raise git.exc.HookExecutionError: """ - hp = hook_path(name, index.repo.git_dir) + hp = _commit_hook_path(name, index) if not os.access(hp, os.X_OK): return @@ -94,8 +105,14 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: if sys.platform == "win32" and not _has_file_extension(hp): # Windows only uses extensions to determine how to open files # (doesn't understand shebangs). Try using bash to run the hook. - relative_hp = Path(hp).relative_to(index.repo.working_dir).as_posix() - cmd = ["bash.exe", relative_hp] + try: + bash_hp = osp.relpath(hp, index.repo.working_dir) + except ValueError: + # Different drives have no relative path on Windows. Git Bash accepts + # an absolute path in this form, although a relative path is preferable + # because it also works with the Windows Subsystem for Linux wrapper. + bash_hp = hp + cmd = ["bash.exe", Path(bash_hp).as_posix()] process = safer_popen( cmd + list(args), diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index d183672db..7984db340 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -49,6 +49,7 @@ Callable, Dict, Iterator, + List, Mapping, Sequence, TYPE_CHECKING, @@ -738,122 +739,132 @@ def update( mrepo = None # END init mrepo + def fetch_remotes(module_repo: "Repo") -> None: + rmts = module_repo.remotes + len_rmts = len(rmts) + for i, remote in enumerate(rmts): + op = FETCH + if i == 0: + op |= BEGIN + # END handle start + + progress.update( + op, + i, + len_rmts, + prefix + "Fetching remote %s of submodule %r" % (remote, self.name), + ) + # =============================== + if not dry_run: + remote.fetch(progress=progress) + # END handle dry-run + # =============================== + if i == len_rmts - 1: + op |= END + # END handle end + progress.update( + op, + i, + len_rmts, + prefix + "Done fetching remote of submodule %r" % self.name, + ) + # END fetch new data + try: # ENSURE REPO IS PRESENT AND UP-TO-DATE ####################################### try: mrepo = self.module() - rmts = mrepo.remotes - len_rmts = len(rmts) - for i, remote in enumerate(rmts): - op = FETCH - if i == 0: - op |= BEGIN - # END handle start - - progress.update( - op, - i, - len_rmts, - prefix + "Fetching remote %s of submodule %r" % (remote, self.name), - ) - # =============================== - if not dry_run: - remote.fetch(progress=progress) - # END handle dry-run - # =============================== - if i == len_rmts - 1: - op |= END - # END handle end - progress.update( - op, - i, - len_rmts, - prefix + "Done fetching remote of submodule %r" % self.name, - ) - # END fetch new data + fetch_remotes(mrepo) except InvalidGitRepositoryError: mrepo = None if not init: return self # END early abort if init is not allowed - # There is no git-repository yet - but delete empty paths. checkout_module_abspath = self.abspath - if not dry_run and osp.isdir(checkout_module_abspath): + module_abspath = self._module_abspath(self.repo, self.path, self.name) + + # ``git submodule deinit`` leaves the repository in + # ``.git/modules`` and empties the checkout. Reconnect that retained + # repository instead of trying to clone over it. + if not dry_run and osp.isdir(module_abspath): try: - os.rmdir(checkout_module_abspath) - except OSError as e: - raise OSError( - "Module directory at %r does already exist and is non-empty" % checkout_module_abspath - ) from e - # END handle OSError - # END handle directory removal - - # Don't check it out at first - nonetheless it will create a local - # branch according to the remote-HEAD if possible. - progress.update( - BEGIN | CLONE, - 0, - 1, - prefix - + "Cloning url '%s' to '%s' in submodule %r" % (self.url, checkout_module_abspath, self.name), - ) - if not dry_run: - if self.url.startswith("."): - url = urllib.parse.urljoin(self.repo.remotes.origin.url + "/", self.url) + git.Repo(module_abspath) + except InvalidGitRepositoryError: + pass else: - url = self.url - mrepo = self._clone_repo( - self.repo, - url, - self.path, - self.name, - n=True, - env=env, - multi_options=clone_multi_options, - allow_unsafe_options=allow_unsafe_options, - allow_unsafe_protocols=allow_unsafe_protocols, + if osp.lexists(checkout_module_abspath) and ( + osp.islink(checkout_module_abspath) + or not osp.isdir(checkout_module_abspath) + or os.listdir(checkout_module_abspath) + ): + raise OSError( + "Module directory at %r does already exist and is non-empty" % checkout_module_abspath + ) + os.makedirs(checkout_module_abspath, exist_ok=True) + self._write_git_file_and_module_config(checkout_module_abspath, module_abspath) + mrepo = git.Repo(checkout_module_abspath) + mrepo.head.reset(mrepo.head.commit, index=True, working_tree=True) + fetch_remotes(mrepo) + with self.repo.config_writer() as writer: + writer.set_value(sm_section(self.name), "url", self.url) + + if mrepo is None: + # There is no git-repository yet - but delete empty paths. + if not dry_run and osp.isdir(checkout_module_abspath): + try: + os.rmdir(checkout_module_abspath) + except OSError as e: + raise OSError( + "Module directory at %r does already exist and is non-empty" % checkout_module_abspath + ) from e + # END handle directory removal + + # Don't check it out at first - nonetheless it will create a local + # branch according to the remote-HEAD if possible. + progress.update( + BEGIN | CLONE, + 0, + 1, + prefix + + "Cloning url '%s' to '%s' in submodule %r" % (self.url, checkout_module_abspath, self.name), ) - # END handle dry-run - progress.update( - END | CLONE, - 0, - 1, - prefix + "Done cloning to %s" % checkout_module_abspath, - ) - - if not dry_run: - # See whether we have a valid branch to check out. - try: - mrepo = cast("Repo", mrepo) - # Find a remote which has our branch - we try to be flexible. - remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name) - local_branch = mkhead(mrepo, self.branch_path) - - # Have a valid branch, but no checkout - make sure we can figure - # that out by marking the commit with a null_sha. - local_branch.set_object(Object(mrepo, self.NULL_BIN_SHA)) - # END initial checkout + branch creation - - # Make sure HEAD is not detached. - mrepo.head.set_reference( - local_branch, - logmsg="submodule: attaching head to %s" % local_branch, + if not dry_run: + if self.url.startswith("."): + url = urllib.parse.urljoin(self.repo.remotes.origin.url + "/", self.url) + else: + url = self.url + mrepo = self._clone_repo( + self.repo, + url, + self.path, + self.name, + n=True, + env=env, + multi_options=clone_multi_options, + allow_unsafe_options=allow_unsafe_options, + allow_unsafe_protocols=allow_unsafe_protocols, ) - mrepo.head.reference.set_tracking_branch(remote_branch) - except (IndexError, InvalidGitRepositoryError): - _logger.warning("Failed to checkout tracking branch %s", self.branch_path) - # END handle tracking branch - - # NOTE: Have to write the repo config file as well, otherwise the - # default implementation will be offended and not update the - # repository. Maybe this is a good way to ensure it doesn't get into - # our way, but we want to stay backwards compatible too... It's so - # redundant! - with self.repo.config_writer() as writer: - writer.set_value(sm_section(self.name), "url", self.url) - # END handle dry_run + progress.update(END | CLONE, 0, 1, prefix + "Done cloning to %s" % checkout_module_abspath) + + if not dry_run: + # See whether we have a valid branch to check out. + try: + mrepo = cast("Repo", mrepo) + remote_branch = find_first_remote_branch(mrepo.remotes, self.branch_name) + local_branch = mkhead(mrepo, self.branch_path) + local_branch.set_object(Object(mrepo, self.NULL_BIN_SHA)) + mrepo.head.set_reference( + local_branch, + logmsg="submodule: attaching head to %s" % local_branch, + ) + mrepo.head.reference.set_tracking_branch(remote_branch) + except (IndexError, InvalidGitRepositoryError): + _logger.warning("Failed to checkout tracking branch %s", self.branch_path) + + with self.repo.config_writer() as writer: + writer.set_value(sm_section(self.name), "url", self.url) # END handle initialization # DETERMINE SHAS TO CHECK OUT @@ -1267,6 +1278,36 @@ def remove( return self + @unbare_repo + def deinit(self, force: bool = False) -> "Submodule": + """Run ``git submodule deinit`` on this submodule. + + This is a thin wrapper around ``git submodule deinit ``, + which unregisters the submodule (removes its entry from + ``.git/config`` and empties the working-tree directory) + without deleting the submodule from ``.gitmodules`` + or its checked-out repository under ``.git/modules/``. + A subsequent :meth:`update` will re-initialize the + submodule from the retained contents. + + :param force: + If ``True``, pass ``--force`` to ``git submodule deinit``. This + allows deinitialization even when the submodule's working tree has + local modifications that would otherwise block the command. + + :return: + self + + :note: + Doesn't work in bare repositories. + """ + args: List[str] = [] + if force: + args.append("--force") + args.extend(["--", str(self.path)]) + self.repo.git.submodule("deinit", *args) + return self + def set_parent_commit(self, commit: Union[Commit_ish, str, None], check: bool = True) -> "Submodule": """Set this instance to use the given commit whose tree is supposed to contain the ``.gitmodules`` blob. diff --git a/git/util.py b/git/util.py index 11c1a0b15..8584b0e7f 100644 --- a/git/util.py +++ b/git/util.py @@ -217,7 +217,7 @@ def rmtree(path: PathLike) -> None: couldn't be deleted are read-only. Windows will not remove them in that case. """ - def handler(function: Callable, path: PathLike, _excinfo: Any) -> None: + def handler(function: Callable[[str], Any], path: str, _excinfo: Any) -> None: """Callback for :func:`shutil.rmtree`. This works as either a ``onexc`` or ``onerror`` style callback. @@ -401,7 +401,7 @@ def _cygexpath(drive: Optional[str], path: str, expand_vars: bool = True) -> str return p_str.replace("\\", "/") -_cygpath_parsers: Tuple[Tuple[Pattern[str], Callable, bool], ...] = ( +_cygpath_parsers: Tuple[Tuple[Pattern[str], Callable[..., str], bool], ...] = ( # See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx # and: https://www.cygwin.com/cygwin-ug-net/using.html#unc-paths ( @@ -508,7 +508,7 @@ def get_user_id() -> str: return "%s@%s" % (getpass.getuser(), platform.node()) -def finalize_process(proc: Union[subprocess.Popen, "Git.AutoInterrupt"], **kwargs: Any) -> None: +def finalize_process(proc: Union["subprocess.Popen[Any]", "Git.AutoInterrupt"], **kwargs: Any) -> None: """Wait for the process (clone, fetch, pull or push) and handle its errors accordingly.""" # TODO: No close proc-streams?? @@ -520,19 +520,21 @@ def expand_path(p: None, expand_vars: bool = ...) -> None: ... @overload -def expand_path(p: PathLike, expand_vars: bool = ...) -> str: +def expand_path(p: PathLike, expand_vars: bool = ...) -> Optional[PathLike]: # TODO: Support for Python 3.5 has been dropped, so these overloads can be improved. ... def expand_path(p: Union[None, PathLike], expand_vars: bool = True) -> Optional[PathLike]: - if isinstance(p, Path): - return p.resolve() + if p is None: + return None try: - p = osp.expanduser(p) # type: ignore[arg-type] + if isinstance(p, Path): + return p.resolve() + expanded_path = osp.expanduser(os.fspath(p)) if expand_vars: - p = osp.expandvars(p) - return osp.normpath(osp.abspath(p)) + expanded_path = osp.expandvars(expanded_path) + return osp.normpath(osp.abspath(expanded_path)) except Exception: return None @@ -767,7 +769,7 @@ class CallableRemoteProgress(RemoteProgress): __slots__ = ("_callable",) - def __init__(self, fn: Callable) -> None: + def __init__(self, fn: Callable[..., Any]) -> None: self._callable = fn super().__init__() @@ -846,7 +848,7 @@ def _main_actor( cls, env_name: str, env_email: str, - config_reader: Union[None, "GitConfigParser", "SectionConstraint"] = None, + config_reader: Union[None, "GitConfigParser", "SectionConstraint[GitConfigParser]"] = None, ) -> "Actor": actor = Actor("", "") user_id = None # We use this to avoid multiple calls to getpass.getuser(). @@ -882,7 +884,9 @@ def default_name() -> str: return actor @classmethod - def committer(cls, config_reader: Union[None, "GitConfigParser", "SectionConstraint"] = None) -> "Actor": + def committer( + cls, config_reader: Union[None, "GitConfigParser", "SectionConstraint[GitConfigParser]"] = None + ) -> "Actor": """ :return: :class:`Actor` instance corresponding to the configured committer. It @@ -897,7 +901,9 @@ def committer(cls, config_reader: Union[None, "GitConfigParser", "SectionConstra return cls._main_actor(cls.env_committer_name, cls.env_committer_email, config_reader) @classmethod - def author(cls, config_reader: Union[None, "GitConfigParser", "SectionConstraint"] = None) -> "Actor": + def author( + cls, config_reader: Union[None, "GitConfigParser", "SectionConstraint[GitConfigParser]"] = None + ) -> "Actor": """Same as :meth:`committer`, but defines the main author. It may be specified in the environment, but defaults to the committer.""" return cls._main_actor(cls.env_author_name, cls.env_author_email, config_reader) @@ -980,11 +986,11 @@ class IndexFileSHA1Writer: __slots__ = ("f", "sha1") - def __init__(self, f: IO) -> None: + def __init__(self, f: IO[bytes]) -> None: self.f = f self.sha1 = make_sha(b"") - def write(self, data: AnyStr) -> int: + def write(self, data: bytes) -> int: self.sha1.update(data) return self.f.write(data) @@ -1181,6 +1187,7 @@ def __new__(cls, id_attr: str, prefix: str = "") -> "IterableList[T_IterableObj] return super().__new__(cls) def __init__(self, id_attr: str, prefix: str = "") -> None: + super().__init__() self._id_attr = id_attr self._prefix = prefix @@ -1210,7 +1217,9 @@ def __getattr__(self, attr: str) -> T_IterableObj: # END for each item return list.__getattribute__(self, attr) - def __getitem__(self, index: Union[SupportsIndex, int, slice, str]) -> T_IterableObj: # type: ignore[override] + def __getitem__( # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride] + self, index: Union[SupportsIndex, int, slice, str] + ) -> T_IterableObj: if isinstance(index, int): return list.__getitem__(self, index) elif isinstance(index, slice): @@ -1288,7 +1297,7 @@ def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_I :return: list(Item,...) list of item instances """ - out_list: IterableList = IterableList(cls._id_attribute_) + out_list: IterableList[T_IterableObj] = IterableList(cls._id_attribute_) out_list.extend(cls.iter_items(repo, *args, **kwargs)) return out_list @@ -1297,7 +1306,8 @@ class IterableClassWatcher(type): """Metaclass that issues :exc:`DeprecationWarning` when :class:`git.util.Iterable` is subclassed.""" - def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None: + def __init__(cls, name: str, bases: Tuple[type, ...], clsdict: Dict[str, Any]) -> None: + super().__init__(name, bases, clsdict) for base in bases: if type(base) is IterableClassWatcher: warnings.warn( diff --git a/pyproject.toml b/pyproject.toml index 149f2dc92..324630002 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,6 +33,17 @@ exclude = ["^git/ext/gitdb"] module = "gitdb.*" ignore_missing_imports = true +[tool.basedpyright] +typeCheckingMode = "standard" +pythonVersion = "3.7" +extraPaths = [ + "git/ext/gitdb", + "git/ext/gitdb/gitdb/ext/smmap", +] +exclude = [ + "git/ext/gitdb", +] + [tool.coverage.run] source = ["git"] diff --git a/test/test_config.py b/test/test_config.py index c4e39db48..361a51fa9 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -194,6 +194,53 @@ def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir): self.assertEqual(git_config.get_value("user", "name"), "safe") self.assertFalse(git_config.has_section("core")) + @with_rw_directory + def test_writer_rejects_unquoted_section_terminators(self, rw_dir): + config_path = osp.join(rw_dir, "config") + bad_sections = ( + "user] [other", + 'user"] [other"', + 'submodule "docs"] [other', + 'submodule "docs] [other', + 'submodule "docs] [other\\', + ) + safe_section = 'submodule "docs]archive"' + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_section in bad_sections: + with pytest.raises(ValueError, match="section name"): + git_config.add_section(bad_section) + with pytest.raises(ValueError, match="section name"): + git_config.set(bad_section, "name", "value") + with pytest.raises(ValueError, match="section name"): + git_config.set_value(bad_section, "name", "value") + with pytest.raises(ValueError, match="section name"): + git_config.add_value(bad_section, "name", "value") + with pytest.raises(ValueError, match="section name"): + git_config.rename_section("user", bad_section) + + git_config.set_value("user", "name", "safe") + git_config.set_value(safe_section, "name", "safe") + self.assertEqual(git_config.get_value(safe_section, "name"), "safe") + + # A closing bracket inside a quoted subsection name is data, not a section terminator. + with open(config_path, "rb") as config_file: + self.assertIn( + b'[submodule "docs]archive"]\n', + config_file.read(), + "a closing bracket within a quoted subsection name should be preserved", + ) + + # Reparse the file to verify that rejected names did not inject an [other] section. + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual( + git_config.get_value("user", "name"), + "safe", + "rejected section names corrupted the existing section", + ) + self.assertFalse(git_config.has_section("other"), "an unsafe section name injected an [other] section") + @with_rw_directory def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir): config_path = osp.join(rw_dir, "config") diff --git a/test/test_index.py b/test/test_index.py index 3be750dbb..881dec19f 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1099,9 +1099,35 @@ class Mocked: def test_run_commit_hook(self, rw_repo): index = rw_repo.index _make_hook(index.repo.git_dir, "fake-hook", "echo 'ran fake hook' >output.txt") - run_commit_hook("fake-hook", index) - output = Path(rw_repo.git_dir, "output.txt").read_text(encoding="utf-8") - self.assertEqual(output, "ran fake hook\n") + output = Path(rw_repo.git_dir, "output.txt") + with mock.patch.object(Repo, "config_level", ("repository",)): + with mock.patch.object(Git, "execute", side_effect=AssertionError("hook lookup must not run git")): + run_commit_hook("fake-hook", index) + self.assertEqual(output.read_text(encoding="utf-8"), "ran fake hook\n") + + output.unlink() + with index.repo.config_writer() as writer: + writer.set_value("core", "hooksPath", "") + run_commit_hook("fake-hook", index) + + self.assertEqual(output.read_text(encoding="utf-8"), "ran fake hook\n") + + @with_rw_directory + def test_run_commit_hook_outside_worktree_on_windows(self, rw_dir): + root = Path(rw_dir).resolve() + repo = Repo.init(root / "repo") + hooks_dir = root / "hooks" + _make_hook(root, "fake-hook", "exit 0") + with repo.config_writer() as writer: + writer.set_value("core", "hooksPath", str(hooks_dir)) + + with mock.patch("git.index.fun.sys.platform", "win32"): + with mock.patch("git.index.fun.safer_popen") as popen, mock.patch("git.index.fun.handle_process_output"): + popen.return_value.returncode = 0 + run_commit_hook("fake-hook", repo.index) + + command = popen.call_args[0][0] + self.assertEqual(command, ["bash.exe", "../hooks/fake-hook"]) @ddt.data((False,), (True,)) @with_rw_directory @@ -1160,6 +1186,32 @@ def test_pre_commit_hook_success(self, rw_repo): _make_hook(index.repo.git_dir, "pre-commit", "exit 0") index.commit("This should not fail") + @pytest.mark.xfail( + type(_win_bash_status) is WinBashStatus.Absent, + reason="Can't run a hook on Windows without bash.exe.", + raises=HookExecutionError, + ) + @pytest.mark.xfail( + type(_win_bash_status) is WinBashStatus.WslNoDistro, + reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", + raises=HookExecutionError, + ) + @with_rw_repo("HEAD") + def test_pre_commit_hook_respects_core_hooks_path(self, rw_repo): + index = rw_repo.index + hooks_dir = Path(index.repo.working_dir, "custom-hooks") + hooks_dir.mkdir() + hp = hooks_dir / "pre-commit" + hp.write_text(HOOKS_SHEBANG + "echo 'ran custom hook' >custom-hook-output.txt", encoding="utf-8") + os.chmod(hp, 0o744) + + with index.repo.config_writer() as writer: + writer.set_value("core", "hooksPath", "custom-hooks") + + index.commit("This should run the custom hook") + output = Path(rw_repo.working_dir, "custom-hook-output.txt").read_text(encoding="utf-8") + self.assertEqual(output, "ran custom hook\n") + @pytest.mark.xfail( type(_win_bash_status) is WinBashStatus.WslNoDistro, reason="Currently uses the bash.exe of WSL, even with no WSL distro installed", diff --git a/test/test_submodule.py b/test/test_submodule.py index 778d22e3f..265ee2ffb 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -713,6 +713,166 @@ def test_iter_items_from_invalid_hash(self): next(it) self.assertIsNone(ctx.exception.value) + @with_rw_directory + def test_deinit_calls_git_submodule(self, rwdir): + repo = git.Repo.init(rwdir) + submodule = Submodule(repo, b"\0" * 20, name="module", path="module") + + with mock.patch.object(Git, "submodule", create=True) as git_submodule: + submodule.deinit() + + git_submodule.assert_called_once_with("deinit", "--", submodule.path) + git_submodule.reset_mock() + + submodule.deinit(force=True) + + git_submodule.assert_called_once_with("deinit", "--force", "--", submodule.path) + + @with_rw_directory + def test_update_after_deinit(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + touch(osp.join(source_path, "file")) + source_repo.index.add(["file"]) + source_repo.index.commit("initial commit") + + parent_path = osp.join(rwdir, "parent") + parent_repo = git.Repo.init(parent_path) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + + submodule.deinit() + assert not submodule.module_exists() + assert osp.isdir(osp.join(parent_repo.git_dir, "modules", submodule.name)) + + submodule.update() + + assert submodule.module_exists() + assert submodule.module().head.commit == source_repo.head.commit + assert osp.isfile(osp.join(submodule.abspath, "file")) + + @with_rw_directory + def test_update_to_latest_revision_after_deinit_fetches_remote(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + source_repo.git.commit(m="initial commit", allow_empty=True) + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + submodule.deinit() + + touch(osp.join(source_path, "new-file")) + source_repo.index.add(["new-file"]) + source_repo.index.commit("advance remote") + + submodule.update(to_latest_revision=True) + + assert submodule.module().head.commit == source_repo.head.commit + assert osp.isfile(osp.join(submodule.abspath, "new-file")) + + @with_rw_directory + def test_update_after_deinit_fetches_new_gitlink_commit(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + source_repo.git.commit(m="initial commit", allow_empty=True) + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + submodule.deinit() + + touch(osp.join(source_path, "new-file")) + source_repo.index.add(["new-file"]) + source_repo.index.commit("advance remote") + parent_repo.git.update_index( + "--cacheinfo", + f"160000,{source_repo.head.commit.hexsha},{submodule.path}", + ) + parent_repo.index.commit("advance submodule") + + submodule = parent_repo.submodule(submodule.name) + submodule.update() + + assert submodule.module().head.commit == source_repo.head.commit + assert osp.isfile(osp.join(submodule.abspath, "new-file")) + + @with_rw_directory + def test_update_after_deinit_refuses_non_empty_checkout(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + tracked_file = osp.join(source_path, "file") + with open(tracked_file, "w") as fp: + fp.write("submodule content") + source_repo.index.add(["file"]) + source_repo.index.commit("initial commit") + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + submodule.deinit() + + checkout_file = osp.join(submodule.abspath, "file") + with open(checkout_file, "w") as fp: + fp.write("user content") + + with pytest.raises(OSError, match="does already exist and is non-empty"): + submodule.update() + + with open(checkout_file) as fp: + assert fp.read() == "user content" + assert not osp.exists(osp.join(submodule.abspath, ".git")) + + @with_rw_directory + def test_update_after_deinit_without_init_does_not_restore_checkout(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + source_repo.git.commit(m="initial commit", allow_empty=True) + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + submodule.deinit() + + assert submodule.update(init=False) is submodule + assert not submodule.module_exists() + assert not osp.exists(osp.join(submodule.abspath, ".git")) + + @with_rw_directory + def test_dry_run_update_after_deinit_does_not_restore_checkout(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + source_repo.git.commit(m="initial commit", allow_empty=True) + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("module", "module", source_path) + parent_repo.index.commit("add submodule") + submodule.deinit() + + assert submodule.update(dry_run=True) is submodule + assert not submodule.module_exists() + assert not osp.exists(osp.join(submodule.abspath, ".git")) + + @with_rw_directory + def test_update_after_deinit_restores_nested_checkout(self, rwdir): + source_path = osp.join(rwdir, "source") + source_repo = git.Repo.init(source_path) + touch(osp.join(source_path, "file")) + source_repo.index.add(["file"]) + source_repo.index.commit("initial commit") + + parent_repo = git.Repo.init(osp.join(rwdir, "parent")) + submodule = parent_repo.create_submodule("nested/module", "deps/module", source_path) + parent_repo.index.commit("add nested submodule") + submodule.deinit() + + submodule.update() + + module_repo = submodule.module() + assert module_repo.head.commit == source_repo.head.commit + assert osp.isfile(osp.join(submodule.abspath, "file")) + assert osp.samefile(module_repo.git_dir, osp.join(parent_repo.git_dir, "modules", submodule.name)) + @with_rw_repo(k_no_subm_tag, bare=False) def test_first_submodule(self, rwrepo): assert len(list(rwrepo.iter_submodules())) == 0