Skip to content

Fix Windows path handling on Python 3.13+ - #2196

Open
Byron wants to merge 2 commits into
mainfrom
fix-windows-path-handling
Open

Fix Windows path handling on Python 3.13+#2196
Byron wants to merge 2 commits into
mainfrom
fix-windows-path-handling

Conversation

@Byron

@Byron Byron commented Jul 27, 2026

Copy link
Copy Markdown
Member

Tasks

  • refackiew
  • unwrinkle

Summary

  • make index and submodule path normalization distinguish drive-rooted, drive-relative, UNC, and device paths on Windows
  • confine normalized paths with commonpath instead of string-prefix checks
  • normalize includeIf gitdir patterns to Git-style separators
  • cover the other isabs consumers whose drive-rooted behavior correctly relies on Windows join semantics
  • remove the Windows 3.13-3.15 CI exclusions and add 3.13-3.15 to tox
  • make the pre-existing tests independent of a local master branch and linked-worktree-specific alternates

This addresses the remaining Windows path checklist from #1955 and closes #2072.

For context, the classifiers mentioned in the issue were introduced by 904aa71 (Declare support for Python 3.13-3.14) and 7d5370d (Add support for Python 3.15), both included in 3.1.56. This PR supplies the Windows path work that those metadata changes did not include.

Windows path coverage

  • paths absolute on the current drive
  • drive-rooted paths such as \directory and /directory
  • drive-relative paths such as C:directory (rejected where repository confinement is required)
  • other-drive, UNC, and device paths
  • traversal and adjacent-prefix escape attempts
  • native and Git-style separators in conditional includes
  • config includes, submodule gitfiles, linked worktree metadata, clone destinations, and Cygwin-path conversion

Validation

  • Python 3.12.13, 3.13.14, 3.14.6, 3.14.6t, 3.15.0b4, and 3.15.0b4t installed locally with uv
  • full test set passed for every interpreter in an independent clone without a local master branch; daemon-free suites ran in parallel and test_remote.py ran serially because its fixed local daemon endpoint cannot be shared across processes
  • Ruff check and format
  • mypy
  • basedpyright
  • all pre-commit hooks

@Byron
Byron force-pushed the fix-windows-path-handling branch from bed2df4 to 8c0966f Compare July 28, 2026 04:10
@Byron
Byron marked this pull request as ready for review July 28, 2026 04:10
Copilot AI review requested due to automatic review settings July 28, 2026 04:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates GitPython’s Windows path handling to account for Python 3.13+ changes (notably around drive-rooted paths), expands coverage across config/includes, index/submodule path confinement, and adjusts CI/tox + tests to validate newer Python versions and avoid assumptions about a local master branch.

Changes:

  • Introduces shared path-kind and confinement helpers and uses them in index/submodule path normalization.
  • Updates conditional include matching to normalize gitdir patterns to Git-style separators and improves repository confinement checks.
  • Expands/adjusts test suite for Windows path kinds and removes reliance on a master branch; enables Python 3.13–3.15 in tox and CI.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tox.ini Adds py313–py315 tox environments.
git/util.py Adds _is_path_rooted and _to_relative_path helper for Git-style, confined relative paths.
git/index/base.py Switches index-relative path normalization to the new confinement helper (with special bare-repo handling).
git/objects/submodule/base.py Uses the new confinement helper for submodule checkout paths and rejects repository-root paths.
git/refs/symbolic.py Adjusts reference path confinement check based on commonpath.
git/config.py Normalizes includeIf gitdir patterns and repository gitdir comparison to forward slashes; tweaks drive-prefix behavior.
git/repo/base.py Reads/writes alternates from common_dir (important for linked worktrees).
.github/workflows/pythonpackage.yml Re-enables Windows testing for Python 3.13–3.15 by removing exclusions.
test/test_index.py Updates expected separators to Git-style and adds Windows path-kind tests for index confinement.
test/test_submodule.py Adds confinement/path-kind tests and a drive-rooted gitfile test on Windows.
test/test_config.py Adds Windows drive-rooted include test and expands conditional include coverage for forward slashes.
test/test_repo.py Removes reliance on master, adds Windows drive-rooted clone/worktree tests, and adds alternates/common_dir test.
test/test_refs.py Removes reliance on master for remote refs and reflog tests.
test/test_commit.py Replaces master with HEAD in commit iteration/trailer tests.
test/test_docs.py Makes docs tests branch-agnostic and updates examples to use active/default branches.
test/test_util.py Adds a Windows drive-rooted cygpath test.
.basedpyright/baseline.json Drops baseline entries corresponding to fixed type-check findings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread git/util.py
Comment on lines +361 to +362
root_abs = osp.abspath(os.fspath(root))
path_abs = osp.abspath(osp.join(root_abs, path_str))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex says: Because the joined result already contains root_abs’s drive or UNC share, abspath() does not consult the process’s current drive.

The existing rooted_inside_path test covers drive-less rooted input. It does not explicitly arrange for a different CWD drive, but that state cannot affect this expression once join() has retained the root’s drive. No implementation change is needed.

