{"thread":{"id":"49304","subject":"git silently ignores include directive with single quotes","startedAt":"2018-09-08T19:00:35Z","lastAt":"2018-09-25T22:03:10Z","messageCount":37,"participants":["Stas Bekman","Martin Ågren","Ævar Arnfjörð Bjarmason","Jeff King","Ramsay Jones","Paul Smith","Junio C Hamano","Jonathan Nieder","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"357708","messageId":"ca2b192e-1722-092e-2c54-d79d21a66ba2@stason.org","threadId":"49304","inReplyTo":null,"subject":"git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T18:58:47Z","receivedAt":"2018-09-08T19:00:35Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"Hi,\n\nOne of the windows users discovered this bug, and I was able to\nreproduce it on linux.\n\nWe are using a custom content filter configuration REPO/.gitconfig which\nneeds to be enabled inside REPO/.git/config:\n\nThis works:\n\n[include]\n path = ../.gitconfig\n\nThis doesn’t:\n\n[include]\n path = '../.gitconfig'\n\nNotice the single quotes around the filename. When this is the case git\nsilently (!) ignores the custom configuration, which is clearly a bug.\n\nI found the easiest to debug this is by using:\n\ngit config --list --show-origin\n\nIn the former case it shows the custom config, in the latter it does not.\n\nYet, git gives no indication of any errors, not even with GIT_TRACE and\nother debug vars.\n\nThe original problem cropped up due to using:\n\n git config --local include.path '../.gitconfig'\n\nwhich on linux stripped the single quotes, but on some windows git bash\nemulation it kept them.\n\nWhat am I suggesting is that git:\n\n(1) should complain if it encounters an invalid configuration and not\nsilently ignore it. It took quite some effort and time to figure the\nculprit.\n\n(2) probably allow the quoted location of the file, but it's much less\nimportant, as it's easy to rectify once git gives user #1\n\nI don't have the details about the windows user setup, but I was able to\nreproduce this bug with git version 2.17.1 on linux.\n\nThank you.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357709","messageId":"CAN0heSroxfcwiJaVgGFTweq=XKAgGsR-E6SeOgsG4m0rzK4dHQ@mail.gmail.com","threadId":"49304","inReplyTo":"ca2b192e-1722-092e-2c54-d79d21a66ba2@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-09-08T19:30:24Z","receivedAt":"2018-09-08T19:33:29Z","isPatch":false,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"Hi Stas\n\nOn Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n> [include]\n> path = '../.gitconfig'\n>\n> Notice the single quotes around the filename. When this is the case git\n> silently (!) ignores the custom configuration, which is clearly a bug.\n\nThanks for reporting and describing out your expectations and what you\nobserved.\n\nActually, there is a test explicitly testing that 'missing include files\nare ignored'. I couldn't find a motivation for this in 9b25a0b52e\n(config: add include directive, 2012-02-06).\n\n> The original problem cropped up due to using:\n>\n> git config --local include.path '../.gitconfig'\n>\n> which on linux stripped the single quotes, but on some windows git bash\n> emulation it kept them.\n\nHuh, I wouldn't have expected them to be kept. You learn something\nnew every day...\n\n> What am I suggesting is that git:\n>\n> (1) should complain if it encounters an invalid configuration and not\n> silently ignore it. It took quite some effort and time to figure the\n> culprit.\n\nSounds reasonable to me, but I might be missing something. I'm cc-ing\nthe original author. Maybe he can recall why he made sure it silently\nignores missing files.\n\n> (2) probably allow the quoted location of the file, but it's much less\n> important, as it's easy to rectify once git gives user #1\n\nI don't think this will work. Allowing quoting for just this one item,\nor for all? Any and all quoting or just at the first and last character?\nWhat about those config items where quotes might legitimately occur,\ni.e., we'd need some escaping? Actually, something like '.gitconfig'\n*with* *those* *quotes* is a valid filename on my machine.\n\nThank you for reporting.\n\nMartin\n"},{"id":"357710","messageId":"2824cc17-b4a4-8821-331f-1768246f2e6b@stason.org","threadId":"49304","inReplyTo":"CAN0heSroxfcwiJaVgGFTweq=XKAgGsR-E6SeOgsG4m0rzK4dHQ@mail.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T19:44:30Z","receivedAt":"2018-09-08T19:44:35Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 12:30 PM, Martin Ågren wrote:\n\n> Actually, there is a test explicitly testing that 'missing include files\n> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n> (config: add include directive, 2012-02-06).\n\nThank you for the follow up, Martin. And discovering that it is by design.\n\nI suppose this could have been done to optimize run-time performance.\nBut there must be a way for a user to validate their custom\nconfiguration. So perhaps there should be a specific directive to do so?\nOne could argue that:\n\n git config --list --show-origin\n\ndoes exactly that. Except it should probably also indicate that some\nconfiguration file or parts of were ignored - and clearly indicate the\nexact nature of the problem. In which case it'd be sufficient.\n\n>> (2) probably allow the quoted location of the file, but it's much less\n>> important, as it's easy to rectify once git gives user #1\n> \n> I don't think this will work. Allowing quoting for just this one item,\n> or for all? Any and all quoting or just at the first and last character?\n> What about those config items where quotes might legitimately occur,\n> i.e., we'd need some escaping? Actually, something like '.gitconfig'\n> *with* *those* *quotes* is a valid filename on my machine.\n\nLet's ignore this sub-issue for now. If we can get git to report when\nsomething is mis-configured, this issue can then be easily resolved.\n\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357711","messageId":"a76c94c6-9fd7-4ed0-be2d-6fc1d021f476@stason.org","threadId":"49304","inReplyTo":"CAN0heSroxfcwiJaVgGFTweq=XKAgGsR-E6SeOgsG4m0rzK4dHQ@mail.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T19:53:55Z","receivedAt":"2018-09-08T19:54:00Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 12:30 PM, Martin Ågren wrote:\n> Hi Stas\n> \n> On Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n>> [include]\n>> path = '../.gitconfig'\n\n> Actually, there is a test explicitly testing that 'missing include files\n> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n> (config: add include directive, 2012-02-06).\n\nAnd also to stress out, that the file is not missing. At least in the\nworld of unix, in particular its many shells, - command line arguments\n\"xyz\", 'xyz', xyz are often deemed to be the same if there are no spaces\nin the word. So that's why it took us a lot of trial and error to even\nconsider the quotes in '../.gitconfig' as a problem. While git deems it\ndifferent, to me:\n\n path = '../.gitconfig'\n path = \"../.gitconfig\"\n path = ../.gitconfig\n\nappear to be the \"same\". So git needs to have a way to say otherwise.\n\nI realize I am going back to the issue of quoting here, after suggesting\nto ignore it. So to clarify I'm bringing it up only in the context of\nwanting git to tell the user what it wants, and not necessarily asking\nto support all the possible ways one could quote a filepath.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357712","messageId":"87bm97rcih.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"CAN0heSroxfcwiJaVgGFTweq=XKAgGsR-E6SeOgsG4m0rzK4dHQ@mail.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-08T19:54:14Z","receivedAt":"2018-09-08T19:54:19Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 08 2018, Martin Ågren wrote:\n\n> Hi Stas\n>\n> On Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n>> [include]\n>> path = '../.gitconfig'\n>>\n>> Notice the single quotes around the filename. When this is the case git\n>> silently (!) ignores the custom configuration, which is clearly a bug.\n>\n> Thanks for reporting and describing out your expectations and what you\n> observed.\n>\n> Actually, there is a test explicitly testing that 'missing include files\n> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n> (config: add include directive, 2012-02-06).\n>\n>> The original problem cropped up due to using:\n>>\n>> git config --local include.path '../.gitconfig'\n>>\n>> which on linux stripped the single quotes, but on some windows git bash\n>> emulation it kept them.\n>\n> Huh, I wouldn't have expected them to be kept. You learn something\n> new every day...\n>\n>> What am I suggesting is that git:\n>>\n>> (1) should complain if it encounters an invalid configuration and not\n>> silently ignore it. It took quite some effort and time to figure the\n>> culprit.\n>\n> Sounds reasonable to me, but I might be missing something. I'm cc-ing\n> the original author. Maybe he can recall why he made sure it silently\n> ignores missing files.\n>\n>> (2) probably allow the quoted location of the file, but it's much less\n>> important, as it's easy to rectify once git gives user #1\n>\n> I don't think this will work. Allowing quoting for just this one item,\n> or for all? Any and all quoting or just at the first and last character?\n> What about those config items where quotes might legitimately occur,\n> i.e., we'd need some escaping? Actually, something like '.gitconfig'\n> *with* *those* *quotes* is a valid filename on my machine.\n\nThe reason missing includes are ignored is that the way this is expected\nto be used is e.g.:\n\n [include]\n path ~/.gitconfig.work\n\nWhere .gitconfig.work is some configuration you're going to drop into\nplace on your $dayjob servers, but not on your personal machine, even\nthough you sync the same ~/.gitconfig everywhere.\n\nA lot of people who use includes rely on this, but I see from this\nthread this should be better documented.\n\nIf we were to make nonexisting files an error, we'd need something like\nan extension of the includeIf syntax added in 3efd0bedc6 (\"config: add\nconditional include\", 2017-03-01) 3efd0bedc6 (\"config: add conditional\ninclude\", 2017-03-01). I.e.:\n\n [includeIfcond \"test -e ~/.gitconfig.work\"]\n path = ~/.gitconfig.work\n\nOr something like that, this is getting increasingly harder to shove\ninto the *.ini config syntax.\n"},{"id":"357713","messageId":"87a7orrc3w.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"a76c94c6-9fd7-4ed0-be2d-6fc1d021f476@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-08T20:02:59Z","receivedAt":"2018-09-08T20:03:06Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 08 2018, Stas Bekman wrote:\n\n> On 2018-09-08 12:30 PM, Martin Ågren wrote:\n>> Hi Stas\n>>\n>> On Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n>>> [include]\n>>> path = '../.gitconfig'\n>\n>> Actually, there is a test explicitly testing that 'missing include files\n>> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n>> (config: add include directive, 2012-02-06).\n>\n> And also to stress out, that the file is not missing. At least in the\n> world of unix, in particular its many shells, - command line arguments\n> \"xyz\", 'xyz', xyz are often deemed to be the same if there are no spaces\n> in the word. So that's why it took us a lot of trial and error to even\n> consider the quotes in '../.gitconfig' as a problem. While git deems it\n> different, to me:\n>\n> path = '../.gitconfig'\n> path = \"../.gitconfig\"\n> path = ../.gitconfig\n>\n> appear to be the \"same\". So git needs to have a way to say otherwise.\n>\n> I realize I am going back to the issue of quoting here, after suggesting\n> to ignore it. So to clarify I'm bringing it up only in the context of\n> wanting git to tell the user what it wants, and not necessarily asking\n> to support all the possible ways one could quote a filepath.\n\nAside from other issues here, in the \"wold of unix\" (not that we only\nuse the git config syntax on those sort of systems) you can't assume\nthat just because some quoting construct works in the shell, that it\nworks the same way in some random config format. If you look in your\n/etc/ you'll find plenty of config formats where you can't use single,\ndouble and no quotes interchangeably, so I don't see what hte confusion\nis with that particular aspect of this.\n\nAlthough as I mentioned in <87bm97rcih.fsf@evledraar.gmail.com> the fact\nthat we ignore missing includes definitely needs to be documented, but\nthat our quoting constructs in our config format behave like they do in\nPOSIX shells I see as a non-issue.\n"},{"id":"357714","messageId":"c0844b98-0fee-9fbd-fedb-883ed88c3ac6@stason.org","threadId":"49304","inReplyTo":"87bm97rcih.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T20:04:37Z","receivedAt":"2018-09-08T20:04:41Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 12:54 PM, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sat, Sep 08 2018, Martin Ågren wrote:\n> \n>> Hi Stas\n>>\n>> On Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n>>> [include]\n>>> path = '../.gitconfig'\n>>>\n>>> Notice the single quotes around the filename. When this is the case git\n>>> silently (!) ignores the custom configuration, which is clearly a bug.\n>>\n>> Thanks for reporting and describing out your expectations and what you\n>> observed.\n>>\n>> Actually, there is a test explicitly testing that 'missing include files\n>> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n>> (config: add include directive, 2012-02-06).\n>>\n>>> The original problem cropped up due to using:\n>>>\n>>> git config --local include.path '../.gitconfig'\n>>>\n>>> which on linux stripped the single quotes, but on some windows git bash\n>>> emulation it kept them.\n>>\n>> Huh, I wouldn't have expected them to be kept. You learn something\n>> new every day...\n>>\n>>> What am I suggesting is that git:\n>>>\n>>> (1) should complain if it encounters an invalid configuration and not\n>>> silently ignore it. It took quite some effort and time to figure the\n>>> culprit.\n>>\n>> Sounds reasonable to me, but I might be missing something. I'm cc-ing\n>> the original author. Maybe he can recall why he made sure it silently\n>> ignores missing files.\n>>\n>>> (2) probably allow the quoted location of the file, but it's much less\n>>> important, as it's easy to rectify once git gives user #1\n>>\n>> I don't think this will work. Allowing quoting for just this one item,\n>> or for all? Any and all quoting or just at the first and last character?\n>> What about those config items where quotes might legitimately occur,\n>> i.e., we'd need some escaping? Actually, something like '.gitconfig'\n>> *with* *those* *quotes* is a valid filename on my machine.\n> \n> The reason missing includes are ignored is that the way this is expected\n> to be used is e.g.:\n> \n> [include]\n> path ~/.gitconfig.work\n> \n> Where .gitconfig.work is some configuration you're going to drop into\n> place on your $dayjob servers, but not on your personal machine, even\n> though you sync the same ~/.gitconfig everywhere.\n\nThank you for clarifying why this is done silently, Ævar. It makes sense\nthen.\n\n> If we were to make nonexisting files an error, we'd need something like\n> an extension of the includeIf syntax added in 3efd0bedc6 (\"config: add\n> conditional include\", 2017-03-01) 3efd0bedc6 (\"config: add conditional\n> include\", 2017-03-01). I.e.:\n> \n> [includeIfcond \"test -e ~/.gitconfig.work\"]\n> path = ~/.gitconfig.work\n> \n> Or something like that, this is getting increasingly harder to shove\n> into the *.ini config syntax.\n\nThis suggestion won't solve the real problem. The real problem is that\ngit can't find '.gitconfig' even though it's there, due to single quotes\naround the filepath. So the suggested check will still ignore the\nconfiguration even if it's there.\n\nThis also leads me to think what if the include path has spaces in it?\n\n path = ~/somewhere on my system/.gitconfig.work\n\nmost people would assume quotes are needed around the filepath.\n\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357715","messageId":"acf93aef-f1f8-1aab-a16d-9655402d445f@stason.org","threadId":"49304","inReplyTo":"87a7orrc3w.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T20:13:00Z","receivedAt":"2018-09-08T20:13:04Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 01:02 PM, Ævar Arnfjörð Bjarmason wrote:\n\n> Aside from other issues here, in the \"wold of unix\" (not that we only\n> use the git config syntax on those sort of systems) you can't assume\n> that just because some quoting construct works in the shell, that it\n> works the same way in some random config format. If you look in your\n> /etc/ you'll find plenty of config formats where you can't use single,\n> double and no quotes interchangeably, so I don't see what hte confusion\n> is with that particular aspect of this.\n> \n> Although as I mentioned in <87bm97rcih.fsf@evledraar.gmail.com> the fact\n> that we ignore missing includes definitely needs to be documented, but\n> that our quoting constructs in our config format behave like they do in\n> POSIX shells I see as a non-issue.\n\nI agree that I should make no such assumptions. Thank you. But it is a\ncross-platform problem. I remind that the original problem came from a\nsimple command:\n\n git config --local include.path '../.gitconfig'\n\nWhich on linux removed the quotes and all was fine, and on windows the\nsame command kept the quotes and the user was tearing his hair out\ntrying to understand why the custom config was ignored.\n\nSo you can say, don't use the quotes in first place. But what if you have:\n\n git config --local include.path 'somewhere on the system/.gitconfig'\n\nyou have to use single or double quotes inside the shell to keep it as a\nsingle argument, yet on some windows set ups it'll result in git\nignoring this configuration directive, as the quotes will end up in git\nconfig file.\n\nI'd say at the very least 'git config' could have an option\n--verify-path or something similar and for it to validate that the path\nis there exactly as it adds it to .git/config at the time of running\nthis command to help the user debug the situation. Of course this won't\nhelp if .git/config is modified manually. But it's a step towards\nsupporting users.\n\nI hope this clarifies the situation.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357716","messageId":"878t4brbgn.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"c0844b98-0fee-9fbd-fedb-883ed88c3ac6@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-08T20:16:56Z","receivedAt":"2018-09-08T20:17:03Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 08 2018, Stas Bekman wrote:\n\n> On 2018-09-08 12:54 PM, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Sat, Sep 08 2018, Martin Ågren wrote:\n>>\n>>> Hi Stas\n>>>\n>>> On Sat, 8 Sep 2018 at 21:00, Stas Bekman wrote:\n>>>> [include]\n>>>> path = '../.gitconfig'\n>>>>\n>>>> Notice the single quotes around the filename. When this is the case git\n>>>> silently (!) ignores the custom configuration, which is clearly a bug.\n>>>\n>>> Thanks for reporting and describing out your expectations and what you\n>>> observed.\n>>>\n>>> Actually, there is a test explicitly testing that 'missing include files\n>>> are ignored'. I couldn't find a motivation for this in 9b25a0b52e\n>>> (config: add include directive, 2012-02-06).\n>>>\n>>>> The original problem cropped up due to using:\n>>>>\n>>>> git config --local include.path '../.gitconfig'\n>>>>\n>>>> which on linux stripped the single quotes, but on some windows git bash\n>>>> emulation it kept them.\n>>>\n>>> Huh, I wouldn't have expected them to be kept. You learn something\n>>> new every day...\n>>>\n>>>> What am I suggesting is that git:\n>>>>\n>>>> (1) should complain if it encounters an invalid configuration and not\n>>>> silently ignore it. It took quite some effort and time to figure the\n>>>> culprit.\n>>>\n>>> Sounds reasonable to me, but I might be missing something. I'm cc-ing\n>>> the original author. Maybe he can recall why he made sure it silently\n>>> ignores missing files.\n>>>\n>>>> (2) probably allow the quoted location of the file, but it's much less\n>>>> important, as it's easy to rectify once git gives user #1\n>>>\n>>> I don't think this will work. Allowing quoting for just this one item,\n>>> or for all? Any and all quoting or just at the first and last character?\n>>> What about those config items where quotes might legitimately occur,\n>>> i.e., we'd need some escaping? Actually, something like '.gitconfig'\n>>> *with* *those* *quotes* is a valid filename on my machine.\n>>\n>> The reason missing includes are ignored is that the way this is expected\n>> to be used is e.g.:\n>>\n>> [include]\n>> path ~/.gitconfig.work\n>>\n>> Where .gitconfig.work is some configuration you're going to drop into\n>> place on your $dayjob servers, but not on your personal machine, even\n>> though you sync the same ~/.gitconfig everywhere.\n>\n> Thank you for clarifying why this is done silently, Ævar. It makes sense\n> then.\n>\n>> If we were to make nonexisting files an error, we'd need something like\n>> an extension of the includeIf syntax added in 3efd0bedc6 (\"config: add\n>> conditional include\", 2017-03-01) 3efd0bedc6 (\"config: add conditional\n>> include\", 2017-03-01). I.e.:\n>>\n>> [includeIfcond \"test -e ~/.gitconfig.work\"]\n>> path = ~/.gitconfig.work\n>>\n>> Or something like that, this is getting increasingly harder to shove\n>> into the *.ini config syntax.\n>\n> This suggestion won't solve the real problem. The real problem is that\n> git can't find '.gitconfig' even though it's there, due to single quotes\n> around the filepath. So the suggested check will still ignore the\n> configuration even if it's there.\n\n...because that's not how the *.ini syntax works. That means to look up\na file called '.gitconfig', as opposed to .gitconfig, ie. one that\nactually starts with a single quote. On POSIX systems filenames can\ninclude all bytes except \\0, so we need some way to include those.\n\nI've just created a 'foo' file (i.e. one that has a 5-chararcer name,\nincluding single quotes), and including it via git's config works, as\nopposed to the filename foo (i.e. the three-character version).\n\nI can see how this is confusing, but we can't have some way to have this\n\"ignore missing\" feature and warn about stuff like 'foo' v.s. \"foo\"\nv.s. foo without carrying some list of quoting constructs deemed to be\nconfusing, and forbidding includes from files that look like that.\n\n> This also leads me to think what if the include path has spaces in it?\n>\n> path = ~/somewhere on my system/.gitconfig.work\n>\n> most people would assume quotes are needed around the filepath.\n"},{"id":"357717","messageId":"877ejvraxh.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"acf93aef-f1f8-1aab-a16d-9655402d445f@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-08T20:28:26Z","receivedAt":"2018-09-08T20:28:32Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 08 2018, Stas Bekman wrote:\n\n> On 2018-09-08 01:02 PM, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Aside from other issues here, in the \"wold of unix\" (not that we only\n>> use the git config syntax on those sort of systems) you can't assume\n>> that just because some quoting construct works in the shell, that it\n>> works the same way in some random config format. If you look in your\n>> /etc/ you'll find plenty of config formats where you can't use single,\n>> double and no quotes interchangeably, so I don't see what hte confusion\n>> is with that particular aspect of this.\n>>\n>> Although as I mentioned in <87bm97rcih.fsf@evledraar.gmail.com> the fact\n>> that we ignore missing includes definitely needs to be documented, but\n>> that our quoting constructs in our config format behave like they do in\n>> POSIX shells I see as a non-issue.\n>\n> I agree that I should make no such assumptions. Thank you. But it is a\n> cross-platform problem. I remind that the original problem came from a\n> simple command:\n>\n> git config --local include.path '../.gitconfig'\n>\n> Which on linux removed the quotes and all was fine, and on windows the\n> same command kept the quotes and the user was tearing his hair out\n> trying to understand why the custom config was ignored.\n>\n> So you can say, don't use the quotes in first place. But what if you have:\n>\n> git config --local include.path 'somewhere on the system/.gitconfig'\n>\n> you have to use single or double quotes inside the shell to keep it as a\n> single argument, yet on some windows set ups it'll result in git\n> ignoring this configuration directive, as the quotes will end up in git\n> config file.\n>\n> I'd say at the very least 'git config' could have an option\n> --verify-path or something similar and for it to validate that the path\n> is there exactly as it adds it to .git/config at the time of running\n> this command to help the user debug the situation. Of course this won't\n> help if .git/config is modified manually. But it's a step towards\n> supporting users.\n>\n> I hope this clarifies the situation.\n\nYeah, some version of this is sensible. There's at least a doc patch in\nhere somewhere, if not some \"warn if missing\" mode.\n\nSo don't take any of this as minimizing that aspect of your bug report.\n\n*But*\n\nThere's just no way that \"git\" the tool can somehow in a sane way rescue\nyou from knowing the quoting rules of the shell on your system, which\ndiffer wildly between the likes of Windows and Linux.\n\nWe guarantee that if you pass us the string \"foo\" it'll work the same\n(for the purposes of config syntax, and most other things on all\nsystems).\n\nWe can't guarantee that just because on one system/shell \"foo\" means the\nsame as 'foo' when you type it into the terminal, but others it doesn't\nthat we'll treat it the same way, it's ultimately up to you to know the\nquoting rules of your system shell.\n\nOn linux/bash I can also do \"git config foo.bar <(some-command)\", and\nthere's some systems where that'll be passed in as though (on\nlinux/bash) we'd passed in:\n\n git config foo.bar \"<(some-command)\"\n\nWhat are we supposed to do with that? In particular in the case where\n\"foo.bar\" is supposed to point to a valid filename, and\n\"<(some-command)\" *is* a valid filename?\n"},{"id":"357718","messageId":"b1658700-dbec-f354-4979-5c3ab341af17@stason.org","threadId":"49304","inReplyTo":"877ejvraxh.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T20:58:59Z","receivedAt":"2018-09-08T20:59:04Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 01:28 PM, Ævar Arnfjörð Bjarmason wrote:\n[...]\n> Yeah, some version of this is sensible. There's at least a doc patch in\n> here somewhere, if not some \"warn if missing\" mode.\n> \n> So don't take any of this as minimizing that aspect of your bug report.\n> \n> *But*\n> \n> There's just no way that \"git\" the tool can somehow in a sane way rescue\n> you from knowing the quoting rules of the shell on your system, which\n> differ wildly between the likes of Windows and Linux.\n\nI understand. All your explanations are perfectly reasonable, Ævar.\nThank you.\n\nYet, there needs to be some way for a user to know that git ignored\nsomething if their configuration doesn't work as expected.\n\n1) I suggest this is done via:\n\n git config --list --show-origin\n\nwhere the new addition would be to also show configuration parts that\nare not active and indicating why it is so.\n\nSo for example currently I get on a valid configuration setup and having\ngit/../.gitconfig in place the following output:\n\n[...]\nfile:/home/stas/.gitconfig mergetool.prompt=false\n[...]\nfile:.git/config include.path=../.gitconfig\n[...]\nfile:.git/../.gitconfig\nfilter.fastai-nbstripout-code.clean=tools/fastai-nbstripout\n[...]\n\nNow, if include.path=../.gitconfig is there and file:.git/../.gitconfig\nis not found, it will indicate that in some way that stands out for the\nuser. Perhaps:\n\n[...]\nfile:/home/stas/.gitconfig mergetool.prompt=false\n[...]\nfile:.git/config include.path=../.gitconfig\n[...]\nfile:.git/../.gitconfig FILE NOT FOUND! Ignored configuration\n[...]\n\nSo that would allow things to work as before, but now we have a way to\ndebug user-side configuration. And of course hoping that the docs would\nindicate that method for debugging configuration problems.\n\nI hope this is a reasonable suggestion that doesn't require any\nmodification on the users' part who rely on this silent ignoring\n\"feature\", yet lending to a configuration debug feature.\n\n2) And a secondary suggestion I mentioned earlier is to also have a flag\nfor git config to validate the path as it is being configured:\n\n git config --local include.path '../.gitconfig' --validate-path\n\nso that on shells that deal with quoting differently, than what git\nexpects, this git command will fail saying:\n\nerror: can't find file:.git/'../.gitconfig'\n\nor at the very least give a warning if we don't want it be fatal. Though\nI see no problem with it being fatal if a user uses a special flag.\n\nI made this second suggestion since it will help users to detect the\nproblem early on. Before they need to search for another debug solution\nsuch as the first one suggested in this email.\n\n3) Finally, it'd be useful to have GIT_TRACE=1 state that so and so\ninclude path wasn't found and was ignored during various 'git whatever'\ncommands.\n\nI am open to any or all of these solutions, or alternative suggestions\nof course.\n\nThank you.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357719","messageId":"20180908211436.GA31560@sigill.intra.peff.net","threadId":"49304","inReplyTo":"87bm97rcih.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-08T21:14:37Z","receivedAt":"2018-09-08T21:14:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 08, 2018 at 09:54:14PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> The reason missing includes are ignored is that the way this is expected\n> to be used is e.g.:\n> \n> [include]\n> path ~/.gitconfig.work\n> \n> Where .gitconfig.work is some configuration you're going to drop into\n> place on your $dayjob servers, but not on your personal machine, even\n> though you sync the same ~/.gitconfig everywhere.\n> \n> A lot of people who use includes rely on this, but I see from this\n> thread this should be better documented.\n\nRight, this was an intentional choice at the time the feature was added,\nto support this kind of feature. I'd note also that it mirrors other\nmisspelled keys. E.g.:\n\n [include]\n psth = whatever\n\nwill also not generate an error. This is also intentional, for two\nreasons:\n\n 1. Git's config format has always been designed to carry extra keys\n used by third-party scripts and porcelain. So we don't actually\n know the complete set of valid keys. (Though you could make an\n argument that git-core could stake out include.* as its own).\n\n 2. It makes using multiple git versions easier in some ways (though\n also harder in others). A config key that isn't known to the\n current version will be quietly ignored.\n\nOf course those things mean that true spelling mistakes are harder to\ncatch as such, because Git doesn't know that's what they are. And here\nI'm talking config _keys_, not values. So I'm just explaining the\nphilosophical thinking that led to the \"missing file is a silent noop\".\nIt doesn't _have_ to behave the same.\n\nThat said, it _does_ behave the same and people are likely depending on\nit at this point. So if we introduce a warning, for example, there needs\nto be some way to suppress it.\n\nProbably:\n\n [include]\n warnOnMissing = false\n path = ...\n\nwould be enough (with the default being \"true\").\n\nYou could even do:\n\n [include]\n warnOnMissing = false\n path = one\n warnOnMissing = true\n path = two\n\nto treat two includes differently (though I'm not sure why you would\nwant to).\n\n> If we were to make nonexisting files an error, we'd need something like\n> an extension of the includeIf syntax added in 3efd0bedc6 (\"config: add\n> conditional include\", 2017-03-01) 3efd0bedc6 (\"config: add conditional\n> include\", 2017-03-01). I.e.:\n> \n> [includeIfcond \"test -e ~/.gitconfig.work\"]\n> path = ~/.gitconfig.work\n> \n> Or something like that, this is getting increasingly harder to shove\n> into the *.ini config syntax.\n\nI think it would be simpler to just introduce a new key that's a variant\nof \"path\". Like:\n\n [include]\n maybePath = ~/.gitconfig.work\n\nThough if it really is just a warning, the \"warnOnMissing\" above would\nmake that unnecessary (and it also scales better if we have to end up\nadding more behavior tweaks in the future).\n\n-Peff\n"},{"id":"357720","messageId":"20180908212256.GB31560@sigill.intra.peff.net","threadId":"49304","inReplyTo":"ca2b192e-1722-092e-2c54-d79d21a66ba2@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-08T21:22:57Z","receivedAt":"2018-09-08T21:23:00Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 08, 2018 at 11:58:47AM -0700, Stas Bekman wrote:\n\n> This doesn’t:\n> \n> [include]\n> path = '../.gitconfig'\n\nSo I think it's been covered elsewhere that single quotes aren't a thing\nin git's config format. I will say that this was actually a minor\nsurprise to me, after a decade of working with the format. ;)\n\nI don't know if it's worth changing now or not It would be\nbackwards-incompatible, but I wonder if we could do it in a sane way.\nE.g., with a rule like:\n\n - if the first non-whitespace character of the value is a\n single-quote, assume the value is quoted and apply normal shell\n rules (i.e., no backslash escapes until the ending single-quote)\n\n - otherwise, single-quotes are not special at all\n\nThat would allow things like:\n\n [diff \"foo\"]\n textconv = some_shell_hackery 'with quotes' | foo\n\nto continue working, but make:\n\n [some]\n path = 'this has \"double quotes\" in it!'\n\ndo what the user probably intended. It would be a regression for anybody\nwho literally has a value that starts with a single-quote, but that\nseems like it would be pretty rare. Or I dunno, maybe people do it on\nWindows to try to protect path-names that get interpreted by the shell.\n\n> The original problem cropped up due to using:\n> \n> git config --local include.path '../.gitconfig'\n> \n> which on linux stripped the single quotes, but on some windows git bash\n> emulation it kept them.\n\nThat sounds like a bug in git bash, if it is not treating single quotes\nin the usual shell way. But I'd also expect such a bug to cause loads of\nproblems in all of the shell scripts. Are you sure it wasn't cmd.exe or\nsome other interpreter?\n\n-Peff\n"},{"id":"357721","messageId":"ad56c575-1211-61d2-daed-5b0da61db738@ramsayjones.plus.com","threadId":"49304","inReplyTo":"20180908211436.GA31560@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2018-09-08T22:10:44Z","receivedAt":"2018-09-08T22:17:20Z","isPatch":false,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 08/09/18 22:14, Jeff King wrote:\n> On Sat, Sep 08, 2018 at 09:54:14PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n>> The reason missing includes are ignored is that the way this is expected\n>> to be used is e.g.:\n>>\n>> [include]\n>> path ~/.gitconfig.work\n>>\n>> Where .gitconfig.work is some configuration you're going to drop into\n>> place on your $dayjob servers, but not on your personal machine, even\n>> though you sync the same ~/.gitconfig everywhere.\n>>\n>> A lot of people who use includes rely on this, but I see from this\n>> thread this should be better documented.\n> \n> Right, this was an intentional choice at the time the feature was added,\n> to support this kind of feature. I'd note also that it mirrors other\n> misspelled keys. E.g.:\n> \n> [include]\n> psth = whatever\n> \n[snip]\n> That said, it _does_ behave the same and people are likely depending on\n> it at this point. So if we introduce a warning, for example, there needs\n> to be some way to suppress it.\n> \n> Probably:\n> \n> [include]\n> warnOnMissing = false\n> path = ...\n\nI was going to suggest, inspired by Makefile syntax, that\n[-include] would not complain if the file was missing ...\nexcept, of course, it's too late for that! ;-)\n\nI suppose [+include] could complain if the file is missing\ninstead, ... dunno.\n\nATB,\nRamsay Jones\n\n"},{"id":"357722","messageId":"87musr7h7q.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"20180908211436.GA31560@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-08T22:32:57Z","receivedAt":"2018-09-08T22:33:51Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 08 2018, Jeff King wrote:\n\n> On Sat, Sep 08, 2018 at 09:54:14PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> The reason missing includes are ignored is that the way this is expected\n>> to be used is e.g.:\n>>\n>> [include]\n>> path ~/.gitconfig.work\n>>\n>> Where .gitconfig.work is some configuration you're going to drop into\n>> place on your $dayjob servers, but not on your personal machine, even\n>> though you sync the same ~/.gitconfig everywhere.\n>>\n>> A lot of people who use includes rely on this, but I see from this\n>> thread this should be better documented.\n>\n> Right, this was an intentional choice at the time the feature was added,\n> to support this kind of feature. I'd note also that it mirrors other\n> misspelled keys. E.g.:\n>\n> [include]\n> psth = whatever\n>\n> will also not generate an error. This is also intentional, for two\n> reasons:\n>\n> 1. Git's config format has always been designed to carry extra keys\n> used by third-party scripts and porcelain. So we don't actually\n> know the complete set of valid keys. (Though you could make an\n> argument that git-core could stake out include.* as its own).\n>\n> 2. It makes using multiple git versions easier in some ways (though\n> also harder in others). A config key that isn't known to the\n> current version will be quietly ignored.\n>\n> Of course those things mean that true spelling mistakes are harder to\n> catch as such, because Git doesn't know that's what they are. And here\n> I'm talking config _keys_, not values. So I'm just explaining the\n> philosophical thinking that led to the \"missing file is a silent noop\".\n> It doesn't _have_ to behave the same.\n>\n> That said, it _does_ behave the same and people are likely depending on\n> it at this point. So if we introduce a warning, for example, there needs\n> to be some way to suppress it.\n>\n> Probably:\n>\n> [include]\n> warnOnMissing = false\n> path = ...\n>\n> would be enough (with the default being \"true\").\n>\n> You could even do:\n>\n> [include]\n> warnOnMissing = false\n> path = one\n> warnOnMissing = true\n> path = two\n>\n> to treat two includes differently (though I'm not sure why you would\n> want to).\n\nI think this is introducing a brand new caveat into our *.ini syntax,\ni.e. that we're sensitive to the order in which we're parsing\n*different* keys.\n\nI.e. we already had the concept that some keys override existing sets\n(e.g. user.name), but not that a x.y=foo controls the behavior of a\nsubsequent a.b=bar, or the other way around.\n\nThis also makes programmatic (via \"git config\") editing of the config\nhard, we'd need to introduce something like:\n\n git config -f ~/.gitconfig a.b bar\n git config -f ~/.gitconfig --before a.b x.y foo\n\nTo set a.b=bar before x.y=foo, or --after or whatever.\n\n>> If we were to make nonexisting files an error, we'd need something like\n>> an extension of the includeIf syntax added in 3efd0bedc6 (\"config: add\n>> conditional include\", 2017-03-01) 3efd0bedc6 (\"config: add conditional\n>> include\", 2017-03-01). I.e.:\n>>\n>> [includeIfcond \"test -e ~/.gitconfig.work\"]\n>> path = ~/.gitconfig.work\n>>\n>> Or something like that, this is getting increasingly harder to shove\n>> into the *.ini config syntax.\n>\n> I think it would be simpler to just introduce a new key that's a variant\n> of \"path\". Like:\n>\n> [include]\n> maybePath = ~/.gitconfig.work\n>\n> Though if it really is just a warning, the \"warnOnMissing\" above would\n> make that unnecessary (and it also scales better if we have to end up\n> adding more behavior tweaks in the future).\n\nYeah, we could do that, and it wouldn't break the model described above,\nWe can make that work, but this would be nasty. E.g. are we going to\ntreat EACCES and ENOENT the same way in this construct?\n"},{"id":"357723","messageId":"7f7e20fd-069d-f227-ce13-811398b52425@stason.org","threadId":"49304","inReplyTo":"20180908212256.GB31560@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-08T22:49:01Z","receivedAt":"2018-09-08T22:49:07Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 02:22 PM, Jeff King wrote:\n[...]\n>> The original problem cropped up due to using:\n>>\n>> git config --local include.path '../.gitconfig'\n>>\n>> which on linux stripped the single quotes, but on some windows git bash\n>> emulation it kept them.\n> \n> That sounds like a bug in git bash, if it is not treating single quotes\n> in the usual shell way. But I'd also expect such a bug to cause loads of\n> problems in all of the shell scripts. Are you sure it wasn't cmd.exe or\n> some other interpreter?\n\nI don't know, Jeff. I think the user said it was first anaconda shell.\nAnd then the user tried gitforwindows with same results. I don't know\nMSwindows at all.\n\nBut it doesn't matter at the end of the day, since we can't cover all\npossible unix shell emulations out there. What matters is that there is\na way to flag the misconfiguration, either by default, or through a\nspecial check - some ideas I suggested in my previous email, but surely\nyou have a much better insight of how to deal with that.\n\nThank you.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357724","messageId":"20180909022931.GA13485@sigill.intra.peff.net","threadId":"49304","inReplyTo":"87musr7h7q.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-09T02:29:32Z","receivedAt":"2018-09-09T02:29:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Sep 09, 2018 at 12:32:57AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > You could even do:\n> >\n> > [include]\n> > warnOnMissing = false\n> > path = one\n> > warnOnMissing = true\n> > path = two\n> >\n> > to treat two includes differently (though I'm not sure why you would\n> > want to).\n> \n> I think this is introducing a brand new caveat into our *.ini syntax,\n> i.e. that we're sensitive to the order in which we're parsing\n> *different* keys.\n> \n> I.e. we already had the concept that some keys override existing sets\n> (e.g. user.name), but not that a x.y=foo controls the behavior of a\n> subsequent a.b=bar, or the other way around.\n\nThis already exists. For example:\n\n echo '[foo]bar = inc' >config.inc\n echo '[foo]bar = main' >config.main\n echo '[include]path = config.inc' >>config.main\n git config -f config.main --includes foo.bar\n\nArriving at that answer requires expanding the include's contents at\nthe exact same spot in the file.\n\nAs far as I know this is the only case, though, so include.path really\nis special. And this would be expanding that specialness to other things\nin include.*. But I'm not sure that is really that big a deal. That\nwarnOnMissing isn't meant to be just a normal key. It's an\norder-sensitive directive for further parsing, just like include.path\nis. Either the parser understands includes (and has to handle these) or\nit doesn't.\n\nSo I'm not worried about any burden on the parsing side, but...\n\n> This also makes programmatic (via \"git config\") editing of the config\n> hard, we'd need to introduce something like:\n> \n> git config -f ~/.gitconfig a.b bar\n> git config -f ~/.gitconfig --before a.b x.y foo\n> \n> To set a.b=bar before x.y=foo, or --after or whatever.\n\nYes, I don't think \"git config include.warnOnMissing true\" would be very\nuseful, because it would generally be added after any includes you have,\nand therefore not affect them.\n\nI think this is generally an issue with include.path, too. My assumption\nhas been that anybody at the level of using includes is probably going\nto be hand-editing anyway.\n\nBut if include.* is special on the parsing side, I don't know that it is\nthat bad to make it special on the writing side, too. I.e., to recognize\n\"git config include.warnOnMissing true\" and always add it at the head of\nany existing include block.\n\nIt certainly _feels_ hacky, but I think it would behave sensibly and\npredictably. And it would just work, as opposed to requiring something\nlike \"--before\", which would be quite a subtle gotcha for somebody to\nforget to use.\n\n> Yeah, we could do that, and it wouldn't break the model described above,\n> We can make that work, but this would be nasty. E.g. are we going to\n> treat EACCES and ENOENT the same way in this construct?\n\nI don't have a strong opinion (after all, I already wrote the behavior I\nthought was reasonable long ago ;) ). So I think it would be up to\nsomebody to propose. We do already report and die on EACCES (and\nbasically any other error except ENOENT). So if we did treat them both\nas a warning, that would be a weakening for EACCES.\n\n-Peff\n"},{"id":"357725","messageId":"20180909023023.GB13485@sigill.intra.peff.net","threadId":"49304","inReplyTo":"7f7e20fd-069d-f227-ce13-811398b52425@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-09T02:30:23Z","receivedAt":"2018-09-09T02:30:27Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 08, 2018 at 03:49:01PM -0700, Stas Bekman wrote:\n\n> On 2018-09-08 02:22 PM, Jeff King wrote:\n> [...]\n> >> The original problem cropped up due to using:\n> >>\n> >> git config --local include.path '../.gitconfig'\n> >>\n> >> which on linux stripped the single quotes, but on some windows git bash\n> >> emulation it kept them.\n> > \n> > That sounds like a bug in git bash, if it is not treating single quotes\n> > in the usual shell way. But I'd also expect such a bug to cause loads of\n> > problems in all of the shell scripts. Are you sure it wasn't cmd.exe or\n> > some other interpreter?\n> \n> I don't know, Jeff. I think the user said it was first anaconda shell.\n> And then the user tried gitforwindows with same results. I don't know\n> MSwindows at all.\n> \n> But it doesn't matter at the end of the day, since we can't cover all\n> possible unix shell emulations out there. What matters is that there is\n> a way to flag the misconfiguration, either by default, or through a\n> special check - some ideas I suggested in my previous email, but surely\n> you have a much better insight of how to deal with that.\n\nYes, I agree this is pretty orthogonal to the config discussion. I was\njust wondering if there was a separate bug to look into. I'm willing to\nshrug and say \"it was probably user error\" until we see more definite\ndetails. Thanks.\n\n-Peff\n"},{"id":"357726","messageId":"20180909023332.GA14762@sigill.intra.peff.net","threadId":"49304","inReplyTo":"ad56c575-1211-61d2-daed-5b0da61db738@ramsayjones.plus.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-09T02:33:33Z","receivedAt":"2018-09-09T02:33:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 08, 2018 at 11:10:44PM +0100, Ramsay Jones wrote:\n\n> > Probably:\n> > \n> > [include]\n> > warnOnMissing = false\n> > path = ...\n> \n> I was going to suggest, inspired by Makefile syntax, that\n> [-include] would not complain if the file was missing ...\n> except, of course, it's too late for that! ;-)\n> \n> I suppose [+include] could complain if the file is missing\n> instead, ... dunno.\n\nI think that's syntactically invalid. At any rate, there are clearly\nthree options for setting a bit:\n\n 1. In the section header (+include, or Ævar's includeIf suggestion).\n\n 2. In another key (which looks pretty clean, but does introduce\n ordering constraints).\n\n 3. In the key name (maybePath or similar).\n\nI don't have a huge preference between them.\n\n-Peff\n"},{"id":"357727","messageId":"066f8a3e-062f-b921-c15d-ce4dd7adf377@stason.org","threadId":"49304","inReplyTo":"d9330ba54fbda54a92a9f4d9320836d88ce9a6e6.camel@mad-scientist.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-09T02:57:37Z","receivedAt":"2018-09-09T02:57:43Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-08 07:51 PM, Paul Smith wrote:\n[...]\n> What I personally think would be more useful would be some sort of\n> \"verbose parsing\" option to git config, that would parse the\n> configuration just as a normal Git command would and show diagnostic\n> output as the entire config is parsed: for each action line the config\n> file name and line number, and the operation performed (and any message\n> about it) would be printed. This could be useful in a variety of\n> situations, for instance to discover conflicts between local, global,\n> and system configuration, easily see where settings are coming from,\n> etc.\n> \n> And as part of this output, when an include file was not present or we\n> didn't have permissions or whatever, an appropriate error message would\n> be generated.\n\nI was thinking along the same lines, Paul - i.e. no need to change\nanything in the config syntax, but to provide better diagnostics.\n\nI quote below what I suggested in an earlier email, but I like Paul's\nidea even better as it'd be useful to many situations and not just the\none that started this thread.\n\n> 1) I suggest this is done via:\n>\n> git config --list --show-origin\n>\n> where the new addition would be to also show configuration parts that\n> are not active and indicating why it is so.\n>\n> So for example currently I get on a valid configuration setup and having\n> git/../.gitconfig in place the following output:\n>\n> [...]\n> file:/home/stas/.gitconfig mergetool.prompt=false\n> [...]\n> file:.git/config include.path=../.gitconfig\n> [...]\n> file:.git/../.gitconfig\n> filter.fastai-nbstripout-code.clean=tools/fastai-nbstripout\n> [...]\n>\n> Now, if include.path=../.gitconfig is there and file:.git/../.gitconfig\n> is not found, it will indicate that in some way that stands out for the\n> user. Perhaps:\n>\n> [...]\n> file:/home/stas/.gitconfig mergetool.prompt=false\n> [...]\n> file:.git/config include.path=../.gitconfig\n> [...]\n> file:.git/../.gitconfig FILE NOT FOUND! Ignored configuration\n> [...]\n>\n> So that would allow things to work as before, but now we have a way to\n> debug user-side configuration. And of course hoping that the docs would\n> indicate that method for debugging configuration problems.\n>\n> I hope this is a reasonable suggestion that doesn't require any\n> modification on the users' part who rely on this silent ignoring\n> \"feature\", yet lending to a configuration debug feature.\n\n\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357728","messageId":"d9330ba54fbda54a92a9f4d9320836d88ce9a6e6.camel@mad-scientist.net","threadId":"49304","inReplyTo":"acf93aef-f1f8-1aab-a16d-9655402d445f@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Paul Smith","fromEmail":"paul@mad-scientist.net","sentAt":"2018-09-09T02:51:08Z","receivedAt":"2018-09-09T03:52:12Z","isPatch":false,"sender":{"key":"paul@mad-scientist.net","avatar":"https://avatars.githubusercontent.com/u/109636?v=4"},"body":"On Sat, 2018-09-08 at 13:13 -0700, Stas Bekman wrote:\n> I remind that the original problem came from a simple command:\n> \n> git config --local include.path '../.gitconfig'\n> \n> Which on linux removed the quotes and all was fine, and on windows\n> the same command kept the quotes and the user was tearing his hair\n> out trying to understand why the custom config was ignored.\n\nI'm quite sure that the user was not using the Git Bash shell when they\nentered this command, but instead using command.com or powershell or\nsome variant of that.\n\nIf you use Git Bash as your shell then quotes will be handled like any\nPOSIX shell and the above will do what (we all) expect.\n\nIf you use command.com and powershell and use single quotes, then the\nsingle quotes will be put into the config file as you observed, because\nthose shells don't deal with single quotes.\n\nYou could use double-quotes, which ARE handled by command.com and\npowershell; in that case they would be stripped out and would not\nappear in the config.\n\n\nIf we were designing from scratch maybe using something like GNU make's\n\"include\" vs \"sinclude\" (silent include--this is another name for the\nalready mentioned \"-include\") would work; maybe \"path\" and \"spath\" or\nsomething. But to make it work right you really want the default\nbehavior to be \"warn if the file is not found\" and have the special\nbehavior be \"quiet if the file is not found\" otherwise it doesn't\nreally help beginners to avoid errors. And that's a backward-\ncompatibility problem. To my mind, adding extra \"check this\" options\nisn't very useful either: these kinds of warnings need to be on by\ndefault to be effective. The beginners, who need them, aren't going to\nremember to add extra options to enable more checking.\n\nWhat I personally think would be more useful would be some sort of\n\"verbose parsing\" option to git config, that would parse the\nconfiguration just as a normal Git command would and show diagnostic\noutput as the entire config is parsed: for each action line the config\nfile name and line number, and the operation performed (and any message\nabout it) would be printed. This could be useful in a variety of\nsituations, for instance to discover conflicts between local, global,\nand system configuration, easily see where settings are coming from,\netc.\n\nAnd as part of this output, when an include file was not present or we\ndidn't have permissions or whatever, an appropriate error message would\nbe generated.\n"},{"id":"357798","messageId":"xmqqr2i1thbs.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"20180908212256.GB31560@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-10T17:04:07Z","receivedAt":"2018-09-10T17:04:13Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sat, Sep 08, 2018 at 11:58:47AM -0700, Stas Bekman wrote:\n>\n>> This doesn’t:\n>> \n>> [include]\n>> path = '../.gitconfig'\n>\n> So I think it's been covered elsewhere that single quotes aren't a thing\n> in git's config format. I will say that this was actually a minor\n> surprise to me, after a decade of working with the format. ;)\n>\n> I don't know if it's worth changing now or not It would be\n> backwards-incompatible, but I wonder if we could do it in a sane way.\n> E.g., with a rule like:\n>\n> - if the first non-whitespace character of the value is a\n> single-quote, assume the value is quoted and apply normal shell\n> rules (i.e., no backslash escapes until the ending single-quote)\n>\n> - otherwise, single-quotes are not special at all\n\nAt least the rule would not force those with ' in the middle of\ntheir family names to surround the user.name with extra double\nquotes, and it would probably be a good and safe practical solution.\nBeing safe \"by magic\" tend to become hard to explain, but in this\ncase the magic part is probably still simple enough.\n\n\n\n\n"},{"id":"357804","messageId":"20180910171422.GA26356@aiede.svl.corp.google.com","threadId":"49304","inReplyTo":"xmqqr2i1thbs.fsf@gitster-ct.c.googlers.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-09-10T17:14:22Z","receivedAt":"2018-09-10T17:14:27Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Jeff King writes:\n\n>> So I think it's been covered elsewhere that single quotes aren't a thing\n>> in git's config format. I will say that this was actually a minor\n>> surprise to me, after a decade of working with the format. ;)\n>>\n>> I don't know if it's worth changing now or not It would be\n>> backwards-incompatible, but I wonder if we could do it in a sane way.\n>> E.g., with a rule like:\n>>\n>> - if the first non-whitespace character of the value is a\n>> single-quote, assume the value is quoted and apply normal shell\n>> rules (i.e., no backslash escapes until the ending single-quote)\n>>\n>> - otherwise, single-quotes are not special at all\n>\n> At least the rule would not force those with ' in the middle of\n> their family names to surround the user.name with extra double\n> quotes, and it would probably be a good and safe practical solution.\n> Being safe \"by magic\" tend to become hard to explain, but in this\n> case the magic part is probably still simple enough.\n\nGiven that today,\n\n\tgit config foo.bar \"'baz'\"\n\nproduces\n\n\t[foo]\n\t\tbar = 'baz'\n\nI don't think this would be safe to do. Since the underlying problem\nis that the latter syntax is confusing, I wonder if we can do the\nfollowing:\n\n 1. Treat single-quote as worth quoting in config.c::write_pair (line\n 2516). This would already help with the original issue, since the\n config would say\n\n\t[foo]\n\t\tbar = \\'baz\\'\n\n allowing a quick diagnosis.\n\n 2. (optional) Warn if a value is surrounded in single-quotes,\n encouraging using backslash to disambiguate.\n\n 3. (optional) Error out if a value is surrounded in single-quotes,\n encouraging using double-quote or backslash, depending on the\n user's intention.\n\n 4. (optional) Start treating wrapping single-quotes specially\n somehow.\n\nI think step 1 is a good idea, but I'm not convinced about any of the\nlater steps.\n\nI also agree with the comments upthread about wanting a way to do a\n'[include] path' that errors out if the file doesn't exist, and maybe\neven starting a transition to repurpose standard [include] path to do\nthat.\n\nThanks,\nJonathan\n"},{"id":"357809","messageId":"xmqqa7optdbs.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"20180910171422.GA26356@aiede.svl.corp.google.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-10T18:30:31Z","receivedAt":"2018-09-10T18:30:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> 1. Treat single-quote as worth quoting in config.c::write_pair (line\n> 2516). This would already help with the original issue, since the\n> config would say\n>\n> \t[foo]\n> \t\tbar = \\'baz\\'\n>\n> allowing a quick diagnosis.\n\nI am mildly against this, as long as you feel that all the remaining\nsteps need to be marked with \"(optional)\", because this will give\nreaders an impression that somehow single-quote is special. If we\ndo not intend to make it special at all, we shouldn't.\n\nIf we do commit to make it special, then this is a very sensible\nfirst step that is backward compatible, of course, and I find that\nthe following steps form a reasonable transition plan, if we intend\nto follow all the way through.\n\n> 2. (optional) Warn if a value is surrounded in single-quotes,\n> encouraging using backslash to disambiguate.\n>\n> 3. (optional) Error out if a value is surrounded in single-quotes,\n> encouraging using double-quote or backslash, depending on the\n> user's intention.\n>\n> 4. (optional) Start treating wrapping single-quotes specially\n> somehow.\n>\n> I think step 1 is a good idea, but I'm not convinced about any of the\n> later steps.\n>\n> I also agree with the comments upthread about wanting a way to do a\n> '[include] path' that errors out if the file doesn't exist, and maybe\n> even starting a transition to repurpose standard [include] path to do\n> that.\n"},{"id":"357811","messageId":"20180910183557.GD26356@aiede.svl.corp.google.com","threadId":"49304","inReplyTo":"xmqqa7optdbs.fsf@gitster-ct.c.googlers.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-09-10T18:35:57Z","receivedAt":"2018-09-10T18:36:03Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder writes:\n\n>> 1. Treat single-quote as worth quoting in config.c::write_pair (line\n>> 2516). This would already help with the original issue, since the\n>> config would say\n>>\n>> \t[foo]\n>> \t\tbar = \\'baz\\'\n>>\n>> allowing a quick diagnosis.\n>\n> I am mildly against this, as long as you feel that all the remaining\n> steps need to be marked with \"(optional)\", because this will give\n> readers an impression that somehow single-quote is special. If we\n> do not intend to make it special at all, we shouldn't.\n\nThat's fair, especially because it would be inconsistent with shell\ncommand language, where single-quote inside double quotes is not\nspecial:\n\n\t$ printf '%s\\n' \"\\'\"\n\t\\'\n\n(I realize that backslash means something different in Git config; I'm\njust saying it would be another source of cognitive dissonance.)\n\nUpdated proposal:\n\n 1. Treat strings starting or ending with single-quote as worth\n quoting in config.c::write_pair (line 3269). This would already\n help with the original issue, since the config would say\n\n\t[foo]\n\t\tbar = \"'baz'\"\n\n allowing a quick diagnosis.\n\n\n 2. (optional) Warn if a value is surrounded in single-quotes,\n encouraging using surrounding double-quotes to disambiguate.\n\n 3. (optional) Error out if a value is surrounded in single-quotes,\n encouraging replacing with or surrounding with double-quote,\n depending on the user's intention.\n\n 4. (optional) Start treating wrapping single-quotes specially\n somehow.\n\nAs before, I think step 1 is a good idea, but I'm not convinced about\nany of the later steps.\n\nThanks,\nJonathan\n"},{"id":"357817","messageId":"xmqq1sa1tb4p.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"20180910183557.GD26356@aiede.svl.corp.google.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-10T19:17:58Z","receivedAt":"2018-09-10T19:18:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Updated proposal:\n>\n> 1. Treat strings starting or ending with single-quote as worth\n> quoting in config.c::write_pair (line 3269). This would already\n> help with the original issue, since the config would say\n>\n> \t[foo]\n> \t\tbar = \"'baz'\"\n>\n> allowing a quick diagnosis.\n\nThis does not change anything essential from Git's point of view\nsince the previous round.\n\nBut I changed my mind anyway ;-) Earlier I said \"if we do not\nintend to make sq special, we shouldn't do #1\", and it still is a\ngood direction to go.\n\nBut making sq special does not have to be making sq a character that\nquotes. A character (or a character sequence) that is *not* quoting\ncan still be special---for example, we can say certain character or\na character sequence *must* be quoted, and make sq such a character.\n\nThat is, even if we stop at your step 3. or step 2., and did not go\nto step 4. (which I think is a bad idea for little gain), we are\nalready treating sq as a special character, and step 1. above is a\nreasonable way to start the transition to that better world.\n\nThe reason why it is a better world that have \"must be quoted, even\nthough they are not quoting characters themselves\" is solely because\nthat would avoid confusion for those who are not familiar with the\nfile format, even when we stop at step #2. In addition to a single\nquote a the beginning of the value, I think two or more SP deserve\nto be such a \"must be quoted\" sequence, i.e. instead of producing\nthis result, which we see with today's Git:\n\n\t$ git config a.test0 \"'foo\"\n\t$ git config a.test1 \"foo bar\" ;# two spaces\n\t$ grep -A2 '\\[a\\]' config\n\t[a]\n\t\ttest0 = 'foo\n\t\ttest1 = foo bar\n\nwe'd produce\n\n\t$ grep -A2 '\\[a\\]' config\n\t[a]\n\t\ttest0 = \"'foo\"\n\t\ttest1 = \"foo bar\"\n\nbut we can still interpret what we have historically written the\nsame way.\n\nI do not know if step #3 is a good idea, and I do not think step #2\nis particularly a good stopping point. Step #1 is probably slightly\na better stopping point if the aim is to avoid user confusion than\nstep #2.\n\n\n> 2. (optional) Warn if a value is surrounded in single-quotes,\n> encouraging using surrounding double-quotes to disambiguate.\n>\n> 3. (optional) Error out if a value is surrounded in single-quotes,\n> encouraging replacing with or surrounding with double-quote,\n> depending on the user's intention.\n>\n> 4. (optional) Start treating wrapping single-quotes specially\n> somehow.\n>\n> As before, I think step 1 is a good idea, but I'm not convinced about\n> any of the later steps.\n>\n> Thanks,\n> Jonathan\n"},{"id":"357824","messageId":"b6446834-04e1-ca65-350e-5847e689e2ea@stason.org","threadId":"49304","inReplyTo":"xmqq1sa1tb4p.fsf@gitster-ct.c.googlers.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-10T19:52:29Z","receivedAt":"2018-09-10T19:52:33Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"To add another report of a similar problem, of silent skipping and not\nof filepath quoting, I found this one:\n\nhttps://stackoverflow.com/questions/31203634/git-clean-filter-python-script-on-windows/52264440#52264440\n\nThe user created .gitconfig and added to .git/config:\n\n[include]\n path = .gitconfig\n\nNot realizing that the two were not in the same folder. And probably\nassuming that .git/config was referring to the root of repository, and\nnot relative to .git/, which is a reasonable assumption.\n\nOf course he had no way of resolving this as git wasn't telling him\nwhere it wasn't finding the file. i.e.\n\nCan't find: ~/myrepo/.git/.gitconfig\n\nwhich would have instantly told him where the problem was.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"357826","messageId":"xmqqk1ntru4r.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"b6446834-04e1-ca65-350e-5847e689e2ea@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-10T20:10:28Z","receivedAt":"2018-09-10T20:10:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stas Bekman writes:\n\n> [include]\n> path = .gitconfig\n>\n> Not realizing that the two were not in the same folder. And probably\n> assuming that .git/config was referring to the root of repository, and\n> not relative to .git/, which is a reasonable assumption.\n>\n> Of course he had no way of resolving this as git wasn't telling him\n> where it wasn't finding the file. i.e.\n>\n> Can't find: ~/myrepo/.git/.gitconfig\n>\n> which would have instantly told him where the problem was.\n\nYeah, I think Peff's idea of introducing a variant of include.path\nthat reports missing file would make sense for such a use case.\n\nThe code needs to turn a relative path to an absolute one by taking\nthe value as path relative to the including configuration file, so\nwe should already have the path to use in the error reporting, if we\nwere to go that route.\n"},{"id":"357911","messageId":"xmqqk1nrojpq.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"20180909023332.GA14762@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-11T20:36:01Z","receivedAt":"2018-09-11T20:36:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think that's syntactically invalid. At any rate, there are clearly\n> three options for setting a bit:\n>\n> 1. In the section header (+include, or Ævar's includeIf suggestion).\n>\n> 2. In another key (which looks pretty clean, but does introduce\n> ordering constraints).\n>\n> 3. In the key name (maybePath or similar).\n>\n> I don't have a huge preference between them.\n\nWhat's the longer term goal for the endgame? Is it acceptable that\ninclude.path will stay to be \"optional include\" for compatibility\nwith users' existing configuration files, and include.requiredpath\nor similar gets introduced to allow people who want to get warned?\nOr do we want the usual multi-step deprecation dance where the first\nphase introduces include.maybepath and include.path starts warning\nagainst missing one, encouraging it to be rewritten to maybepath?\n\nI have mild preference against #2, as I suspect that the ordering\nconstraints makes it harder to understand to end users. Between #1\nand #3, there wouldn't be much difference, whether the endgame is\n\"add a stricter variant that is opt in\" or \"migrate to a stricter\ndefault\".\n\n\n"},{"id":"357914","messageId":"20180911205709.GA25828@sigill.intra.peff.net","threadId":"49304","inReplyTo":"xmqqk1nrojpq.fsf@gitster-ct.c.googlers.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-11T20:57:10Z","receivedAt":"2018-09-11T20:57:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 11, 2018 at 01:36:01PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I think that's syntactically invalid. At any rate, there are clearly\n> > three options for setting a bit:\n> >\n> > 1. In the section header (+include, or Ævar's includeIf suggestion).\n> >\n> > 2. In another key (which looks pretty clean, but does introduce\n> > ordering constraints).\n> >\n> > 3. In the key name (maybePath or similar).\n> >\n> > I don't have a huge preference between them.\n> \n> What's the longer term goal for the endgame? Is it acceptable that\n> include.path will stay to be \"optional include\" for compatibility\n> with users' existing configuration files, and include.requiredpath\n> or similar gets introduced to allow people who want to get warned?\n> Or do we want the usual multi-step deprecation dance where the first\n> phase introduces include.maybepath and include.path starts warning\n> against missing one, encouraging it to be rewritten to maybepath?\n\nI don't see much point in introducing include.requiredPath. It might be\nuseful for people who want to be extra-careful with their includes, but\nit would not really help users who simply made a spelling error and\ndidn't know how to debug it.\n\nSo switching the default for include.path and providing an escape hatch\nseems like the more useful path (if we indeed want to do one of these;\nadding better debugging like GIT_TRACE_CONFIG is yet another option).\n\nAs far as deprecation, it depends on what the new behavior is. If it is\nsimply that include.path will generate a warning on a missing file, I\ndon't think there is any point in a multi-step dance. The endgame is a\nwarning, which is no different than the deprecated-stage behavior. :)\n\nIf the endgame is to die(), then I'd agree that there should be a\nwarning in the middle.\n\nBetween all those things I mentioned (or simply leaving it as-is), I\nreally don't have a strong feeling. I hoped people who did would\ngenerate a patch to give something concrete to review.\n\n> I have mild preference against #2, as I suspect that the ordering\n> constraints makes it harder to understand to end users. Between #1\n> and #3, there wouldn't be much difference, whether the endgame is\n> \"add a stricter variant that is opt in\" or \"migrate to a stricter\n> default\".\n\nThe thing that #2 buys you is that multiple such bits could be combined.\nIf we imagine that later there is another choice to make in interpreting\ninclude.path with two options, \"foo\" and \"bar\", then we would be stuck\nwith:\n\n include.maybeFooPath\n include.maybeBarPath\n include.fooPath\n include.barPath\n\nand of course it only gets worse with a third one. Whereas with\nindependent options, you can do:\n\n [include]\n warnOnMissing = false\n otherPreference = bar\n path = ...\n\nI dunno. The combinatorics might not be too bad if we document the\nrequired order, and actually code the parsing side like:\n\n if (!skip_prefix(key, \"maybe\", &key))\n\twarn_on_missing = 0;\n if (!skip_prefix(key, \"foo\", &key))\n other_pref = \"foo\";\n ...and so on\n\nIt's kind of hacky, but it does encode the bits into the name. As long\nas they remain bits, and not, say, arbitrary strings.\n\n-Peff\n"},{"id":"358739","messageId":"29173fd8-ce72-0927-9bfe-786442dfd82c@stason.org","threadId":"49304","inReplyTo":"20180911205709.GA25828@sigill.intra.peff.net","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-23T22:48:01Z","receivedAt":"2018-09-23T22:48:05Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"Apologies for I don't know how this project manages issues, so I'm not\nsure whether it is my responsibility to make sure this issue gets\nresolved, or do you have some tracking mechanism where you have it\nregistered? There is no rush, I'm asking because the discussion about\nthis issue has suddenly dropped about 2 weeks ago, hence my ping.\n\nThank you.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"358789","messageId":"87efdik3ie.fsf@evledraar.gmail.com","threadId":"49304","inReplyTo":"29173fd8-ce72-0927-9bfe-786442dfd82c@stason.org","subject":"Re: git silently ignores include directive with single quotes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-24T21:08:09Z","receivedAt":"2018-09-24T21:08:14Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Sep 23 2018, Stas Bekman wrote:\n\n> Apologies for I don't know how this project manages issues, so I'm not\n> sure whether it is my responsibility to make sure this issue gets\n> resolved, or do you have some tracking mechanism where you have it\n> registered? There is no rush, I'm asking because the discussion about\n> this issue has suddenly dropped about 2 weeks ago, hence my ping.\n\nPosting to this mailing list is generally how it's done, see\nhttps://github.com/git/git/blame/v2.19.0/README.md#L30-L37\n\nGit's a project worked on by a bunch of people who're either doing it as\na hobby, or are otherwise busy chasing stuff they're planning to work\non.\n\nThat doesn't mean the issue you reported doesn't matter, just that\nrealistically we have thousands of issues big and small at any given\ntime, and any new reported issue competes with those. There's always too\nmuch to do, and too little time to do it.\n\nPersonally, I'm interested enough in this to muse about how it could be\nfixed / what sort of general issues it exposes, but not enough to tackle\nit myself, the general silence for a couple of weeks means a lot of\npeople share that sentiment (or care even less).\n\nThat doesn't mean this issue doesn't matter, or that it couldn't be\naddressed in some way. I just wanted to try to give you some fair &\nrealistic summary of what's going on.\n\nThe best way to fix stuff in git that you can't interest others in is to\ndo it yourself. Take a look at Documentation/SubmittingPatches in the\ngit.git repository for how to do that.\n\nIn particular, starting by clarifying the docs around this as I\nsuggested upthread might be a good and easy start to your first\ncontribution to git!\n\nI hope that helps.\n"},{"id":"358802","messageId":"20180924222416.5240-1-philipoakley@iee.org","threadId":"49304","inReplyTo":"29173fd8-ce72-0927-9bfe-786442dfd82c@stason.org","subject":"[PATCH 0/1] Re: git silently ignores include directive with single quotes","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2018-09-24T22:24:15Z","receivedAt":"2018-09-24T22:32:51Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Rather than attaching the problem with code, I decided to simply update\nthe config file documentation.\n\nAs the userbase expands the documentation will need to be more comprehensive\nabout exclusions and omissions, along with better highlighting for core\nareas.\n\nI would be useful if Stas could comment on whether these changes would\nhave assisted in debugging the faulty config file. \n\nPhilip Oakley (1):\n config doc: highlight the name=value syntax\n\n Documentation/config.txt | 16 ++++++++++++----\n 1 file changed, 12 insertions(+), 4 deletions(-)\n\n-- \n2.17.1.windows.2\n\n"},{"id":"358803","messageId":"20180924222416.5240-2-philipoakley@iee.org","threadId":"49304","inReplyTo":"29173fd8-ce72-0927-9bfe-786442dfd82c@stason.org","subject":"[PATCH 1/1] config doc: highlight the name=value syntax","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2018-09-24T22:24:16Z","receivedAt":"2018-09-24T22:32:51Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Stas Bekman reported [1] that Git config was not accepting single quotes\naround a filename as may have been expected by shell users.\n\nHighlight the 'name = value' syntax with its own heading. Clarify that\nsingle quotes are not special here. Also point to this paragraph in the\n'include' section regarding pathnames.\n\nIn addition clarify that missing include file paths are not an error, but\nrather an implicit 'if found' for include files.\n\n[1] https://public-inbox.org/git/ca2b192e-1722-092e-2c54-d79d21a66ba2@stason.org/\n\nReported-by: Stas Bekman \nSigned-off-by: Philip Oakley \n---\n Documentation/config.txt | 16 ++++++++++++----\n 1 file changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 1264d91fa3..b65fd6138d 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -19,8 +19,8 @@ characters and `-`, and must start with an alphabetic character. Some\n variables may appear multiple times; we say then that the variable is\n multivalued.\n \n-Syntax\n-~~~~~~\n+Config file Syntax\n+~~~~~~~~~~~~~~~~~~\n \n The syntax is fairly flexible and permissive; whitespaces are mostly\n ignored. The '#' and ';' characters begin comments to the end of line,\n@@ -56,6 +56,9 @@ syntax, the subsection name is converted to lower-case and is also\n compared case sensitively. These subsection names follow the same\n restrictions as section names.\n \n+Variable name/value syntax\n+^^^^^^^^^^^^^^^^^^^^^^^^^^\n+\n All the other lines (and the remainder of the line after the section\n header) are recognized as setting variables, in the form\n 'name = value' (or just 'name', which is a short-hand to say that\n@@ -69,7 +72,8 @@ stripped. Leading whitespaces after 'name =', the remainder of the\n line after the first comment character '#' or ';', and trailing\n whitespaces of the line are discarded unless they are enclosed in\n double quotes. Internal whitespaces within the value are retained\n-verbatim.\n+verbatim. Single quotes are not special and form part of the\n+variable's value.\n \n Inside double quotes, double quote `\"` and backslash `\\` characters\n must be escaped: use `\\\"` for `\"` and `\\\\` for `\\`.\n@@ -89,10 +93,14 @@ each other with the exception that `includeIf` sections may be ignored\n if their condition does not evaluate to true; see \"Conditional includes\"\n below.\n \n+Both the `include` and `includeIf` sections implicitly apply an 'if found'\n+condition to the given path names.\n+\n You can include a config file from another by setting the special\n `include.path` (or `includeIf.*.path`) variable to the name of the file\n to be included. The variable takes a pathname as its value, and is\n-subject to tilde expansion. These variables can be given multiple times.\n+subject to tilde expansion and the value syntax detailed above.\n+These variables can be given multiple times.\n \n The contents of the included file are inserted immediately, as if they\n had been found at the location of the include directive. If the value of the\n-- \n2.17.1.windows.2\n\n"},{"id":"358804","messageId":"fc846d71-e3cb-6eb7-587c-2abeac1a7383@stason.org","threadId":"49304","inReplyTo":"20180924222416.5240-1-philipoakley@iee.org","subject":"Re: [PATCH 0/1] Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-24T23:05:40Z","receivedAt":"2018-09-24T23:05:46Z","isPatch":true,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-24 03:24 PM, Philip Oakley wrote:\n> Rather than attaching the problem with code, I decided to simply update\n> the config file documentation.\n> \n> As the userbase expands the documentation will need to be more comprehensive\n> about exclusions and omissions, along with better highlighting for core\n> areas.\n> \n> I would be useful if Stas could comment on whether these changes would\n> have assisted in debugging the faulty config file. \n\nThank you for writing this doc patch, Philip.\n\nThe documentation improvement would be most useful in conjunction with a\nan improved debugging/tracing facility. So that a user can see what git\nis seeing. Once a user sees that their configuration is broken then they\ncan peruse the improved documentation to find why it is broken. Without\nthe debugging ability, the docs would help but it'll be a much longer\njourney, since words like:\n\n\"Single quotes are not special and form part of the variable's value.\"\n\naren't necessarily going to stand out as an indicator of a potential\nproblem, when you won't think twice that quotes could even be a suspect,\neven though the docs say so explicitly.\n\nA trace saying:\n\n\"./.git/'.gitconfig'\" is not found\n\nwould speak volumes and be self-documenting.\n\nIn lieu of that, the docs would be need to have more examples.\n\nHere are the potential expansions to the patch you shared:\n\n1. \"Single quotes are not special and form part of the variable's value.\nFor example, if the configuration includes:\n\n include = '.gitconfig'\n\nthen git will look for \"'.gitconfig'\", single quotes included. Also note\nthat it'll look for the file relative to \"REPO/.git/\", hence it'll look\nfor \"REPO/.git/'.gitconfig'\", which is most likely incorrect, since you\ncan't check in files under \"REPO/.git/\". The correct configuration for\nincluding \"REPO/.gitconfig\" is:\n\n include = ../.gitconfig\n\n2. Same with:\n\n\"Both the `include` and `includeIf` sections implicitly apply an 'if\nfound' condition to the given path names.\"\n\nTo a user this would be a difficult statement to make sense of. An\nexample would fix that:\n\n\"Both the `include` and `includeIf` sections implicitly apply an 'if\nfound' condition to the given path names. For example, if the\nconfiguration includes:\n\n include = ../.gitconfig\n\nand git finds \"REPO/.gitconfig\", it will include its configuration. If\ngit can't find it, it will silently ignore this include statement until\nthis file appears. It has been designed this way to allow for optional\nuser-specific configuration facilities.\"\n\nThank you.\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"358808","messageId":"087d2065-6ba0-87b7-2a6f-bf2726aef829@stason.org","threadId":"49304","inReplyTo":"87efdik3ie.fsf@evledraar.gmail.com","subject":"Re: git silently ignores include directive with single quotes","fromName":"Stas Bekman","fromEmail":"stas@stason.org","sentAt":"2018-09-24T23:20:52Z","receivedAt":"2018-09-24T23:20:55Z","isPatch":false,"sender":{"key":"stas@stason.org","avatar":null},"body":"On 2018-09-24 02:08 PM, Ævar Arnfjörð Bjarmason wrote:\n[...]\n\n> Posting to this mailing list is generally how it's done\n\nThank you, Ævar, for clarifying that there is no issue tracker for the\ngit project.\n\n> The best way to fix stuff in git that you can't interest others in is to\n> do it yourself. Take a look at Documentation/SubmittingPatches in the\n> git.git repository for how to do that.\n\nBased on the initial rich discussion this post created I had a feeling\nthat there was a lot of interest. But you're correct, it's easier to\nshare one's thought and patches take a lot more effort and time.\n\n> In particular, starting by clarifying the docs around this as I\n> suggested upthread might be a good and easy start to your first\n> contribution to git!\n\nThat's an excellent idea. Philip started the process and hopefully it\nwill lead to better documentation of the issue.\n\nThanks again.\n\n\n-- \n________________________________________________\nStas Bekman <'))))>< <'))))><\nhttps://stasosphere.com https://chestofbooks.com\nhttps://experientialsexlab.com https://stason.org\nhttps://stasosphere.com/experience-life/my-books\n"},{"id":"358879","messageId":"xmqqlg7pnskm.fsf@gitster-ct.c.googlers.com","threadId":"49304","inReplyTo":"20180924222416.5240-2-philipoakley@iee.org","subject":"Re: [PATCH 1/1] config doc: highlight the name=value syntax","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-25T22:03:05Z","receivedAt":"2018-09-25T22:03:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Philip Oakley writes:\n\n> +Variable name/value syntax\n> +^^^^^^^^^^^^^^^^^^^^^^^^^^\n> +\n> All the other lines (and the remainder of the line after the section\n> header) are recognized as setting variables, in the form\n> 'name = value' (or just 'name', which is a short-hand to say that\n> @@ -69,7 +72,8 @@ stripped. Leading whitespaces after 'name =', the remainder of the\n> line after the first comment character '#' or ';', and trailing\n> whitespaces of the line are discarded unless they are enclosed in\n> double quotes. Internal whitespaces within the value are retained\n> -verbatim.\n> +verbatim. Single quotes are not special and form part of the\n> +variable's value.\n> \n> Inside double quotes, double quote `\"` and backslash `\\` characters\n> must be escaped: use `\\\"` for `\"` and `\\\\` for `\\`.\n\nHmph. This feels a bit backwards. \n\nThe original paragraph is horrible in that there is no clear mention\nthat a pair of dq can be used to quote (which primarily is useful if\nyour value have leading or trailing whitespaces); the closest hint\nis \"enclosed in double quotes\" we see in the pre-context. The added\nsentence singles out sq but it is unclear why it is necessary to\ncall out that it is not special---the readers can legitimately\nwonder if backquotes are special or not and why.\n\nI wonder if this is easier to understand:\n\n diff --git a/Documentation/config.txt b/Documentation/config.txt\n index ad0f4510c3..5eebd539df 100644\n --- a/Documentation/config.txt\n +++ b/Documentation/config.txt\n @@ -61,12 +61,16 @@ the variable is the boolean \"true\").\n The variable names are case-insensitive, allow only alphanumeric characters\n and `-`, and must start with an alphabetic character.\n\n +The value part can have segments that are enclosed in a pair of\n +double quotes (note: other kinds of quoting character pairs are not\n +special)--the double quotes are stripped from the value.\n +\n A line that defines a value can be continued to the next line by\n ending it with a `\\`; the backquote and the end-of-line are\n stripped. Leading whitespaces after 'name =', the remainder of the\n line after the first comment character '#' or ';', and trailing\n -whitespaces of the line are discarded unless they are enclosed in\n -double quotes. Internal whitespaces within the value are retained\n +whitespaces of the line are discarded.\n +Internal whitespaces within the value are retained\n verbatim.\n\n Inside double quotes, double quote `\"` and backslash `\\` characters\n\n> @@ -89,10 +93,14 @@ each other with the exception that `includeIf` sections may be ignored\n> if their condition does not evaluate to true; see \"Conditional includes\"\n> below.\n> \n> +Both the `include` and `includeIf` sections implicitly apply an 'if found'\n> +condition to the given path names.\n> +\n\nMentioning that missing target file is not an error is definitely an\nimprovement. I've never viewed it as applying \"if found\" condition\nmyself, but it is not wrong per-se to do so, I would think.\n\n> You can include a config file from another by setting the special\n> `include.path` (or `includeIf.*.path`) variable to the name of the file\n> to be included. The variable takes a pathname as its value, and is\n> -subject to tilde expansion. These variables can be given multiple times.\n> +subject to tilde expansion and the value syntax detailed above.\n> +These variables can be given multiple times.\n\nI have a mild suspicion that this adds negative value. Singling out\nthat \"[include] path = ...\" follows the usual value syntax makes\nthe readers wonder if there are some \"[section] variable = ...\" that\ndoes not follow the value syntax that they have to be aware of and\ncareful about.\n"}]}