Validate every fspath with tests

Author: George-Ogden

git/index/base.py

@@ -434,7 +434,7 @@ def raise_exc(e: Exception) -> NoReturn:
                 #   characters.
                 if abs_path not in resolved_paths:
                     for f in self._iter_expand_paths(glob.glob(abs_path)):
-                        yield os.fspath(f).replace(rs, "")
+                        yield str(f).replace(rs, "")
                     continue
             # END glob handling
             try:
@@ -569,7 +569,7 @@ def resolve_blobs(self, iter_blobs: Iterator[Blob]) -> "IndexFile":
         for blob in iter_blobs:
             stage_null_key = (blob.path, 0)
             if stage_null_key in self.entries:
-                raise ValueError("Path %r already exists at stage 0" % os.fspath(blob.path))
+                raise ValueError("Path %r already exists at stage 0" % str(blob.path))
             # END assert blob is not stage 0 already
 
             # Delete all possible stages.
@@ -652,15 +652,14 @@ def _to_relative_path(self, path: PathLike) -> PathLike:
 
         :raise ValueError:
         """
-        path = os.fspath(path)
         if not osp.isabs(path):
             return path
         if self.repo.bare:
             raise InvalidGitRepositoryError("require non-bare repository")
         if not osp.normpath(path).startswith(os.fspath(self.repo.working_tree_dir)):
             raise ValueError("Absolute path %r is not in git repository at %r" % (path, self.repo.working_tree_dir))
         result = os.path.relpath(path, self.repo.working_tree_dir)
-        if path.endswith(os.sep) and not result.endswith(os.sep):
+        if os.fspath(path).endswith(os.sep) and not result.endswith(os.sep):
             result += os.sep
         return result
 
@@ -1364,7 +1363,7 @@ def make_exc() -> GitCommandError:
                     if not folder.endswith("/"):
                         folder += "/"
                     for entry in self.entries.values():
-                        if os.fspath(entry.path).startswith(folder):
+                        if entry.path.startswith(folder):
                             p = entry.path
                             self._write_path_to_stdin(proc, p, p, make_exc, fprogress, read_from_stdout=False)
                             checked_out_files.append(p)

git/index/fun.py

@@ -87,7 +87,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None:
         return
 
     env = os.environ.copy()
-    env["GIT_INDEX_FILE"] = safe_decode(index.path)
+    env["GIT_INDEX_FILE"] = safe_decode(os.fspath(index.path))
     env["GIT_EDITOR"] = ":"
     cmd = [hp]
     try:
@@ -167,7 +167,7 @@ def write_cache(
         beginoffset = tell()
         write(entry.ctime_bytes)  # ctime
         write(entry.mtime_bytes)  # mtime
-        path_str = os.fspath(entry.path)
+        path_str = str(entry.path)
         path: bytes = force_bytes(path_str, encoding=defenc)
         plen = len(path) & CE_NAMEMASK  # Path length
         assert plen == len(path), "Path %s too long to fit into index" % entry.path

git/index/typ.py

@@ -58,9 +58,9 @@ def __init__(self, paths: Sequence[PathLike]) -> None:
 
     def __call__(self, stage_blob: Tuple[StageType, Blob]) -> bool:
         blob_pathlike: PathLike = stage_blob[1].path
-        blob_path = Path(blob_pathlike)
+        blob_path: Path = blob_pathlike if isinstance(blob_pathlike, Path) else Path(blob_pathlike)
         for pathlike in self.paths:
-            path = Path(pathlike)
+            path: Path = pathlike if isinstance(pathlike, Path) else Path(pathlike)
             # TODO: Change to use `PosixPath.is_relative_to` once Python 3.8 is no
             # longer supported.
             filter_parts = path.parts

git/index/util.py

@@ -37,8 +37,8 @@ class TemporaryFileSwap:
     __slots__ = ("file_path", "tmp_file_path")
 
     def __init__(self, file_path: PathLike) -> None:
-        self.file_path = os.fspath(file_path)
-        dirname, basename = osp.split(self.file_path)
+        self.file_path = file_path
+        dirname, basename = osp.split(file_path)
         fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname)
         os.close(fd)
         with contextlib.suppress(OSError):  # It may be that the source does not exist.
@@ -106,7 +106,7 @@ def git_working_dir(func: Callable[..., _T]) -> Callable[..., _T]:
     @wraps(func)
     def set_git_working_dir(self: "IndexFile", *args: Any, **kwargs: Any) -> _T:
         cur_wd = os.getcwd()
-        os.chdir(os.fspath(self.repo.working_tree_dir))
+        os.chdir(self.repo.working_tree_dir)
         try:
             return func(self, *args, **kwargs)
         finally:

git/objects/submodule/base.py

@@ -352,7 +352,7 @@ def _clone_repo(
             module_abspath_dir = osp.dirname(module_abspath)
             if not osp.isdir(module_abspath_dir):
                 os.makedirs(module_abspath_dir)
-            module_checkout_path = osp.join(os.fspath(repo.working_tree_dir), path)
+            module_checkout_path = osp.join(repo.working_tree_dir, path)
 
         if url.startswith("../"):
             remote_name = cast("RemoteReference", repo.active_branch.tracking_branch()).remote_name
@@ -1016,7 +1016,7 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool =
             return self
         # END handle no change
 
-        module_checkout_abspath = join_path_native(os.fspath(self.repo.working_tree_dir), module_checkout_path)
+        module_checkout_abspath = join_path_native(str(self.repo.working_tree_dir), module_checkout_path)
         if osp.isfile(module_checkout_abspath):
             raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath)
         # END handle target files
@@ -1229,7 +1229,7 @@ def remove(
                     wtd = mod.working_tree_dir
                     del mod  # Release file-handles (Windows).
                     gc.collect()
-                    rmtree(wtd)
+                    rmtree(str(wtd))
                 # END delete tree if possible
             # END handle force
 
@@ -1313,7 +1313,7 @@ def set_parent_commit(self, commit: Union[Commit_ish, str, None], check: bool =
         # If check is False, we might see a parent-commit that doesn't even contain the
         # submodule anymore. in that case, mark our sha as being NULL.
         try:
-            self.binsha = pctree[os.fspath(self.path)].binsha
+            self.binsha = pctree[str(self.path)].binsha
         except KeyError:
             self.binsha = self.NULL_BIN_SHA
 
@@ -1395,7 +1395,7 @@ def rename(self, new_name: str) -> "Submodule":
             destination_module_abspath = self._module_abspath(self.repo, self.path, new_name)
             source_dir = mod.git_dir
             # Let's be sure the submodule name is not so obviously tied to a directory.
-            if os.fspath(destination_module_abspath).startswith(os.fspath(mod.git_dir)):
+            if str(destination_module_abspath).startswith(str(mod.git_dir)):
                 tmp_dir = self._module_abspath(self.repo, self.path, str(uuid.uuid4()))
                 os.renames(source_dir, tmp_dir)
                 source_dir = tmp_dir

git/refs/head.py

@@ -8,7 +8,6 @@
 
 __all__ = ["HEAD", "Head"]
 
-import os
 from git.config import GitConfigParser, SectionConstraint
 from git.exc import GitCommandError
 from git.util import join_path
@@ -45,7 +44,6 @@ class HEAD(SymbolicReference):
     __slots__ = ()
 
     def __init__(self, repo: "Repo", path: PathLike = _HEAD_NAME) -> None:
-        path = os.fspath(path)
         if path != self._HEAD_NAME:
             raise ValueError("HEAD instance must point to %r, got %r" % (self._HEAD_NAME, path))
         super().__init__(repo, path)

git/refs/log.py

@@ -4,7 +4,6 @@
 __all__ = ["RefLog", "RefLogEntry"]
 
 from mmap import mmap
-import os
 import os.path as osp
 import re
 import time as _time
@@ -168,7 +167,7 @@ def __init__(self, filepath: Union[PathLike, None] = None) -> None:
         """Initialize this instance with an optional filepath, from which we will
         initialize our data. The path is also used to write changes back using the
         :meth:`write` method."""
-        self._path = None if filepath is None else os.fspath(filepath)
+        self._path = filepath
         if filepath is not None:
             self._read_from_file()
         # END handle filepath

git/refs/symbolic.py

@@ -4,6 +4,7 @@
 __all__ = ["SymbolicReference"]
 
 import os
+from pathlib import Path
 
 from gitdb.exc import BadName, BadObject
 
@@ -76,10 +77,10 @@ class SymbolicReference:
 
     def __init__(self, repo: "Repo", path: PathLike, check_path: bool = False) -> None:
         self.repo = repo
-        self.path = os.fspath(path)
+        self.path: PathLike = path
 
     def __str__(self) -> str:
-        return self.path
+        return os.fspath(self.path)
 
     def __repr__(self) -> str:
         return '<git.%s "%s">' % (self.__class__.__name__, self.path)
@@ -103,7 +104,7 @@ def name(self) -> str:
             In case of symbolic references, the shortest assumable name is the path
             itself.
         """
