{"thread":{"id":"51039","subject":"Git build on antique PowerMac","startedAt":"2019-05-05T19:43:23Z","lastAt":"2019-05-11T00:19:10Z","messageCount":9,"participants":["Jeffrey Walton","SZEDER Gábor","Eric Sunshine","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"374928","messageId":"CAH8yC8nTyq5Fjc-6fUNn1iwkCO9i1Vrc0vFqxEVg4Eor3h-0Lw@mail.gmail.com","threadId":"51039","inReplyTo":null,"subject":"Git build on antique PowerMac","fromName":"Jeffrey Walton","fromEmail":"noloader@gmail.com","sentAt":"2019-05-05T19:42:45Z","receivedAt":"2019-05-05T19:43:23Z","isPatch":false,"sender":{"key":"noloader@gmail.com","avatar":null},"body":"Hi Everyone,\n\nI have a PowerMac I use for testing. It provides several testing\ndifferentiators, like OS X 10.5, Bash 3.2, GCC 4.0.1, Apple cc-tools\nlinker, and big-endian PowerPC. (I think Gentoo provides a Linux image\nfor the hardware, but I don't use it).\n\nThe Git libraries and programs build fine out of the box. They also\nseem to work as expected once installed.\n\nThe pain point is the self self tests, which I have never been able to\nbuild (or execute). I'd like to close this gap.\n\nmake -C templates SHELL_PATH='/bin/sh' PERL_PATH='/usr/bin/perl'\n: no custom templates yet\nmake -C t/ all\nrm -f -r 'test-results'\nreadline() on unopened filehandle test_must_fail\nrun_sub_test_lib_test at check-non-portable-shell.pl line 34.\nModification of a read-only value attempted at\ncheck-non-portable-shell.pl line 34.\nmake[1]: *** [test-lint-shell-syntax] Error 255\n\nDoes anyone want to take a shot at this antique system? I can provide\nSSH access with root.\n\nJeff\n"},{"id":"374931","messageId":"20190505201452.GL14763@szeder.dev","threadId":"51039","inReplyTo":"CAH8yC8nTyq5Fjc-6fUNn1iwkCO9i1Vrc0vFqxEVg4Eor3h-0Lw@mail.gmail.com","subject":"Re: Git build on antique PowerMac","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-05-05T20:14:52Z","receivedAt":"2019-05-05T20:14:58Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, May 05, 2019 at 03:42:45PM -0400, Jeffrey Walton wrote:\n> I have a PowerMac I use for testing. It provides several testing\n> differentiators, like OS X 10.5, Bash 3.2, GCC 4.0.1, Apple cc-tools\n> linker, and big-endian PowerPC. (I think Gentoo provides a Linux image\n> for the hardware, but I don't use it).\n> \n> The Git libraries and programs build fine out of the box. They also\n> seem to work as expected once installed.\n> \n> The pain point is the self self tests, which I have never been able to\n> build (or execute). I'd like to close this gap.\n> \n> make -C templates SHELL_PATH='/bin/sh' PERL_PATH='/usr/bin/perl'\n> : no custom templates yet\n> make -C t/ all\n> rm -f -r 'test-results'\n> readline() on unopened filehandle test_must_fail\n> run_sub_test_lib_test at check-non-portable-shell.pl line 34.\n> Modification of a read-only value attempted at\n> check-non-portable-shell.pl line 34.\n> make[1]: *** [test-lint-shell-syntax] Error 255\n\nYou could try:\n\n make -C t all TEST_LINT=\n\nThat error comes from one of the \"lint\" targets that are supposed to\ncatch some common errors that we repeatedly made in our tests. This\nlinter is enabled by default, but this command disables it.\n\nYeah, it's not the proper solution, but I would think that you don't\nneed them on that antique PowerMac anyway.\n\n"},{"id":"374933","messageId":"CAPig+cSzsmeHp1S5WG3TVXEyZKt9eVrsCsWEkpwbNUZK_SJ53g@mail.gmail.com","threadId":"51039","inReplyTo":"CAH8yC8nTyq5Fjc-6fUNn1iwkCO9i1Vrc0vFqxEVg4Eor3h-0Lw@mail.gmail.com","subject":"Re: Git build on antique PowerMac","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-05T20:47:22Z","receivedAt":"2019-05-05T20:47:36Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, May 5, 2019 at 3:43 PM Jeffrey Walton wrote:\n> I have a PowerMac I use for testing. [...]\n> make -C t/ all\n> readline() on unopened filehandle test_must_fail\n> run_sub_test_lib_test at check-non-portable-shell.pl line 34.\n> Modification of a read-only value attempted at\n> check-non-portable-shell.pl line 34.\n> make[1]: *** [test-lint-shell-syntax] Error 255\n>\n> Does anyone want to take a shot at this antique system? I can provide\n> SSH access with root.\n\nAs the author of the code triggering that problem, I can take a look\nat it. (You already have my public SSH key on your Solaris box.)\n\nGiven [1], I can see (I guess) why it's complaining about modification\nof a read-only variable. However, the unopened filehandle complaint\nlooks odd.\n\n[1]: https://www.perlmonks.org/?node_id=570712#default_unlocalized\n"},{"id":"375191","messageId":"87mujwh98z.fsf@evledraar.gmail.com","threadId":"51039","inReplyTo":"CAPig+cSzsmeHp1S5WG3TVXEyZKt9eVrsCsWEkpwbNUZK_SJ53g@mail.gmail.com","subject":"Re: Git build on antique PowerMac","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-08T22:27:56Z","receivedAt":"2019-05-08T22:28:04Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, May 05 2019, Eric Sunshine wrote:\n\n> On Sun, May 5, 2019 at 3:43 PM Jeffrey Walton wrote:\n>> I have a PowerMac I use for testing. [...]\n>> make -C t/ all\n>> readline() on unopened filehandle test_must_fail\n>> run_sub_test_lib_test at check-non-portable-shell.pl line 34.\n>> Modification of a read-only value attempted at\n>> check-non-portable-shell.pl line 34.\n>> make[1]: *** [test-lint-shell-syntax] Error 255\n>>\n>> Does anyone want to take a shot at this antique system? I can provide\n>> SSH access with root.\n>\n> As the author of the code triggering that problem, I can take a look\n> at it. (You already have my public SSH key on your Solaris box.)\n>\n> Given [1], I can see (I guess) why it's complaining about modification\n> of a read-only variable. However, the unopened filehandle complaint\n> looks odd.\n>\n> [1]: https://www.perlmonks.org/?node_id=570712#default_unlocalized\n\nIt's because \"while (<>) { readline }\" didn't use to be supported until\n5.10, and he's on 5.8.\n\n $ seq 1 4 | ./miniperl -we 'print $]; while (<>) { chomp; print \"$.: $_ -> \" . readline }; print \"\\n\"'\n readline() on unopened filehandle 1: 1 -> at -e line 1.\n Use of uninitialized value in concatenation (.) or string at -e line 1.\n readline() on unopened filehandle 2: 2 -> at -e line 1.\n readline() on unopened filehandle 3: 3 -> at -e line 1.\n readline() on unopened filehandle 4: 4 -> at -e line 1.\n 5.009004\n $ seq 1 4 | perl -we 'print $]; while (<>) { chomp; print \"$.: $_ -> \" . readline }; print \"\\n\"'\n 5.0280011: 1 -> 2\n 3: 3 -> 4\n"},{"id":"375209","messageId":"CAPig+cTM-ObsvvNy8uDmO1mS_KfcjppeRBMe=pnM5GkvS5ZVgQ@mail.gmail.com","threadId":"51039","inReplyTo":"87mujwh98z.fsf@evledraar.gmail.com","subject":"Re: Git build on antique PowerMac","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-09T09:10:53Z","receivedAt":"2019-05-09T09:11:08Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, May 8, 2019 at 6:28 PM Ævar Arnfjörð Bjarmason wrote:\n> On Sun, May 05 2019, Eric Sunshine wrote:\n> > On Sun, May 5, 2019 at 3:43 PM Jeffrey Walton wrote:\n> >> readline() on unopened filehandle test_must_fail\n> >\n> > Given [1], I can see (I guess) why it's complaining about modification\n> > of a read-only variable. However, the unopened filehandle complaint\n> > looks odd.\n>\n> It's because \"while (<>) { readline }\" didn't use to be supported until\n> 5.10, and he's on 5.8.\n\nThanks for researching this. Now it makes sense.\n"},{"id":"375214","messageId":"20190509102037.27044-1-sunshine@sunshineco.com","threadId":"51039","inReplyTo":"CAH8yC8nTyq5Fjc-6fUNn1iwkCO9i1Vrc0vFqxEVg4Eor3h-0Lw@mail.gmail.com","subject":"[PATCH] check-non-portable-shell: support Perl versions older than 5.10","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-09T10:20:37Z","receivedAt":"2019-05-09T10:21:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For thoroughness when checking for one-shot environment variable\nassignments at shell function call sites, check-non-portable-shell\nstitches together incomplete lines (those ending with backslash). This\nallows it to correctly flag such undesirable usage even when the\nvariable assignment and function call are split across lines, for\nexample:\n\n FOO=bar \\\n func\n\nwhere 'func' is a shell function.\n\nThe stitching is accomplished like this:\n\n while (<>) {\n chomp;\n # stitch together incomplete lines (those ending with \"\\\")\n while (s/\\\\$//) {\n $_ .= readline;\n chomp;\n }\n # detect unportable/undesirable shell constructs\n ...\n }\n\nAlthough this implementation is well supported in reasonably modern Perl\nversions (5.10 and later), it fails in a couple ways with older versions\n(such as Perl 5.8 shipped with ancient Mac OS 10.5).\n\nIn particular, in older Perl versions, 'readline' is not connected to\nthe file handle associated with the \"magic\" while (<>) {...} construct,\nso 'readline' throws a \"readline() on unopened filehandle\" error.\nFurthermore, $_ assigned by the outer while-loop is read-only, so the\nattempt to modify it via \"$_ .= readline\" in the inner while-loop fails\nwith a \"Modification of a read-only value\" error.\n\nAvoid both problems by collecting the stitched-together line in a\nvariable other than $_ and dropping the inner loop entirely.\n\nSigned-off-by: Eric Sunshine \n---\n t/check-non-portable-shell.pl | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 166d64d4a2..60e607ba42 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -27,14 +27,14 @@ sub err {\n \tclose $f;\n }\n \n+my $line = '';\n while (<>) {\n \tchomp;\n+\t$line .= $_;\n \t# stitch together incomplete lines (those ending with \"\\\")\n-\twhile (s/\\\\$//) {\n-\t\t$_ .= readline;\n-\t\tchomp;\n-\t}\n+\tnext if $line =~ s/\\\\$//;\n \n+\tlocal $_ = $line;\n \t/\\bcp\\s+-a/ and err 'cp -a is not portable';\n \t/\\bsed\\s+-[^efn]\\s+/ and err 'sed option not portable (use only -n, -e, -f)';\n \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n@@ -48,6 +48,7 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/^\\s*([A-Z0-9_]+=(\\w+|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n+\t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n }\n-- \n2.21.0.1084.g81c186ecd2\n\n"},{"id":"375222","messageId":"87ftpnhknn.fsf@evledraar.gmail.com","threadId":"51039","inReplyTo":"20190509102037.27044-1-sunshine@sunshineco.com","subject":"Re: [PATCH] check-non-portable-shell: support Perl versions older than 5.10","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-09T12:33:48Z","receivedAt":"2019-05-09T12:33:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, May 09 2019, Eric Sunshine wrote:\n\n> For thoroughness when checking for one-shot environment variable\n> assignments at shell function call sites, check-non-portable-shell\n> stitches together incomplete lines (those ending with backslash). This\n> allows it to correctly flag such undesirable usage even when the\n> variable assignment and function call are split across lines, for\n> example:\n>\n> FOO=bar \\\n> func\n>\n> where 'func' is a shell function.\n>\n> The stitching is accomplished like this:\n>\n> while (<>) {\n> chomp;\n> # stitch together incomplete lines (those ending with \"\\\")\n> while (s/\\\\$//) {\n> $_ .= readline;\n> chomp;\n> }\n> # detect unportable/undesirable shell constructs\n> ...\n> }\n>\n> Although this implementation is well supported in reasonably modern Perl\n> versions (5.10 and later), it fails in a couple ways with older versions\n> (such as Perl 5.8 shipped with ancient Mac OS 10.5).\n>\n> In particular, in older Perl versions, 'readline' is not connected to\n> the file handle associated with the \"magic\" while (<>) {...} construct,\n> so 'readline' throws a \"readline() on unopened filehandle\" error.\n> Furthermore, $_ assigned by the outer while-loop is read-only, so the\n> attempt to modify it via \"$_ .= readline\" in the inner while-loop fails\n> with a \"Modification of a read-only value\" error.\n>\n> Avoid both problems by collecting the stitched-together line in a\n> variable other than $_ and dropping the inner loop entirely.\n>\n> Signed-off-by: Eric Sunshine \n> ---\n> t/check-non-portable-shell.pl | 9 +++++----\n> 1 file changed, 5 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index 166d64d4a2..60e607ba42 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -27,14 +27,14 @@ sub err {\n> \tclose $f;\n> }\n>\n> +my $line = '';\n> while (<>) {\n> \tchomp;\n> +\t$line .= $_;\n> \t# stitch together incomplete lines (those ending with \"\\\")\n> -\twhile (s/\\\\$//) {\n> -\t\t$_ .= readline;\n> -\t\tchomp;\n> -\t}\n> +\tnext if $line =~ s/\\\\$//;\n>\n> +\tlocal $_ = $line;\n> \t/\\bcp\\s+-a/ and err 'cp -a is not portable';\n> \t/\\bsed\\s+-[^efn]\\s+/ and err 'sed option not portable (use only -n, -e, -f)';\n> \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n> @@ -48,6 +48,7 @@ sub err {\n> \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n> \t/^\\s*([A-Z0-9_]+=(\\w+|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n> \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n> +\t$line = '';\n> \t# this resets our $. for each file\n> \tclose ARGV if eof;\n> }\n\nThis fix is fine, but just for the record: There's no problem with\nassigning to $_, it just throws an error about $_ *because* of the\nreadline() issue, i.e. it'll fail, clobber $_ to a read-only value, and\noff we go.\n\nSo just assigning to $_ is fine, and you don't need to localize it.\n\nAnyway, I tested this on 5.8, it works, then looked at the output and\nwondered if I could improve it, came up with this:\n\n diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n index 60e607ba42..d5fd0a3050 100755\n --- a/t/check-non-portable-shell.pl\n +++ b/t/check-non-portable-shell.pl\n @@ -8,13 +8,26 @@\n\n my $exit_code=0;\n my %func;\n +my $start_nr = 0;\n +my $line = '';\n\n sub err {\n \tmy $msg = shift;\n -\ts/^\\s+//;\n -\ts/\\s+$//;\n -\ts/\\s+/ /g;\n -\tprint \"$ARGV:$.: error: $msg: $_\\n\";\n +\tif (/\\n/) {\n +\t\t$. = $start_nr;\n +\t\tmy ($ws) = $_ =~ /^(\\s+)/;\n +\t\tfor (split /^/) {\n +\t\t\ts/^\\Q$ws\\E//;\n +\t\t\tprint \"$ARGV:$.: error: $msg: $_\";\n +\t\t\t$.++;\n +\t\t}\n +\t\tprint \"\\n\";\n +\t} else {\n +\t\ts/^\\s+//;\n +\t\ts/\\s+$//;\n +\t\ts/\\s+/ /g;\n +\t\tprint \"$ARGV:$.: error: $msg: $_\\n\";\n +\t}\n \t$exit_code = 1;\n }\n\n @@ -27,14 +40,16 @@ sub err {\n \tclose $f;\n }\n\n -my $line = '';\n while (<>) {\n \tchomp;\n -\t$line .= $_;\n \t# stitch together incomplete lines (those ending with \"\\\")\n -\tnext if $line =~ s/\\\\$//;\n -\n -\tlocal $_ = $line;\n +\tif (s/\\\\$//) {\n +\t\t$start_nr ||= $.;\n +\t\t$line .= \"$_\\n\";\n +\t\tnext;\n +\t} else {\n +\t\t$_ = $line . $_;\n +\t}\n \t/\\bcp\\s+-a/ and err 'cp -a is not portable';\n \t/\\bsed\\s+-[^efn]\\s+/ and err 'sed option not portable (use only -n, -e, -f)';\n \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n @@ -48,7 +63,11 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/^\\s*([A-Z0-9_]+=(\\w+|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n +\n +\t# No longer spanning lines\n +\t$start_nr = 0;\n \t$line = '';\n +\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n }\n diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\n index c03054c538..b4af7032ad 100755\n --- a/t/t0000-basic.sh\n +++ b/t/t0000-basic.sh\n @@ -156,8 +156,11 @@ test_expect_success 'pretend we have a fully passing test suite' \"\n \"\n\n test_expect_success 'pretend we have a partially passing test suite' \"\n -\ttest_must_fail run_sub_test_lib_test \\\n -\t\tpartial-pass '2/3 tests passing' <<-\\\\EOF &&\n +\ttest_must_fail penis run_sub_test_lib_test \\\n +\t\tpartial-pass '2/3 tests passing' <<-\\\\EOF \\\n +\t\tpartial-pass '2/3 tests passing' <<-\\\\EOF \\\n +\t\tpartial-pass '2/3 tests passing' <<-\\\\EOF \\\n +\t\tcp -a hi there &&\n \ttest_expect_success 'passing test #1' 'true'\n \ttest_expect_success 'failing test #2' 'false'\n \ttest_expect_success 'passing test #3' 'true'\n diff --git a/t/t0001-init.sh b/t/t0001-init.sh\n index 1f462204ea..a25ac208e5 100755\n --- a/t/t0001-init.sh\n +++ b/t/t0001-init.sh\n @@ -122,6 +122,7 @@ test_expect_success 'plain bare with GIT_WORK_TREE' '\n\n test_expect_success 'GIT_DIR bare' '\n \tmkdir git-dir-bare.git &&\n +\tcp -a foo bar &&\n \tGIT_DIR=git-dir-bare.git git init &&\n \tcheck_config git-dir-bare.git true unset\n '\n\nI.e. now for these multi-line issues we'll print the whole offending\nmulti-line invocation\n\n $ ~/g/perl/miniperl -I ~/g/perl/lib check-non-portable-shell.pl t[0-9]*.sh\n t0000-basic.sh:159: error: cp -a is not portable: test_must_fail penis run_sub_test_lib_test\n t0000-basic.sh:160: error: cp -a is not portable: partial-pass '2/3 tests passing' <<-\\\\EOF\n t0000-basic.sh:161: error: cp -a is not portable: partial-pass '2/3 tests passing' <<-\\\\EOF\n t0000-basic.sh:162: error: cp -a is not portable: partial-pass '2/3 tests passing' <<-\\\\EOF\n t0000-basic.sh:163: error: cp -a is not portable: cp -a hi there &&\n t0001-init.sh:125: error: cp -a is not portable: cp -a foo bar &&\n\nI figured it was better than the current output just squashing such a\nlong line together, i.e. it'll print this now (before/after this patch):\n\n $ ~/g/perl/miniperl -I ~/g/perl/lib check-non-portable-shell.pl t[0-9]*.sh\n t0000-basic.sh:163: error: cp -a is not portable: test_must_fail penis run_sub_test_lib_test partial-pass '2/3 tests passing' <<-\\\\EOF partial-pass '2/3 tests passing' <<-\\\\EOF partial-pass '2/3 tests passing' <<-\\\\EOF cp -a hi there &&\n t0001-init.sh:125: error: cp -a is not portable: cp -a foo bar &&\n\nThere's ways to make that WIP patch shorter etc. I wasn't trying to golf\nit, also I think we can get rid of that s/\\s+$// if we split up lines\nlike this.\n"},{"id":"375316","messageId":"CAPig+cR30dnurYq=G_Ki2Nf7vFCkebcR+egvhbGE5w8XSBKQBA@mail.gmail.com","threadId":"51039","inReplyTo":"87ftpnhknn.fsf@evledraar.gmail.com","subject":"Re: [PATCH] check-non-portable-shell: support Perl versions older than 5.10","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-10T20:39:58Z","receivedAt":"2019-05-10T20:40:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, May 9, 2019 at 8:33 AM Ævar Arnfjörð Bjarmason wrote:\n> On Thu, May 09 2019, Eric Sunshine wrote:\n> > Although this implementation is well supported in reasonably modern Perl\n> > versions (5.10 and later), it fails in a couple ways with older versions\n> > (such as Perl 5.8 shipped with ancient Mac OS 10.5).\n> > [...]\n> > Signed-off-by: Eric Sunshine \n>\n> This fix is fine, but just for the record: There's no problem with\n> assigning to $_, it just throws an error about $_ *because* of the\n> readline() issue, i.e. it'll fail, clobber $_ to a read-only value, and\n> off we go.\n> So just assigning to $_ is fine, and you don't need to localize it.\n\nThanks for the clarification. I'll drop the misleading discussion of\n$_ from the commit message and eliminate $_ localization from the\npatch proper.\n\n> Anyway, I tested this on 5.8, it works, then looked at the output and\n> wondered if I could improve it, came up with this:\n> [...large WIP patch snipped...]\n> I figured it was better than the current output just squashing such a\n> long line together [...]\n\nThat could be done, though it is outside the scope of this\nportability-fix patch.\n\nAlso, the existing output with its sufficiently clear problem\nexplanations is probably \"good enough\" to clue in the developer of new\ntest code as to what needs fixing, so I'm also not sure that it's\nworth that much effort to prettify what should be a relatively rare\nwarning message from code which has not yet made it into the project.\nI wouldn't be opposed to someone working on it, but I don't plan on\ntackling it myself.\n"},{"id":"375330","messageId":"20190511001853.23011-1-sunshine@sunshineco.com","threadId":"51039","inReplyTo":"20190509102037.27044-1-sunshine@sunshineco.com","subject":"[PATCH v2] check-non-portable-shell: support Perl versions older than 5.10","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-05-11T00:18:53Z","receivedAt":"2019-05-11T00:19:10Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"For thoroughness when checking for one-shot environment variable\nassignments at shell function call sites, check-non-portable-shell\nstitches together incomplete lines (those ending with backslash). This\nallows it to correctly flag such undesirable usage even when the\nvariable assignment and function call are split across lines, for\nexample:\n\n FOO=bar \\\n func\n\nwhere 'func' is a shell function.\n\nThe stitching is accomplished like this:\n\n while (<>) {\n chomp;\n # stitch together incomplete lines (those ending with \"\\\")\n while (s/\\\\$//) {\n $_ .= readline;\n chomp;\n }\n # detect unportable/undesirable shell constructs\n ...\n }\n\nAlthough this implementation is well supported in reasonably modern Perl\nversions (5.10 and later), it fails with older versions (such as Perl\n5.8 shipped with ancient Mac OS 10.5). In particular, in older Perl\nversions, 'readline' is not connected to the file handle associated with\nthe \"magic\" while (<>) {...} construct, so 'readline' throws a\n\"readline() on unopened filehandle\" error. Work around this problem by\ndropping readline() and instead incorporating the stitching of\nincomplete lines directly into the existing while (<>) {...} loop.\n\nHelped-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Eric Sunshine \n---\n\nThis is a re-roll of [1]. The only change since v1 is to drop\nlocalization of $_ as suggested by Ævar in [2], and rewriting the commit\nmessage to avoid misleadingly talking about $_ as being problematic.\n\n[1]: http://public-inbox.org/git/20190509102037.27044-1-sunshine@sunshineco.com/\n[2]: http://public-inbox.org/git/87ftpnhknn.fsf@evledraar.gmail.com/\n\n t/check-non-portable-shell.pl | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 166d64d4a2..38bfeebd88 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -27,14 +27,14 @@ sub err {\n \tclose $f;\n }\n \n+my $line = '';\n while (<>) {\n \tchomp;\n+\t$line .= $_;\n \t# stitch together incomplete lines (those ending with \"\\\")\n-\twhile (s/\\\\$//) {\n-\t\t$_ .= readline;\n-\t\tchomp;\n-\t}\n+\tnext if $line =~ s/\\\\$//;\n \n+\t$_ = $line;\n \t/\\bcp\\s+-a/ and err 'cp -a is not portable';\n \t/\\bsed\\s+-[^efn]\\s+/ and err 'sed option not portable (use only -n, -e, -f)';\n \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n@@ -48,6 +48,7 @@ sub err {\n \t/\\bexport\\s+[A-Za-z0-9_]*=/ and err '\"export FOO=bar\" is not portable (use FOO=bar && export FOO)';\n \t/^\\s*([A-Z0-9_]+=(\\w+|([\"']).*?\\3)\\s+)+(\\w+)/ and exists($func{$4}) and\n \t\terr '\"FOO=bar shell_func\" assignment extends beyond \"shell_func\"';\n+\t$line = '';\n \t# this resets our $. for each file\n \tclose ARGV if eof;\n }\n-- \n2.21.0.1090.g8b4b5564af\n\n"}]}