Skip to content

config: resolve relative includes from relative config paths#2123

Draft
officialasishkumar wants to merge 2 commits intogitpython-developers:mainfrom
officialasishkumar:fix-relative-config-include
Draft

config: resolve relative includes from relative config paths#2123
officialasishkumar wants to merge 2 commits intogitpython-developers:mainfrom
officialasishkumar:fix-relative-config-include

Conversation

@officialasishkumar
Copy link
Copy Markdown

Fixes #2103

Summary

  • Normalize path-based config inputs before using them for include cycle detection.
  • Resolve relative include.path entries from that normalized source path when the root config file was supplied as a relative path.
  • Add regression coverage with a relative root config and a relative include cycle.

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.py
  • git diff --check

Copilot AI review requested due to automatic review settings April 11, 2026 19:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.path values 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.

File Description
git/config.py Normalizes config paths for seen and uses an absolute source path to resolve relative include paths without asserting the caller provided an absolute root path.
test/test_config.py Adds a regression test ensuring relative root config paths correctly resolve relative includes and avoid include-cycle failures.

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

Copy link
Copy Markdown
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

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 officialasishkumar force-pushed the fix-relative-config-include branch from 424760d to 483f0c6 Compare April 13, 2026 03:42
@officialasishkumar
Copy link
Copy Markdown
Author

@Byron Added the nested-directory include cycle coverage in 483f0c6: the relative root config now includes subdir/b, and that nested file includes ../a. Validation passed with test/test_config.py, Ruff, mypy on git/config.py, and git diff --check.

@Byron Byron marked this pull request as draft April 13, 2026 04:23
fpa = osp.join(rw_dir, "a")
nested_dir = osp.join(rw_dir, "subdir")
os.makedirs(nested_dir)
fpb = osp.join(nested_dir, "b")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

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.

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.

Relative path include in .gitconfig throws AssertionError

3 participants