{"thread":{"id":"53356","subject":"[PATCH] doc: fix quoting bug in credential cache example","startedAt":"2020-04-30T14:36:52Z","lastAt":"2020-05-01T15:11:43Z","messageCount":18,"participants":["douglas.fuller@gmail.com","Junio C Hamano","Jeff King","Andreas Schwab"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"396661","messageId":"63f35287c9ced4d674f938bedd439aefa6c46f41.camel@gmail.com","threadId":"53356","inReplyTo":null,"subject":"[PATCH] doc: fix quoting bug in credential cache example","fromName":"","fromEmail":"douglas.fuller@gmail.com","sentAt":"2020-04-30T14:36:47Z","receivedAt":"2020-04-30T14:36:52Z","isPatch":true,"sender":{"key":"douglas.fuller@gmail.com","avatar":null},"body":"Unquoted semicolons are considered shell argument separators, quote\nthem so the example works correctly.\n\nSigned-off-by: Douglas Fuller <douglas.fuller@gmail.com>\n---\n Documentation/gitcredentials.txt | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Documentation/gitcredentials.txt\nb/Documentation/gitcredentials.txt\nindex 1814d2d23c..fb16c71cfe 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -229,7 +229,7 @@ foo --bar=\"whitespace arg\"\n /path/to/my/helper --with-arguments\n \n # or you can specify your own shell snippet\n-!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n+\"!f() { echo password=`cat $HOME/.secret`; }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to\nspecify.\n-- \n2.26.2\n\n\n"},{"id":"396676","messageId":"xmqqr1w4sv5t.fsf@gitster.c.googlers.com","threadId":"53356","inReplyTo":"63f35287c9ced4d674f938bedd439aefa6c46f41.camel@gmail.com","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-30T18:09:02Z","receivedAt":"2020-04-30T18:09:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"douglas.fuller@gmail.com writes:\n\n> Unquoted semicolons are considered shell argument separators, quote\n> them so the example works correctly.\n\nI think what you wanted to do might make sense, but the above\njustification is totally incorrect.\n\n>  # or you can specify your own shell snippet\n> -!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n> +\"!f() { echo password=`cat $HOME/.secret`; }; f\"\n\nThis is one of the examples shown, each shows possible value that\ncan be given to credential.helper variable.  Reproducing them fully:\n\n    # run \"git credential-foo\"\n    foo\n\n    # same as above, but pass an argument to the helper\n    foo --bar=baz\n\n    # the arguments are parsed by the shell, so use shell\n    # quoting if necessary\n    foo --bar=\"whitespace arg\"\n\n    # you can also use an absolute path, which will not use the git wrapper\n    /path/to/my/helper --with-arguments\n\n    # or you can specify your own shell snippet\n    !f() { echo \"password=`cat $HOME/.secret`\"; }; f\n\nThese are examples of values, and how they may have to be quoted in\nvarious environments is not discussed here.  \n\nWe will not want a patch that says that the second example is wrong\nbecause \"spaces separate arguments in shell and a string with a\nspace in it must be quoted\", i.e.\n\n    $ git -c credential.helper=\"foo --bar=baz\" frotz\n\nand does this\n\n     # same as above, but pass an argument to the helper\n    -foo --bar=baz\n    +\"foo --bar=baz\"\n\nbecause the quoting convention would be different depending on where\nit appears.  In a .git/config file, i.e.\n\n    [credential]\n\thelper = foo --bar=baz\n\nis perfectly fine without quoting.\n\n    $ git -c credential.helper='!f() { echo \"password=`cat ...`\"; }; f' frotz\n\nwould be how you would pass a one-shot config from shell.\n\nNow, the reason why I said what you did is correct but the\njustification is wrong is because the semicolon does pose a problem\nin the .git/config file.  In fact\n\n    [credential]\n\thelper = !f() { echo \"password=`cat ...`\"; }; f\n\nwould *NOT* work, because semicolon introduces a comment in the\nconfiguration file.\n\nFor this particular case, you can just do\n\n    [credential]\n\thelper = !echo password=`cat $HOME/.secret`\n\nwithout any quoting issues, though.\n\nHaving said all that, I think we should clarify what these sample\nstrings are in the introductory text in the examples.  \n\nI've always thought that they are illustrating possible values and\nhow to express that value in the context the values appear in is up\nto the readers who learn what values to write in this document (and\nthey learn from manual for shell to learn the shell quoting\nconvention and manual for 'git config' to learn the config quoting\nconvention).  Hence my initial reaction to your patch was \"shell?\nQuoting for shell is outside the scope of the explanation here\".\n\nOn the other hand, for anybody who assumes that these examples are\nliterally showing what you write after \"[credential] helper = \" in\nthe configuration file, the example clearly is wrong and dq may be\nneeded (but yours is also wrong, in that it loses double quotes\naround the argument to echo; if ~/.secret file had a tab in it, the\nhelper will now yield a wrong password and you won't be able to log\nin).\n\n"},{"id":"396714","messageId":"20200501055738.GB23665@coredump.intra.peff.net","threadId":"53356","inReplyTo":"xmqqr1w4sv5t.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T05:57:38Z","receivedAt":"2020-05-01T05:57:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 30, 2020 at 11:09:02AM -0700, Junio C Hamano wrote:\n\n> [...]\n> Having said all that, I think we should clarify what these sample\n> strings are in the introductory text in the examples.  \n\nYou already said everything I was going to. :)\n\n> I've always thought that they are illustrating possible values and\n> how to express that value in the context the values appear in is up\n> to the readers who learn what values to write in this document (and\n> they learn from manual for shell to learn the shell quoting\n> convention and manual for 'git config' to learn the config quoting\n> convention).  Hence my initial reaction to your patch was \"shell?\n> Quoting for shell is outside the scope of the explanation here\".\n> \n> On the other hand, for anybody who assumes that these examples are\n> literally showing what you write after \"[credential] helper = \" in\n> the configuration file, the example clearly is wrong and dq may be\n> needed (but yours is also wrong, in that it loses double quotes\n> around the argument to echo; if ~/.secret file had a tab in it, the\n> helper will now yield a wrong password and you won't be able to log\n> in).\n\nYes, they were definitely meant as: here are the raw values you would\nwant to use, and it is up to you to figure out how to get that into a\nconfig file (whether on the cmdline via \"git config\" or editing the file\nyourself).\n\nI think we can either clarify that with a note at the beginning of the\nlist, or we can just present it as config, like:\n\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 1814d2d23c..c756ecb8fd 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -216,20 +216,25 @@ Here are some example specifications:\n \n ----------------------------------------------------\n # run \"git credential-foo\"\n-foo\n+[credential]\n+helper = foo\n \n # same as above, but pass an argument to the helper\n-foo --bar=baz\n+[credential]\n+helper = foo --bar=baz\n \n # the arguments are parsed by the shell, so use shell\n # quoting if necessary\n-foo --bar=\"whitespace arg\"\n+[credential]\n+helper = \"foo --bar='whitespace arg'\"\n \n # you can also use an absolute path, which will not use the git wrapper\n-/path/to/my/helper --with-arguments\n+[credential]\n+helper = /path/to/my/helper --with-arguments\n \n # or you can specify your own shell snippet\n-!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n+[credential]\n+helper = \"!f() { echo \\\"password=`cat $HOME/.secret`\\\"; }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to specify.\n\nIt may be easier to just use double-quotes consistently, even for ones\nthat do not need it, to give readers one less thing to wonder about.\n\n-Peff\n"},{"id":"396716","messageId":"20200501061917.GC23665@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501055738.GB23665@coredump.intra.peff.net","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:19:17Z","receivedAt":"2020-05-01T06:19:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 01, 2020 at 01:57:38AM -0400, Jeff King wrote:\n\n> It may be easier to just use double-quotes consistently, even for ones\n> that do not need it, to give readers one less thing to wonder about.\n\nSo here's a patch that does that. I also noticed a few other\ndeficiencies in that final example, which are fixed in the second patch.\n\nI hope I'm not stealing Douglas's thunder. :)\n\n  [1/2]: gitcredentials(7): clarify quoting of helper examples\n  [2/2]: gitcredentials(7): make shell-snippet example more realistic\n\n Documentation/gitcredentials.txt | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\n-Peff\n"},{"id":"396717","messageId":"20200501062039.GA25603@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501061917.GC23665@coredump.intra.peff.net","subject":"[PATCH 1/2] gitcredentials(7): clarify quoting of helper examples","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:20:39Z","receivedAt":"2020-05-01T06:20:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We give several helper config examples, but don't make clear that these\nare raw values. It's up to the user to add the appropriate quoting to\nput them into a config file (either by running with \"git config\" and\nquoting against the shell, or by adding double-quotes as appropriate\nwithin the git-config file).\n\nLet's flesh them out as full config blocks, which makes the syntax more\nclear (and makes it possible for people to just cut-and-paste them as a\nstarting point). I added double-quotes to any values larger than a\nsingle word. That isn't strictly necessary in all cases, but it\nsidesteps explaining the rules about exactly when you need to quote a\nvalue.\n\nThe existing quotes can be converted to single-quotes in one instance,\nand simply omitted in another for simplicity (unless you have odd\nwhitespace in your password, the shell whitespace-splitting behavior is\nfine). I also swapped out backticks for our preferred $().\n\nReported-by: douglas.fuller@gmail.com\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/gitcredentials.txt | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 1814d2d23c..8a20343dd7 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -216,20 +216,25 @@ Here are some example specifications:\n \n ----------------------------------------------------\n # run \"git credential-foo\"\n-foo\n+[credential]\n+helper = foo\n \n # same as above, but pass an argument to the helper\n-foo --bar=baz\n+[credential]\n+helper = \"foo --bar=baz\"\n \n # the arguments are parsed by the shell, so use shell\n # quoting if necessary\n-foo --bar=\"whitespace arg\"\n+[credential]\n+helper = \"foo --bar='whitespace arg'\"\n \n # you can also use an absolute path, which will not use the git wrapper\n-/path/to/my/helper --with-arguments\n+[credential]\n+helper = \"/path/to/my/helper --with-arguments\"\n \n # or you can specify your own shell snippet\n-!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n+[credential]\n+helper = \"!f() { echo password=$(cat $HOME/.secret); }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to specify.\n-- \n2.26.2.933.gdf62622942\n\n"},{"id":"396718","messageId":"xmqq368kp457.fsf@gitster.c.googlers.com","threadId":"53356","inReplyTo":"20200501055738.GB23665@coredump.intra.peff.net","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-01T06:20:52Z","receivedAt":"2020-05-01T06:20:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think we can either clarify that with a note at the beginning of the\n> list, or we can just present it as config, like:\n>\n> diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\n> index 1814d2d23c..c756ecb8fd 100644\n> --- a/Documentation/gitcredentials.txt\n> +++ b/Documentation/gitcredentials.txt\n> @@ -216,20 +216,25 @@ Here are some example specifications:\n>  \n>  ----------------------------------------------------\n>  # run \"git credential-foo\"\n> -foo\n> +[credential]\n> +helper = foo\n\nI like this style of \"clarification\"; it makes it clear that we are\nexplaining the value in the config-file syntax.\n\n>  # or you can specify your own shell snippet\n> -!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n> +[credential]\n> +helper = \"!f() { echo \\\"password=`cat $HOME/.secret`\\\"; }; f\"\n\nBut I do not think the \"tentative shell function\" trick is\nnecessary.  I personally think it is oversold ;-)  For this\nparticular one,\n\n    [credential]\n        helper = !echo \\\"password=`cat $HOME/.secret`\\\"\n\nshould be sufficient, perhaps?  You do not even need to understand\nhow shell functions work to understand it.\n\nAlso, \"git config\" indents the \"var = value\" lines, so it would look\nmore natural if we indented our examples in a similar way.\n\nThanks.\n"},{"id":"396719","messageId":"20200501062316.GB25603@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501061917.GC23665@coredump.intra.peff.net","subject":"[PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:23:16Z","receivedAt":"2020-05-01T06:23:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's an example of using your own bit of shell to act as a credential\nhelper, but it's not very realistic:\n\n - It's stupid to hand out your secret password to _every_ host. In the\n   real world you'd use the config-matcher to limit it to a particular\n   host.\n\n - We never provided a username. We can easily do that in another config\n   option (you can do it in the helper, too, but this is much more\n   readable).\n\n - We were sending the secret even for store/erase operations. This\n   is OK because Git would just ignore it, but a real system would\n   probably be unlocking a password store, which you wouldn't want to do\n   more than necessary.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is in fact very close to what's in my own ~/.gitconfig, except that\nI swap out \"cat\" for the \"pass\" tool.\n\n Documentation/gitcredentials.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 8a20343dd7..63b20fc6a5 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -233,8 +233,9 @@ helper = \"foo --bar='whitespace arg'\"\n helper = \"/path/to/my/helper --with-arguments\"\n \n # or you can specify your own shell snippet\n-[credential]\n-helper = \"!f() { echo password=$(cat $HOME/.secret); }; f\"\n+[credential \"https://example.com\"]\n+username = your_user\n+helper = \"!f() { test $1 = get && echo password=$(cat $HOME/.secret); }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to specify.\n-- \n2.26.2.933.gdf62622942\n"},{"id":"396720","messageId":"20200501062552.GC25603@coredump.intra.peff.net","threadId":"53356","inReplyTo":"xmqq368kp457.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:25:52Z","receivedAt":"2020-05-01T06:25:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 30, 2020 at 11:20:52PM -0700, Junio C Hamano wrote:\n\n> >  # or you can specify your own shell snippet\n> > -!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n> > +[credential]\n> > +helper = \"!f() { echo \\\"password=`cat $HOME/.secret`\\\"; }; f\"\n> \n> But I do not think the \"tentative shell function\" trick is\n> necessary.  I personally think it is oversold ;-)  For this\n> particular one,\n> \n>     [credential]\n>         helper = !echo \\\"password=`cat $HOME/.secret`\\\"\n> \n> should be sufficient, perhaps?  You do not even need to understand\n> how shell functions work to understand it.\n\nThat will print:\n\n  password=your-password get\n\nSee the patches I just sent for a better example. :)\n\n> Also, \"git config\" indents the \"var = value\" lines, so it would look\n> more natural if we indented our examples in a similar way.\n\nWe do earlier, too.\n\nI can re-indent the patches I just set, but I'll hold off on sending in\ncase you have any other comment.\n\n-Peff\n"},{"id":"396721","messageId":"xmqqy2qcnpb4.fsf@gitster.c.googlers.com","threadId":"53356","inReplyTo":"20200501062316.GB25603@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-01T06:26:39Z","receivedAt":"2020-05-01T06:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> There's an example of using your own bit of shell to act as a credential\n> helper, but it's not very realistic:\n>\n>  - It's stupid to hand out your secret password to _every_ host. In the\n>    real world you'd use the config-matcher to limit it to a particular\n>    host.\n>\n>  - We never provided a username. We can easily do that in another config\n>    option (you can do it in the helper, too, but this is much more\n>    readable).\n>\n>  - We were sending the secret even for store/erase operations. This\n>    is OK because Git would just ignore it, but a real system would\n>    probably be unlocking a password store, which you wouldn't want to do\n>    more than necessary.\n\nAll of them make sense, but I do not think we want to encourage that\nloose style of passing unquoted argument to echo to lose embedded\n$IFS spaces that is not a SP.\n\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This is in fact very close to what's in my own ~/.gitconfig, except that\n> I swap out \"cat\" for the \"pass\" tool.\n>\n>  Documentation/gitcredentials.txt | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\n> index 8a20343dd7..63b20fc6a5 100644\n> --- a/Documentation/gitcredentials.txt\n> +++ b/Documentation/gitcredentials.txt\n> @@ -233,8 +233,9 @@ helper = \"foo --bar='whitespace arg'\"\n>  helper = \"/path/to/my/helper --with-arguments\"\n>  \n>  # or you can specify your own shell snippet\n> -[credential]\n> -helper = \"!f() { echo password=$(cat $HOME/.secret); }; f\"\n> +[credential \"https://example.com\"]\n> +username = your_user\n> +helper = \"!f() { test $1 = get && echo password=$(cat $HOME/.secret); }; f\"\n>  ----------------------------------------------------\n>  \n>  Generally speaking, rule (3) above is the simplest for users to specify.\n"},{"id":"396722","messageId":"20200501063207.GA26128@coredump.intra.peff.net","threadId":"53356","inReplyTo":"xmqqy2qcnpb4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:32:07Z","receivedAt":"2020-05-01T06:32:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 30, 2020 at 11:26:39PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > There's an example of using your own bit of shell to act as a credential\n> > helper, but it's not very realistic:\n> >\n> >  - It's stupid to hand out your secret password to _every_ host. In the\n> >    real world you'd use the config-matcher to limit it to a particular\n> >    host.\n> >\n> >  - We never provided a username. We can easily do that in another config\n> >    option (you can do it in the helper, too, but this is much more\n> >    readable).\n> >\n> >  - We were sending the secret even for store/erase operations. This\n> >    is OK because Git would just ignore it, but a real system would\n> >    probably be unlocking a password store, which you wouldn't want to do\n> >    more than necessary.\n> \n> All of them make sense, but I do not think we want to encourage that\n> loose style of passing unquoted argument to echo to lose embedded\n> $IFS spaces that is not a SP.\n\nYou mean dropping the quotes in the first patch?\n\nDoing:\n\n  echo \"password=$(cat $HOME/.secret)\"\n\nalready eats some trailing whitespace, though I guess if you have\nnewlines in your password you are beyond help anyway.\n\nI can add back in the quoted \\\", though it does make the code slightly\nharder to read.\n\n-Peff\n"},{"id":"396723","messageId":"20200501063513.GB26128@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501063207.GA26128@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T06:35:13Z","receivedAt":"2020-05-01T06:35:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 01, 2020 at 02:32:07AM -0400, Jeff King wrote:\n\n> > All of them make sense, but I do not think we want to encourage that\n> > loose style of passing unquoted argument to echo to lose embedded\n> > $IFS spaces that is not a SP.\n> \n> You mean dropping the quotes in the first patch?\n> \n> Doing:\n> \n>   echo \"password=$(cat $HOME/.secret)\"\n> \n> already eats some trailing whitespace, though I guess if you have\n> newlines in your password you are beyond help anyway.\n> \n> I can add back in the quoted \\\", though it does make the code slightly\n> harder to read.\n\nOr did you mean passing $1 in the test call? It definitely isn't good\nshell practice, but we know that we're getting a single-word action from\nGit, per the protocol.\n\nFully quoting, it looks like this:\n\n  helper = \"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n\nwhich IMHO is getting a little hard to read. I think that's part of why\nI gave such an unfinished example in the first place. :)\n\n-Peff\n"},{"id":"396724","messageId":"87tv10i0ll.fsf@linux-m68k.org","threadId":"53356","inReplyTo":"20200501062039.GA25603@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2020-05-01T07:19:18Z","receivedAt":"2020-05-01T07:19:25Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Mai 01 2020, Jeff King wrote:\n\n>  # or you can specify your own shell snippet\n> -!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n> +[credential]\n> +helper = \"!f() { echo password=$(cat $HOME/.secret); }; f\"\n\nThat now lacks a pair of quotes around the argument of echo.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"396725","messageId":"20200501073113.GA27392@coredump.intra.peff.net","threadId":"53356","inReplyTo":"87tv10i0ll.fsf@linux-m68k.org","subject":"Re: [PATCH 1/2] gitcredentials(7): clarify quoting of helper examples","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T07:31:13Z","receivedAt":"2020-05-01T07:31:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 01, 2020 at 09:19:18AM +0200, Andreas Schwab wrote:\n\n> On Mai 01 2020, Jeff King wrote:\n> \n> >  # or you can specify your own shell snippet\n> > -!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n> > +[credential]\n> > +helper = \"!f() { echo password=$(cat $HOME/.secret); }; f\"\n> \n> That now lacks a pair of quotes around the argument of echo.\n\nRight, as discussed in the commit message.\n\n-Peff\n"},{"id":"396726","messageId":"20200501073222.GB27392@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501063513.GB26128@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T07:32:22Z","receivedAt":"2020-05-01T07:32:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 01, 2020 at 02:35:13AM -0400, Jeff King wrote:\n\n> On Fri, May 01, 2020 at 02:32:07AM -0400, Jeff King wrote:\n> \n> > > All of them make sense, but I do not think we want to encourage that\n> > > loose style of passing unquoted argument to echo to lose embedded\n> > > $IFS spaces that is not a SP.\n> > \n> > You mean dropping the quotes in the first patch?\n> > \n> > Doing:\n> > \n> >   echo \"password=$(cat $HOME/.secret)\"\n> > \n> > already eats some trailing whitespace, though I guess if you have\n> > newlines in your password you are beyond help anyway.\n> > \n> > I can add back in the quoted \\\", though it does make the code slightly\n> > harder to read.\n> \n> Or did you mean passing $1 in the test call? It definitely isn't good\n> shell practice, but we know that we're getting a single-word action from\n> Git, per the protocol.\n> \n> Fully quoting, it looks like this:\n> \n>   helper = \"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n> \n> which IMHO is getting a little hard to read. I think that's part of why\n> I gave such an unfinished example in the first place. :)\n\nSo I dunno. That's ugly, but I don't think is worth nitpicking over\nmore. So here it is with indentation and full-on quoting.\n\n  [1/2]: gitcredentials(7): clarify quoting of helper examples\n  [2/2]: gitcredentials(7): make shell-snippet example more realistic\n\n Documentation/gitcredentials.txt | 16 +++++++++++-----\n 1 file changed, 11 insertions(+), 5 deletions(-)\n\n-Peff\n"},{"id":"396728","messageId":"20200501073305.GA27675@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501073222.GB27392@coredump.intra.peff.net","subject":"[PATCH v2 1/2] gitcredentials(7): clarify quoting of helper examples","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T07:33:05Z","receivedAt":"2020-05-01T07:33:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We give several helper config examples, but don't make clear that these\nare raw values. It's up to the user to add the appropriate quoting to\nput them into a config file (either by running with \"git config\" and\nquoting against the shell, or by adding double-quotes as appropriate\nwithin the git-config file).\n\nLet's flesh them out as full config blocks, which makes the syntax more\nclear (and makes it possible for people to just cut-and-paste them as a\nstarting point). I added double-quotes to any values larger than a\nsingle word. That isn't strictly necessary in all cases, but it\nsidesteps explaining the rules about exactly when you need to quote a\nvalue.\n\nThe existing quotes can be converted to single-quotes in one instance,\nand backslash-esccaped in the other. I also swapped out backticks for\nour preferred $().\n\nReported-by: douglas.fuller@gmail.com\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/gitcredentials.txt | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 1814d2d23c..8127dfcd2f 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -216,20 +216,25 @@ Here are some example specifications:\n \n ----------------------------------------------------\n # run \"git credential-foo\"\n-foo\n+[credential]\n+\thelper = foo\n \n # same as above, but pass an argument to the helper\n-foo --bar=baz\n+[credential]\n+\thelper = \"foo --bar=baz\"\n \n # the arguments are parsed by the shell, so use shell\n # quoting if necessary\n-foo --bar=\"whitespace arg\"\n+[credential]\n+\thelper = \"foo --bar='whitespace arg'\"\n \n # you can also use an absolute path, which will not use the git wrapper\n-/path/to/my/helper --with-arguments\n+[credential]\n+\thelper = \"/path/to/my/helper --with-arguments\"\n \n # or you can specify your own shell snippet\n-!f() { echo \"password=`cat $HOME/.secret`\"; }; f\n+[credential]\n+\thelper = \"!f() { echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to specify.\n-- \n2.26.2.933.gdf62622942\n\n"},{"id":"396729","messageId":"20200501073311.GB27675@coredump.intra.peff.net","threadId":"53356","inReplyTo":"20200501073222.GB27392@coredump.intra.peff.net","subject":"[PATCH v2 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-05-01T07:33:11Z","receivedAt":"2020-05-01T07:33:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There's an example of using your own bit of shell to act as a credential\nhelper, but it's not very realistic:\n\n - It's stupid to hand out your secret password to _every_ host. In the\n   real world you'd use the config-matcher to limit it to a particular\n   host.\n\n - We never provided a username. We can easily do that in another config\n   option (you can do it in the helper, too, but this is much more\n   readable).\n\n - We were sending the secret even for store/erase operations. This\n   is OK because Git would just ignore it, but a real system would\n   probably be unlocking a password store, which you wouldn't want to do\n   more than necessary.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Documentation/gitcredentials.txt | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt\nindex 8127dfcd2f..0d0f7149bd 100644\n--- a/Documentation/gitcredentials.txt\n+++ b/Documentation/gitcredentials.txt\n@@ -233,8 +233,9 @@ Here are some example specifications:\n \thelper = \"/path/to/my/helper --with-arguments\"\n \n # or you can specify your own shell snippet\n-[credential]\n-\thelper = \"!f() { echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n+[credential \"https://example.com\"]\n+\tusername = your_user\n+\thelper = \"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n ----------------------------------------------------\n \n Generally speaking, rule (3) above is the simplest for users to specify.\n-- \n2.26.2.933.gdf62622942\n"},{"id":"396752","messageId":"448114a7ab365f81971bfd9dd75b781f4145b098.camel@gmail.com","threadId":"53356","inReplyTo":"20200501061917.GC23665@coredump.intra.peff.net","subject":"Re: [PATCH] doc: fix quoting bug in credential cache example","fromName":"","fromEmail":"douglas.fuller@gmail.com","sentAt":"2020-05-01T14:40:09Z","receivedAt":"2020-05-01T14:40:15Z","isPatch":true,"sender":{"key":"douglas.fuller@gmail.com","avatar":null},"body":"On Fri, 2020-05-01 at 02:19 -0400, Jeff King wrote:\n> On Fri, May 01, 2020 at 01:57:38AM -0400, Jeff King wrote:\n> \n> > It may be easier to just use double-quotes consistently, even for\n> > ones\n> > that do not need it, to give readers one less thing to wonder\n> > about.\n> \n> So here's a patch that does that. I also noticed a few other\n> deficiencies in that final example, which are fixed in the second\n> patch.\n> \n> I hope I'm not stealing Douglas's thunder. :)\n\nMy one-line patch definitely wasn't thunderous -- I didn't even know\nhow to write a comment in a config file. As posited upthread, I did\ninterpret the example as presenting config file syntax, so I got\ntripped up by the semicolon there. This patch makes it more clear.\n\nI use git (thanks!) without having looked at the code, so I figured I\nwas a good example of the target audience for this doc. Thanks for\nmaking it more clear.\n\nCheers,\n--Doug\n\n>   [1/2]: gitcredentials(7): clarify quoting of helper examples\n>   [2/2]: gitcredentials(7): make shell-snippet example more realistic\n> \n>  Documentation/gitcredentials.txt | 16 +++++++++++-----\n>  1 file changed, 11 insertions(+), 5 deletions(-)\n> \n> -Peff\n\n"},{"id":"396754","messageId":"xmqqr1w3ofko.fsf@gitster.c.googlers.com","threadId":"53356","inReplyTo":"20200501063513.GB26128@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] gitcredentials(7): make shell-snippet example more realistic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-05-01T15:11:35Z","receivedAt":"2020-05-01T15:11:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> I can add back in the quoted \\\", though it does make the code slightly\n>> harder to read.\n>\n> Or did you mean passing $1 in the test call? It definitely isn't good\n> shell practice, but we know that we're getting a single-word action from\n> Git, per the protocol.\n\nI meant former.  I actually am having a second thought.\n\nI do not think this page should be the canonical place for config\nand shell syntax lessons, but the value we have been using as an\nexample is a good candidate that an end user would encounter in the\nreal life and has need to do so.  \n\nPerhaps it deserves some comment next to it?\n\n; For the specified site, use \"your_user\" as the username, and\n; invoke the helper, which is a short shell script.  Note that the\n; value of the helper variable is enclosed in a double-quote pair,\n; because it has a semicolon, which will cause the rest of the line\n; discarded as a comment unless quoted.  The shell script in turn\n; needs to quote various pieces of it in double quotes, each of\n; which needs to be escaped with a backslash.\n\n[credential \"https://example.com\"]\n    username = your_user\n    helper = \"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n\n\nThe more I think about it, the worse it becomes X-<\n\nDo we expect that most of our users are comfortable editing\n~/.gitconfig in their editor, or do they mostly work with the \"git\nconfig --global\" command?\n\nI have a feeling that my wishful-thinking answer, which is \"former\",\nis not true in the real world.  Which means they not only need to\nquote for the parser of the configuration file, but they then need\nto quote that for the shell X-<.\n\n    $ git config --global credential.https://example.com.helper \\\n\t'\"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"'\n\nI wonder if it helps the users to have something that guides them to\nfigure out how they do the above.\n\n> Fully quoting, it looks like this:\n>\n>   helper = \"!f() { test \\\"$1\\\" = get && echo \\\"password=$(cat $HOME/.secret)\\\"; }; f\"\n>\n> which IMHO is getting a little hard to read. I think that's part of why\n> I gave such an unfinished example in the first place. :)\n\nYes, \"these are values, go quote them as necessary\" is certainly a\nlot more attractive position to take.  But apparently that wasn't\nall that helpful.\n\nThanks.\n\n"}]}