diff --git a/.github/workflows/dependencies.yml b/.github/workflows/dependencies.yml index b4046dd9b..4eff39b3c 100644 --- a/.github/workflows/dependencies.yml +++ b/.github/workflows/dependencies.yml @@ -48,7 +48,7 @@ jobs: persist-credentials: false - name: Install uv - uses: astral-sh/setup-uv@c18668ad3cf93ea998bef934396af7bb5c839dc7 # v10.2.0 + uses: astral-sh/setup-uv@1c37ad07a6a961277cf70c0d37d6f313000f5884 # v10.3.0 with: enable-cache: false version: "0.12.13" # Released on 2026-09-10 diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 3fbbe2ec6..94630dbfc 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -54,7 +54,7 @@ jobs: - name: Set up WSL (Windows) if: matrix.os-type == 'windows' - uses: Vampire/setup-wsl@v7.0.0 + uses: Vampire/setup-wsl@v7.2.0 with: wsl-version: 1 diff --git a/AGENTS.md b/AGENTS.md index 6b2963cf4..cdffa4764 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,8 @@ Before starting work, read and follow [CONTRIBUTING.md](CONTRIBUTING.md), including the [Prevent agent impersonation](CONTRIBUTING.md#prevent-agent-impersonation) -section governing identification when communicating through a person's account. +section governing agent identification and separation of unaltered user statements +when communicating through a person's account. # Commit messages diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 76f276323..1a64a701d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,6 +27,13 @@ example in issue or PR descriptions and comments. AI assistance that does not re the person as the speaker, such as proofreading or wording polish, does not require identification. +Even when identifying themselves, agents must not assert on a person's behalf that +that person performed an action, such as reviewing or approving a PR. The person +must make any such statement themselves. If it is included alongside agent-authored +content, it must be supplied by the person and preserved verbatim in a clearly +labeled, separate user-authored section. Agents must not draft, paraphrase, or embed +such statements in their own narration. + 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. diff --git a/VERSION b/VERSION index 944880fa1..e4604e3af 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0 +3.2.1 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 8447228db..95178088d 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -10,7 +10,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-83vg-56qc-22m7 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.2.1 @@ -35,7 +35,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-fx3j-rwgx-fr94 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.2.0 @@ -48,7 +48,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-59cr-6r3x-644w If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.62 @@ -60,7 +60,7 @@ A fixup release to avoid accidental removal of public class regex on Actor. It's now deprecated instead. If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.61 @@ -75,7 +75,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-239g-whfq-7xj9 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.60 @@ -92,7 +92,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-7833-fr7j-v32q If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.59 @@ -110,7 +110,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-hmq2-w58f-27jc If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.58 @@ -124,7 +124,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-539m-9xh6-q6rr If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.57 @@ -137,7 +137,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-p538-c434-8v24 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.56 @@ -150,7 +150,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-94p4-4cq8-9g67 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.55 @@ -165,7 +165,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-fjr4-x663-mwxc If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.54 @@ -177,7 +177,7 @@ 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. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.53 diff --git a/doc/source/conf.py b/doc/source/conf.py index 809762483..032e55cfe 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -111,6 +111,11 @@ # This file should be a Windows icon file (.ico) being 16x16 or 32x32 pixels large. # html_favicon = None +# Do not document the attributes holding a class's namespace. Their reprs embed +# frozensets whose order depends on string hashing, which makes the generated +# HTML vary between builds. +autodoc_default_options = {"exclude-members": "__dict__, __weakref__"} + # Add any paths that contain custom static files (such as style sheets) here, relative # to this directory. They are copied after the builtin static files, so a file named # "default.css" will overwrite the builtin "default.css". diff --git a/git/cmd.py b/git/cmd.py index 3ddb9ae7f..1d1ca909a 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1940,9 +1940,15 @@ def _prepare_ref(self, ref: object) -> bytes: else: refstr = ref - if not refstr.endswith("\n"): - refstr += "\n" - return refstr.encode(defenc) + # A line feed terminates a request, so one object name must be one line. An + # embedded one would queue a second request on the persistent command while + # only one response line is read back, leaving every later call one response + # behind, answered with the header of an object it did not ask for. + if refstr.endswith("\n"): + refstr = refstr[:-1] + if "\n" in refstr: + raise ValueError("Object name %r contains a line feed" % refstr) + return (refstr + "\n").encode(defenc) def _get_persistent_cmd(self, attr_name: str, cmd_name: str, *args: Any, **kwargs: Any) -> "Git.AutoInterrupt": cur_val = getattr(self, attr_name) diff --git a/git/config.py b/git/config.py index a3437d525..7109c09c3 100644 --- a/git/config.py +++ b/git/config.py @@ -1018,8 +1018,8 @@ def _string_to_value(self, valuestr: Union[str, None]) -> Union[int, float, str, for numtype in types: try: val = numtype(valuestr) - # truncated value ? - if val != float(valuestr): + # Reject NaN without coercing exact integer values to float. + if numtype is float and val != float(valuestr): continue return val except (ValueError, TypeError): diff --git a/git/diff.py b/git/diff.py index b28a29529..123c22840 100644 --- a/git/diff.py +++ b/git/diff.py @@ -733,12 +733,17 @@ def _handle_diff_line(lines_bytes: bytes, repo: "Repo", index: DiffIndex["Diff"] # Discard everything before the first colon, and the colon itself. _, _, lines = lines.partition(":") - for line in lines.split("\x00:"): - if not line: + # Each record is a metadata field starting with a colon, followed by one path, + # or two for copies and renames, each terminated by NUL. A path may itself + # start with a colon or contain a line feed, so read the fields in order + # instead of splitting at colons. + fields = iter(lines.rstrip("\n").split("\x00")) + for meta in fields: + if not meta: # The line data is empty, skip. continue - meta, _, path = line.partition("\x00") - path = path.rstrip("\x00") + if meta.startswith(":"): + meta = meta[1:] a_blob_id: Optional[str] b_blob_id: Optional[str] old_mode, new_mode, a_blob_id, b_blob_id, _change_type = meta.split(None, 4) @@ -748,7 +753,7 @@ def _handle_diff_line(lines_bytes: bytes, repo: "Repo", index: DiffIndex["Diff"] change_type: Lit_change_type = cast(Lit_change_type, _change_type[0]) score_str = "".join(_change_type[1:]) score = int(score_str) if score_str.isdigit() else None - path = path.strip("\n") + path = next(fields, "") a_path = path.encode(defenc) b_path = path.encode(defenc) deleted_file = False @@ -767,13 +772,9 @@ def _handle_diff_line(lines_bytes: bytes, repo: "Repo", index: DiffIndex["Diff"] new_file = True elif change_type == "C": copied_file = True - a_path_str, b_path_str = path.split("\x00", 1) - a_path = a_path_str.encode(defenc) - b_path = b_path_str.encode(defenc) + b_path = next(fields, "").encode(defenc) elif change_type == "R": - a_path_str, b_path_str = path.split("\x00", 1) - a_path = a_path_str.encode(defenc) - b_path = b_path_str.encode(defenc) + b_path = next(fields, "").encode(defenc) rename_from, rename_to = a_path, b_path elif change_type == "T": # Nothing to do. @@ -817,17 +818,17 @@ def _index_from_raw_format(cls, repo: "Repo", proc: "Popen") -> "DiffIndex[Diff] # :100644 100644 687099101... 37c5e30c8... M .gitignore index: "DiffIndex" = DiffIndex() + text_list: List[bytes] = [] stderr_list: List[bytes] = [] def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> None: finalize_process(proc, stderr=b"".join(stderr_list)) + # Output is read line by line, but paths may contain line feeds, so parse + # the records only once all of it has been read. handle_process_output( - proc, - lambda byt: cls._handle_diff_line(byt, repo, index), - stderr_list.append, - finalize_process_with_stderr, - decode_streams=False, + proc, text_list.append, stderr_list.append, finalize_process_with_stderr, decode_streams=False ) + cls._handle_diff_line(b"".join(text_list), repo, index) return index diff --git a/git/index/base.py b/git/index/base.py index 4ecd0c5b1..17a722c21 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -525,17 +525,17 @@ def _write_path_to_stdin( the piped-in files are processed anyway and just in time. :note: - Newlines are essential here, git's behaviour is somewhat inconsistent on - this depending on the version, hence we try our best to deal with newlines - carefully. Usually the last newline will not be sent, instead we will close - stdin to break the pipe. + Paths are NUL-terminated, so the command has to run with ``-z``. A path can + contain a line feed, and with line-feed separation git would read such a + path as two paths and act on files that were never passed. git also unquotes + a line-feed separated path that begins with a double quote. """ fprogress(filepath, False, item) rval: Union[None, str] = None if proc.stdin is not None: try: - proc.stdin.write(("%s\n" % filepath).encode(defenc)) + proc.stdin.write(("%s\0" % filepath).encode(defenc)) except OSError as e: # Pipe broke, usually because some error happened. raise fmakeexc() from e @@ -728,6 +728,8 @@ def _preprocess_add_items( else: raise TypeError("Invalid Type: %r" % item) # END for each item + # Source paths must be safe to read, but their recorded names may be rewritten. + # Apply mode-dependent restrictions to the final entries in add(). for entry in entries: _validate_repo_path(entry.path) return paths, entries @@ -1026,7 +1028,7 @@ def handle_null_entries(self: "IndexFile") -> None: # FINALIZE # Add the new entries to this instance. for entry in entries_added: - _validate_repo_path(entry.path) + _validate_repo_path(entry.path, entry.mode) for entry in entries_added: self.entries[(entry.path, 0)] = IndexEntry.from_base(entry) @@ -1450,6 +1452,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik # initialization. self.entries # noqa: B018 + args.append("-z") args.append("--stdin") kwargs["as_process"] = True kwargs["istream"] = subprocess.PIPE diff --git a/git/index/fun.py b/git/index/fun.py index 929038f1f..70c5a7e50 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -198,6 +198,11 @@ 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. + # Prefer Bash associated with GitPython's selected Git installation. If + # that layout is not recognized, use an explicitly configured non-system + # PATH entry. Preserve the bare fallback for installations that previously + # relied on WSL or another CreateProcess-resolved Bash. + found_bash = _git_for_windows_bash() or _which_from_path("bash.exe") try: bash_hp = osp.relpath(hp, index.repo.working_dir) except ValueError: @@ -205,12 +210,16 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: # 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 - # Prefer Bash associated with GitPython's selected Git installation. If - # that layout is not recognized, use an explicitly configured non-system - # PATH entry. Preserve the bare fallback for installations that previously - # relied on WSL or another CreateProcess-resolved Bash. - bash_executable = _git_for_windows_bash() or _which_from_path("bash.exe") or "bash.exe" - cmd = [bash_executable, Path(bash_hp).as_posix()] + else: + # Bash resolves a relative path in its own view of the filesystem, + # which differs from Windows' across a mount point: Git Bash mounts + # the user's temporary directory as /tmp, so from a working tree + # there, ".." leads elsewhere. A path leaving the working tree is + # thus passed whole to a Bash found above, which accepts it; the + # bare fallback may be WSL's, which does not, and keeps it relative. + if found_bash and bash_hp.startswith(os.pardir + os.sep): + bash_hp = hp + cmd = [found_bash or "bash.exe", Path(bash_hp).as_posix()] process = safer_popen( cmd + list(args), @@ -279,7 +288,7 @@ def write_cache( # Body for entry in entries: - _validate_repo_path(entry.path) + _validate_repo_path(entry.path, entry.mode) beginoffset = tell() write(entry.ctime_bytes) # ctime write(entry.mtime_bytes) # mtime @@ -394,7 +403,7 @@ def read_cache( if terminator != b"\0": raise ValueError("Unterminated index entry path") path = path_bytes.decode(defenc) - _validate_repo_path(path) + _validate_repo_path(path, mode) real_size = (tell() - beginoffset + 7) & ~7 padding_size = beginoffset + real_size - tell() @@ -462,7 +471,7 @@ def write_tree_from_cache( """ if si == 0: for entry in entries[sl]: - _validate_repo_path(entry.path) + _validate_repo_path(entry.path, entry.mode) tree_items: List["TreeCacheTup"] = [] ci = sl.start @@ -510,7 +519,7 @@ def write_tree_from_cache( def _tree_entry_to_baseindexentry(tree_entry: "TreeCacheTup", stage: int) -> BaseIndexEntry: - _validate_repo_path(tree_entry[2]) + _validate_repo_path(tree_entry[2], tree_entry[1]) return BaseIndexEntry((tree_entry[1], tree_entry[0], stage << CE_STAGESHIFT, tree_entry[2])) diff --git a/git/objects/commit.py b/git/objects/commit.py index 307b8c59b..3239079c1 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -662,6 +662,10 @@ def create_from_tree( :return: :class:`Commit` object representing the new commit. + :raise ValueError: + If the name or email of the author or committer contains ``<``, ``>`` or a + line feed, as these would change the identity headers of the commit. + :note: Additional information about the committer and author are taken from the environment or from the git configuration. See :manpage:`git-commit-tree(1)` @@ -786,6 +790,16 @@ def create_from_tree( # { Serializable Implementation def _serialize(self, stream: BytesIO) -> "Commit": + # An identity is written as "name date" on a single header line, so a + # line feed or an angle bracket inside a name or email moves those boundaries: + # it can add header lines, end the headers early, or present another email. + # Git drops these three characters when it writes an identity; refuse them + # here before anything is written. + for actor in (self.author, self.committer): + for value in (actor.name, actor.email): + if value and any(char in value for char in "<>\n"): + raise ValueError("Commit identity %r must not contain '<', '>' or a line feed" % value) + write = stream.write write(("tree %s\n" % self.tree).encode("ascii")) for p in self.parents: @@ -882,8 +896,19 @@ def _deserialize(self, stream: BytesIO) -> "Commit": enc = next_line buf = enc.strip() while buf: - if buf[0:10] == b"encoding ": - self.encoding = buf[buf.find(b" ") + 1 :].decode(self.encoding, "ignore") + if buf[0:9] == b"encoding ": + raw_encoding = buf[9:] + try: + # Codec names are ASCII, independent of the content encoding. + encoding = raw_encoding.decode("ascii") + # Probe one byte: decoding b"" skips the codec lookup entirely. + b" ".decode(encoding, "replace") + except (LookupError, ValueError): + # Reject malformed names and non-text codecs without changing + # the current encoding (the default unless a valid header was read). + _logger.error("Ignoring invalid commit encoding %r", raw_encoding) + else: + self.encoding = encoding elif buf[0:7] == b"gpgsig ": sig_lines = [buf[buf.find(b" ") + 1 :] + b"\n"] is_next_header = False diff --git a/git/objects/fun.py b/git/objects/fun.py index ff1bc8483..41ddecafd 100644 --- a/git/objects/fun.py +++ b/git/objects/fun.py @@ -39,11 +39,11 @@ # --------------------------------------------------- -def _validate_tree_entry_name(name: str) -> None: +def _validate_tree_entry_name(name: str, mode: Union[int, None] = None) -> None: if "/" in name: raise ValueError("Tree entry names must not contain '/' characters") # A tree name is a component, not a rooted path; a colon cannot select a drive. - _validate_repo_path("tree/" + name) + _validate_repo_path("tree/" + name, mode) def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer"], Union[int, None]]) -> None: @@ -82,7 +82,7 @@ def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer name_bytes = name.encode(defenc) else: name_bytes = name # type: ignore[unreachable] # check runtime types - is always str? - _validate_tree_entry_name(safe_decode(name_bytes)) + _validate_tree_entry_name(safe_decode(name_bytes), mode) write(b"".join((mode_str, b" ", name_bytes, b"\0", binsha))) # END for each item @@ -112,7 +112,7 @@ def tree_entries_from_data(data: bytes) -> List[EntryTup]: if name_end < 0 or name_end + 21 > len(data): raise ValueError("Truncated tree entry") name = safe_decode(bytes(data[mode_end + 1 : name_end])) - _validate_tree_entry_name(name) + _validate_tree_entry_name(name, mode) offset = name_end + 21 out.append((bytes(data[name_end + 1 : offset]), mode, name)) return out diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 74fbd39fe..00bb7e226 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -12,7 +12,6 @@ import shlex import stat import sys -import urllib.parse import uuid from io import BytesIO from pathlib import Path @@ -57,8 +56,12 @@ from .util import ( SubmoduleConfigParser, + _is_absolute_path, + _url_is_local_not_ssh, find_first_remote_branch, + is_relative_url, mkhead, + resolve_relative_url, sm_name, sm_section, ) @@ -68,7 +71,7 @@ if TYPE_CHECKING: from git.index import IndexFile from git.objects.commit import Commit - from git.refs import Head, RemoteReference + from git.refs import Head from git.repo import Repo # ----------------------------------------------------------------------------- @@ -428,13 +431,8 @@ def _clone_repo( if not osp.isdir(module_abspath_dir): os.makedirs(module_abspath_dir) - if url.startswith("../"): - remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name - repo_remote_url = repo.remote(remote_name).url - url = os.path.join(repo_remote_url, url) - clone = git.Repo.clone_from( - url, + cls._resolve_url(repo, url), module_checkout_path, allow_unsafe_options=allow_unsafe_options, allow_unsafe_protocols=allow_unsafe_protocols, @@ -746,8 +744,9 @@ def add( ) # END verify url - ## See #525 for ensuring git URLs in config-files are valid under Windows. - url = Git.polish_url(url, expand_vars=False) + # Relative submodule URLs must keep their prefix to resolve against the parent remote. + if not is_relative_url(url): + url = Git.polish_url(url, expand_vars=False) # It's important to add the URL to the parent config, to let `git submodule` know. # Otherwise there is a '-' character in front of the submodule listing: @@ -982,13 +981,9 @@ def fetch_remotes(module_repo: "Repo") -> None: if not dry_run: if no_fetch: raise ValueError("Missing module at %r but fetching is disabled" % self.path) from None - 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.url, self.path, self.name, n=True, @@ -1738,6 +1733,31 @@ def url(self) -> str: """:return: The url to the repository our submodule's repository refers to""" return self._url + @staticmethod + def _resolve_url(repo: "Repo", url: str) -> str: + """Resolve relative URLs against the tracking remote, then ``origin``, + falling back to the superproject's directory the way git does.""" + if not is_relative_url(url): + return url + remote_name = "origin" + if not repo.head.is_detached: + with repo.active_branch.config_reader() as reader: + remote_name = reader.get("remote", fallback="origin") + try: + remote_url = repo.remote(remote_name).url + except ValueError: + remote_url = os.fspath(repo.working_dir) + if remote_url == ".": + remote_url = os.fspath(repo.working_dir) + resolved = resolve_relative_url(remote_url, url) + if resolved and _url_is_local_not_ssh(resolved) and not _is_absolute_path(resolved): + resolved = os.path.join(repo.working_dir, resolved) + return resolved + + def _resolved_url(self) -> str: + """:return: :attr:`url` resolved against the superproject's default remote""" + return self._resolve_url(self.repo, self.url) + @property def parent_commit(self) -> "Commit": """ diff --git a/git/objects/submodule/root.py b/git/objects/submodule/root.py index a41c50009..c1fb4d537 100644 --- a/git/objects/submodule/root.py +++ b/git/objects/submodule/root.py @@ -264,10 +264,13 @@ def update( # type: ignore[override] nn = "__new_origin__" smm = sm.module() rmts = smm.remotes + # Relative urls are resolved against the superproject's remote, + # which is what the module's remotes hold. + sm_url = sm._resolved_url() # Don't do anything if we already have the url we search in # place. - if len([r for r in rmts if r.url == sm.url]) == 0: + if len([r for r in rmts if r.url == sm_url]) == 0: progress.update( BEGIN | URLCHANGE, i, @@ -277,38 +280,31 @@ def update( # type: ignore[override] if not dry_run: previous_remote = None - for remote in rmts: - if remote.url == psm.url: - previous_remote = remote - break - # END if urls match - # END for each remote + if len(rmts) == 1: + previous_remote = rmts[0] + else: + psm_url = psm._resolved_url() + for remote in rmts: + if remote.url == psm_url: + previous_remote = remote + break + # END if urls match + # END for each remote - # If we didn't find a matching remote, but have exactly - # one, we can safely use this one. if previous_remote is None: - if len(rmts) == 1: - previous_remote = rmts[0] - else: - # If we have not found any remote with the - # original URL we may not have a name. This is a - # special case, and its okay to fail here. - # Alternatively we could just generate a unique - # name and leave all existing ones in place. - raise InvalidGitRepositoryError( - "Couldn't find original remote-repo at url %r" % psm.url - ) - # END handle one single remote + raise InvalidGitRepositoryError( + "Couldn't find original remote-repo at url %r" % psm.url + ) # END handle check we found a remote if no_fetch: # A new remote would have no cached refs. Preserve # the existing refs and tracking configuration for # offline updates instead of replacing the remote. - previous_remote.set_url(git.Git.polish_url(sm.url, expand_vars=False)) + previous_remote.set_url(git.Git.polish_url(sm_url, expand_vars=False)) else: assert nn not in [r.name for r in rmts] - smr = smm.create_remote(nn, sm.url) + smr = smm.create_remote(nn, sm_url) smr.fetch(progress=progress) # If we have a tracking branch, it should be available diff --git a/git/objects/submodule/util.py b/git/objects/submodule/util.py index c021510d8..9856e39ce 100644 --- a/git/objects/submodule/util.py +++ b/git/objects/submodule/util.py @@ -6,10 +6,13 @@ "sm_name", "mkhead", "find_first_remote_branch", + "is_relative_url", + "resolve_relative_url", "SubmoduleConfigParser", ] from io import BytesIO +import sys import weakref import git @@ -63,6 +66,97 @@ def find_first_remote_branch(remotes: Sequence["Remote"], branch_name: str) -> " raise InvalidGitRepositoryError("Didn't find remote branch '%r' in any of the given remotes" % branch_name) +def _is_dir_sep(char: str) -> bool: + return char == "/" or (sys.platform == "win32" and char == "\\") + + +def _starts_with_dot_slash(url: str) -> bool: + return len(url) >= 2 and url[0] == "." and _is_dir_sep(url[1]) + + +def _starts_with_dot_dot_slash(url: str) -> bool: + return len(url) >= 3 and url[0:2] == ".." and _is_dir_sep(url[2]) + + +def _has_dos_drive_prefix(url: str) -> bool: + return sys.platform == "win32" and len(url) >= 2 and url[0].isalpha() and url[1] == ":" + + +def _is_absolute_path(url: str) -> bool: + return (len(url) >= 1 and _is_dir_sep(url[0])) or _has_dos_drive_prefix(url) + + +def _url_is_local_not_ssh(url: str) -> bool: + colon = url.find(":") + slash = url.find("/") + return colon == -1 or (slash != -1 and slash < colon) or _has_dos_drive_prefix(url) + + +def _find_last_dir_sep(url: str) -> int: + for index in range(len(url) - 1, -1, -1): + if _is_dir_sep(url[index]): + return index + return -1 + + +def is_relative_url(url: str) -> bool: + """:return: ``True`` if `url` starts with ``./`` or ``../``, which git resolves + against the superproject's remote rather than treating as a path""" + return _starts_with_dot_slash(url) or _starts_with_dot_dot_slash(url) + + +def resolve_relative_url(remote_url: str, url: str) -> str: + """Resolve a submodule URL that starts with ``./`` or ``../`` against the + superproject's remote URL, the way git's ``relative_url`` does. + + Git treats a relative submodule URL like a relative directory: each leading + ``../`` removes the last component of `remote_url`, whether that is a path + component, the host of a URL, or the host of an scp-like ``host:path`` address. + The remainder is appended with ``/``, or with ``:`` once an scp-like host was + reached. A URL that is not relative is returned unchanged. + + :raise ValueError: + If more components are stripped than `remote_url` has, which git rejects with + "cannot strip one component off url". + """ + if not is_relative_url(url): + return url + + remote = remote_url + if remote and _is_dir_sep(remote[-1]): + remote = remote[:-1] + + is_relative = _url_is_local_not_ssh(remote) and not _is_absolute_path(remote) + if is_relative and not _starts_with_dot_slash(remote) and not _starts_with_dot_dot_slash(remote): + remote = "./" + remote + + colonsep = False + while url: + if _starts_with_dot_dot_slash(url): + url = url[3:] + separator = _find_last_dir_sep(remote) + if separator != -1: + remote = remote[:separator] + elif ":" in remote: + remote = remote[: remote.rfind(":")] + colonsep = True + elif is_relative or remote == ".": + raise ValueError("cannot strip one component off url %r" % remote) + else: + remote = "." + elif _starts_with_dot_slash(url): + url = url[2:] + else: + break + + resolved = remote + (":" if colonsep else "/") + url + if url.endswith("/"): + resolved = resolved[:-1] + if _starts_with_dot_slash(resolved): + resolved = resolved[2:] + return resolved + + # } END utilities # { Classes diff --git a/git/objects/tag.py b/git/objects/tag.py index e08d6ef10..2dbf79f1b 100644 --- a/git/objects/tag.py +++ b/git/objects/tag.py @@ -106,7 +106,16 @@ def _set_cache_(self, attr: str) -> None: """Cache all our attributes at once.""" if attr in TagObject.__slots__: ostream = self.repo.odb.stream(self.binsha) - lines: List[str] = ostream.read().decode(defenc, "replace").splitlines() + # Git ends a header only at a line feed, but str.splitlines() also + # breaks at "\r", "\v", "\f", U+2028 and similar. Splitting a tag + # object that way lets an untrusted "tag" header containing such a + # character shift every later header by a line. Split at "\n" only, + # and drop the trailing empty element so tags that use line feeds + # parse exactly as before. + data = ostream.read().decode(defenc, "replace") + lines: List[str] = data.split("\n") + if lines and lines[-1] == "": + lines.pop() _obj, hexsha = lines[0].split(" ") _type_token, type_name = lines[1].split(" ") diff --git a/git/objects/tree.py b/git/objects/tree.py index d2a5859df..96e263530 100644 --- a/git/objects/tree.py +++ b/git/objects/tree.py @@ -110,7 +110,7 @@ def add(self, sha: bytes, mode: int, name: str, force: bool = False) -> "TreeMod :return: self """ - _validate_tree_entry_name(name) + _validate_tree_entry_name(name, mode) if (mode >> 12) not in Tree._map_id_to_type: raise ValueError("Invalid object type according to mode %o" % mode) diff --git a/git/util.py b/git/util.py index 75677b7d3..874b764d5 100644 --- a/git/util.py +++ b/git/util.py @@ -29,63 +29,62 @@ if sys.platform == "win32": __all__.append("to_native_path_windows") -from abc import abstractmethod import contextlib -from functools import wraps import getpass import logging import ntpath import os import os.path as osp -from pathlib import Path import platform import re import shutil import stat import subprocess import time -from urllib.parse import urlsplit, urlunsplit import warnings - -# NOTE: Unused imports can be improved now that CI testing has fully resumed. Some of -# these be used indirectly through other GitPython modules, which avoids having to write -# gitdb all the time in their imports. They are not in __all__, at least currently, -# because they could be removed or changed at any time, and so should not be considered -# conceptually public to code outside GitPython. Linters of course do not like it. -from gitdb.util import ( - LazyMixin, # noqa: F401 - LockedFD, # noqa: F401 - bin_to_hex, # noqa: F401 - file_contents_ro, # noqa: F401 - file_contents_ro_filepath, # noqa: F401 - hex_to_bin, # noqa: F401 - make_sha, - to_bin_sha, # noqa: F401 - to_hex_sha, # noqa: F401 -) +from abc import abstractmethod +from functools import wraps +from pathlib import Path # typing --------------------------------------------------------- - from typing import ( + IO, + TYPE_CHECKING, Any, AnyStr, Callable, Dict, Generator, - IO, Iterator, List, Optional, Pattern, Sequence, Tuple, - TYPE_CHECKING, Type, TypeVar, Union, cast, overload, ) +from urllib.parse import urlsplit, urlunsplit + +# NOTE: Unused imports can be improved now that CI testing has fully resumed. Some of +# these be used indirectly through other GitPython modules, which avoids having to write +# gitdb all the time in their imports. They are not in __all__, at least currently, +# because they could be removed or changed at any time, and so should not be considered +# conceptually public to code outside GitPython. Linters of course do not like it. +from gitdb.util import ( + LazyMixin, # noqa: F401 + LockedFD, # noqa: F401 + bin_to_hex, # noqa: F401 + file_contents_ro, # noqa: F401 + file_contents_ro_filepath, # noqa: F401 + hex_to_bin, # noqa: F401 + make_sha, + to_bin_sha, # noqa: F401 + to_hex_sha, # noqa: F401 +) if TYPE_CHECKING: from git.cmd import Git @@ -94,9 +93,9 @@ from git.repo.base import Repo from git.types import ( + HSH_TD, Files_TD, Has_id_attribute, - HSH_TD, Literal, PathLike, Protocol, @@ -385,12 +384,27 @@ def _to_relative_path(root: PathLike, path: PathLike) -> str: "", "", "\u200c\u200d\u200e\u200f\u202a\u202b\u202c\u202d\u202e\u206a\u206b\u206c\u206d\u206e\u206f\ufeff" ) +# Match Git's is_ntfs_dotgitmodules in path.c on a lowercased, trimmed name. +# Besides gitmod~1..4, fallback aliases have exactly eight ASCII characters: +# a shrinking prefix of "gi7eba", "~", and digits with no leading zero. +# Explicit digit counts avoid accepting shorter/longer names or Unicode digits. +_NTFS_DOTGITMODULES_SHORT_NAME = re.compile( + r"(?:gitmod~[1-4]|gi7eba~[1-9]|gi7eb~[1-9][0-9]|gi7e~[1-9][0-9]{2}|" + r"gi7~[1-9][0-9]{3}|gi~[1-9][0-9]{4}|g~[1-9][0-9]{5}|~[1-9][0-9]{6})" +) + -def _validate_repo_path(path: PathLike) -> None: +def _validate_repo_path(path: PathLike, mode: Union[int, None] = None) -> None: """Reject unsafe tree/index paths without normalizing away their components. Protect Git metadata aliases on NTFS and HFS even when writing on another platform. Other POSIX filename characters, including newlines, remain valid. + + :param mode: + Mode of the index or tree entry the path belongs to, where one is known. + Git refuses a symbolic link that aliases ``.gitmodules``, since the + submodule configuration would then be read through the link, so that + name is only rejected once the mode says the entry is a link. """ name = os.fspath(path) if not name or "\0" in name or ntpath.splitdrive(name)[0] or name.startswith("/"): @@ -406,6 +420,13 @@ def _validate_repo_path(path: PathLike) -> None: hfs_name = part.translate(_HFS_IGNORABLES).lower() if ntfs_name in (".git", "git~1") or hfs_name == ".git": raise ValueError("Repository path aliases Git metadata: %r" % name) + if mode is not None and stat.S_ISLNK(mode): + if ( + ntfs_name == ".gitmodules" + or hfs_name == ".gitmodules" + or _NTFS_DOTGITMODULES_SHORT_NAME.fullmatch(ntfs_name) is not None + ): + raise ValueError("Symbolic link aliases the submodule configuration: %r" % name) def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool: @@ -1086,7 +1107,7 @@ def _list_from_string(cls, repo: "Repo", text: str) -> "Stats": "lines": insertions + deletions, "change_type": change_type, } - hsh["files"][filename.strip()] = files_dict + hsh["files"][filename] = files_dict return Stats(hsh["total"], hsh["files"]) @@ -1249,8 +1270,8 @@ class BlockingLockFile(LockFile): specified timeout. :note: - If the directory containing the lock was removed, an exception will be raised - during the blocking period, preventing hangs as the lock can never be obtained. + Only an existing lock causes retries. Other errors, including a removed or + unwritable parent directory, are raised immediately. """ __slots__ = ("_check_interval", "_max_block_time") @@ -1276,7 +1297,7 @@ def __init__( def _obtain_lock(self) -> None: """This method blocks until it obtained the lock, or raises :exc:`IOError` if it - ran out of time or if the parent directory was not available anymore. + ran out of time or lock creation failed for a reason other than an existing lock. If this method returns, you are guaranteed to own the lock. """ @@ -1286,10 +1307,14 @@ def _obtain_lock(self) -> None: try: super()._obtain_lock() except OSError as e: - # synity check: if the directory leading to the lockfile is not - # readable anymore, raise an exception + # LockFile retains the creation error as the cause of its OSError. + # Only an existing lock is contention; other errors must not be retried. + if not isinstance(e.__cause__, FileExistsError): + raise + + # Abort if the directory containing the existing lock has disappeared. curtime = time.time() - if not osp.isdir(osp.dirname(self._lock_file_path())): + if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir): msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % ( self._lock_file_path(), curtime - starttime, @@ -1361,7 +1386,7 @@ def __contains__(self, attr: object) -> bool: # Otherwise make a full name search. try: - getattr(self, cast(str, attr)) # Use cast to silence mypy. + self.__getattr__(cast(str, attr)) # Use cast to silence mypy. return True except (AttributeError, TypeError): return False @@ -1373,7 +1398,7 @@ def __getattr__(self, attr: str) -> T_IterableObj: if getattr(item, self._id_attr) == attr: return item # END for each item - return list.__getattribute__(self, attr) + raise AttributeError(f"No item found with id {attr}") def __getitem__( # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride] self, index: Union[SupportsIndex, int, slice, str] @@ -1384,7 +1409,7 @@ def __getitem__( # type: ignore[override] # pyright: ignore[reportIncompatible raise ValueError("Index should be an int or str") else: try: - return getattr(self, cast(str, index)) + return self.__getattr__(cast(str, index)) except AttributeError as e: raise IndexError(f"No item found with id {self._prefix}{index}") from e # END handle getattr diff --git a/test/test_commit.py b/test/test_commit.py index b5baf0e33..a22daa87a 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -17,6 +17,7 @@ from git import Actor, Commit, Repo from git.exc import UnsafeOptionError +from git.index.typ import BaseIndexEntry from git.objects.util import tzoffset, utc from git.repo.fun import touch @@ -166,6 +167,39 @@ def check_entries(d, has_change_type=False): self.assertEqual(commit.committer_tz_offset, 14400, commit.committer_tz_offset) self.assertEqual(commit.message, "initial project\n") + @with_rw_directory + def test_stats_preserve_filename_spaces(self, rw_dir): + repo = Repo.init(rw_dir) + actor = Actor("Test", "test@example.com") + paths = ("name", " name", "name ", " name ", " ") + # Store blobs directly so trailing spaces work on Windows as well. + for multiplier, change_type in ((1, "A"), (2, "M")): + for count, path in enumerate(paths, 1): + data = b"line\n" * count * multiplier + blob = repo.odb.store(IStream("blob", len(data), BytesIO(data))) + repo.index.add([BaseIndexEntry((0o100644, blob.binsha, 0, path))]) + commit = repo.index.commit("Update files", author=actor, committer=actor) + stats = commit.stats + assert stats.total == {"insertions": 15, "deletions": 0, "lines": 15, "files": 5} + assert stats.files == { + path: {"insertions": count, "deletions": 0, "lines": count, "change_type": change_type} + for count, path in enumerate(paths, 1) + } + empty_stats = repo.index.commit("No changes", author=actor, committer=actor).stats + assert empty_stats.files == {} + assert empty_stats.total == {"insertions": 0, "deletions": 0, "lines": 0, "files": 0} + + # Avoid filesystem path normalization of space-only names on Windows. + index = repo.index + index.entries.clear() + index.write() + removed_stats = repo.index.commit("Remove files", author=actor, committer=actor).stats + assert removed_stats.files == { + path: {"insertions": 0, "deletions": 2 * count, "lines": 2 * count, "change_type": "D"} + for count, path in enumerate(paths, 1) + } + assert removed_stats.total == {"insertions": 0, "deletions": 30, "lines": 30, "files": 5} + def test_renames(self): commit = self.rorepo.commit("185d847ec7647fd2642a82d9205fb3d07ea71715") files = commit.stats.files @@ -424,6 +458,104 @@ def test_invalid_commit(self): self.assertEqual(cmt.author.name, "E.Azer Ko�o�o�oculu", cmt.author.name) self.assertEqual(cmt.author.email, "azer@kodfabrik.com", cmt.author.email) + def test_deserialization_declared_encoding(self): + # The "encoding" header names the encoding of the identity and message bytes. + data = ( + b"tree 3385f7b1e5f91930a3bd3e07105e113fca29981d\n" + b"author Andr\xe9 1112591163 +0200\n" + b"committer Andr\xe9 1112591163 +0200\n" + b"encoding ISO-8859-1\n" + b"\n" + b"R\xe9sum\xe9\n" + ) + cmt = self.rorepo.commit() + cmt._deserialize(BytesIO(data)) + + self.assertEqual(cmt.encoding, "ISO-8859-1") + self.assertEqual(cmt.author.name, "André") + self.assertEqual(cmt.committer.name, "André") + self.assertEqual(cmt.message, "Résumé\n") + + def test_deserialization_unknown_declared_encoding(self): + # An encoding Python has no text codec for falls back to the default encoding. + data = ( + b"tree 3385f7b1e5f91930a3bd3e07105e113fca29981d\n" + b"author Andr\xe9 1112591163 +0200\n" + b"committer Andr\xe9 1112591163 +0200\n" + b"encoding pony-speak\n" + b"\n" + b"R\xe9sum\xe9\n" + ) + cmt = self.rorepo.commit() + cmt._deserialize(BytesIO(data)) + + self.assertEqual(cmt.encoding, cmt.default_encoding) + self.assertEqual(cmt.author.name, "Andr�") + self.assertEqual(cmt.message, "R�sum�\n") + + def test_deserialization_multiple_encoding_headers(self): + for headers in ( + b"encoding utf-16-be\nencoding ISO-8859-1\n", + b"encoding ISO-8859-1\nencoding utf-\xff8\n", + ): + with self.subTest(headers=headers): + data = ( + b"tree 3385f7b1e5f91930a3bd3e07105e113fca29981d\n" + b"author Andr\xe9 1112591163 +0200\n" + b"committer Andr\xe9 1112591163 +0200\n" + headers + b"\n" + b"R\xe9sum\xe9\n" + ) + cmt = self.rorepo.commit() + cmt._deserialize(BytesIO(data)) + + self.assertEqual(cmt.encoding, "ISO-8859-1") + self.assertEqual(cmt.author.name, "André") + self.assertEqual(cmt.committer.name, "André") + self.assertEqual(cmt.message, "Résumé\n") + + @with_rw_directory + def test_identity_cannot_alter_headers(self, rw_dir): + """A name or email must not add header lines or present another identity.""" + rw_repo = Repo.init(osp.join(rw_dir, "test_identity_headers")) + path = osp.join(str(rw_repo.working_tree_dir), "hello.txt") + touch(path) + rw_repo.index.add([path]) + tree = rw_repo.index.write_tree() + service = Actor("Service", "service@example.com") + forged = "committer Forged 0 +0000" + + for name, email in ( + # A line feed ends the header line, so the remainder would become headers + # of its own, which Git reads before the committer written after them. + ("User 0 +0000\n" + forged, "user@example.com"), + ("User", "user@example.com> 0 +0000\n" + forged), + # Angle brackets delimit the email, so these would present another one. + ("Forged ", "user@example.com"), + ("User", "forged@example.com> ", "user@example.com"), + ("User", "10L20sH", 0, 0, 0, 0, 0, 0, 0o100644, 0, 0, 0, b"a" * 20, min(len(name), 0xFFF)) + name + entry = struct.pack(">10L20sH", 0, 0, 0, 0, 0, 0, mode, 0, 0, 0, b"a" * 20, min(len(name), 0xFFF)) + name entry += b"\0" * (8 - len(entry) % 8) data = b"DIRC" + struct.pack(">LL", 2, 1) + entry return data + sha1(data).digest() @@ -340,6 +340,68 @@ def test_index_reader_and_writer_reject_unsafe_paths(self, path): with pytest.raises(ValueError): write_cache([entry], BytesIO()) + @ddt.data( + ".gitmodules", + ".GITMODULES", + ".gitmodules.", + ".gitmodules ", + ".gi\u200ctmodules", + "gitmod~1", + "gitmod~4", + "gi7eba~1", + "gi7eba~9", + "GI7EB~10", + "GI7EB~99", + "GI7E~100", + "GI7E~999", + "GI7~1000", + "GI7~9999", + "GI~10000", + "GI~99999", + "G~100000", + "G~999999", + "~1000000", + "~9999999", + "GI7EB~10. ", + "GI7E~100:$DATA", + "sub/~1000000", + "sub/.gitmodules", + ) + def test_index_reader_and_writer_reject_gitmodules_symlinks(self, path): + """An entry that turns .gitmodules into a symbolic link is rejected, while the + same name stays valid for a regular file. The spellings are those + `git update-index --add --cacheinfo 120000,,` refuses on git 2.52.0.""" + with pytest.raises(ValueError): + read_cache(BytesIO(_raw_index(path, mode=0o120000))) + with pytest.raises(ValueError): + write_cache([IndexEntry((0o120000, b"a" * 20, 0, path))], BytesIO()) + + assert next(iter(read_cache(BytesIO(_raw_index(path)))[1])) == (path, 0) + stream = BytesIO() + write_cache([IndexEntry((0o100644, b"a" * 20, 0, path))], stream) + stream.seek(0) + assert next(iter(read_cache(stream)[1])) == (path, 0) + + @ddt.data("GI7EB~10", "GI7E~100", "GI7~1000", "GI~10000", "G~100000", "~1000000", "~9999999") + @with_rw_directory + def test_index_add_and_write_tree_reject_gitmodules_fallback_symlinks(self, rw_dir, path): + with Repo.init(rw_dir) as repo: + binsha = repo.odb.store(IStream("blob", 6, BytesIO(b"target"))).binsha + index = repo.index + for item in (Blob(repo, binsha, 0o120000, path), BaseIndexEntry((0o120000, binsha, 0, path))): + with pytest.raises(ValueError, match="submodule configuration"): + index.add([item], write=False) + assert not index.entries + + index.entries[(path, 0)] = IndexEntry((0o120000, binsha, 0, path)) + with pytest.raises(ValueError, match="submodule configuration"): + index.write_tree() + assert not Path(index.path).exists() + + index.add([BaseIndexEntry((0o100644, binsha, 0, path))]) + assert repo.index.entries[(path, 0)].mode == 0o100644 + assert index.write_tree()[path].mode == 0o100644 + def test_valid_unusual_index_names_round_trip(self): names = ["a b", "a\nb", "a\tb", "name:value", "dir/.gitignore", "café"] if os.name != "nt": @@ -381,20 +443,26 @@ def _cmp_tree_index(self, tree, index): @with_rw_repo("0.1.6") def test_index_lock_handling(self, rw_repo): - def add_bad_blob(): - rw_repo.index.add([Blob(rw_repo, b"f" * 20, "bad-permissions", "foo")]) - - try: - ## First, fail on purpose adding into index. - add_bad_blob() - except Exception as ex: - assert "required argument is not an integer" in str(ex) - - ## The second time should not fail due to stray lock file. - try: - add_bad_blob() - except Exception as ex: - assert "index.lock' could not be obtained" not in str(ex) + index = rw_repo.index + index_path = Path(index.path) + lock_path = Path(str(index_path) + ".lock") + before = index_path.read_bytes() + + def fail_serialize(stream, ignore_extension_data): + assert lock_path.exists() + stream.write(b"partial index") + raise OSError("simulated index write failure") + + with mock.patch.object(IndexFile, "_serialize", side_effect=fail_serialize): + for _ in range(2): + with pytest.raises(OSError, match="simulated index write failure"): + index.write() + assert not lock_path.exists() + assert index_path.read_bytes() == before + + index.add([Blob(rw_repo, b"f" * 20, 0o100644, "foo")]) + assert not lock_path.exists() + assert rw_repo.index.entries[("foo", 0)].mode == 0o100644 @with_rw_repo("0.1.6") def test_read_tree_methods_reject_index_output(self, rw_repo): @@ -1221,6 +1289,60 @@ def test_staging_rejects_unsafe_object_paths_even_without_writing(self, rw_dir, index.add([item], write=False, **kwargs) assert not index.entries + @ddt.data(*product(("path", "blob", "entry", "stored-blob", "stored-entry"), (False, True), (False, True))) + @ddt.unpack + @with_rw_directory + def test_staging_gitmodules_symlink_check_uses_rewritten_path(self, rw_dir, kind, unsafe_destination, write): + with Repo.init(rw_dir) as repo: + source, destination = ("safe-link", ".gitmodules") if unsafe_destination else (".gitmodules", "safe-link") + binsha = Blob.NULL_BIN_SHA + if kind.startswith("stored-"): + binsha = repo.odb.store(IStream("blob", 6, BytesIO(b"target"))).binsha + else: + try: + (Path(rw_dir) / source).symlink_to("target") + except OSError: + pytest.skip("Symlinks unavailable") + if kind == "path": + item = source + elif kind.endswith("blob"): + item = Blob(repo, binsha, 0o120000, source) + else: + item = BaseIndexEntry((0o120000, binsha, 0, source)) + index = repo.index + rewriter = mock.Mock(return_value=destination) + if unsafe_destination: + with pytest.raises(ValueError, match="submodule configuration"): + index.add([item], path_rewriter=rewriter, write=write) + assert not index.entries + assert not Path(index.path).exists() + else: + added = index.add([item], path_rewriter=rewriter, write=write) + assert [(entry.path, entry.mode) for entry in added] == [(destination, 0o120000)] + assert set(index.entries) == {(destination, 0)} + assert index.write_tree()[destination].mode == 0o120000 + if write: + assert repo.index.entries[(destination, 0)].mode == 0o120000 + rewriter.assert_called_once() + assert rewriter.call_args[0][0].path == source + + @ddt.data(*product(("blob", "entry"), ("../outside", ".git/config"))) + @ddt.unpack + @with_rw_directory + def test_staging_rewriter_cannot_sanitize_unsafe_source_paths(self, rw_dir, kind, path): + with Repo.init(rw_dir) as repo: + item = ( + Blob(repo, b"a" * 20, 0o120000, path) + if kind == "blob" + else BaseIndexEntry((0o120000, b"a" * 20, 0, path)) + ) + index = repo.index + rewriter = mock.Mock(return_value="safe-link") + with pytest.raises(ValueError): + index.add([item], path_rewriter=rewriter, write=False) + rewriter.assert_not_called() + assert not index.entries + @with_rw_directory def test_staging_root_preserves_symlinks_and_skips_git_metadata(self, rw_dir): tmp_path = Path(rw_dir) @@ -1391,6 +1513,7 @@ def test_run_commit_hook_outside_worktree_on_windows(self, rw_dir): repo = Repo.init(root / "repo") hooks_dir = root / "hooks" _make_hook(root, "fake-hook", "exit 0") + _make_hook(repo.working_dir, "fake-hook", "exit 0") system_root = root / "Windows" system_bash = system_root / "System32" / "bash.exe" git_executable = root / "Git" / "cmd" / "git.exe" @@ -1399,24 +1522,37 @@ def test_run_commit_hook_outside_worktree_on_windows(self, rw_dir): executable.parent.mkdir(parents=True) executable.touch() executable.chmod(0o755) - with repo.config_writer() as writer: - writer.set_value("core", "hooksPath", str(hooks_dir)) + + def command(hooks_path, exec_path): + with repo.config_writer() as writer: + writer.set_value("core", "hooksPath", hooks_path) + with mock.patch("git.index.fun.sys.platform", "win32"), mock.patch.object( + Git, "GIT_PYTHON_GIT_EXECUTABLE", "git" + ), mock.patch.dict(os.environ, {"SystemRoot": str(system_root)}), mock.patch( + "git.index.fun.os.get_exec_path", return_value=exec_path + ): + 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) + return popen.call_args[0][0] # Model a normal Windows PATH: System32 (containing the WSL launcher) comes # before Git's cmd directory, while Git's Bash is not itself on PATH. This # exercises both Git-installation discovery and shell selection without # mocking either resolver's answer. - with mock.patch("git.index.fun.sys.platform", "win32"), mock.patch.object( - Git, "GIT_PYTHON_GIT_EXECUTABLE", "git" - ), mock.patch.dict(os.environ, {"SystemRoot": str(system_root)}), mock.patch( - "git.index.fun.os.get_exec_path", return_value=["", str(system_bash.parent), str(git_executable.parent)] - ): - 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, [str(git_bash), "../hooks/fake-hook"]) + with_git = ["", str(system_bash.parent), str(git_executable.parent)] + # A hook outside the working tree is passed by its absolute path: a relative + # one is resolved in Bash's own view of the filesystem, where a mount point + # (Git Bash's /tmp) can make ".." lead elsewhere. + self.assertEqual(command(str(hooks_dir), with_git), [str(git_bash), (hooks_dir / "fake-hook").as_posix()]) + # A hook inside the working tree keeps its relative path. + self.assertEqual(command("hooks", with_git), [str(git_bash), "hooks/fake-hook"]) + # So does one outside it for the bare fallback, which may be WSL's bash.exe, + # unable to open a Windows absolute path. + without_git = ["", str(system_bash.parent)] + self.assertEqual(command(str(hooks_dir), without_git), ["bash.exe", "../hooks/fake-hook"]) @with_rw_directory def test_windows_bash_lookup_respects_explicit_current_directory_in_path(self, rw_dir): @@ -1549,6 +1685,28 @@ def test_pre_commit_hook_respects_core_hooks_path(self, rw_repo): 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.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_directory + def test_pre_commit_hook_outside_worktree(self, rw_dir): + repo = Repo.init(osp.join(rw_dir, "repo")) + hp = _make_hook(rw_dir, "pre-commit", "echo 'ran outside hook' >outside-hook-output.txt") + + with repo.config_writer() as writer: + writer.set_value("core", "hooksPath", osp.dirname(hp)) + + repo.index.commit("This should run the hook outside the working tree") + output = Path(repo.working_dir, "outside-hook-output.txt").read_text(encoding="utf-8") + self.assertEqual(output, "ran outside 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", @@ -1756,6 +1914,27 @@ def test_checkout_pathlike(self, tmp_path, path_type, absolute, directory, conta for name, data in files.items(): assert (tmp_path / name).read_bytes() == data + @pytest.mark.skipif(os.name == "nt", reason="Line feeds and quotes are not valid Windows filenames") + def test_checkout_sends_each_path_as_one_record(self, tmp_path): + with Repo.init(tmp_path) as repo: + nested = tmp_path / "nested" + nested.mkdir() + (nested / "first\noutside").write_bytes(b"nested") + (tmp_path / "outside").write_bytes(b"committed") + (tmp_path / '"quoted"').write_bytes(b"quoted") + repo.index.add(["nested", "outside", '"quoted"']) + + (nested / "first\noutside").unlink() + (tmp_path / '"quoted"').unlink() + (tmp_path / "outside").write_bytes(b"local") + + checked_out = {"nested/first\noutside", '"quoted"'} + assert set(repo.index.checkout(["nested", '"quoted"'], force=True)) == checked_out + assert (nested / "first\noutside").read_bytes() == b"nested" + assert (tmp_path / '"quoted"').read_bytes() == b"quoted" + # Neither "nested/first" nor "outside" was requested. + assert (tmp_path / "outside").read_bytes() == b"local" + class TestIndexUtils: @pytest.mark.parametrize("file_path_type", [str, Path]) diff --git a/test/test_refs.py b/test/test_refs.py index 2edb7b766..42bf5200e 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -7,10 +7,12 @@ import os.path as osp import tempfile import unittest +from io import BytesIO from itertools import chain from pathlib import Path from unittest.mock import patch +from gitdb import IStream from gitdb.exc import BadName import git.refs as refs @@ -139,6 +141,31 @@ def test_tags_author(self): tagger_name = tagobj.tagger.name assert tagger_name == "Michael Trier" + def test_tag_headers_split_at_line_feeds_only(self): + # A "tag" header value carrying a U+2028 (or any separator that + # str.splitlines() honors but git does not) must not shift the later + # headers. Git ends a header only at "\n", so GitPython must read the + # whole value as the tag name and the following line as the tagger, + # exactly as "git cat-file tag" does. See the TagObject parsing fix. + with tempfile.TemporaryDirectory() as tmp_dir: + with self._repo_with_initial_commit(Path(tmp_dir)) as repo: + commit_hexsha = repo.head.commit.hexsha + payload = ( + f"object {commit_hexsha}\n" + "type commit\n" + "tag v1
tagger Linus Torvalds 1700000000 +0000\n" + "tagger Mallory 1700000001 +0000\n" + "\n" + "release\n" + ).encode("utf-8") + istream = repo.odb.store(IStream("tag", len(payload), BytesIO(payload))) + tagobj = TagObject(repo, istream.binsha) + + assert tagobj.tag == "v1
tagger Linus Torvalds 1700000000 +0000" + assert tagobj.tagger.name == "Mallory" + assert tagobj.tagger.email == "m@evil.example" + assert tagobj.message == "release" + def test_tags(self): # Tag refs can point to tag objects or to commits. s = set() diff --git a/test/test_submodule.py b/test/test_submodule.py index 45b285c55..c09f0a8e1 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -26,6 +26,7 @@ ) from git.objects.submodule.base import Submodule from git.objects.submodule.root import RootModule, RootUpdateProgress +from git.objects.submodule.util import resolve_relative_url from git.repo.fun import find_submodule_git_dir, touch from git.util import HIDE_WINDOWS_KNOWN_ERRORS, cwd, join_path_native, to_native_path_linux @@ -52,6 +53,138 @@ def _patch_git_config(name, value): yield +@pytest.mark.parametrize("state", ["unborn", "untracked", "detached"]) +@pytest.mark.parametrize("remote", [None, ".", "origin"], ids=["working-tree", "local-origin", "origin"]) +def test_add_relative_submodule_without_tracking_remote(tmp_path, state, remote): + with git.Repo.init(tmp_path / "parent") as parent: + if state != "unborn": + parent.index.commit("Initial commit") + if state == "detached": + parent.head.reference = parent.head.commit + base = tmp_path / "remote" if remote == "origin" else tmp_path / "parent" + if remote is not None: + parent.create_remote("origin", str(base) if remote == "origin" else ".") + with git.Repo.init(base / "source") as source: + source.git.symbolic_ref("HEAD", "refs/heads/master") + source.index.commit("Initial commit") + submodule = parent.create_submodule("module", "module", "./source") + assert submodule.url == "./source" + with submodule.module() as module: + assert module.head.commit == source.head.commit + assert Path(module.remotes.origin.url).samefile(source.working_tree_dir) + parent.index.commit("Add submodule") + submodule.remove(configuration=False) + submodule.update(init=True) + with submodule.module() as module: + assert module.head.commit == source.head.commit + assert Path(module.remotes.origin.url).samefile(source.working_tree_dir) + + +@pytest.mark.parametrize("url", ["./source", "../source", "./../source"]) +def test_add_preserves_relative_url_on_cygwin(tmp_path, url): + with git.Repo.init(tmp_path / "parent") as parent, git.Repo.init(tmp_path / "source") as source: + source.index.commit("Initial commit") + with mock.patch.object(Submodule, "_clone_repo", return_value=source): + with mock.patch.object(Git, "is_cygwin", return_value=True): + submodule = parent.create_submodule("module", "module", url) + assert submodule.url == url + with submodule.config_reader() as reader: + assert reader.get_value("url") == url + with parent.config_reader() as reader: + assert reader.get_value('submodule "module"', "url") == url + + +def test_update_resolves_local_relative_remote_once(tmp_path): + with git.Repo.init(tmp_path / "parent") as parent, git.Repo.init(tmp_path / "repos" / "child.git") as source: + source.git.symbolic_ref("HEAD", "refs/heads/master") + source.index.commit("Initial commit") + parent.create_remote("origin", "../repos/super.git") + parent.head.reference.set_tracking_branch(git.RemoteReference(parent, "refs/remotes/origin/master")) + submodule = Submodule( + parent, + source.head.commit.binsha, + name="module", + path="module", + url="../child.git", + branch_path="refs/heads/master", + ) + submodule.update(init=True) + with submodule.module() as module: + assert module.head.commit == source.head.commit + assert Path(module.remotes.origin.url).samefile(source.working_tree_dir) + module.remotes.origin.fetch() + + +@pytest.mark.parametrize("no_fetch", [False, True]) +def test_root_update_anchors_relative_remote_url(tmp_path, no_fetch): + with git.Repo.init(tmp_path / "repos" / "source.git") as source, git.Repo.init(tmp_path / "parent") as parent: + source.git.symbolic_ref("HEAD", "refs/heads/master") + source.index.commit("Initial commit") + with git.Repo.clone_from(source.working_tree_dir, tmp_path / "repos" / "replacement.git") as replacement: + parent.create_remote("origin", "../repos/super.git") + submodule = parent.create_submodule("module", "module", source.working_tree_dir) + with submodule.config_writer() as writer: + writer.set_value("url", "../source.git") + previous = parent.index.commit("Record relative URL") + with submodule.config_writer() as writer: + writer.set_value("url", "../replacement.git") + parent.index.commit("Change relative URL") + + RootModule(parent).update(previous_commit=previous, no_fetch=no_fetch) + + with submodule.module() as module: + assert Path(module.remotes.origin.url).samefile(replacement.working_tree_dir) + assert module.head.commit == replacement.head.commit + module.remotes.origin.fetch() + + +@pytest.mark.parametrize("no_fetch", [False, True]) +def test_root_update_uses_single_remote_without_resolving_old_url(tmp_path, no_fetch): + with git.Repo.init(tmp_path / "source") as source, git.Repo.init(tmp_path / "parent") as parent: + source.git.symbolic_ref("HEAD", "refs/heads/master") + source.index.commit("Initial commit") + with git.Repo.clone_from(source.working_tree_dir, tmp_path / "replacement") as replacement: + parent.create_remote("origin", "../super.git") + submodule = parent.create_submodule("module", "module", source.working_tree_dir) + with submodule.config_writer() as writer: + writer.set_value("url", "../../lib.git") + previous = parent.index.commit("Record historical URL") + with submodule.config_writer() as writer: + writer.set_value("url", replacement.working_tree_dir) + parent.index.commit("Replace historical URL") + + RootModule(parent).update(previous_commit=previous, no_fetch=no_fetch) + + with submodule.module() as module: + assert Path(module.remotes.origin.url).samefile(replacement.working_tree_dir) + assert module.head.commit == replacement.head.commit + + +@pytest.mark.parametrize("dry_run", [False, True]) +def test_root_update_skips_old_url_resolution_when_remote_matches(tmp_path, dry_run): + with git.Repo.init(tmp_path / "source") as source, git.Repo.init(tmp_path / "parent") as parent: + source.index.commit("Initial commit") + submodule = parent.create_submodule("module", "module", source.working_tree_dir) + with submodule.config_writer() as writer: + writer.set_value("url", "../source") + previous = parent.index.commit("Record relative submodule URL") + with submodule.config_writer() as writer: + writer.set_value("url", Git.polish_url(source.working_tree_dir)) + parent.index.commit("Use existing remote URL") + + resolve_url = Submodule._resolved_url + + def resolve_current_url(sm): + assert sm.url != "../source", "Previous URL does not need resolving" + return resolve_url(sm) + + with mock.patch.object(Submodule, "_resolved_url", autospec=True, side_effect=resolve_current_url): + RootModule(parent).update(previous_commit=previous, no_fetch=True, dry_run=dry_run) + with submodule.module() as module: + assert module.remotes.origin.url == Git.polish_url(source.working_tree_dir) + assert module.head.commit == source.head.commit + + @pytest.mark.parametrize( "name", ["module", "$GITPYTHON_TEST_SECRET", "prefix-${GITPYTHON_TEST_SECRET}-suffix", "%GITPYTHON_TEST_SECRET%"] ) @@ -2416,3 +2549,72 @@ def test_submodule_update_relative_url(self, rwdir): has_module = any(sm.name == "module" for sm in cloned_repo.submodules) assert has_module, "Relative submodule was not updated properly" + + def test_resolve_relative_url_like_git(self): + # Expected values are what `git submodule init` records for the same pair. + cases = [ + ("ssh://host/org/super.git", "./sub", "ssh://host/org/super.git/sub"), + ("ssh://host/org/super.git", "../sub", "ssh://host/org/sub"), + ("ssh://host:22/org/super.git", "../sub", "ssh://host:22/org/sub"), + ("git@host:org/super.git", "./sub", "git@host:org/super.git/sub"), + ("git@host:org/super.git", "../sub", "git@host:org/sub"), + ("git@host:org/super.git", "../../sub", "git@host:sub"), + ("https://host/org/super.git", "./sub/", "https://host/org/super.git/sub"), + ("https://host/org/super.git/", "././sub", "https://host/org/super.git/sub"), + ("https://host/org/super.git", ".././sub", "https://host/org/sub"), + ("https://host/org/super.git", "../../../sub", "https://sub"), + ("file:///srv/git/super.git", "../sub", "file:///srv/git/sub"), + ("/srv/git/super", "../sub", "/srv/git/sub"), + ("/srv/git/super", "../../../../sub", "sub"), + ("//share/super", "../../sub", "//sub"), + ("foo/bar", "../sub", "foo/sub"), + ("./foo", "../sub", "sub"), + ("./super.git", "../", ""), + ("ssh://host/org/super.git", "./", "ssh://host/org/super.git/"), + ("ssh://host/org/super.git", "../", "ssh://host/org/"), + ("../foo/bar", "../sub", "../foo/sub"), + ("ssh://host/org/super.git", ".sub", ".sub"), + ("ssh://host/org/super.git", "https://other/sub", "https://other/sub"), + ] + for remote_url, url, expected in cases: + assert resolve_relative_url(remote_url, url) == expected, (remote_url, url) + + for remote_url, url in [("foo", "../../sub"), ("git@host:super.git", "../../../sub"), ("../foo", "../../sub")]: + with self.assertRaisesRegex(ValueError, "cannot strip one component"): + resolve_relative_url(remote_url, url) + + @with_rw_directory + def test_update_resolves_relative_url_against_remote(self, rwdir): + parent = git.Repo.init(osp.join(rwdir, "parent")) + parent.create_remote("origin", "ssh://git@host/org/super.git") + os.mkdir(osp.join(parent.working_tree_dir, "planted")) + + for url, expected in [ + ("./planted", "ssh://git@host/org/super.git/planted"), + ("../planted", "ssh://git@host/org/planted"), + ]: + submodule = Submodule(parent, Submodule.NULL_BIN_SHA, name="module", path="module", url=url) + with mock.patch.object(git.Repo, "clone_from", side_effect=AssertionError("stop")) as clone_from: + with pytest.raises(AssertionError, match="stop"): + submodule.update(init=True) + assert clone_from.call_args.args[0] == expected + + @with_rw_directory + def test_clone_repo_resolves_relative_url_against_tracking_remote(self, rwdir): + parent = git.Repo.init(osp.join(rwdir, "parent")) + parent.index.commit("initial") + parent.create_remote("origin", "git@host:other/super.git") + for remote_name in ("upstream", "team/upstream"): + parent.create_remote(remote_name, "git@host:org/super.git") + with parent.head.reference.config_writer() as writer: + writer.set_value("remote", remote_name) + writer.set_value("merge", "refs/heads/main") + + for url, expected in [ + ("./planted", "git@host:org/super.git/planted"), + ("../planted", "git@host:org/planted"), + ]: + with mock.patch.object(git.Repo, "clone_from", side_effect=AssertionError("stop")) as clone_from: + with pytest.raises(AssertionError, match="stop"): + Submodule._clone_repo(parent, url, "module", "module") + assert clone_from.call_args.args[0] == expected diff --git a/test/test_tree.py b/test/test_tree.py index 803f1f381..3a85405bd 100644 --- a/test/test_tree.py +++ b/test/test_tree.py @@ -138,6 +138,113 @@ def test_tree_names_are_checked_at_construction_and_serialization(self, name): with pytest.raises(ValueError): tree_to_stream([(b"a" * 20, 0o100644, name)], BytesIO().write) + @ddt.data( + ".gitmodules", + ".GITMODULES", + ".gitmodules ", + ".gi\u200ctmodules", + "gitmod~1", + "gitmod~2", + "gitmod~3", + "GITMOD~4", + "gi7eba~1", + "GI7EBA~9", + "GI7EB~10", + "GI7EB~11", + "GI7EB~99", + "GI7E~100", + "GI7E~101", + "GI7E~999", + "GI7~1000", + "GI7~9999", + "GI~10000", + "GI~99999", + "G~100000", + "G~999999", + "~1000000", + "~9999999", + "Gi7Eb~42", + "gi7e~120", + "GITMOD~4 . ", + "GI7EB~10. ", + "GI7E~100:$DATA", + "~1000000 . :$DATA", + ) + def test_gitmodules_symlink_entries_are_rejected(self, name): + """A symbolic link named like the submodule configuration would make Git read + it from outside the repository, so such an entry is refused in both + directions. A regular file with the same name is the normal case.""" + symlink_mode = 0o120000 + cache = [] + with pytest.raises(ValueError): + TreeModifier(cache).add(b"a" * 20, symlink_mode, name) + assert not cache + with pytest.raises(ValueError): + tree_to_stream([(b"a" * 20, symlink_mode, name)], BytesIO().write) + raw = b"120000 " + name.encode() + b"\0" + b"a" * 20 + with pytest.raises(ValueError): + tree_entries_from_data(raw) + + TreeModifier(cache).add(b"a" * 20, 0o100644, name) + assert cache == [(b"a" * 20, 0o100644, name)] + data = BytesIO() + tree_to_stream(cache, data.write) + assert tree_entries_from_data(data.getvalue()) == cache + + @ddt.data( + "gitmod~0", + "gitmod~5", + "gitmod~10", + "GI7EBA~", + "GI7EBA~0", + "GI7EBA~~1", + "GI7EBA~X", + "GI7EBA~10", + "Gx7EBA~1", + "GI7EBX~1", + "GI7EB~1", + "GI7EB~01", + "GI7EB~1X", + "GI7EB~100", + "GI7E~10", + "GI7E~010", + "GI7E~1000", + "GI7~100", + "GI7~0100", + "GI7~10000", + "GI~1000", + "GI~01000", + "GI~100000", + "G~10000", + "G~010000", + "G~1000000", + "~100000", + "~0100000", + "~10000000", + "GI7EBA~\u0661", + "GI7EB~1\uff10", + "GI7EB~10x", + "GI7EB~10.x", + " GI7EB~10", + "GI7EB~10\n", + "GI7EB~10\t", + "GI7EB~10x:$DATA", + ".gitmodules x", + ".gitmodules .x", + ".gitmodules,:$DATA", + ) + def test_gitmodules_short_name_near_misses_round_trip(self, name): + """Only exact aliases are forbidden, and only for symbolic links.""" + for mode in (0o100644, 0o120000): + cache = [] + TreeModifier(cache).add(b"a" * 20, mode, name) + assert cache == [(b"a" * 20, mode, name)] + data = BytesIO() + tree_to_stream(cache, data.write) + raw = ("%o " % mode).encode() + name.encode() + b"\0" + b"a" * 20 + assert data.getvalue() == raw + assert tree_entries_from_data(raw) == cache + def test_traverse(self): root = self.rorepo.tree("0.1.6") num_recursive = 0 diff --git a/test/test_util.py b/test/test_util.py index ca8e006f3..ca592aaf7 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -4,7 +4,7 @@ # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ import ast -from datetime import datetime +import errno import os import pathlib import pickle @@ -14,6 +14,7 @@ import tempfile import threading import time +from datetime import datetime from unittest import SkipTest, mock import ddt @@ -37,12 +38,11 @@ LockFile, cygpath, decygpath, - is_cygwin_git, get_user_id, + is_cygwin_git, remove_password_if_present, rmtree, ) - from test.lib import TestBase, requires_symlinks, with_rw_repo @@ -862,3 +862,122 @@ def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int(b"100644") == 0o100644 assert mode_str_to_int("644") == 0o644 assert mode_str_to_int(b"120000") == 0o120000 + + +@pytest.mark.parametrize("name", ["sort", "count", "append", "_prefix", "__class__", "ordinary"]) +@pytest.mark.parametrize("prefix", ["", "refs/"]) +def test_iterable_list_name_lookup_ignores_list_attributes(name, prefix): + values = IterableList("name", prefix) + member = _Member(prefix + name) + values.append(member) + + assert values[name] is member + assert name in values + assert member in values + assert values[0] is member + assert callable(values.append) + assert callable(values.sort) + + del values[name] + assert name not in values + with pytest.raises(IndexError, match="No item found with id"): + values[name] + + +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +@pytest.mark.parametrize("relative", [True, False]) +def test_blocking_lock_waits_for_existing_holder(tmp_path, monkeypatch, path_type, relative): + monkeypatch.chdir(tmp_path) + path = path_type("resource" if relative else tmp_path / "resource") + owner = LockFile(path) + waiter = BlockingLockFile(path, check_interval_s=0.01, max_block_time_s=1) + owner._obtain_lock() + try: + with mock.patch.object(time, "sleep", side_effect=lambda _: owner._release_lock()) as sleep: + waiter._obtain_lock() + sleep.assert_called_once_with(0.01) + assert waiter._has_lock() + finally: + owner._release_lock() + waiter._release_lock() + + +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + owner = LockFile(path_type("resource")) + waiter = BlockingLockFile(path_type("resource"), max_block_time_s=0) + owner._obtain_lock() + try: + with pytest.raises(OSError, match="Waited .* seconds for lock"): + waiter._obtain_lock() + finally: + owner._release_lock() + + +def test_blocking_lock_still_rejects_missing_directory(tmp_path): + waiter = BlockingLockFile(tmp_path / "missing" / "resource") + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, FileNotFoundError) + assert not waiter._has_lock() + + +@pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows") +@pytest.mark.parametrize( + "error_number", [errno.EACCES, errno.ENOENT, errno.ENOTDIR, errno.EROFS, errno.ENOSPC, errno.EIO] +) +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +@pytest.mark.parametrize("relative", [True, False]) +def test_blocking_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number, path_type, relative): + monkeypatch.chdir(tmp_path) + path = path_type("resource" if relative else tmp_path / "resource") + waiter = BlockingLockFile(path) + error = OSError(error_number, os.strerror(error_number), waiter._lock_file_path()) + with mock.patch.object(os, "open", side_effect=error) as open_lock: + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + open_lock.assert_called_once() + assert exc_info.value.__cause__ is error + assert str(exc_info.value) == str(error) + assert not waiter._has_lock() + assert not os.path.lexists(waiter._lock_file_path()) + + +@pytest.mark.skipif(os.name != "posix", reason="Directory write permissions require POSIX") +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_rejects_unwritable_cwd(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + original_mode = tmp_path.stat().st_mode + tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR) + try: + if os.access(".", os.W_OK): + pytest.skip("Current user can write to read-only directories") + assert os.path.isdir(".") + waiter = BlockingLockFile(path_type("resource")) + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, PermissionError) + assert not waiter._has_lock() + finally: + tmp_path.chmod(original_mode) + + +@pytest.mark.skipif(os.name != "posix", reason="Removing the current directory requires POSIX") +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_rejects_removed_cwd(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + tmp_path.rmdir() + waiter = BlockingLockFile(path_type("resource")) + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, FileNotFoundError) + assert not waiter._has_lock()