config: resolve relative includes from relative config paths#2123
config: resolve relative includes from relative config paths#2123officialasishkumar wants to merge 2 commits intogitpython-developers:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes relative [include] path = ... handling when the root config file is provided as a relative path (regression for #2103), by normalizing config file paths for both include-cycle detection and relative-include resolution.
Changes:
- Normalize initial config file inputs to absolute, normalized paths for include cycle detection.
- Resolve relative
include.pathvalues using the normalized absolute source config path (instead of asserting the source path is already absolute). - Add a regression test covering a relative root config path with a relative include cycle.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
It looks like Windows has problems, but that might point to some logic error.
There should also be a test that actually validates that cycle-detection, by putting one of the include files in a directory.
Normalize path-based config inputs before using them for include cycle checks. This lets GitConfigParser resolve relative include.path values when the root config file was provided as a relative path, without re-reading the same config under a different spelling. Add regression coverage with a relative root config and a relative include cycle.
officialasishkumar
force-pushed
the
fix-relative-config-include
branch
from
424760d to
483f0c6
Compare
There was a problem hiding this comment.
One of the included files need to be in a subdirectory, and both need to be named "a" so the relative name is the same for an actual cycle check.
Byron
left a comment
Byron
left a comment
There was a problem hiding this comment.
Windows still fails, also the cycle check isn't addressed. It seems like this is a bot account and I don't want to spend more time here.
Will close if the PR isn't green and has its issues addressed next time it comes out of draft.
Fixes #2103
Summary
include.pathentries from that normalized source path when the root config file was supplied as a relative path.Testing
.venv/bin/pytest --no-cov -q test/test_config.py.venv/bin/pytest --no-cov -q test/test_config.py test/test_repo.py::TestRepo::test_config_reader.venv/bin/pytest --no-cov -q test/test_refs.py::TestRefs::test_set_tracking_branch_with_import.venv/bin/ruff check git/config.py test/test_config.py.venv/bin/ruff format --check git/config.py test/test_config.py.venv/bin/mypy git/config.pygit diff --check