Skip to content

Commit fb8a0bd

Browse files
Byroncodex
andcommitted
fix: match Git config names case-insensitively (#2240)
Mostly a rubber-stamp, impl seems sane and tests seem to cover the important bits. <!-- agent --> GitConfigParser required exact section and option spelling, so valid Git configuration such as core.BigName could not be read as CORE.bigname. Differently cased sections and options also stayed separate, causing lookups to miss later values and writers to create duplicate settings. Index the ordered multi-dictionary by normalized names while retaining the first spelling in storage. Lowercase the section/option portion only; quoted subsection names remain case-sensitive. The shared mapping covers the inherited ConfigParser accessors, multivalue reads, and mutations without scanning all stored names. Case variants now merge in read order, and enumeration and write-back use the first spelling for each name. Normalize include section matching and remote discovery as well, while keeping include conditions and remote names case-sensitive. Add regressions for case variants, duplicate values, implicit booleans, quoted subsections, spelling-preserving writes, removal and renaming, included files, and remote discovery. The three new regression tests failed before the fix. Extend the existing setlast check to cover mixed case and clearing the name index. The behavior follows Documentation/config.adoc and the mixed-case and subsection tests in t/t1300-config.sh from the local Git reference at 1630431f326e15fcde608827b5ff38422528eb59. Regression comparisons with git config --get and --get-all used Git 2.50.1 (Apple Git-155). Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
1 parent 696e1cb commit fb8a0bd

3 files changed

Lines changed: 146 additions & 19 deletions

File tree

git/config.py

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
CONFIG_LEVELS: ConfigLevels_Tup = ("system", "user", "global", "repository")
6565
"""The configuration level of a configuration file."""
6666

67-
CONDITIONAL_INCLUDE_REGEXP = re.compile(r"(?<=includeIf )\"(gitdir|gitdir/i|onbranch|hasconfig:remote\.\*\.url):(.+)\"")
67+
CONDITIONAL_INCLUDE_REGEXP = re.compile(r"(?<=includeif )\"(gitdir|gitdir/i|onbranch|hasconfig:remote\.\*\.url):(.+)\"")
6868
"""Section pattern to detect conditional includes.
6969
7070
See: https://git-scm.com/docs/git-config#_conditional_includes
@@ -203,41 +203,67 @@ def __exit__(self, exception_type: str, exception_value: str, traceback: str) ->
203203
self._config.__exit__(exception_type, exception_value, traceback)
204204

205205

206+
def _normalize_name(name: str) -> str:
207+
"""Fold section and option names, leaving quoted subsections unchanged."""
208+
prefix, separator, subsection = name.partition('"')
209+
return prefix.lower() + separator + subsection
210+
211+
206212
class _OMD(OrderedDict_OMD):
207-
"""Ordered multi-dict."""
213+
"""Ordered multi-dict matching config names while retaining their first spelling."""
214+
215+
def __init__(self, *args: Any, **kwargs: Any) -> None:
216+
self._keymap: Dict[str, str] = {}
217+
super().__init__(*args, **kwargs)
218+
219+
def _key(self, key: str) -> str:
220+
stored = self._keymap.get(_normalize_name(key), key)
221+
return stored if super().__contains__(stored) else key
222+
223+
def __contains__(self, key: object) -> bool:
224+
return isinstance(key, str) and super().__contains__(self._key(key))
225+
226+
def __delitem__(self, key: str) -> None:
227+
super().__delitem__(self._key(key))
228+
del self._keymap[_normalize_name(key)]
208229

209230
def __setitem__(self, key: str, value: _T) -> None:
210-
super().__setitem__(key, [value])
231+
self.setall(key, [value])
232+
233+
def clear(self) -> None:
234+
super().clear()
235+
self._keymap.clear()
211236

212237
def add(self, key: str, value: Any) -> None:
213238
if key not in self:
214-
super().__setitem__(key, [value])
239+
self[key] = value
215240
return
216241

217-
super().__getitem__(key).append(value)
242+
self.getall(key).append(value)
218243

219244
def setall(self, key: str, values: List[_T]) -> None:
245+
key = self._key(key)
220246
super().__setitem__(key, values)
247+
self._keymap[_normalize_name(key)] = key
221248

222249
def __getitem__(self, key: str) -> Any:
223-
return super().__getitem__(key)[-1]
250+
return super().__getitem__(self._key(key))[-1]
224251

225252
def getlast(self, key: str) -> Any:
226-
return super().__getitem__(key)[-1]
253+
return self[key]
227254

228255
def setlast(self, key: str, value: Any) -> None:
229256
if key not in self:
230-
super().__setitem__(key, [value])
257+
self[key] = value
231258
return
232259

233-
prior = super().__getitem__(key)
234-
prior[-1] = value
260+
self.getall(key)[-1] = value
235261

236262
def get(self, key: str, default: Union[_T, None] = None) -> Union[_T, None]:
237-
return super().get(key, [default])[-1]
263+
return super().get(self._key(key), [default])[-1]
238264

239265
def getall(self, key: str) -> List[_T]:
240-
return super().__getitem__(key)
266+
return super().__getitem__(self._key(key))
241267

242268
def items(self) -> List[Tuple[str, _T]]: # type: ignore[override]
243269
"""List of (key, last value for key)."""
@@ -286,8 +312,9 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder):
286312
other instances to write concurrently.
287313
288314
:note:
289-
The config is case-sensitive even when queried, hence section and option names
290-
must match perfectly.
315+
Section and option names are case-insensitive; quoted subsection names are
316+
case-sensitive. Names retain their first spelling when enumerated or written.
317+
Case variants are merged, preserving all values in the order they are read.
291318
292319
:note:
293320
If used as a context manager, this will release the locked file.
@@ -641,10 +668,11 @@ def _all_items(section: str) -> List[Tuple[str, str]]:
641668
paths = []
642669

643670
for section in self.sections():
644-
if section == "include":
671+
normalized_section = _normalize_name(section)
672+
if normalized_section == "include":
645673
paths += _all_items(section)
646674

647-
match = CONDITIONAL_INCLUDE_REGEXP.search(section)
675+
match = CONDITIONAL_INCLUDE_REGEXP.search(normalized_section)
648676
if match is None or self._repo is None:
649677
continue
650678

git/remote.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,7 @@ def exists(self) -> bool:
632632
def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Iterator["Remote"]:
633633
""":return: Iterator yielding :class:`Remote` objects of the given repository"""
634634
for section in repo.config_reader("repository").sections():
635-
if not section.startswith("remote "):
635+
if not section.lower().startswith("remote "):
636636
continue
637637
lbound = section.find('"')
638638
rbound = section.rfind('"')

test/test_config.py

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
import pytest
1515

16-
from git import GitConfigParser
16+
from git import GitConfigParser, Repo
1717
from git.compat import defenc
1818
from git.config import _OMD, cp
1919
from git.util import cwd, rmfile
@@ -103,6 +103,102 @@ def test_includes_order(self):
103103
except AssertionError as e:
104104
raise SkipTest("Known failure -- included values are not in effect right away") from e
105105

106+
@with_rw_directory
107+
def test_case_insensitive_names(self, rw_dir):
108+
config_path = osp.join(rw_dir, "config")
109+
with open(config_path, "wb") as config_file:
110+
config_file.write(
111+
b"[core]\n\tBigName = 1\n"
112+
b"[CoRe]\n\tbigname = 2\n\tFlag\n"
113+
b'[REMOTE "Origin"]\n\tUrl = upper\n'
114+
b'[remote "origin"]\n\tURL = lower\n'
115+
)
116+
117+
with GitConfigParser(config_path) as config:
118+
for section in ("core", "CORE", "CoRe"):
119+
for option in ("BigName", "bigname", "BIGNAME"):
120+
self.assertTrue(config.has_section(section))
121+
self.assertTrue(config.has_option(section, option))
122+
self.assertEqual(config.get(section, option), "2")
123+
self.assertEqual(config.getint(section, option), 2)
124+
self.assertEqual(config.get_value(section, option), 2)
125+
self.assertEqual(config.get_values(section, option), [1, 2])
126+
self.assertIs(config.getboolean(section, "FLAG"), True)
127+
self.assertEqual(config.sections(), ["core", 'REMOTE "Origin"', 'remote "origin"'])
128+
self.assertEqual(config.items("CORE"), [("BigName", "2"), ("Flag", None)])
129+
self.assertEqual(config.items_all("CORE"), [("BigName", ["1", "2"]), ("Flag", [None])])
130+
self.assertIn("BigName", config.options("CORE"))
131+
self.assertEqual(config.get('remote "Origin"', "URL"), "upper")
132+
self.assertEqual(config.get('REMOTE "origin"', "url"), "lower")
133+
self.assertFalse(config.has_section('remote "ORIGIN"'))
134+
with self.assertRaises(cp.NoSectionError):
135+
config.get('remote "ORIGIN"', "url")
136+
137+
git_config = ["git", "config", "--file", config_path]
138+
self.assertEqual(subprocess.check_output(git_config + ["--get-all", "CORE.BIGNAME"]), b"1\n2\n")
139+
self.assertEqual(subprocess.check_output(git_config + ["--get", "remote.Origin.URL"]), b"upper\n")
140+
self.assertEqual(subprocess.check_output(git_config + ["--get", "REMOTE.origin.url"]), b"lower\n")
141+
142+
@with_rw_directory
143+
def test_case_insensitive_writes_preserve_spelling(self, rw_dir):
144+
config_path = osp.join(rw_dir, "config")
145+
content = b'[CoRe]\n\tBigName = 1\n[REMOTE "Origin"]\n\tUrl = upper\n'
146+
with open(config_path, "wb") as config_file:
147+
config_file.write(content)
148+
149+
with GitConfigParser(config_path, read_only=False) as config:
150+
config.set_value("core", "bigname", 1)
151+
with open(config_path, "rb") as config_file:
152+
self.assertEqual(config_file.read(), content)
153+
with self.assertRaises(cp.DuplicateSectionError):
154+
config.add_section("CORE")
155+
config.set("CORE", "BIGNAME", "3")
156+
config.add_value("core", "bigname", 4)
157+
config.set_value("CORE", "NewKey", "new")
158+
self.assertEqual(config.items_all("core"), [("BigName", ["3", "4"]), ("NewKey", ["new"])])
159+
self.assertEqual(config.get_values("CORE", "BIGNAME"), [3, 4])
160+
self.assertTrue(config.remove_option("CORE", "NEWKEY"))
161+
self.assertFalse(config.has_option("core", "newkey"))
162+
config.set_value("core", "newkey", "again")
163+
self.assertIn(("newkey", "again"), config.items("CORE"))
164+
self.assertTrue(config.remove_option("CORE", "NEWKEY"))
165+
config.rename_section('remote "Origin"', 'Remote "Other"')
166+
self.assertEqual(config.get('REMOTE "Other"', "URL"), "upper")
167+
self.assertTrue(config.remove_section('REMOTE "Other"'))
168+
169+
with open(config_path, "rb") as config_file:
170+
self.assertEqual(config_file.read(), b"[CoRe]\n\tBigName = 3\n\tBigName = 4\n")
171+
with GitConfigParser(config_path) as config:
172+
self.assertEqual(config.get_values("CORE", "bigname"), [3, 4])
173+
174+
@with_rw_directory
175+
def test_case_insensitive_includes_and_remotes(self, rw_dir):
176+
with Repo.init(rw_dir) as repo:
177+
config_path = osp.join(repo.git_dir, "config")
178+
with open(config_path, "ab") as config_file:
179+
config_file.write(
180+
b'[REMOTE "Origin"]\n\tURL = upper\n'
181+
b'[Remote "origin"]\n\tUrl = lower\n'
182+
b"[core]\n\tBigName = 1\n"
183+
b"[INCLUDE]\n\tPaTh = included\n"
184+
b'[INCLUDEIF "onbranch:*"]\n\tPATH = conditional\n'
185+
b'[INCLUDEIF "ONBRANCH:*"]\n\tpath = wrong-case\n'
186+
)
187+
for filename, content in (
188+
("included", b"[CORE]\n\tBIGNAME = 2\n"),
189+
("conditional", b"[core]\n\tBranchName = 3\n"),
190+
("wrong-case", b"[core]\n\tbigname = 4\n"),
191+
):
192+
with open(osp.join(repo.git_dir, filename), "wb") as config_file:
193+
config_file.write(content)
194+
195+
with repo.config_reader("repository") as config:
196+
self.assertEqual(config.get_values("CORE", "bigname"), [1, 2])
197+
self.assertEqual(config.get_value("CORE", "branchname"), 3)
198+
self.assertEqual([remote.name for remote in repo.remotes], ["Origin", "origin"])
199+
self.assertEqual(repo.remote("Origin").config_reader.get("url"), "upper")
200+
self.assertEqual(repo.remote("origin").config_reader.get("URL"), "lower")
201+
106202
@with_rw_directory
107203
def test_lock_reentry(self, rw_dir):
108204
fpl = osp.join(rw_dir, "l")
@@ -1119,6 +1215,9 @@ def test_setlast(self):
11191215
omd.setlast("key", "value1")
11201216
self.assertEqual(omd["key"], "value1")
11211217
self.assertEqual(omd.getall("key"), ["value1"])
1122-
omd.setlast("key", "value2")
1218+
omd.setlast("KEY", "value2")
11231219
self.assertEqual(omd["key"], "value2")
11241220
self.assertEqual(omd.getall("key"), ["value2"])
1221+
omd.clear()
1222+
omd.setall("KEY", ["value3"])
1223+
self.assertEqual(omd.items_all(), [("KEY", ["value3"])])

0 commit comments

Comments
 (0)