Merge pull request #2196 from gitpython-developers/fix-windows-path-handling

Fix Windows path handling on Python 3.13+

Author: Byron

.basedpyright/baseline.json

@@ -887,22 +887,6 @@
                     "lineCount": 1
                 }
             },
-            {
-                "code": "reportPossiblyUnboundVariable",
-                "range": {
-                    "startColumn": 22,
-                    "endColumn": 37,
-                    "lineCount": 1
-                }
-            },
-            {
-                "code": "reportPossiblyUnboundVariable",
-                "range": {
-                    "startColumn": 22,
-                    "endColumn": 37,
-                    "lineCount": 1
-                }
-            },
             {
                 "code": "reportReturnType",
                 "range": {

.github/workflows/pythonpackage.yml

@@ -25,16 +25,6 @@ jobs:
           python-version: "3.14t"
         - os-type: macos
           python-version: "3.15t"
-        - os-type: windows
-          python-version: "3.13"  # FIXME: Fix and enable Python 3.13-3.15 on Windows (#1955).
-        - os-type: windows
-          python-version: "3.14"
-        - os-type: windows
-          python-version: "3.14t"
-        - os-type: windows
-          python-version: "3.15"
-        - os-type: windows
-          python-version: "3.15t"
         include:
         - os-ver: latest
         - os-type: ubuntu

git/config.py

@@ -577,8 +577,12 @@ def _all_items(section: str) -> List[Tuple[str, str]]:
 
             if keyword in ["gitdir", "gitdir/i"]:
                 value = osp.expanduser(value)
+                git_dir = os.fspath(self._repo.git_dir) if self._repo.git_dir else None
+                if sys.platform == "win32":
+                    git_dir = git_dir.replace("\\", "/") if git_dir else None
 
-                if not any(value.startswith(s) for s in ["./", "/"]):
+                drive, _tail = osp.splitdrive(value)
+                if not drive and not any(value.startswith(s) for s in ["./", "/"]):
                     value = "**/" + value
                 if value.endswith("/"):
                     value += "**"
@@ -590,9 +594,8 @@ def _all_items(section: str) -> List[Tuple[str, str]]:
                         lambda m: f"[{m.group().lower()!r}{m.group().upper()!r}]",
                         value,
                     )
-                if self._repo.git_dir:
-                    if fnmatch.fnmatchcase(os.fspath(self._repo.git_dir), value):
-                        paths += _all_items(section)
+                if git_dir and fnmatch.fnmatchcase(git_dir, value):
+                    paths += _all_items(section)
 
             elif keyword == "onbranch":
                 try:

git/index/base.py

@@ -34,6 +34,8 @@
     LockedFD,
     join_path_native,
     file_contents_ro,
+    _is_path_rooted,
+    _to_relative_path,
     to_native_path_linux,
     unbare_repo,
     to_bin_sha,
@@ -58,6 +60,7 @@
     Any,
     BinaryIO,
     Callable,
+    cast,
     Dict,
     Generator,
     IO,
@@ -655,16 +658,12 @@ def _to_relative_path(self, path: PathLike) -> PathLike:
 
         :raise ValueError:
         """
-        if not osp.isabs(path):
-            return path
         if self.repo.bare:
-            raise InvalidGitRepositoryError("require non-bare repository")
-        if not osp.normpath(path).startswith(str(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 os.fspath(path).endswith(os.sep) and not result.endswith(os.sep):
-            result += os.sep
-        return result
+            drive, _tail = osp.splitdrive(os.fspath(path))
+            if drive or _is_path_rooted(path):
+                raise InvalidGitRepositoryError("paths with a drive or root require a non-bare repository")
+            return path
+        return _to_relative_path(cast(PathLike, self.repo.working_tree_dir), path)
 
     def _preprocess_add_items(
         self, items: Union[PathLike, Sequence[Union[PathLike, Blob, BaseIndexEntry, "Submodule"]]]

git/objects/submodule/base.py

@@ -29,6 +29,7 @@
 from git.util import (
     IterableList,
     RemoteProgress,
+    _to_relative_path,
     join_path_native,
     rmtree,
     to_native_path_linux,
@@ -392,23 +393,14 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike:
         :raise ValueError:
             If path is not contained in the parent repository's working tree.
         """
-        path = to_native_path_linux(path)
+        if parent_repo.working_tree_dir:
+            path = _to_relative_path(parent_repo.working_tree_dir, path)
+        else:
+            path = to_native_path_linux(path)
         if path.endswith("/"):
             path = path[:-1]
-        # END handle trailing slash
-
-        if osp.isabs(path) and parent_repo.working_tree_dir:
-            working_tree_linux = to_native_path_linux(parent_repo.working_tree_dir)
-            if not path.startswith(working_tree_linux):
-                raise ValueError(
-                    "Submodule checkout path '%s' needs to be within the parents repository at '%s'"
-                    % (working_tree_linux, path)
-                )
-            path = path[len(working_tree_linux.rstrip("/")) + 1 :]
-            if not path:
-                raise ValueError("Absolute submodule path '%s' didn't yield a valid relative path" % path)
-            # END verify converted relative path makes sense
-        # END convert to a relative path
+        if not path or path == ".":
+            raise ValueError("Submodule checkout path must not be the repository root")
 
         return path
 

git/refs/symbolic.py

@@ -119,7 +119,7 @@ def _get_validated_path(base: PathLike, path: PathLike) -> str:
             common_path = os.path.commonpath([base_path, abs_path])
         except ValueError as e:
             raise ValueError("Reference path %r escapes the repository" % path) from e
-        if os.path.normcase(common_path) != os.path.normcase(base_path):
+        if common_path != base_path:
             raise ValueError("Reference path %r escapes the repository" % path)
         return abs_path
 

git/repo/base.py

@@ -961,8 +961,7 @@ def _get_alternates(self) -> List[str]:
         :return:
             List of strings being pathnames of alternates
         """
-        if self.git_dir:
-            alternates_path = osp.join(self.git_dir, "objects", "info", "alternates")
+        alternates_path = osp.join(self.common_dir, "objects", "info", "alternates")
 
         if osp.exists(alternates_path):
             with open(alternates_path, "rb") as f:

git/util.py

@@ -315,6 +315,68 @@ def join_path_native(a: PathLike, *p: PathLike) -> PathLike:
     return to_native_path(join_path(a, *p))
 
 
+def _is_path_rooted(path: PathLike) -> bool:
+    r"""Whether ``path`` has a root, including one encoded in a UNC drive.
+
+    On Windows, ``\directory`` is rooted on the current drive without being
+    absolute, while ``C:\directory`` has both a drive and a root. In contrast,
+    ``directory`` and the drive-relative ``C:directory`` have no root.
+    UNC paths are rooted: ``\\server\share`` stores the share in the drive
+    returned by :func:`os.path.splitdrive`, while ``\\server\share\directory``
+    additionally has a rooted tail.
+    On POSIX, which has no drive concept, this simply distinguishes absolute
+    paths such as ``/directory`` from relative paths such as ``directory``.
+    """
+    drive, tail = osp.splitdrive(os.fspath(path))
+    separators = (os.sep,) if os.altsep is None else (os.sep, os.altsep)
+    return tail.startswith(separators) or drive.startswith(separators)
+
+
+def _to_relative_path(root: PathLike, path: PathLike) -> str:
+    r"""Return a normalized Git-style path confined to ``root``.
+
+    A Windows path such as ``\directory`` is rooted but not absolute. Resolve it
+    against the drive of ``root`` rather than treating it as relative to ``root``.
+    Drive-relative paths such as ``C:directory`` are rejected because their meaning
+    depends on process-global per-drive state.
+
+    For example, with ``root`` set to ``C:\repo`` on Windows:
+
+    * ``directory\file`` -> ``directory/file``
+    * ``directory\`` -> ``directory/``
+    * ``C:\repo\directory\file`` -> ``directory/file``
+    * ``\repo\directory\file`` -> ``directory/file``
+    * ``C:directory\file`` -> :exc:`ValueError`
+    * ``C:\other\file`` -> :exc:`ValueError`
+
+    On POSIX, ``/repo/directory/file`` under ``/repo`` similarly becomes
+    ``directory/file``. A trailing separator is preserved as a Git-style ``/``.
+    """
+    path_str = os.fspath(path)
+    if not path_str:
+        return path_str
+
+    drive, _tail = osp.splitdrive(path_str)
+    rooted = _is_path_rooted(path_str)
+    if drive and not rooted:
+        raise ValueError("Drive-relative path %r is not supported" % path_str)
+
+    root_abs = osp.abspath(os.fspath(root))
+    path_abs = osp.abspath(osp.join(root_abs, path_str))
+    try:
+        common_path = osp.commonpath([root_abs, path_abs])
+    except ValueError as e:
+        raise ValueError("Path %r is not in repository at %r" % (path_str, root_abs)) from e
+    if common_path != root_abs:
+        raise ValueError("Path %r is not in repository at %r" % (path_str, root_abs))
+
+    relative_path = to_native_path_linux(osp.relpath(path_abs, root_abs))
+    separators = (os.sep,) if os.altsep is None else (os.sep, os.altsep)
+    if path_str.endswith(separators) and relative_path != "." and not relative_path.endswith("/"):
+        relative_path += "/"
+    return relative_path
+
+
 def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool:
     """Make sure that the directory pointed to by path exists.
 

test/test_commit.py

@@ -276,14 +276,14 @@ def test_iteration(self):
         assert ltd_commits and len(ltd_commits) < len(all_commits)
 
         # Show commits of multiple paths, resulting in a union of commits.
-        less_ltd_commits = list(Commit.iter_items(self.rorepo, "master", paths=("CHANGES", "AUTHORS")))
+        less_ltd_commits = list(Commit.iter_items(self.rorepo, "HEAD", paths=("CHANGES", "AUTHORS")))
         assert len(ltd_commits) < len(less_ltd_commits)
 
         class Child(Commit):
             def __init__(self, *args, **kwargs):
                 super().__init__(*args, **kwargs)
 
-        child_commits = list(Child.iter_items(self.rorepo, "master", paths=("CHANGES", "AUTHORS")))
+        child_commits = list(Child.iter_items(self.rorepo, "HEAD", paths=("CHANGES", "AUTHORS")))
         assert type(child_commits[0]) is Child
 
     def test_iter_items(self):
@@ -536,7 +536,7 @@ def test_trailers(self):
             ),
         ]
         for msg in msgs:
