{"thread":{"id":"47032","subject":"[PATCH] t0000: check whether the shell supports the \"local\" keyword","startedAt":"2017-10-26T08:19:12Z","lastAt":"2017-10-30T17:39:48Z","messageCount":6,"participants":["Michael Haggerty","Eric Sunshine","Jacob Keller","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"331075","messageId":"6ecab31e7ed05f5e79ecd454b133a2bfa6ac9ab7.1509005669.git.mhagger@alum.mit.edu","threadId":"47032","inReplyTo":null,"subject":"[PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-10-26T08:18:53Z","receivedAt":"2017-10-26T08:19:12Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Add a test balloon to see if we get complaints from anybody who is\nusing a shell that doesn't support the \"local\" keyword. If so, this\ntest can be reverted. If not, we might want to consider using \"local\"\nin shell code throughout the git code base.\n\nSigned-off-by: Michael Haggerty \n---\nThis has been discussed on the mailing list [1,2].\n\nMichael\n\n[1] https://public-inbox.org/git/CAPig+cRLB=dGD=+Af=yYL3M709LRpeUrtvcDLo9iBKYy2HAW-w@mail.gmail.com/\n[2] https://public-inbox.org/git/20160601163747.GA10721@sigill.intra.peff.net/\n\n t/t0000-basic.sh | 25 +++++++++++++++++++++++++\n 1 file changed, 25 insertions(+)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 1aa5093f36..7fd87dd544 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -20,6 +20,31 @@ modification *should* take notice and update the test vectors here.\n \n . ./test-lib.sh\n \n+try_local_x () {\n+\tlocal x=\"local\" &&\n+\techo \"$x\"\n+}\n+\n+# This test is an experiment to check whether any Git users are using\n+# Shells that don't support the \"local\" keyword. \"local\" is not\n+# POSIX-standard, but it is very widely supported by POSIX-compliant\n+# shells, and if it doesn't cause problems for people, we would like\n+# to be able to use it in Git code.\n+#\n+# For now, this is the only test that requires \"local\". If your shell\n+# fails this test, you can ignore the failure, but please report the\n+# problem to the Git mailing list , as it might\n+# convince us to continue avoiding the use of \"local\".\n+test_expect_success 'verify that the running shell supports \"local\"' '\n+\tx=\"notlocal\" &&\n+\techo \"local\" >expected1 &&\n+\ttry_local_x >actual1 &&\n+\ttest_cmp expected1 actual1 &&\n+\techo \"notlocal\" >expected2 &&\n+\techo \"$x\" >actual2 &&\n+\ttest_cmp expected2 actual2\n+'\n+\n ################################################################\n # git init has been done in an empty repository.\n # make sure it is empty.\n-- \n2.14.1\n\n"},{"id":"331076","messageId":"CAPig+cTv4YW0m0PLH+UucEHjgQkbCsOunPrkKVDrPQXNkd=GAg@mail.gmail.com","threadId":"47032","inReplyTo":"6ecab31e7ed05f5e79ecd454b133a2bfa6ac9ab7.1509005669.git.mhagger@alum.mit.edu","subject":"Re: [PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-10-26T08:28:48Z","receivedAt":"2017-10-26T08:28:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Oct 26, 2017 at 4:18 AM, Michael Haggerty wrote:\n> Add a test balloon to see if we get complaints from anybody who is\n> using a shell that doesn't support the \"local\" keyword. If so, this\n> test can be reverted. If not, we might want to consider using \"local\"\n> in shell code throughout the git code base.\n\nI would guess that the number of people who actually run the Git test\nsuite is microscopic compared to the number of people who use Git\nitself. It is not clear, therefore, that lack of reports of failure of\nthe new test would imply that \"local\" can safely be used throughout\nthe Git code base. At best, it might indicate that \"local\" can be used\nin the tests.\n\nOr, am I missing something?\n\n> Signed-off-by: Michael Haggerty \n> ---\n> This has been discussed on the mailing list [1,2].\n>\n> Michael\n>\n> [1] https://public-inbox.org/git/CAPig+cRLB=dGD=+Af=yYL3M709LRpeUrtvcDLo9iBKYy2HAW-w@mail.gmail.com/\n> [2] https://public-inbox.org/git/20160601163747.GA10721@sigill.intra.peff.net/\n>\n> t/t0000-basic.sh | 25 +++++++++++++++++++++++++\n> 1 file changed, 25 insertions(+)\n>\n> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n> index 1aa5093f36..7fd87dd544 100755\n> --- a/t/t0000-basic.sh\n> +++ b/t/t0000-basic.sh\n> @@ -20,6 +20,31 @@ modification *should* take notice and update the test vectors here.\n>\n> . ./test-lib.sh\n>\n> +try_local_x () {\n> + local x=\"local\" &&\n> + echo \"$x\"\n> +}\n> +\n> +# This test is an experiment to check whether any Git users are using\n> +# Shells that don't support the \"local\" keyword. \"local\" is not\n> +# POSIX-standard, but it is very widely supported by POSIX-compliant\n> +# shells, and if it doesn't cause problems for people, we would like\n> +# to be able to use it in Git code.\n> +#\n> +# For now, this is the only test that requires \"local\". If your shell\n> +# fails this test, you can ignore the failure, but please report the\n> +# problem to the Git mailing list , as it might\n> +# convince us to continue avoiding the use of \"local\".\n> +test_expect_success 'verify that the running shell supports \"local\"' '\n> + x=\"notlocal\" &&\n> + echo \"local\" >expected1 &&\n> + try_local_x >actual1 &&\n> + test_cmp expected1 actual1 &&\n> + echo \"notlocal\" >expected2 &&\n> + echo \"$x\" >actual2 &&\n> + test_cmp expected2 actual2\n> +'\n> +\n> ################################################################\n> # git init has been done in an empty repository.\n> # make sure it is empty.\n> --\n> 2.14.1\n"},{"id":"331077","messageId":"CA+P7+xoCKTaG9kV2T9YUHvagHVzD6v7A=neLzF3Qj1q8Fi0u-w@mail.gmail.com","threadId":"47032","inReplyTo":"CAPig+cTv4YW0m0PLH+UucEHjgQkbCsOunPrkKVDrPQXNkd=GAg@mail.gmail.com","subject":"Re: [PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2017-10-26T08:40:46Z","receivedAt":"2017-10-26T08:41:16Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Oct 26, 2017 at 1:28 AM, Eric Sunshine wrote:\n> On Thu, Oct 26, 2017 at 4:18 AM, Michael Haggerty wrote:\n>> Add a test balloon to see if we get complaints from anybody who is\n>> using a shell that doesn't support the \"local\" keyword. If so, this\n>> test can be reverted. If not, we might want to consider using \"local\"\n>> in shell code throughout the git code base.\n>\n> I would guess that the number of people who actually run the Git test\n> suite is microscopic compared to the number of people who use Git\n> itself. It is not clear, therefore, that lack of reports of failure of\n> the new test would imply that \"local\" can safely be used throughout\n> the Git code base. At best, it might indicate that \"local\" can be used\n> in the tests.\n>\n> Or, am I missing something?\n>\n\nI don't think you're missing anything. I think the idea here is: \"do\nany users who actively run the test suite care if we start using\nlocal\". I don't think the goal is to allow use of local in non-test\nsuite code. At least, that's not how I interpreted it.\n\nThus it's fine to be only as part of a test and see if anyone\ncomplains, since the only people affected would be those which\nactually run the test suite...\n\nChanging our requirement for regular shell scripts we ship seems a lot\ntrickier to gauge.\n\nThanks,\nJake\n"},{"id":"331079","messageId":"9a900b29-0974-9ec8-d15b-73c22c197327@alum.mit.edu","threadId":"47032","inReplyTo":"CA+P7+xoCKTaG9kV2T9YUHvagHVzD6v7A=neLzF3Qj1q8Fi0u-w@mail.gmail.com","subject":"Re: [PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2017-10-26T08:47:56Z","receivedAt":"2017-10-26T08:48:09Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/26/2017 10:40 AM, Jacob Keller wrote:\n> On Thu, Oct 26, 2017 at 1:28 AM, Eric Sunshine wrote:\n>> On Thu, Oct 26, 2017 at 4:18 AM, Michael Haggerty wrote:\n>>> Add a test balloon to see if we get complaints from anybody who is\n>>> using a shell that doesn't support the \"local\" keyword. If so, this\n>>> test can be reverted. If not, we might want to consider using \"local\"\n>>> in shell code throughout the git code base.\n>>\n>> I would guess that the number of people who actually run the Git test\n>> suite is microscopic compared to the number of people who use Git\n>> itself. It is not clear, therefore, that lack of reports of failure of\n>> the new test would imply that \"local\" can safely be used throughout\n>> the Git code base. At best, it might indicate that \"local\" can be used\n>> in the tests.\n>>\n>> Or, am I missing something?\n>>\n> \n> I don't think you're missing anything. I think the idea here is: \"do\n> any users who actively run the test suite care if we start using\n> local\". I don't think the goal is to allow use of local in non-test\n> suite code. At least, that's not how I interpreted it.\n> \n> Thus it's fine to be only as part of a test and see if anyone\n> complains, since the only people affected would be those which\n> actually run the test suite...\n> \n> Changing our requirement for regular shell scripts we ship seems a lot\n> trickier to gauge.\n\nActually, I would hope that if this experiment is successful that we can\nuse \"local\" in production code, too.\n\nThe proper question isn't \"what fraction of Git users run the test\nsuite?\", because I agree with Eric that that is microscopic. The correct\nquestion is \"on what fraction of platforms where Git will be run has the\ntest suite been run by *somebody*?\", and I think (I hope!) that that\nfraction is quite high.\n\nReally...if you are compiling Git on a platform that is so deviant or\narchaic that it doesn't have a reasonable Shell, and you don't even\nbother running the test suite, you kindof deserve your fate, don't you?\n\nMichael\n"},{"id":"331131","messageId":"xmqqd159e6go.fsf@gitster.mtv.corp.google.com","threadId":"47032","inReplyTo":"CA+P7+xoCKTaG9kV2T9YUHvagHVzD6v7A=neLzF3Qj1q8Fi0u-w@mail.gmail.com","subject":"Re: [PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-27T01:15:51Z","receivedAt":"2017-10-27T01:15:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> I don't think you're missing anything. I think the idea here is: \"do\n> any users who actively run the test suite care if we start using\n> local\". I don't think the goal is to allow use of local in non-test\n> suite code. At least, that's not how I interpreted it.\n>\n> Thus it's fine to be only as part of a test and see if anyone\n> complains, since the only people affected would be those which\n> actually run the test suite...\n>\n> Changing our requirement for regular shell scripts we ship seems a lot\n> trickier to gauge.\n\nYup, that matches my expectations for what we would gain out of this\nchange.\n\n"},{"id":"331391","messageId":"20171030173941.bye4vyi3jmkzxfr5@sigill.intra.peff.net","threadId":"47032","inReplyTo":"6ecab31e7ed05f5e79ecd454b133a2bfa6ac9ab7.1509005669.git.mhagger@alum.mit.edu","subject":"Re: [PATCH] t0000: check whether the shell supports the \"local\" keyword","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-10-30T17:39:41Z","receivedAt":"2017-10-30T17:39:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 26, 2017 at 10:18:53AM +0200, Michael Haggerty wrote:\n\n> Add a test balloon to see if we get complaints from anybody who is\n> using a shell that doesn't support the \"local\" keyword. If so, this\n> test can be reverted. If not, we might want to consider using \"local\"\n> in shell code throughout the git code base.\n> \n> Signed-off-by: Michael Haggerty \n> ---\n> This has been discussed on the mailing list [1,2].\n\nThanks for following up, this looks nice and thorough to me.\n\n-Peff\n"}]}