{"thread":{"id":"46050","subject":"[PATCH] doc: filter-branch does not require re-export of vars","startedAt":"2017-05-26T17:37:18Z","lastAt":"2017-05-29T04:58:59Z","messageCount":7,"participants":["Andreas Heiduk","Jeff King","Junio C Hamano","Samuel Lijin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"320828","messageId":"20170526173654.4238-1-asheiduk@gmail.com","threadId":"46050","inReplyTo":null,"subject":"[PATCH] doc: filter-branch does not require re-export of vars","fromName":"Andreas Heiduk","fromEmail":"asheiduk@gmail.com","sentAt":"2017-05-26T17:36:54Z","receivedAt":"2017-05-26T17:37:18Z","isPatch":true,"sender":{"key":"asheiduk@gmail.com","avatar":"https://avatars.githubusercontent.com/u/9371344?v=4"},"body":"The function `set_ident` in `filter-branch` exported the variables\nGIT_(AUTHOR|COMMITTER)_(NAME|EMAIL|DATE) at least since 6f6826c52b in 2007.\nTherefore the filter scripts don't need to re-eport them again.\n\nSigned-off-by: Andreas Heiduk \n---\n Documentation/git-filter-branch.txt | 5 +----\n 1 file changed, 1 insertion(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt\nindex 6e4bb0220..7b695dbb7 100644\n--- a/Documentation/git-filter-branch.txt\n+++ b/Documentation/git-filter-branch.txt\n@@ -86,8 +86,7 @@ OPTIONS\n \tThis filter may be used if you only need to modify the environment\n \tin which the commit will be performed. Specifically, you might\n \twant to rewrite the author/committer name/email/time environment\n-\tvariables (see linkgit:git-commit-tree[1] for details). Do not forget\n-\tto re-export the variables.\n+\tvariables (see linkgit:git-commit-tree[1] for details).\n \n --tree-filter ::\n \tThis is the filter for rewriting the tree and its contents.\n@@ -340,12 +339,10 @@ git filter-branch --env-filter '\n \tif test \"$GIT_AUTHOR_EMAIL\" = \"root@localhost\"\n \tthen\n \t\tGIT_AUTHOR_EMAIL=john@example.com\n-\t\texport GIT_AUTHOR_EMAIL\n \tfi\n \tif test \"$GIT_COMMITTER_EMAIL\" = \"root@localhost\"\n \tthen\n \t\tGIT_COMMITTER_EMAIL=john@example.com\n-\t\texport GIT_COMMITTER_EMAIL\n \tfi\n ' -- --all\n --------------------------------------------------------\n-- \n2.13.0\n\n"},{"id":"320831","messageId":"20170526183702.jrjkykhldz74pquq@sigill.intra.peff.net","threadId":"46050","inReplyTo":"20170526173654.4238-1-asheiduk@gmail.com","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-26T18:37:02Z","receivedAt":"2017-05-26T18:37:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 26, 2017 at 07:36:54PM +0200, Andreas Heiduk wrote:\n\n> The function `set_ident` in `filter-branch` exported the variables\n> GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL|DATE) at least since 6f6826c52b in 2007.\n> Therefore the filter scripts don't need to re-eport them again.\n\nSome old shells keep separate values for the internal and exporter\nversions of variables. I.e., this:\n\n foo=one\n export foo\n foo=two\n\nwould continue to export $foo as \"one\", even though it is \"two\" inside\nthe script.\n\nHowever, I think POSIX mandates the behavior you'd expect. And the only\nshell I know that misbehaves in this way is Solaris /bin/sh, which we\nhave already declared too broken to support. According to\n\n https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Limitations-of-Builtins.html#export\n\nit sounds like there are some other antique shells which may do the\nsame (it doesn't cover this case explicitly, but the \"coexist\" cases it\nmentions are likely to behave in this way).\n\nAt this point, I'd be inclined to remove the text as you suggest and\neither make a small note at the bottom of the page, or just omit it\nentirely and assume that anybody on an old non-POSIX shell can fend for\nthemselves.\n\n-Peff\n"},{"id":"320911","messageId":"xmqqlgpga69n.fsf@gitster.mtv.corp.google.com","threadId":"46050","inReplyTo":"20170526183702.jrjkykhldz74pquq@sigill.intra.peff.net","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-28T23:52:36Z","receivedAt":"2017-05-28T23:52:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, May 26, 2017 at 07:36:54PM +0200, Andreas Heiduk wrote:\n>\n>> The function `set_ident` in `filter-branch` exported the variables\n>> GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL|DATE) at least since 6f6826c52b in 2007.\n>> Therefore the filter scripts don't need to re-eport them again.\n>\n> Some old shells keep separate values for the internal and exporter\n> versions of variables. I.e., this:\n>\n> foo=one\n> export foo\n> foo=two\n>\n> would continue to export $foo as \"one\", even though it is \"two\" inside\n> the script.\n\nYup, that sounds like a grandfather's war story at this point,\nthough ;-)\n\n> However, I think POSIX mandates the behavior you'd expect. ...\n> ...\n> At this point, I'd be inclined to remove the text as you suggest and\n> either make a small note at the bottom of the page, or just omit it\n> entirely and assume that anybody on an old non-POSIX shell can fend for\n> themselves.\n\nSounds good to me. Thanks.\n"},{"id":"320917","messageId":"CAJZjrdVjeeHUN_biza-MA9iqP5Y5CO3DkLaGG-Bq1w5oo5tW_g@mail.gmail.com","threadId":"46050","inReplyTo":"20170526183702.jrjkykhldz74pquq@sigill.intra.peff.net","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Samuel Lijin","fromEmail":"sxlijin@gmail.com","sentAt":"2017-05-29T01:35:30Z","receivedAt":"2017-05-29T01:36:18Z","isPatch":true,"sender":{"key":"sxlijin@gmail.com","avatar":"https://gravatar.com/avatar/01777bf1eae64e2b4dca97dcac182a6abbcf6fd8cb4d5b8fa33edf9f8cc21746?d=mp&s=160"},"body":"On Fri, May 26, 2017 at 2:37 PM, Jeff King wrote:\n>\n> On Fri, May 26, 2017 at 07:36:54PM +0200, Andreas Heiduk wrote:\n>\n> > The function `set_ident` in `filter-branch` exported the variables\n> > GIT_(AUTHOR|COMMITTER)_(NAME|EMAIL|DATE) at least since 6f6826c52b in 2007.\n> > Therefore the filter scripts don't need to re-eport them again.\n>\n> Some old shells keep separate values for the internal and exporter\n> versions of variables. I.e., this:\n>\n> foo=one\n> export foo\n> foo=two\n>\n> would continue to export $foo as \"one\", even though it is \"two\" inside\n> the script.\n>\n> However, I think POSIX mandates the behavior you'd expect. And the only\n> shell I know that misbehaves in this way is Solaris /bin/sh, which we\n> have already declared too broken to support.\n\nOff-topic, but where is this explicitly documented?\n\n> According to\n>\n> https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Limitations-of-Builtins.html#export\n>\n> it sounds like there are some other antique shells which may do the\n> same (it doesn't cover this case explicitly, but the \"coexist\" cases it\n> mentions are likely to behave in this way).\n>\n> At this point, I'd be inclined to remove the text as you suggest and\n> either make a small note at the bottom of the page, or just omit it\n> entirely and assume that anybody on an old non-POSIX shell can fend for\n> themselves.\n>\n> -Peff\n"},{"id":"320923","messageId":"20170529042005.xgjx2kuuelak62sl@sigill.intra.peff.net","threadId":"46050","inReplyTo":"CAJZjrdVjeeHUN_biza-MA9iqP5Y5CO3DkLaGG-Bq1w5oo5tW_g@mail.gmail.com","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-29T04:20:06Z","receivedAt":"2017-05-29T04:20:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 28, 2017 at 09:35:30PM -0400, Samuel Lijin wrote:\n\n> > However, I think POSIX mandates the behavior you'd expect. And the only\n> > shell I know that misbehaves in this way is Solaris /bin/sh, which we\n> > have already declared too broken to support.\n> \n> Off-topic, but where is this explicitly documented?\n\nI couldn't find a place that mentioned it explicitly, but POSIX defines\nthe concept as \"the export attribute\" of the variables. Which implies to\nme that the bit is tied to the variable itself, not its value.\n\nIt also says that the flag is cleared when a variable is unset, so:\n\n foo=one\n export foo\n sh -c 'echo $foo should be one'\n unset foo\n foo=two\n sh -c 'echo $foo is not exported'\n\nThat could potentially affect somebody writing a filter-branch snippet,\nbut presumably if they are using \"unset\" they know what they're doing.\n\n-Peff\n"},{"id":"320925","messageId":"xmqqa85w8eym.fsf@gitster.mtv.corp.google.com","threadId":"46050","inReplyTo":"CAJZjrdVjeeHUN_biza-MA9iqP5Y5CO3DkLaGG-Bq1w5oo5tW_g@mail.gmail.com","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-05-29T04:27:45Z","receivedAt":"2017-05-29T04:27:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel Lijin writes:\n\n>> However, I think POSIX mandates the behavior you'd expect. And the only\n>> shell I know that misbehaves in this way is Solaris /bin/sh, which we\n>> have already declared too broken to support.\n>\n> Off-topic, but where is this explicitly documented?\n\nOne link I had readily available was\n\n https://public-inbox.org/git/7vei5qtnc5.fsf@alter.siamese.dyndns.org/\n\nbut there may be older discussions that were the actual process of\nour officially having \"written its /bin/sh off as broken and\nunusable\" if you dig further in the list archive.\n"},{"id":"320926","messageId":"20170529045843.ynbsfrwnefoyxwrm@sigill.intra.peff.net","threadId":"46050","inReplyTo":"xmqqa85w8eym.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] doc: filter-branch does not require re-export of vars","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-05-29T04:58:44Z","receivedAt":"2017-05-29T04:58:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 29, 2017 at 01:27:45PM +0900, Junio C Hamano wrote:\n\n> Samuel Lijin writes:\n> \n> >> However, I think POSIX mandates the behavior you'd expect. And the only\n> >> shell I know that misbehaves in this way is Solaris /bin/sh, which we\n> >> have already declared too broken to support.\n> >\n> > Off-topic, but where is this explicitly documented?\n> \n> One link I had readily available was\n> \n> https://public-inbox.org/git/7vei5qtnc5.fsf@alter.siamese.dyndns.org/\n> \n> but there may be older discussions that were the actual process of\n> our officially having \"written its /bin/sh off as broken and\n> unusable\" if you dig further in the list archive.\n\nAh, I had taken Samuel's question as \"where is the POSIX behavior\ndocumented\". :)\n\nThe link above is a good explanation of the $() problem on Solaris. It\nalso doesn't do ${parameter#word}. I couldn't find a specific moment of\noutlawing that /bin/sh, but there are several mentions of problems with\nit going back to the beginning. Here's one from 2007:\n\n http://public-inbox.org/git/7vabt9sasl.fsf@assigned-by-dhcp.cox.net/\n\nWe had already given up on it by then.\n\nI don't think there's anything in CodingGuidelines (though t/README does\ncall it \"broken\"), though it explicitly endorses $(), which rules out\nSolaris /bin/sh.\n\nI think we found in the last year or two that there are some old\nversions of ksh that don't like our scripts (definitely ksh88, and maybe\nksh93?). Some light reading for the curious:\n\n http://public-inbox.org/git/CALR6jEhviK9KZxR6R6xzkZ5EAO-RjWj3xYah_DOSDXhEjYsT-A@mail.gmail.com/\n\n http://public-inbox.org/git/CALR6jEjWjJA0X2qXsxqObqc_yxrgX87LYf8cmJ0MmJFF6PkmTQ@mail.gmail.com/\n\n http://public-inbox.org/git/xmqqinxt3kwq.fsf@gitster.mtv.corp.google.com/\n\nThe problems are around backslash-escaping, signal exit codes, and\ndoubled parentheses. There are some patches, but I think we decided not\nto pursue it (I couldn't find that exact decision, but it's referenced\nlater on).\n\n-Peff\n"}]}