-            commit = copy.copy(self.rorepo.commit("master"))
+            commit = copy.copy(self.rorepo.commit("HEAD"))
             commit.message = msg
             assert commit.trailers_list == [
                 (KEY_1, VALUE_1_1),
@@ -559,13 +559,13 @@ def test_trailers(self):
         ]
 
         for msg in msgs:
-            commit = copy.copy(self.rorepo.commit("master"))
+            commit = copy.copy(self.rorepo.commit("HEAD"))
             commit.message = msg
             assert commit.trailers_list == []
             assert commit.trailers_dict == {}
 
         # Check that only the last key value paragraph is evaluated.
-        commit = copy.copy(self.rorepo.commit("master"))
+        commit = copy.copy(self.rorepo.commit("HEAD"))
         commit.message = f"Subject\n\nMultiline\nBody\n\n{KEY_1}: {VALUE_1_1}\n\n{KEY_2}: {VALUE_2}\n"
         assert commit.trailers_list == [(KEY_2, VALUE_2)]
         assert commit.trailers_dict == {KEY_2: [VALUE_2]}

test/test_config.py

@@ -14,7 +14,7 @@
 
 from git import GitConfigParser
 from git.config import _OMD, cp
-from git.util import rmfile
+from git.util import cwd, rmfile
 
 from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory
 