Comment thread git/refs/symbolic.py
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:

@Byron Byron Jul 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I removed based on the codex note, but maybe ultimately it's safer to do this, and it's something it has copied from symbolic.py ... even though on case-sensitive filesystems it most definitely is overly restrictive to fold then separate paths.

Copilot AI review requested due to automatic review settings July 28, 2026 04:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Byron
Byron marked this pull request as draft July 28, 2026 04:29
@EliahKagan
EliahKagan requested a review from Copilot July 28, 2026 04:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

git/refs/symbolic.py:123

  • On Windows, comparing common_path and base_path with raw string equality can incorrectly reject valid paths when only casing differs (case-insensitive filesystem) or when realpath() normalizes the drive letter. Previously this check used normcase() to avoid false positives; removing it can make _get_validated_path raise ValueError for refs that are actually inside the repo.
        base_path = os.path.realpath(os.fspath(base))
        abs_path = os.path.realpath(os.path.join(base_path, path))
        try:
            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 common_path != base_path:
            raise ValueError("Reference path %r escapes the repository" % path)

git/util.py:363

  • _to_relative_path() claims to resolve drive-rooted paths like \\dir against the drive of root, but abspath(join(root_abs, path_str)) drops the drive and then resolves against the process current drive. If root is on a different drive than the current working directory, \\dir will be resolved to the wrong drive, and confinement checks will be incorrect.
    root_abs = osp.abspath(os.fspath(root))
    path_abs = osp.abspath(osp.join(root_abs, path_str))
    try:

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

git/index/base.py:660

  • This method can now raise :exc:InvalidGitRepositoryError (bare repo + rooted/drive path), but the docstring only mentions :exc:ValueError. Please document the additional exception so error handling is discoverable from the API contract.
        :raise ValueError:
        """

git/objects/submodule/base.py:381

  • The docstring says ValueError is only raised when the path is outside the working tree, but the implementation also rejects paths that refer to the working tree root (".") after normalization. Documenting that behavior will make this constraint clearer.
        :raise ValueError:
            If path is not contained in the parent repository's working tree.

git/index/base.py:666

  • The docstring above still describes this as making paths relative to the repo's "git directory", but the new implementation delegates to git.util._to_relative_path(..., working_tree_dir) and returns a Git-style (forward-slash) path. Please update the return description so it matches the current behavior.
        return _to_relative_path(cast(PathLike, self.repo.working_tree_dir), path)

Byron and others added 2 commits July 28, 2026 08:38
This commit also makes the test-suite independent of the presence of a
local `master` branch.

<!-- agent -->

- make index and submodule path normalization distinguish drive-rooted, drive-relative, UNC, and device paths on Windows
- confine normalized paths with `commonpath` instead of string-prefix checks
- normalize `includeIf gitdir` patterns to Git-style separators
- cover the other `isabs` consumers whose drive-rooted behavior correctly relies on Windows join semantics
- remove the Windows 3.13-3.15 CI exclusions and add 3.13-3.15 to tox
- make the pre-existing tests independent of a local `master` branch and linked-worktree-specific alternates

For context, the classifiers mentioned in the issue #2072 were introduced by 904aa71
(`Declare support for Python 3.13-3.14`) and 7d5370d (`Add support for Python
3.15`), both included in 3.1.56. This PR supplies the Windows path work that
those metadata changes did not include.

- paths absolute on the current drive
- drive-rooted paths such as `\directory` and `/directory`
- drive-relative paths such as `C:directory` (rejected where repository confinement is required)
- other-drive, UNC, and device paths
- traversal and adjacent-prefix escape attempts
- native and Git-style separators in conditional includes
- config includes, submodule gitfiles, linked worktree metadata, clone destinations, and Cygwin-path conversion

- Python 3.12.13, 3.13.14, 3.14.6, 3.14.6t, 3.15.0b4, and 3.15.0b4t installed locally with uv
- full test set passed for every interpreter in an independent clone without a
  local `master` branch; daemon-free suites ran in parallel and `test_remote.py`
  ran serially because its static local daemon endpoint cannot be shared across
  processes
- Ruff check and format
- mypy
- basedpyright
- all pre-commit hooks

Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
- Preserve POSIX backslashes in include patterns
- Accept UNC share roots as rooted paths
- treat includeIf patterns correctly on Windows, expecting forward slashes
- Recognize UNC roots on older Python versions

Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
@Byron
Byron force-pushed the fix-windows-path-handling branch from ecdce93 to 464da19 Compare July 28, 2026 06:57
@Byron
Byron marked this pull request as ready for review July 28, 2026 07:05
Copilot AI review requested due to automatic review settings July 28, 2026 07:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Byron
Byron requested a review from EliahKagan July 28, 2026 07:11
@Byron

Byron commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Alright, to me it seems good to go, and I will wait a week or so for feedback, assuming there are no major concerns after that and that everything can be forward fixed, with a new release following. A week could easily be more though as I will leave it to chance that I rediscover the issue (which is expected, at least if other GitPython work happens).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Request to Declare Python 3.13 Compatibility on PyPI

2 participants