-        return self.path
+        return os.fspath(self.path)
 
     @property
     def abspath(self) -> PathLike:
@@ -212,7 +213,7 @@ def _check_ref_name_valid(ref_path: PathLike) -> None:
             raise ValueError(f"Invalid reference '{ref_path}': references cannot end with a forward slash (/)")
         elif previous == "@" and one_before_previous is None:
             raise ValueError(f"Invalid reference '{ref_path}': references cannot be '@'")
-        elif any(component.endswith(".lock") for component in os.fspath(ref_path).split("/")):
+        elif any(component.endswith(".lock") for component in Path(ref_path).parts):
             raise ValueError(
                 f"Invalid reference '{ref_path}': references cannot have slash-separated components that end with"
                 " '.lock'"
@@ -235,7 +236,7 @@ def _get_ref_info_helper(
         tokens: Union[None, List[str], Tuple[str, str]] = None
         repodir = _git_dir(repo, ref_path)
         try:
-            with open(os.path.join(repodir, os.fspath(ref_path)), "rt", encoding="UTF-8") as fp:
+            with open(os.path.join(repodir, ref_path), "rt", encoding="UTF-8") as fp:
                 value = fp.read().rstrip()
             # Don't only split on spaces, but on whitespace, which allows to parse lines like:
             # 60b64ef992065e2600bfef6187a97f92398a9144                branch 'master' of git-server:/path/to/repo
@@ -706,7 +707,7 @@ def _create(
         if not force and os.path.isfile(abs_ref_path):
             target_data = str(target)
             if isinstance(target, SymbolicReference):
-                target_data = target.path
+                target_data = os.fspath(target.path)
             if not resolve:
                 target_data = "ref: " + target_data
             with open(abs_ref_path, "rb") as fd:
@@ -930,4 +931,4 @@ def from_path(cls: Type[T_References], repo: "Repo", path: PathLike) -> T_Refere
 
     def is_remote(self) -> bool:
         """:return: True if this symbolic reference points to a remote branch"""
-        return self.path.startswith(self._remote_common_path_default + "/")
+        return os.fspath(self.path).startswith(self._remote_common_path_default + "/")

git/repo/base.py

@@ -126,7 +126,8 @@ class Repo:
     working_dir: PathLike
     """The working directory of the git command."""
 
-    _working_tree_dir: Optional[str] = None
+    # stored as string for easier processing, but annotated as path for clearer intention
+    _working_tree_dir: Optional[PathLike] = None
 
     git_dir: PathLike
     """The ``.git`` repository directory."""
@@ -215,13 +216,13 @@ def __init__(
         epath = path or os.getenv("GIT_DIR")
         if not epath:
             epath = os.getcwd()
+        epath = os.fspath(epath)
         if Git.is_cygwin():
             # Given how the tests are written, this seems more likely to catch Cygwin
             # git used from Windows than Windows git used from Cygwin. Therefore
             # changing to Cygwin-style paths is the relevant operation.
-            epath = cygpath(os.fspath(epath))
+            epath = cygpath(epath)
 
-        epath = os.fspath(epath)
         if expand_vars and re.search(self.re_envvars, epath):
             warnings.warn(
                 "The use of environment variables in paths is deprecated"
@@ -306,7 +307,7 @@ def __init__(
             self._working_tree_dir = None
         # END working dir handling
 
-        self.working_dir: str = self._working_tree_dir or self.common_dir
+        self.working_dir: PathLike = self._working_tree_dir or self.common_dir
         self.git = self.GitCommandWrapperType(self.working_dir)
 
         # Special handling, in special times.
@@ -366,7 +367,7 @@ def description(self, descr: str) -> None:
             fp.write((descr + "\n").encode(defenc))
 
     @property
-    def working_tree_dir(self) -> Optional[str]:
+    def working_tree_dir(self) -> Optional[PathLike]:
         """
         :return:
             The working tree directory of our git repository.
@@ -554,7 +555,7 @@ def tag(self, path: PathLike) -> TagReference:
 
     @staticmethod
     def _to_full_tag_path(path: PathLike) -> str:
-        path_str = os.fspath(path)
+        path_str = str(path)
         if path_str.startswith(TagReference._common_path_default + "/"):
             return path_str
         if path_str.startswith(TagReference._common_default + "/"):
@@ -1355,6 +1356,7 @@ def _clone(
     ) -> "Repo":
         odbt = kwargs.pop("odbt", odb_default_type)
 
+        # url may be a path and this has no effect if it is a string
         url = os.fspath(url)
         path = os.fspath(path)
 

git/repo/fun.py

@@ -62,7 +62,6 @@ def is_git_dir(d: PathLike) -> bool:
         clearly indicates that we don't support it. There is the unlikely danger to
         throw if we see directories which just look like a worktree dir, but are none.
     """
-    d = os.fspath(d)
     if osp.isdir(d):
         if (osp.isdir(osp.join(d, "objects")) or "GIT_OBJECT_DIRECTORY" in os.environ) and osp.isdir(
             osp.join(d, "refs")