@@ -374,6 +374,22 @@ def test_config_relative_path_include(self, rw_dir):
         with GitConfigParser(relative_config_path, read_only=True) as cr:
             assert cr.get_value("included", "value") == "included"
 
+    @pytest.mark.skipif(os.name != "nt", reason="Specifically for Windows drive-rooted paths.")
+    @with_rw_directory
+    def test_config_drive_rooted_path_include(self, rw_dir):
+        with cwd(rw_dir):
+            included_path = osp.join(rw_dir, "included")
+            with GitConfigParser(included_path, read_only=False) as cw:
+                cw.set_value("included", "value", "included")
+
+            _drive, rooted_included_path = osp.splitdrive(included_path)
+            config_path = osp.join(rw_dir, "config")
+            with GitConfigParser(config_path, read_only=False) as cw:
+                cw.set_value("include", "path", rooted_included_path)
+
+            with GitConfigParser(config_path, read_only=True) as cr:
+                assert cr.get_value("included", "value") == "included"
+
     @with_rw_directory
     def test_multiple_include_paths_with_same_key(self, rw_dir):
         """Test that multiple 'path' entries under [include] are all respected.
@@ -411,15 +427,11 @@ def test_multiple_include_paths_with_same_key(self, rw_dir):
             assert cr.get_value("user", "name") == "from-inc1"
             assert cr.get_value("core", "bar") == "from-inc2"
 
-    @pytest.mark.xfail(
-        sys.platform == "win32",
-        reason='Second config._has_includes() assertion fails (for "config is included if path is matching git_dir")',
-        raises=AssertionError,
-    )
     @with_rw_directory
     def test_conditional_includes_from_git_dir(self, rw_dir):
         # Initiate repository path.
         git_dir = osp.join(rw_dir, "target1", "repo1")
+        git_dir_pattern = git_dir.replace("\\", "/")
         os.makedirs(git_dir)
 
         # Initiate mocked repository.
@@ -431,29 +443,42 @@ def test_conditional_includes_from_git_dir(self, rw_dir):
         template = '[includeIf "{}:{}"]\n    path={}\n'
 
         with open(path1, "w") as stream:
+            # on Windows, this writes a backslash pattern.
             stream.write(template.format("gitdir", git_dir, path2))
 
         # Ensure that config is ignored if no repo is set.
         with GitConfigParser(path1) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
-        # Ensure that config is included if path is matching git_dir.
+        # Git uses forward slashes in gitdir patterns on every platform:
+        # backslashes escape the next pattern character rather than separate
+        # path components. On Windows, GitPython therefore normalizes git_dir
+        # to forward slashes but leaves this backslash pattern unchanged, so
+        # the two do not match and no path is included.
+        with GitConfigParser(path1, repo=repo, merge_includes=False) as config:
+            expected_paths = [] if sys.platform == "win32" else [("path", path2)]
+            assert config._included_paths() == expected_paths
+
+        # Ensure that Git's forward-slash syntax matches native Windows paths.
+        with open(path1, "w") as stream:
+            stream.write(template.format("gitdir", git_dir_pattern, path2))
+
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
         # Ensure that config is ignored if case is incorrect.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", git_dir.upper(), path2))
+            stream.write(template.format("gitdir", git_dir_pattern.upper(), path2))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
         # Ensure that config is included if case is ignored.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir/i", git_dir.upper(), path2))
+            stream.write(template.format("gitdir/i", git_dir_pattern.upper(), path2))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
@@ -483,6 +508,20 @@ def test_conditional_includes_from_git_dir(self, rw_dir):
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
+    @with_rw_directory
+    def test_conditional_includes_do_not_treat_backslashes_as_separators(self, rw_dir):
+        git_dir = osp.join(rw_dir, "target", "repo")
+        repo = mock.Mock(git_dir=git_dir)
+        config_path = osp.join(rw_dir, "config")
+        included_path = osp.join(rw_dir, "included")
+        pattern = git_dir.replace("\\", "/").replace("/target/repo", R"/target\repo")
+
+        with open(config_path, "w") as stream:
+            stream.write(f'[includeIf "gitdir:{pattern}"]\n    path={included_path}\n')
+
+        with GitConfigParser(config_path, repo=repo, merge_includes=False) as config:
+            assert config._included_paths() == []
+
     @with_rw_directory
     def test_conditional_includes_from_branch_name(self, rw_dir):
         # Initiate mocked branch.