Allow relative config paths with includes#2169
Merged
Merged
Conversation
GitConfigParser asserted while resolving a relative include whenever the top-level configuration file had itself been supplied as a relative path. A regression test constructs that combination and verifies the included value is available. Normalize path-based inputs to absolute paths before reading them. This lets relative includes resolve against the containing file and ensures cycle detection uses the same canonical spelling for top-level and included paths. This matches Git's documented include behavior and the relative and chained-relative coverage in t/t1305-config-include.sh. Validation: - uv run pytest -q test/test_config.py - uv run pre-commit run --files git/config.py test/test_config.py - uv run mypy git/config.py - git diff --check - full pytest: 631 passed, 75 skipped, 34 unrelated environment failures (uninitialized nested submodules and missing legacy master branch)
Member
Author
|
Merged due to simplicity, without waiting for CI which basically doesn't finish today. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Normalize config parser input paths so relative includes resolve correctly when the top-level config file is provided as a relative path (avoiding assertion/cycle-detection inconsistencies).
Changes:
- Normalize
GitConfigParser.read()input file paths to absolute paths before processing includes. - Add a regression test covering relative top-level config path with a relative
[include] path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/test_config.py | Adds coverage for relative config path + relative include behavior. |
| git/config.py | Normalizes initial config paths to absolute to make include resolution and cycle detection consistent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| files_to_read = list(self._file_or_files) | ||
| # END ensure we have a copy of the paths to handle | ||
|
|
||
| files_to_read = [osp.abspath(path) if isinstance(path, (str, os.PathLike)) else path for path in files_to_read] |
Comment on lines
+313
to
+325
| @with_rw_directory | ||
| def test_config_relative_path_include(self, rw_dir): | ||
| included_path = osp.join(rw_dir, "included") | ||
| with GitConfigParser(included_path, read_only=False) as cw: | ||
| cw.set_value("included", "value", "included") | ||
|
|
||
| config_path = osp.join(rw_dir, "config") | ||
| with GitConfigParser(config_path, read_only=False) as cw: | ||
| cw.set_value("include", "path", "included") | ||
|
|
||
| relative_config_path = osp.relpath(config_path) | ||
| with GitConfigParser(relative_config_path, read_only=True) as cr: | ||
| assert cr.get_value("included", "value") == "included" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
GPT-5.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Summary
GitPython already supports relative include paths, but raised an
AssertionErrorwhen the top-level configuration file was itself passed as a relative path. Normalize path-based parser inputs before reading so relative includes resolve against their containing config file and cycle detection uses consistent absolute paths.Fixes #2103
Git behavior reference
This matches Git documentation and the relative and chained-relative include cases in
t/t1305-config-include.sh.Validation
uv run pytest -q test/test_config.py(31 passed, 1 skipped)uv run pre-commit run --files git/config.py test/test_config.pyuv run mypy git/config.pygit diff --checkmasterbranch