{"thread":{"id":"59894","subject":"BUG: config.c:129: kvi should not be set while parsing a config source","startedAt":"2023-06-21T23:46:00Z","lastAt":"2023-06-23T19:23:09Z","messageCount":3,"participants":["Jesse Kasky","Derrick Stolee","Glen Choo"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"478672","messageId":"CAJSLrw6qhHj8Kxrqhp7xN=imTHgg79QB9Fxa9XpdZYFnBKhkvA@mail.gmail.com","threadId":"59894","inReplyTo":null,"subject":"BUG: config.c:129: kvi should not be set while parsing a config source","fromName":"Jesse Kasky","fromEmail":"jkasky@slack-corp.com","sentAt":"2023-06-21T23:45:44Z","receivedAt":"2023-06-21T23:46:00Z","isPatch":false,"sender":{"key":"jkasky@slack-corp.com","avatar":null},"body":"Thank you for filling out a Git bug report!\nPlease answer the following questions to help us understand your issue.\n\nWhat did you do before the bug happened? (Steps to reproduce your issue)\n\ngit clone --no-checkout --sparse --filter=blob:none --depth=1 <repo> <dir>\ncd <dir>\ngit sparse-checkout add <dir1> <dir2>\ngit fetch --depth=1 origin <commit>\n\nWhat did you expect to happen? (Expected behavior)\n\nExpected the fetch to complete\n\nWhat happened instead? (Actual behavior)\n\nReceived the following error:\n\nBUG: config.c:129: kvi should not be set while parsing a config source\n[1]    5842 abort      /opt/homebrew/bin/git fetch --depth=1 origin\n\nWhat's different between what you expected and what actually happened?\n\nDid not expect to encounter an abort.\n\nAnything else you want to add:\n\nTwo items I came across while trying to troubleshoot:\nhttps://github.com/git/git/commit/9828453ff00b330c57daa3a8b672cbb5f0cdce34\nhttps://github.com/Homebrew/homebrew-cask-fonts/issues/7718\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.41.0\ncpu: arm64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Darwin 22.5.0 Darwin Kernel Version 22.5.0: Mon Apr 24 20:52:24\nPDT 2023; root:xnu-8796.121.2~5/RELEASE_ARM64_T6000 arm64\ncompiler info: clang: 14.0.3 (clang-1403.0.22.14.1)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]\n"},{"id":"478732","messageId":"6961779f-9fd8-28cb-6046-cc24b6869cbc@github.com","threadId":"59894","inReplyTo":"CAJSLrw6qhHj8Kxrqhp7xN=imTHgg79QB9Fxa9XpdZYFnBKhkvA@mail.gmail.com","subject":"Re: BUG: config.c:129: kvi should not be set while parsing a config source","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2023-06-23T13:36:53Z","receivedAt":"2023-06-23T13:37:02Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/21/2023 7:45 PM, Jesse Kasky wrote:\n> Thank you for filling out a Git bug report!\n> Please answer the following questions to help us understand your issue.\n> \n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n> git clone --no-checkout --sparse --filter=blob:none --depth=1 <repo> <dir>\n\nI'll note the oddity of using a blobless clone filter alongside a\nshallow clone. This is something we are not super-likely to do\nvery often, so I could see how it is skipped by our test suite.\n\n> cd <dir>\n> git sparse-checkout add <dir1> <dir2>\n> git fetch --depth=1 origin <commit>\n\nEspecially this fetch after the clone. Shallow clones are better to\nonly clone, then throw away. Don't fetch from them again. That said,\nwe shouldn't be hitting a BUG statement.\n\n> Received the following error:\n> \n> BUG: config.c:129: kvi should not be set while parsing a config source\n> [1]    5842 abort      /opt/homebrew/bin/git fetch --depth=1 origin\n\nI've come across this error while playing around with things in\nthe config space, and the case I figured out was due to nested\niterations over config.\n\nIn my case, I was adding dynamic config loading when certain\nglobal variables were used, and some were used during config\nparsing causing this nesting.\n\nThat's where I would start investigating, but don't have time to\ndig in more than to contribute my experience with this BUG\nstatement.\n\n> Two items I came across while trying to troubleshoot:\n> https://github.com/git/git/commit/9828453ff00b330c57daa3a8b672cbb5f0cdce34\n\nI have CC'd Glen, the author of that commit, and an experienced\ncontributor in this space.\n\nThanks,\n-Stolee\n"},{"id":"478740","messageId":"kl6lh6qyrnjm.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"59894","inReplyTo":"6961779f-9fd8-28cb-6046-cc24b6869cbc@github.com","subject":"Re: BUG: config.c:129: kvi should not be set while parsing a config source","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2023-06-23T19:22:53Z","receivedAt":"2023-06-23T19:23:09Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Thanks Jesse for the bug report, and thanks Stolee for looping me in!\n\nDerrick Stolee <derrickstolee@github.com> writes:\n\n> On 6/21/2023 7:45 PM, Jesse Kasky wrote:\n>> Thank you for filling out a Git bug report!\n>> Please answer the following questions to help us understand your issue.\n>> \n>> What did you do before the bug happened? (Steps to reproduce your issue)\n>> \n>> git clone --no-checkout --sparse --filter=blob:none --depth=1 <repo> <dir>\n>> cd <dir>\n>> git sparse-checkout add <dir1> <dir2>\n>> git fetch --depth=1 origin <commit>\n>> Received the following error:\n>> \n>> BUG: config.c:129: kvi should not be set while parsing a config source\n>> [1]    5842 abort      /opt/homebrew/bin/git fetch --depth=1 origin\n\nIt took me some fiddling around to find a reproducing case, because, as\nit turns out, the choice of repo is important. This bug occurs when you\nhave submodules + partial clone, _and_ .gitmodules has not been fetched\nyet (e.g. because --no-checkout skipped the fetch).\n\nE.g. you can see this for yourself with:\n\n  rm -fr test-breakage &&\n  git clone --filter=blob:none --no-checkout https://github.com/git/git test-breakage &&\n  cd test-breakage &&\n  git fetch\n\nThis BUG() assertion is meant to guard against reading config in an\nunsafe nested way - the information about the config source we are\nreading (i.e. the key_value_info/kvi) is global state in the config\nmachinery, and certain kinds of nesting will produce undefined\nbehavior if we try to read it.\n\nWhat happens is that \"git fetch\" tries to read .gitmodules, which\ntriggers a partial clone fetch because .gitmodules is missing. But to do\nthis, the partial clone code reads config to figure out where to fetch\nfrom, leading to a nested config-read-in-a-config-read.\n\nFor Git devs: the 'unsafe' thing is reading a config set while reading a\nconfig file/params/blob/etc. In this case, we are trying to read\nrepo_config() (the config set in the_repository->config) while reading\n.gitmodules from a blob. Reading from files while reading files is safe\n(e.g. that's what we do when evaluating includes).\n\nThis should go away in my in-flight series:\n\n  https://lore.kernel.org/git/pull.1497.v3.git.git.1687290231.gitgitgadget@gmail.com\n\nbecause that removes all of the problematic global state. That's a very\nbig refactor, so I'm not sure we should wait until that goes in to fix\nthis bug. And while I'd like to fix this bug now, I don't see a point in\nmaking the existing code 'safer' by making small changes in the\nmachinery that will just go away anyway.\n\nSo I think the best fix is for me to get rid of the BUG() check and go\nback to the 'unsafe' behavior. It won't matter in this specific scenario\n(since we don't actually need the config source information AFAICT), and\nat least it doesn't make _other_ use cases worse off than before.\n\n> I've come across this error while playing around with things in\n> the config space, and the case I figured out was due to nested\n> iterations over config.\n>\n> In my case, I was adding dynamic config loading when certain\n> global variables were used, and some were used during config\n> parsing causing this nesting.\n\nPresumably this is how you were coming across this bug too: since your\nexperiments added config set lookups, maybe some of them were happening\nin the middle of reading a config file.\n"}]}