{"thread":{"id":"49200","subject":"[PATCH] tests: fix and add lint for non-portable head -c N","startedAt":"2018-08-23T09:14:41Z","lastAt":"2018-08-28T20:17:14Z","messageCount":33,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Jeff King","Eric Sunshine","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"356350","messageId":"20180823091427.1756-1-avarab@gmail.com","threadId":"49200","inReplyTo":null,"subject":"[PATCH] tests: fix and add lint for non-portable head -c N","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T09:14:27Z","receivedAt":"2018-08-23T09:14:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"head -c BYTES\" option is non-portable (not in POSIX[1]). Change\nsuch invocations to use the test_copy_bytes wrapper added in\n48860819e8 (\"t9300: factor out portable \"head -c\" replacement\",\n2016-06-30).\n\nThis fixes a test added in 9d2e330b17 (\"ewah_read_mmap: bounds-check\nmmap reads\", 2018-06-14), which has been breaking\nt5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports\nalready have a similar workaround after their upgrade to 2.18.0[2].\n\nI have not tested this on IRIX, but according to 4de0bbd898 (\"t9300:\nuse perl \"head -c\" clone in place of \"dd bs=1 count=16000\" kluge\",\n2010-12-13) this invocation would have broken things there too.\n\nAlso, change a valgrind-specific codepath in test-lib.sh to use this\nwrapper. Given where valgrind runs I don't think this would ever\nbecome a portability issue in practice, but it's easier to just use\nthe wrapper than introduce some exception for the \"make test-lint\"\ncheck being added here.\n\n1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html\n2. https://github.com/openbsd/ports/commit/08d5d82eaefe5cf2f125ecc0c6a57df9cf91350c#diff-f7d3c4fabeed1691620d608f1534f5e5\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nJunio: Even though this isn't a 2.19.0-rc0 regression I think it makes\nsense for 2.19.0. The fix is trivial, and it'll unbreak (at least some\nof) the tests on stock git on OpenBSD.\n\nOpenBSD guys: If you CC the git mailing list when you find you need to\napply patches like these, we're happy to fix this more pro-actively. I\njust happened to be testing the upcoming 2.19.0 on OpenBSD and spotted\nthis.\n\n t/check-non-portable-shell.pl | 1 +\n t/t5310-pack-bitmaps.sh | 2 +-\n t/test-lib.sh | 4 ++--\n 3 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex d5823f71d8..94a7e6165e 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -35,6 +35,7 @@ sub err {\n \t\tchomp;\n \t}\n \n+\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n \t/\\bsed\\s+-i/ and err 'sed -i is not portable';\n \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 557bd0d0c0..7bff7923f2 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n \tgit rev-list --use-bitmap-index --count --all >expect &&\n \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n \ttest_when_finished \"rm -f $bitmap\" &&\n-\thead -c 512 <$bitmap >$bitmap.tmp &&\n+\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n \tmv -f $bitmap.tmp $bitmap &&\n \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n \ttest_cmp expect actual &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8bb0f4348e..44288cbb59 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -867,7 +867,7 @@ then\n \t\t# handle only executables, unless they are shell libraries that\n \t\t# need to be in the exec-path.\n \t\ttest -x \"$1\" ||\n-\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n+\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n \t\treturn;\n \n \t\tbase=$(basename \"$1\")\n@@ -882,7 +882,7 @@ then\n \t\t# do not override scripts\n \t\tif test -x \"$symlink_target\" &&\n \t\t test ! -d \"$symlink_target\" &&\n-\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n \t\tthen\n \t\t\tsymlink_target=../valgrind.sh\n \t\tfi\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356353","messageId":"87a7pdfltp.fsf@evledraar.gmail.com","threadId":"49200","inReplyTo":"20180823091427.1756-1-avarab@gmail.com","subject":"Test failures on OpenBSD","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T09:56:18Z","receivedAt":"2018-08-23T09:56:24Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is on OpenBSD 6.2 amd64 with my \"tests: fix and add lint for\nnon-portable head -c N\" patch (which fixes one failure).\n\n $ for t in t1305-config-include.sh t1308-config-set.sh t5004-archive-corner-cases.sh t5552-skipping-fetch-negotiator.sh; do ./$t --no-color -v -x 2>&1 | grep -B10 \"^not ok\"; done\n \n + cd bar\n + echo [includeIf \"gitdir:bar/\"]path=bar7\n + >> .git/config \n + echo [test]seven=7\n + > .git/bar7 \n + echo 7\n + > expect \n + git config test.seven\n + > actual \n error: last command exited with $?=1\n not ok 27 - conditional include, gitdir matching symlink\n --\n + cd bar\n + echo [includeIf \"gitdir/i:BAR/\"]path=bar8\n + >> .git/config \n + echo [test]eight=8\n + > .git/bar8 \n + echo 8\n + > expect \n + git config test.eight\n + > actual \n error: last command exited with $?=1\n not ok 28 - conditional include, gitdir matching symlink, icase\n \ttest_cmp expect actual\n \n + echo Error (-1) reading configuration file a-directory.\n + > expect \n + mkdir a-directory\n + test_expect_code 2 test-tool config configset_get_value foo.bar a-directory\n + 2> output \n Value not found for \"foo.bar\"\n test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory\n error: last command exited with $?=1\n not ok 23 - proper error on directory \"files\"\n + make_dir extract\n + tar xf subtree-all.tar -C extract\n tar: Cannot identify format. Searching...\n tar: End of archive volume 1 reached\n tar: Sorry, unable to determine archive format.\n error: last command exited with $?=1\n + rm -rf extract\n + exit 1\n + eval_ret=1\n + :\n not ok 9 - archive empty subtree with no pathspec\n --\n + make_dir extract\n + tar xf subtree-path.tar -C extract\n tar: Cannot identify format. Searching...\n tar: End of archive volume 1 reached\n tar: Sorry, unable to determine archive format.\n error: last command exited with $?=1\n + rm -rf extract\n + exit 1\n + eval_ret=1\n + :\n not ok 10 - archive empty subtree by direct pathspec\n 'git [...] -- [...]'\n fatal: ambiguous argument 'c7': unknown revision or path not in the working tree.\n Use '--' to separate paths from revisions, like this:\n 'git [...] -- [...]'\n No have c7 (c7)\n error: last command exited with $?=1\n + test_unconfig -C client fetch.negotiationalgorithm\n + exit 1\n + eval_ret=1\n + :\n not ok 1 - commits with no parents are sent regardless of skip distance\n --\n Author: A U Thor \n 1 file changed, 1 insertion(+)\n create mode 100644 to_fetch.t\n + git init client\n Initialized empty Git repository in /home/avar/g/git/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/\n + seq 11\n ./t5552-skipping-fetch-negotiator.sh: seq: not found\n + git -C client checkout c5\n error: pathspec 'c5' did not match any file(s) known to git\n error: last command exited with $?=1\n not ok 3 - when two skips collide, favor the larger one\n --\n + git init client\n Initialized empty Git repository in /home/avar/g/git/t/trash directory.t5552-skipping-fetch-negotiator/client/.git/\n + seq 8\n ./t5552-skipping-fetch-negotiator.sh: seq: not found\n + seq 19\n ./t5552-skipping-fetch-negotiator.sh: seq: not found\n + pwd\n + git -C server fetch --no-tags /home/avar/g/git/t/trash directory.t5552-skipping-fetch-negotiator/client b1:refs/heads/b1\n fatal: Couldn't find remote ref b1\n error: last command exited with $?=128\n not ok 6 - do not send \"have\" with ancestors of commits that server ACKed\n\nFull output at https://gitlab.com/snippets/1747801\n\nSome of this, like the t5552-skipping-fetch-negotiator.sh failures are\nnew in 2.19.0 (those go away with s/seq/test_seq). But some are\nolder. E.g. the archive corner cases failure is becuse that test wants\nunzip & GNU tar, which OpenBSD upstream has patched already:\nhttps://github.com/openbsd/ports/blob/master/devel/git/patches/patch-t_test-lib_sh\n"},{"id":"356371","messageId":"20180823152502.3886-1-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823091427.1756-1-avarab@gmail.com","subject":"[PATCH v2 1/2] tests: fix and add lint for non-portable head -c N","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T15:25:01Z","receivedAt":"2018-08-23T15:25:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"head -c BYTES\" option is non-portable (not in POSIX[1]). Change\nsuch invocations to use the test_copy_bytes wrapper added in\n48860819e8 (\"t9300: factor out portable \"head -c\" replacement\",\n2016-06-30).\n\nThis fixes a test added in 9d2e330b17 (\"ewah_read_mmap: bounds-check\nmmap reads\", 2018-06-14), which has been breaking\nt5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports\nalready have a similar workaround after their upgrade to 2.18.0[2].\n\nI have not tested this on IRIX, but according to 4de0bbd898 (\"t9300:\nuse perl \"head -c\" clone in place of \"dd bs=1 count=16000\" kluge\",\n2010-12-13) this invocation would have broken things there too.\n\nAlso, change a valgrind-specific codepath in test-lib.sh to use this\nwrapper. Given where valgrind runs I don't think this would ever\nbecome a portability issue in practice, but it's easier to just use\nthe wrapper than introduce some exception for the \"make test-lint\"\ncheck being added here.\n\n1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html\n2. https://github.com/openbsd/ports/commit/08d5d82eaefe5cf2f125ecc0c6a57df9cf91350c#diff-f7d3c4fabeed1691620d608f1534f5e5\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5310-pack-bitmaps.sh | 2 +-\n t/test-lib.sh | 4 ++--\n 3 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex d5823f71d8..c8f10d40a1 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -41,6 +41,7 @@ sub err {\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (use type)';\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n+\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\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\"';\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 557bd0d0c0..7bff7923f2 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n \tgit rev-list --use-bitmap-index --count --all >expect &&\n \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n \ttest_when_finished \"rm -f $bitmap\" &&\n-\thead -c 512 <$bitmap >$bitmap.tmp &&\n+\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n \tmv -f $bitmap.tmp $bitmap &&\n \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n \ttest_cmp expect actual &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8bb0f4348e..44288cbb59 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -867,7 +867,7 @@ then\n \t\t# handle only executables, unless they are shell libraries that\n \t\t# need to be in the exec-path.\n \t\ttest -x \"$1\" ||\n-\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n+\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n \t\treturn;\n \n \t\tbase=$(basename \"$1\")\n@@ -882,7 +882,7 @@ then\n \t\t# do not override scripts\n \t\tif test -x \"$symlink_target\" &&\n \t\t test ! -d \"$symlink_target\" &&\n-\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n \t\tthen\n \t\t\tsymlink_target=../valgrind.sh\n \t\tfi\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356372","messageId":"20180823152502.3886-2-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823091427.1756-1-avarab@gmail.com","subject":"[PATCH v2 2/2] tests: fix and add lint for non-portable seq","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T15:25:02Z","receivedAt":"2018-08-23T15:25:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"GNU seq is not a POSIX command, and doesn't exist on\ne.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 (\"tests:\nIntroduce test_seq\", 2012-08-04), but use of it keeps coming back,\ne.g. in the recently added \"fetch negotiator\" tests being added here.\n\nSo let's also add a check to \"make test-lint\". The regex is aiming to\ncapture the likes of $(seq ..) and \"seq\" as a stand-alone command,\nwithout capturing some existing cases where we e.g. have files called\n\"seq\".\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nNow with a fix & check in v2 for the seq issue mentioned in\nhttps://public-inbox.org/git/87a7pdfltp.fsf@evledraar.gmail.com/\n\n t/check-non-portable-shell.pl | 1 +\n t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n 3 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex c8f10d40a1..75f38298d7 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -42,6 +42,7 @@ sub err {\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n+\t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\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\"';\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nindex 5ad5bece55..30857b84a8 100755\n--- a/t/t5552-skipping-fetch-negotiator.sh\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -46,7 +46,7 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 7)\n+\tfor i in $(test_seq 7)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -89,7 +89,7 @@ test_expect_success 'when two skips collide, favor the larger one' '\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 11)\n+\tfor i in $(test_seq 11)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -168,14 +168,14 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 8)\n+\tfor i in $(test_seq 8)\n \tdo\n \t\tgit -C client checkout --orphan b$i &&\n \t\ttest_commit -C client b$i.c0\n \tdone &&\n-\tfor j in $(seq 19)\n+\tfor j in $(test_seq 19)\n \tdo\n-\t\tfor i in $(seq 8)\n+\t\tfor i in $(test_seq 8)\n \t\tdo\n \t\t\tgit -C client checkout b$i &&\n \t\t\ttest_commit -C client b$i.c$j\n@@ -205,7 +205,7 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \n \t# fetch-pack should thus not send any more commits in the b1 branch, but\n \t# should still send the others (in this test, just check b2).\n-\tfor i in $(seq 0 8)\n+\tfor i in $(test_seq 0 8)\n \tdo\n \t\thave_not_sent b1.c$i\n \tdone &&\ndiff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh\nindex a73c55a47e..d1ccc22331 100755\n--- a/t/t5703-upload-pack-ref-in-want.sh\n+++ b/t/t5703-upload-pack-ref-in-want.sh\n@@ -176,7 +176,7 @@ test_expect_success 'setup repos for change-while-negotiating test' '\n \t\tgit clone \"http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n@@ -289,7 +289,7 @@ test_expect_success 'setup repos for fetching with ref-in-want tests' '\n \t\tgit clone \"file://$REPO\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356374","messageId":"xmqqmutdjdid.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"20180823091427.1756-1-avarab@gmail.com","subject":"Re: [PATCH] tests: fix and add lint for non-portable head -c N","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T15:42:18Z","receivedAt":"2018-08-23T15:42:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Junio: Even though this isn't a 2.19.0-rc0 regression I think it makes\n> sense for 2.19.0. The fix is trivial, and it'll unbreak (at least some\n> of) the tests on stock git on OpenBSD.\n\nI would have been a bit more sympathetic if this were recent\nregression (say, 2.18 or so), even if it is not new in this cycle,\nand the patch applied to the target track cleanly (say, maint or\nmaint-2.17), but seeing that some of these are quite old, dating\nback to 2009, I am not sure I am enthused.\n\nThe changes certainly look trivial. Let's apply.\n\nThanks.\n\n>\n> OpenBSD guys: If you CC the git mailing list when you find you need to\n> apply patches like these, we're happy to fix this more pro-actively. I\n> just happened to be testing the upcoming 2.19.0 on OpenBSD and spotted\n> this.\n>\n> t/check-non-portable-shell.pl | 1 +\n> t/t5310-pack-bitmaps.sh | 2 +-\n> t/test-lib.sh | 4 ++--\n> 3 files changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index d5823f71d8..94a7e6165e 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -35,6 +35,7 @@ sub err {\n> \t\tchomp;\n> \t}\n> \n> +\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n> \t/\\bsed\\s+-i/ and err 'sed -i is not portable';\n> \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n> \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\n> index 557bd0d0c0..7bff7923f2 100755\n> --- a/t/t5310-pack-bitmaps.sh\n> +++ b/t/t5310-pack-bitmaps.sh\n> @@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n> \tgit rev-list --use-bitmap-index --count --all >expect &&\n> \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n> \ttest_when_finished \"rm -f $bitmap\" &&\n> -\thead -c 512 <$bitmap >$bitmap.tmp &&\n> +\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n> \tmv -f $bitmap.tmp $bitmap &&\n> \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n> \ttest_cmp expect actual &&\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 8bb0f4348e..44288cbb59 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -867,7 +867,7 @@ then\n> \t\t# handle only executables, unless they are shell libraries that\n> \t\t# need to be in the exec-path.\n> \t\ttest -x \"$1\" ||\n> -\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n> +\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n> \t\treturn;\n> \n> \t\tbase=$(basename \"$1\")\n> @@ -882,7 +882,7 @@ then\n> \t\t# do not override scripts\n> \t\tif test -x \"$symlink_target\" &&\n> \t\t test ! -d \"$symlink_target\" &&\n> -\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n> +\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n> \t\tthen\n> \t\t\tsymlink_target=../valgrind.sh\n> \t\tfi\n"},{"id":"356377","messageId":"xmqqin41jd05.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"87a7pdfltp.fsf@evledraar.gmail.com","subject":"Re: Test failures on OpenBSD","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T15:53:14Z","receivedAt":"2018-08-23T15:53:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> + echo Error (-1) reading configuration file a-directory.\n> + > expect \n> + mkdir a-directory\n> + test_expect_code 2 test-tool config configset_get_value foo.bar a-directory\n> + 2> output \n> Value not found for \"foo.bar\"\n> test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory\n> error: last command exited with $?=1\n> not ok 23 - proper error on directory \"files\"\n\nA blind guess. is this \"BSD allows opening a FILE* on a directory\nand reading bytes from it\" aka FREAD_READS_DIRECTORIES?\n\nI wonder why FreeBSD sets it but not OpenBSD in config.mak.uname.\n\n> + make_dir extract\n> + tar xf subtree-all.tar -C extract\n> tar: Cannot identify format. Searching...\n> tar: End of archive volume 1 reached\n> tar: Sorry, unable to determine archive format.\n> error: last command exited with $?=1\n\nPerhaps modern tar elements like extended headers are not supported\non the platform-supplied \"tar\"? \"make TAR=gnutar\"?\n\n> + seq 11\n> ./t5552-skipping-fetch-negotiator.sh: seq: not found\n\nThis must use test_seq.\n\n t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nindex 5ad5bece55..30857b84a8 100755\n--- a/t/t5552-skipping-fetch-negotiator.sh\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -46,7 +46,7 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 7)\n+\tfor i in $(test_seq 7)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -89,7 +89,7 @@ test_expect_success 'when two skips collide, favor the larger one' '\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 11)\n+\tfor i in $(test_seq 11)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -168,14 +168,14 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 8)\n+\tfor i in $(test_seq 8)\n \tdo\n \t\tgit -C client checkout --orphan b$i &&\n \t\ttest_commit -C client b$i.c0\n \tdone &&\n-\tfor j in $(seq 19)\n+\tfor j in $(test_seq 19)\n \tdo\n-\t\tfor i in $(seq 8)\n+\t\tfor i in $(test_seq 8)\n \t\tdo\n \t\t\tgit -C client checkout b$i &&\n \t\t\ttest_commit -C client b$i.c$j\n@@ -205,7 +205,7 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \n \t# fetch-pack should thus not send any more commits in the b1 branch, but\n \t# should still send the others (in this test, just check b2).\n-\tfor i in $(seq 0 8)\n+\tfor i in $(test_seq 0 8)\n \tdo\n \t\thave_not_sent b1.c$i\n \tdone &&\n"},{"id":"356378","messageId":"xmqqefepjcam.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"Re: [PATCH v2 2/2] tests: fix and add lint for non-portable seq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T16:08:33Z","receivedAt":"2018-08-23T16:08:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> GNU seq is not a POSIX command, and doesn't exist on\n\ns/GNU //; the command did not even originate there, but came from V8\nand/or Plan9 IIRC.\n\n> e.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 (\"tests:\n> Introduce test_seq\", 2012-08-04), but use of it keeps coming back,\n> e.g. in the recently added \"fetch negotiator\" tests being added here.\n>\n> So let's also add a check to \"make test-lint\". The regex is aiming to\n> capture the likes of $(seq ..) and \"seq\" as a stand-alone command,\n> without capturing some existing cases where we e.g. have files called\n> \"seq\".\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n>\n> Now with a fix & check in v2 for the seq issue mentioned in\n> https://public-inbox.org/git/87a7pdfltp.fsf@evledraar.gmail.com/\n\nThanks.\n\n> t/check-non-portable-shell.pl | 1 +\n> t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n> t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n> 3 files changed, 9 insertions(+), 8 deletions(-)\n>\n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index c8f10d40a1..75f38298d7 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -42,6 +42,7 @@ sub err {\n> \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n> \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n> \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n> +\t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n\nLooking at $(wc -l) thing a few lines above, I am not sure if this\ndeviation is a good idea. Wouldn't /\\bseq\\s/ be sufficient?\n\n"},{"id":"356379","messageId":"20180823161150.GA29579@sigill.intra.peff.net","threadId":"49200","inReplyTo":"20180823152502.3886-1-avarab@gmail.com","subject":"Re: [PATCH v2 1/2] tests: fix and add lint for non-portable head -c N","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-08-23T16:11:51Z","receivedAt":"2018-08-23T16:11:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Aug 23, 2018 at 03:25:01PM +0000, Ævar Arnfjörð Bjarmason wrote:\n\n> The \"head -c BYTES\" option is non-portable (not in POSIX[1]). Change\n> such invocations to use the test_copy_bytes wrapper added in\n> 48860819e8 (\"t9300: factor out portable \"head -c\" replacement\",\n> 2016-06-30).\n> \n> This fixes a test added in 9d2e330b17 (\"ewah_read_mmap: bounds-check\n> mmap reads\", 2018-06-14), which has been breaking\n> t5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports\n> already have a similar workaround after their upgrade to 2.18.0[2].\n\nHeh, I even considered using this when writing that test. But the reason\nI introduced test_copy_bytes is not because the target platform did not\nhave \"head -c\" at all, but because some tests need very specific\nbuffering guarantees when reading from a shared pipe.\n\nThat said, if OpenBSD's \"head\" doesn't have \"-c\" at all, I'm fine with\nthis as a fix (and it sounds like we know that IRIX lacks it, too).\n\n> Also, change a valgrind-specific codepath in test-lib.sh to use this\n> wrapper. Given where valgrind runs I don't think this would ever\n> become a portability issue in practice, but it's easier to just use\n> the wrapper than introduce some exception for the \"make test-lint\"\n> check being added here.\n\nWhen working on 9d2e330b17, I recall finding these other \"head -c\"\ninvocations in the test suite when I did 9d2e330b17 and took them as\nevidence that it was OK to use in vanilla cases. So even if these sites\ndon't affect any platforms in practice, I think it's worth it to ban\n\"head -c\" completely.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> t/check-non-portable-shell.pl | 1 +\n> t/t5310-pack-bitmaps.sh | 2 +-\n> t/test-lib.sh | 4 ++--\n> 3 files changed, 4 insertions(+), 3 deletions(-)\n\nPatch itself looks good to me. Thanks.\n\n-Peff\n"},{"id":"356385","messageId":"874lflf18n.fsf@evledraar.gmail.com","threadId":"49200","inReplyTo":"xmqqefepjcam.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/2] tests: fix and add lint for non-portable seq","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T17:20:56Z","receivedAt":"2018-08-23T17:21:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 23 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> GNU seq is not a POSIX command, and doesn't exist on\n>\n> s/GNU //; the command did not even originate there, but came from V8\n> and/or Plan9 IIRC.\n>\n>> e.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 (\"tests:\n>> Introduce test_seq\", 2012-08-04), but use of it keeps coming back,\n>> e.g. in the recently added \"fetch negotiator\" tests being added here.\n>>\n>> So let's also add a check to \"make test-lint\". The regex is aiming to\n>> capture the likes of $(seq ..) and \"seq\" as a stand-alone command,\n>> without capturing some existing cases where we e.g. have files called\n>> \"seq\".\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>>\n>> Now with a fix & check in v2 for the seq issue mentioned in\n>> https://public-inbox.org/git/87a7pdfltp.fsf@evledraar.gmail.com/\n>\n> Thanks.\n>\n>> t/check-non-portable-shell.pl | 1 +\n>> t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n>> t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n>> 3 files changed, 9 insertions(+), 8 deletions(-)\n>>\n>> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n>> index c8f10d40a1..75f38298d7 100755\n>> --- a/t/check-non-portable-shell.pl\n>> +++ b/t/check-non-portable-shell.pl\n>> @@ -42,6 +42,7 @@ sub err {\n>> \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n>> \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n>> \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n>> +\t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n>\n> Looking at $(wc -l) thing a few lines above, I am not sure if this\n> deviation is a good idea. Wouldn't /\\bseq\\s/ be sufficient?\n\nAs alluded to in the commit message that will find cases like:\n\n $ git grep -P '\\bseq\\b' -- t | grep -P -v '(?:\\$\\(seq|^\\s*seq\\b)'\n t/perf/p3400-rebase.sh:16: seq 1000 >>unrelated-file$i &&\n t/perf/p3400-rebase.sh:21: seq 1000 | tac >>unrelated-file$i &&\n t/t3404-rebase-interactive.sh:1227: test_seq 5 | sed \"s/$double/&&/\" >seq &&\n t/t3404-rebase-interactive.sh:1228: git add seq &&\n t/t3404-rebase-interactive.sh:1230: git commit -m seq-$double\n t/t3404-rebase-interactive.sh:1232: git tag seq-onto &&\n t/t3404-rebase-interactive.sh:1234: git cherry-pick seq-onto &&\n t/t3404-rebase-interactive.sh:1236: test_must_fail env FAKE_LINES= git rebase -i seq-onto &&\n t/t3404-rebase-interactive.sh:1239: git diff --exit-code seq-onto &&\n t/t4022-diff-rewrite.sh:69: test_seq 1 99 >seq &&\n t/t4022-diff-rewrite.sh:70: printf 100 >>seq &&\n t/t4022-diff-rewrite.sh:71: git add seq &&\n t/t4022-diff-rewrite.sh:72: git commit seq -m seq\n t/t4022-diff-rewrite.sh:76: test_seq 1 5 >seq &&\n t/t4022-diff-rewrite.sh:77: test_seq 9331 9420 >>seq &&\n t/t4022-diff-rewrite.sh:78: test_seq 96 100 >>seq\n t/t4022-diff-rewrite.sh:82: git diff -B seq >res &&\n t/t4022-diff-rewrite.sh:87: git diff seq >res &&\n t/t4022-diff-rewrite.sh:93: git diff -B seq >res &&\n t/t4150-am.sh:933: git format-patch -2 --stdout >seq.patch &&\n t/t4150-am.sh:945: test_must_fail git am -3 seq.patch &&\n t/t4150-am.sh:955: test_must_fail git am -3 seq.patch &&\n t/t5520-pull.sh:295: git checkout -b seq &&\n t/t5520-pull.sh:296: test_seq 5 >seq.txt &&\n t/t5520-pull.sh:297: git add seq.txt &&\n t/t5520-pull.sh:299: git commit -m \"Add seq.txt\" &&\n t/t5520-pull.sh:300: echo 6 >>seq.txt &&\n t/t5520-pull.sh:302: git commit -m \"Append to seq.txt\" seq.txt &&\n t/t5520-pull.sh:304: echo conflicting >>seq.txt &&\n t/t5520-pull.sh:306: git commit -m \"Create conflict\" seq.txt &&\n t/t5520-pull.sh:307: test_must_fail git pull --rebase . seq 2>err >out &&\n"},{"id":"356386","messageId":"8736v5f12j.fsf@evledraar.gmail.com","threadId":"49200","inReplyTo":"xmqqmutdjdid.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] tests: fix and add lint for non-portable head -c N","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T17:24:36Z","receivedAt":"2018-08-23T17:24:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Aug 23 2018, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Junio: Even though this isn't a 2.19.0-rc0 regression I think it makes\n>> sense for 2.19.0. The fix is trivial, and it'll unbreak (at least some\n>> of) the tests on stock git on OpenBSD.\n>\n> I would have been a bit more sympathetic if this were recent\n> regression (say, 2.18 or so), even if it is not new in this cycle,\n> and the patch applied to the target track cleanly (say, maint or\n> maint-2.17),\n\nYeah it's not a big deal if it's not in 2.19.\n\n> but seeing that some of these are quite old, dating back to 2009, I am\n> not sure I am enthused.\n\nNote that those parts are the \"while I'm at it\" valgrind-specific parts,\ni.e. parts of the code that likely never ran on anything except a GNU\ntoolchain.\n\n> The changes certainly look trivial. Let's apply.\n>\n> Thanks.\n>\n>>\n>> OpenBSD guys: If you CC the git mailing list when you find you need to\n>> apply patches like these, we're happy to fix this more pro-actively. I\n>> just happened to be testing the upcoming 2.19.0 on OpenBSD and spotted\n>> this.\n>>\n>> t/check-non-portable-shell.pl | 1 +\n>> t/t5310-pack-bitmaps.sh | 2 +-\n>> t/test-lib.sh | 4 ++--\n>> 3 files changed, 4 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n>> index d5823f71d8..94a7e6165e 100755\n>> --- a/t/check-non-portable-shell.pl\n>> +++ b/t/check-non-portable-shell.pl\n>> @@ -35,6 +35,7 @@ sub err {\n>> \t\tchomp;\n>> \t}\n>>\n>> +\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n>> \t/\\bsed\\s+-i/ and err 'sed -i is not portable';\n>> \t/\\becho\\s+-[neE]/ and err 'echo with option is not portable (use printf)';\n>> \t/^\\s*declare\\s+/ and err 'arrays/declare not portable';\n>> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\n>> index 557bd0d0c0..7bff7923f2 100755\n>> --- a/t/t5310-pack-bitmaps.sh\n>> +++ b/t/t5310-pack-bitmaps.sh\n>> @@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n>> \tgit rev-list --use-bitmap-index --count --all >expect &&\n>> \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n>> \ttest_when_finished \"rm -f $bitmap\" &&\n>> -\thead -c 512 <$bitmap >$bitmap.tmp &&\n>> +\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n>> \tmv -f $bitmap.tmp $bitmap &&\n>> \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n>> \ttest_cmp expect actual &&\n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index 8bb0f4348e..44288cbb59 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -867,7 +867,7 @@ then\n>> \t\t# handle only executables, unless they are shell libraries that\n>> \t\t# need to be in the exec-path.\n>> \t\ttest -x \"$1\" ||\n>> -\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n>> +\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n>> \t\treturn;\n>>\n>> \t\tbase=$(basename \"$1\")\n>> @@ -882,7 +882,7 @@ then\n>> \t\t# do not override scripts\n>> \t\tif test -x \"$symlink_target\" &&\n>> \t\t test ! -d \"$symlink_target\" &&\n>> -\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n>> +\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n>> \t\tthen\n>> \t\t\tsymlink_target=../valgrind.sh\n>> \t\tfi\n"},{"id":"356396","messageId":"20180823203604.5564-1-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 0/5] OpenBSD & AIX etc. portability fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:35:59Z","receivedAt":"2018-08-23T20:36:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This grew a bit more. I'm going to stop poking at this for now. The\ntests are still broken on OpenBSD (3-5 broken) and on AIX something\nlike 20-30 are broken, but this makes it slightly better.\n\nÆvar Arnfjörð Bjarmason (5):\n tests: fix and add lint for non-portable head -c N\n tests: fix and add lint for non-portable seq\n tests: use shorter here-docs in chainlint.sed for AIX sed\n tests: fix version-specific portability issue in Perl JSON\n tests: fix and add lint for non-portable grep --file\n\n t/chainlint.sed | 8 ++++----\n t/check-non-portable-shell.pl | 3 +++\n t/t0019/parse_json.perl | 3 +++\n t/t5310-pack-bitmaps.sh | 2 +-\n t/t5318-commit-graph.sh | 2 +-\n t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n t/test-lib.sh | 4 ++--\n 8 files changed, 22 insertions(+), 16 deletions(-)\n\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356397","messageId":"20180823203604.5564-2-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 1/5] tests: fix and add lint for non-portable head -c N","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:36:00Z","receivedAt":"2018-08-23T20:36:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"head -c BYTES\" option is non-portable (not in POSIX[1]). Change\nsuch invocations to use the test_copy_bytes wrapper added in\n48860819e8 (\"t9300: factor out portable \"head -c\" replacement\",\n2016-06-30).\n\nThis fixes a test added in 9d2e330b17 (\"ewah_read_mmap: bounds-check\nmmap reads\", 2018-06-14), which has been breaking\nt5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports\nalready have a similar workaround after their upgrade to 2.18.0[2].\n\nI have not tested this on IRIX, but according to 4de0bbd898 (\"t9300:\nuse perl \"head -c\" clone in place of \"dd bs=1 count=16000\" kluge\",\n2010-12-13) this invocation would have broken things there too.\n\nAlso, change a valgrind-specific codepath in test-lib.sh to use this\nwrapper. Given where valgrind runs I don't think this would ever\nbecome a portability issue in practice, but it's easier to just use\nthe wrapper than introduce some exception for the \"make test-lint\"\ncheck being added here.\n\n1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html\n2. https://github.com/openbsd/ports/commit/08d5d82eaefe5cf2f125ecc0c6a57df9cf91350c#diff-f7d3c4fabeed1691620d608f1534f5e5\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5310-pack-bitmaps.sh | 2 +-\n t/test-lib.sh | 4 ++--\n 3 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex d5823f71d8..c8f10d40a1 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -41,6 +41,7 @@ sub err {\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (use type)';\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n+\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\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\"';\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 557bd0d0c0..7bff7923f2 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n \tgit rev-list --use-bitmap-index --count --all >expect &&\n \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n \ttest_when_finished \"rm -f $bitmap\" &&\n-\thead -c 512 <$bitmap >$bitmap.tmp &&\n+\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n \tmv -f $bitmap.tmp $bitmap &&\n \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n \ttest_cmp expect actual &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8bb0f4348e..44288cbb59 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -867,7 +867,7 @@ then\n \t\t# handle only executables, unless they are shell libraries that\n \t\t# need to be in the exec-path.\n \t\ttest -x \"$1\" ||\n-\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n+\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n \t\treturn;\n \n \t\tbase=$(basename \"$1\")\n@@ -882,7 +882,7 @@ then\n \t\t# do not override scripts\n \t\tif test -x \"$symlink_target\" &&\n \t\t test ! -d \"$symlink_target\" &&\n-\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n \t\tthen\n \t\t\tsymlink_target=../valgrind.sh\n \t\tfi\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356398","messageId":"20180823203604.5564-3-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 2/5] tests: fix and add lint for non-portable seq","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:36:01Z","receivedAt":"2018-08-23T20:36:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The seq command is not in POSIX, and doesn't exist on\ne.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 (\"tests:\nIntroduce test_seq\", 2012-08-04), but use of it keeps coming back,\ne.g. in the recently added \"fetch negotiator\" tests being added here.\n\nSo let's also add a check to \"make test-lint\". The regex is aiming to\ncapture the likes of $(seq ..) and \"seq\" as a stand-alone command,\nwithout capturing some existing cases where we e.g. have files called\n\"seq\", as \\bseq\\b would do.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n 3 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex c8f10d40a1..75f38298d7 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -42,6 +42,7 @@ sub err {\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n+\t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\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\"';\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nindex 5ad5bece55..30857b84a8 100755\n--- a/t/t5552-skipping-fetch-negotiator.sh\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -46,7 +46,7 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 7)\n+\tfor i in $(test_seq 7)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -89,7 +89,7 @@ test_expect_success 'when two skips collide, favor the larger one' '\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 11)\n+\tfor i in $(test_seq 11)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -168,14 +168,14 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 8)\n+\tfor i in $(test_seq 8)\n \tdo\n \t\tgit -C client checkout --orphan b$i &&\n \t\ttest_commit -C client b$i.c0\n \tdone &&\n-\tfor j in $(seq 19)\n+\tfor j in $(test_seq 19)\n \tdo\n-\t\tfor i in $(seq 8)\n+\t\tfor i in $(test_seq 8)\n \t\tdo\n \t\t\tgit -C client checkout b$i &&\n \t\t\ttest_commit -C client b$i.c$j\n@@ -205,7 +205,7 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \n \t# fetch-pack should thus not send any more commits in the b1 branch, but\n \t# should still send the others (in this test, just check b2).\n-\tfor i in $(seq 0 8)\n+\tfor i in $(test_seq 0 8)\n \tdo\n \t\thave_not_sent b1.c$i\n \tdone &&\ndiff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh\nindex a73c55a47e..d1ccc22331 100755\n--- a/t/t5703-upload-pack-ref-in-want.sh\n+++ b/t/t5703-upload-pack-ref-in-want.sh\n@@ -176,7 +176,7 @@ test_expect_success 'setup repos for change-while-negotiating test' '\n \t\tgit clone \"http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n@@ -289,7 +289,7 @@ test_expect_success 'setup repos for fetching with ref-in-want tests' '\n \t\tgit clone \"file://$REPO\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356399","messageId":"20180823203604.5564-4-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 3/5] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:36:02Z","receivedAt":"2018-08-23T20:36:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the portability of chainlint by using shorter here-docs. On\nAIX sed will complain about:\n\n sed: 0602-417 The label :hereslurp is greater than eight\n characters\n\nAs noted in [1] there's still a remaining recently introduced\nportability issue also introduced in 878f988350 (\"t/test-lib: teach\n--chain-lint to detect broken &&-chains in subshells\", 2018-07-11), so\nunder AIX the tests must be run with GIT_TEST_CHAIN_LINT=0.\n\nI don't know how to solve the other issue, and this gets us some of\nthe way to GIT_TEST_CHAIN_LINT=1 working again on AIX.\n\n1. https://public-inbox.org/git/871sapezba.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/chainlint.sed | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/t/chainlint.sed b/t/chainlint.sed\nindex 8544df38df..2333705b27 100644\n--- a/t/chainlint.sed\n+++ b/t/chainlint.sed\n@@ -97,11 +97,11 @@\n /<<[ \t]*[-\\\\']*[A-Za-z0-9_]/ {\n \ts/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1<]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n \t\ts/\\n.*$//\n-\t\tbhereslurp\n+\t\tbhered\n \t}\n \ts/^<[^>]*>//\n \ts/\\n.*$//\n@@ -283,11 +283,11 @@ bfolded\n :heredoc\n s/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1<]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n \ts/\\n.*$//\n-\tbhereslurpsub\n+\tbheredsub\n }\n s/^<[^>]*>//\n s/\\n.*$//\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356400","messageId":"20180823203604.5564-5-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 4/5] tests: fix version-specific portability issue in Perl JSON","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:36:03Z","receivedAt":"2018-08-23T20:36:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The test guarded by PERLJSON added in 75459410ed (\"json_writer: new\nroutines to create JSON data\", 2018-07-13) assumed that a JSON boolean\nvalue like \"true\" or \"false\" would be represented as \"1\" or \"0\" in\nPerl.\n\nThis behavior can't be relied upon, e.g. with JSON.pm 2.50 and\nJSON::PP A JSON::PP::Boolean object will be represented as \"true\" or\n\"false\". To work around this let's check if we have any refs left\nafter we check for hashes and arrays, assume those are JSON objects,\nand coerce them to a known boolean value.\n\nThe behavior of this test still looks odd to me. Why implement our own\nad-hoc encoder just for some one-off test, as opposed to say Perl's\nown Data::Dumper with Sortkeys et al? But with this change it works,\nso let's leave it be.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0019/parse_json.perl | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t0019/parse_json.perl b/t/t0019/parse_json.perl\nindex ca4e5bfa78..fea87fb81b 100644\n--- a/t/t0019/parse_json.perl\n+++ b/t/t0019/parse_json.perl\n@@ -34,6 +34,9 @@ sub dump_item {\n } elsif (ref($value) eq 'HASH') {\n \tprint \"$label_in hash\\n\";\n \tdump_hash($label_in, $value);\n+ } elsif (ref $value) {\n+\tmy $bool = $value ? 1 : 0;\n+\tprint \"$label_in $bool\\n\";\n } elsif (defined $value) {\n \tprint \"$label_in $value\\n\";\n } else {\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356401","messageId":"20180823203604.5564-6-avarab@gmail.com","threadId":"49200","inReplyTo":"20180823152502.3886-2-avarab@gmail.com","subject":"[PATCH v3 5/5] tests: fix and add lint for non-portable grep --file","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-23T20:36:04Z","receivedAt":"2018-08-23T20:36:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The --file option to grep isn't in POSIX[1], but -f is[1]. Let's check\nfor that in the future, and fix the portability regression in\nf237c8b6fe (\"commit-graph: implement git-commit-graph write\",\n2018-04-02) that broke e.g. AIX.\n\n1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5318-commit-graph.sh | 2 +-\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 75f38298d7..b45bdac688 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -43,6 +43,7 @@ sub err {\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n \t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n+\t/\\bgrep\\b.*--file\\b/ and err 'grep --file FILE is not portable (use grep -f FILE)';\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\"';\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 3c1ffad491..0c500f7ca2 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -134,7 +134,7 @@ test_expect_success 'Add one more commit' '\n \tgit branch commits/8 &&\n \tls $objdir/pack | grep idx >existing-idx &&\n \tgit repack &&\n-\tls $objdir/pack| grep idx | grep -v --file=existing-idx >new-idx\n+\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n '\n \n # Current graph structure:\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356402","messageId":"xmqqa7pcizmn.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"20180823203604.5564-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/5] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T20:42:08Z","receivedAt":"2018-08-23T20:42:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Improve the portability of chainlint by using shorter here-docs. On\n> AIX sed will complain about:\n>\n> sed: 0602-417 The label :hereslurp is greater than eight\n> characters\n>\n> As noted in [1] there's still a remaining recently introduced\n> portability issue also introduced in 878f988350 (\"t/test-lib: teach\n> --chain-lint to detect broken &&-chains in subshells\", 2018-07-11), so\n> under AIX the tests must be run with GIT_TEST_CHAIN_LINT=0.\n>\n> I don't know how to solve the other issue, and this gets us some of\n> the way to GIT_TEST_CHAIN_LINT=1 working again on AIX.\n>\n> 1. https://public-inbox.org/git/871sapezba.fsf@evledraar.gmail.com/\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n\nI'll globally do s/here-doc/label/ while queueing.\n\nPOSIX says \"The implementation shall support label arguments\nrecognized as unique up to at least 8 bytes\", so replacing these\nlabels to shorter strings makes perfect sense.\n\nWill queue; thanks.\n\n> t/chainlint.sed | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/chainlint.sed b/t/chainlint.sed\n> index 8544df38df..2333705b27 100644\n> --- a/t/chainlint.sed\n> +++ b/t/chainlint.sed\n> @@ -97,11 +97,11 @@\n> /<<[ \t]*[-\\\\']*[A-Za-z0-9_]/ {\n> \ts/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1< \ts/[ \t]*< -\t:hereslurp\n> +\t:hered\n> \tN\n> \t/^<\\([^>]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n> \t\ts/\\n.*$//\n> -\t\tbhereslurp\n> +\t\tbhered\n> \t}\n> \ts/^<[^>]*>//\n> \ts/\\n.*$//\n> @@ -283,11 +283,11 @@ bfolded\n> :heredoc\n> s/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1< s/[ \t]*< -:hereslurpsub\n> +:heredsub\n> N\n> /^<\\([^>]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n> \ts/\\n.*$//\n> -\tbhereslurpsub\n> +\tbheredsub\n> }\n> s/^<[^>]*>//\n> s/\\n.*$//\n"},{"id":"356403","messageId":"xmqq6000izil.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"20180823203604.5564-6-avarab@gmail.com","subject":"Re: [PATCH v3 5/5] tests: fix and add lint for non-portable grep --file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-23T20:44:34Z","receivedAt":"2018-08-23T20:44:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> The --file option to grep isn't in POSIX[1], but -f is[1]. Let's check\n> for that in the future, and fix the portability regression in\n> f237c8b6fe (\"commit-graph: implement git-commit-graph write\",\n> 2018-04-02) that broke e.g. AIX.\n>\n> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n\nThanks.\n\n> t/check-non-portable-shell.pl | 1 +\n> t/t5318-commit-graph.sh | 2 +-\n> 2 files changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n> index 75f38298d7..b45bdac688 100755\n> --- a/t/check-non-portable-shell.pl\n> +++ b/t/check-non-portable-shell.pl\n> @@ -43,6 +43,7 @@ sub err {\n> \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n> \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n> \t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n> +\t/\\bgrep\\b.*--file\\b/ and err 'grep --file FILE is not portable (use grep -f FILE)';\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> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index 3c1ffad491..0c500f7ca2 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -134,7 +134,7 @@ test_expect_success 'Add one more commit' '\n> \tgit branch commits/8 &&\n> \tls $objdir/pack | grep idx >existing-idx &&\n> \tgit repack &&\n> -\tls $objdir/pack| grep idx | grep -v --file=existing-idx >new-idx\n> +\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n> '\n> \n> # Current graph structure:\n"},{"id":"356406","messageId":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","threadId":"49200","inReplyTo":"20180823203604.5564-4-avarab@gmail.com","subject":"Re: [PATCH v3 3/5] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-23T20:56:03Z","receivedAt":"2018-08-23T20:56:16Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Aug 23, 2018 at 4:36 PM Ævar Arnfjörð Bjarmason\n wrote:\n> As noted in [1] there's still a remaining recently introduced\n> portability issue also introduced in 878f988350 (\"t/test-lib: teach\n> --chain-lint to detect broken &&-chains in subshells\", 2018-07-11), so\n> under AIX the tests must be run with GIT_TEST_CHAIN_LINT=0.\n>\n> I don't know how to solve the other issue, and this gets us some of\n> the way to GIT_TEST_CHAIN_LINT=1 working again on AIX.\n\nDoes unindenting the comment, as I suggested in [1], fix the remaining\nproblem for you?\n\n[1]: https://public-inbox.org/git/CAPig+cTTbU5HFMKgNyrxTp3+kcK46-Fn=4ZH6zDt1oQChAc3KA@mail.gmail.com/\n"},{"id":"356456","messageId":"b1dc8c6a-1f42-b92f-8544-9593b6bd298f@gmail.com","threadId":"49200","inReplyTo":"xmqq6000izil.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 5/5] tests: fix and add lint for non-portable grep --file","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-08-24T13:49:41Z","receivedAt":"2018-08-24T13:49:49Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 8/23/2018 4:44 PM, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> The --file option to grep isn't in POSIX[1], but -f is[1]. Let's check\n>> for that in the future, and fix the portability regression in\n>> f237c8b6fe (\"commit-graph: implement git-commit-graph write\",\n>> 2018-04-02) that broke e.g. AIX.\n>>\n>> 1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n> Thanks.\n\nYes, thanks! I'll keep this in mind for the future.\n\n>> t/check-non-portable-shell.pl | 1 +\n>> t/t5318-commit-graph.sh | 2 +-\n>> 2 files changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\n>> index 75f38298d7..b45bdac688 100755\n>> --- a/t/check-non-portable-shell.pl\n>> +++ b/t/check-non-portable-shell.pl\n>> @@ -43,6 +43,7 @@ sub err {\n>> \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n>> \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n>> \t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n>> +\t/\\bgrep\\b.*--file\\b/ and err 'grep --file FILE is not portable (use grep -f FILE)';\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>> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n>> index 3c1ffad491..0c500f7ca2 100755\n>> --- a/t/t5318-commit-graph.sh\n>> +++ b/t/t5318-commit-graph.sh\n>> @@ -134,7 +134,7 @@ test_expect_success 'Add one more commit' '\n>> \tgit branch commits/8 &&\n>> \tls $objdir/pack | grep idx >existing-idx &&\n>> \tgit repack &&\n>> -\tls $objdir/pack| grep idx | grep -v --file=existing-idx >new-idx\n>> +\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n>> '\n>> \n>> # Current graph structure:\n"},{"id":"356462","messageId":"20180824152016.20286-1-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 0/6] OpenBSD & AIX etc. portability fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:10Z","receivedAt":"2018-08-24T15:20:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Aug 23 2018, Eric Sunshine wrote:\n\n> On Thu, Aug 23, 2018 at 4:36 PM Ævar Arnfjörð Bjarmason\n> wrote:\n>> As noted in [1] there's still a remaining recently introduced\n>> portability issue also introduced in 878f988350 (\"t/test-lib: teach\n>> --chain-lint to detect broken &&-chains in subshells\", 2018-07-11), so\n>> under AIX the tests must be run with GIT_TEST_CHAIN_LINT=0.\n>>\n>> I don't know how to solve the other issue, and this gets us some of\n>> the way to GIT_TEST_CHAIN_LINT=1 working again on AIX.\n>\n> Does unindenting the comment, as I suggested in [1], fix the remaining\n> problem for you?\n>\n> [1]: https://public-inbox.org/git/CAPig+cTTbU5HFMKgNyrxTp3+kcK46-Fn=4ZH6zDt1oQChAc3KA@mail.gmail.com/\n\nI didn't notice Eric's E-mail before I sent my v3, so going back and\ntesting this revealed two things:\n\n 1) Yes, his suggestion works\n 2) AIX sed will complain about one thing at a time, so we had a lot\n more of these \"labels too long\" problems once I got past fixing\n that issue.\n\nSo here's a version, which as noted in 4/6 makes GIT_TEST_CHAIN_LINT=1\nfully work on AIX again.\n\nAs an aside, the reason I have access to AIX is because I requested\naccess to the GCC compile farm as suggested by someone on-list here\nthe other day: https://cfarm.tetaneutral.net/\n\nThey accepted my account request on the basis that I was going to hack\non git & perl on those boxes, so if anyone else here is interested in\ntesting stuff for portability...\n\nÆvar Arnfjörð Bjarmason (6):\n tests: fix and add lint for non-portable head -c N\n tests: fix and add lint for non-portable seq\n tests: fix comment syntax in chainlint.sed for AIX sed\n tests: use shorter here-docs in chainlint.sed for AIX sed\n tests: fix version-specific portability issue in Perl JSON\n tests: fix and add lint for non-portable grep --file\n\n t/chainlint.sed | 59 ++++++++++++++--------------\n t/check-non-portable-shell.pl | 3 ++\n t/t0019/parse_json.perl | 3 ++\n t/t5310-pack-bitmaps.sh | 2 +-\n t/t5318-commit-graph.sh | 2 +-\n t/t5552-skipping-fetch-negotiator.sh | 12 +++---\n t/t5703-upload-pack-ref-in-want.sh | 4 +-\n t/test-lib.sh | 4 +-\n 8 files changed, 47 insertions(+), 42 deletions(-)\n\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356463","messageId":"20180824152016.20286-2-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 1/6] tests: fix and add lint for non-portable head -c N","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:11Z","receivedAt":"2018-08-24T15:20:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The \"head -c BYTES\" option is non-portable (not in POSIX[1]). Change\nsuch invocations to use the test_copy_bytes wrapper added in\n48860819e8 (\"t9300: factor out portable \"head -c\" replacement\",\n2016-06-30).\n\nThis fixes a test added in 9d2e330b17 (\"ewah_read_mmap: bounds-check\nmmap reads\", 2018-06-14), which has been breaking\nt5310-pack-bitmaps.sh on OpenBSD since 2.18.0. The OpenBSD ports\nalready have a similar workaround after their upgrade to 2.18.0[2].\n\nI have not tested this on IRIX, but according to 4de0bbd898 (\"t9300:\nuse perl \"head -c\" clone in place of \"dd bs=1 count=16000\" kluge\",\n2010-12-13) this invocation would have broken things there too.\n\nAlso, change a valgrind-specific codepath in test-lib.sh to use this\nwrapper. Given where valgrind runs I don't think this would ever\nbecome a portability issue in practice, but it's easier to just use\nthe wrapper than introduce some exception for the \"make test-lint\"\ncheck being added here.\n\n1. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/head.html\n2. https://github.com/openbsd/ports/commit/08d5d82eaefe5cf2f125ecc0c6a57df9cf91350c#diff-f7d3c4fabeed1691620d608f1534f5e5\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5310-pack-bitmaps.sh | 2 +-\n t/test-lib.sh | 4 ++--\n 3 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex d5823f71d8..c8f10d40a1 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -41,6 +41,7 @@ sub err {\n \t/^\\s*[^#]\\s*which\\s/ and err 'which is not portable (use type)';\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n+\t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\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\"';\ndiff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh\nindex 557bd0d0c0..7bff7923f2 100755\n--- a/t/t5310-pack-bitmaps.sh\n+++ b/t/t5310-pack-bitmaps.sh\n@@ -335,7 +335,7 @@ test_expect_success 'truncated bitmap fails gracefully' '\n \tgit rev-list --use-bitmap-index --count --all >expect &&\n \tbitmap=$(ls .git/objects/pack/*.bitmap) &&\n \ttest_when_finished \"rm -f $bitmap\" &&\n-\thead -c 512 <$bitmap >$bitmap.tmp &&\n+\ttest_copy_bytes 512 <$bitmap >$bitmap.tmp &&\n \tmv -f $bitmap.tmp $bitmap &&\n \tgit rev-list --use-bitmap-index --count --all >actual 2>stderr &&\n \ttest_cmp expect actual &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 8bb0f4348e..44288cbb59 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -867,7 +867,7 @@ then\n \t\t# handle only executables, unless they are shell libraries that\n \t\t# need to be in the exec-path.\n \t\ttest -x \"$1\" ||\n-\t\ttest \"# \" = \"$(head -c 2 <\"$1\")\" ||\n+\t\ttest \"# \" = \"$(test_copy_bytes 2 <\"$1\")\" ||\n \t\treturn;\n \n \t\tbase=$(basename \"$1\")\n@@ -882,7 +882,7 @@ then\n \t\t# do not override scripts\n \t\tif test -x \"$symlink_target\" &&\n \t\t test ! -d \"$symlink_target\" &&\n-\t\t test \"#!\" != \"$(head -c 2 < \"$symlink_target\")\"\n+\t\t test \"#!\" != \"$(test_copy_bytes 2 <\"$symlink_target\")\"\n \t\tthen\n \t\t\tsymlink_target=../valgrind.sh\n \t\tfi\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356464","messageId":"20180824152016.20286-3-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 2/6] tests: fix and add lint for non-portable seq","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:12Z","receivedAt":"2018-08-24T15:20:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The seq command is not in POSIX, and doesn't exist on\ne.g. OpenBSD. We've had the test_seq wrapper since d17cf5f3a3 (\"tests:\nIntroduce test_seq\", 2012-08-04), but use of it keeps coming back,\ne.g. in the recently added \"fetch negotiator\" tests being added here.\n\nSo let's also add a check to \"make test-lint\". The regex is aiming to\ncapture the likes of $(seq ..) and \"seq\" as a stand-alone command,\nwithout capturing some existing cases where we e.g. have files called\n\"seq\", as \\bseq\\b would do.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5552-skipping-fetch-negotiator.sh | 12 ++++++------\n t/t5703-upload-pack-ref-in-want.sh | 4 ++--\n 3 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex c8f10d40a1..75f38298d7 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -42,6 +42,7 @@ sub err {\n \t/\\btest\\s+[^=]*==/ and err '\"test a == b\" is not portable (use =)';\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n+\t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\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\"';\ndiff --git a/t/t5552-skipping-fetch-negotiator.sh b/t/t5552-skipping-fetch-negotiator.sh\nindex 5ad5bece55..30857b84a8 100755\n--- a/t/t5552-skipping-fetch-negotiator.sh\n+++ b/t/t5552-skipping-fetch-negotiator.sh\n@@ -46,7 +46,7 @@ test_expect_success 'commits with no parents are sent regardless of skip distanc\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 7)\n+\tfor i in $(test_seq 7)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -89,7 +89,7 @@ test_expect_success 'when two skips collide, favor the larger one' '\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 11)\n+\tfor i in $(test_seq 11)\n \tdo\n \t\ttest_commit -C client c$i\n \tdone &&\n@@ -168,14 +168,14 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \ttest_commit -C server to_fetch &&\n \n \tgit init client &&\n-\tfor i in $(seq 8)\n+\tfor i in $(test_seq 8)\n \tdo\n \t\tgit -C client checkout --orphan b$i &&\n \t\ttest_commit -C client b$i.c0\n \tdone &&\n-\tfor j in $(seq 19)\n+\tfor j in $(test_seq 19)\n \tdo\n-\t\tfor i in $(seq 8)\n+\t\tfor i in $(test_seq 8)\n \t\tdo\n \t\t\tgit -C client checkout b$i &&\n \t\t\ttest_commit -C client b$i.c$j\n@@ -205,7 +205,7 @@ test_expect_success 'do not send \"have\" with ancestors of commits that server AC\n \n \t# fetch-pack should thus not send any more commits in the b1 branch, but\n \t# should still send the others (in this test, just check b2).\n-\tfor i in $(seq 0 8)\n+\tfor i in $(test_seq 0 8)\n \tdo\n \t\thave_not_sent b1.c$i\n \tdone &&\ndiff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh\nindex a73c55a47e..d1ccc22331 100755\n--- a/t/t5703-upload-pack-ref-in-want.sh\n+++ b/t/t5703-upload-pack-ref-in-want.sh\n@@ -176,7 +176,7 @@ test_expect_success 'setup repos for change-while-negotiating test' '\n \t\tgit clone \"http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n@@ -289,7 +289,7 @@ test_expect_success 'setup repos for fetching with ref-in-want tests' '\n \t\tgit clone \"file://$REPO\" \"$LOCAL_PRISTINE\" &&\n \t\tcd \"$LOCAL_PRISTINE\" &&\n \t\tgit checkout -b side &&\n-\t\tfor i in $(seq 1 33); do test_commit s$i; done &&\n+\t\tfor i in $(test_seq 1 33); do test_commit s$i; done &&\n \n \t\t# Add novel commits to upstream\n \t\tgit checkout master &&\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356465","messageId":"20180824152016.20286-4-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 3/6] tests: fix comment syntax in chainlint.sed for AIX sed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:13Z","receivedAt":"2018-08-24T15:20:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change a comment in chainlint.sed to appease AIX sed, which would\npreviously print this error:\n\n sed: # stash for later printing is not a recognized function\n\n1. https://public-inbox.org/git/CAPig+cTTbU5HFMKgNyrxTp3+kcK46-Fn=4ZH6zDt1oQChAc3KA@mail.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/chainlint.sed | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/t/chainlint.sed b/t/chainlint.sed\nindex 8544df38df..dcb4b333ed 100644\n--- a/t/chainlint.sed\n+++ b/t/chainlint.sed\n@@ -131,9 +131,8 @@ b\n b\n \n :subshell\n-# bare \"(\" line?\n+# bare \"(\" line? -- stash for later printing\n /^[ \t]*([\t]*$/ {\n-\t# stash for later printing\n \th\n \tbnextline\n }\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356466","messageId":"20180824152016.20286-5-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 4/6] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:14Z","receivedAt":"2018-08-24T15:20:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the portability of chainlint by using shorter here-docs. On\nAIX sed will complain about:\n\n sed: 0602-417 The label :hereslurp is greater than eight\n characters\n\nThis, in combination with the previous fix to this file makes\nGIT_TEST_CHAIN_LINT=1 (which is the default) working again on AIX\nwithout issues, and the \"gmake check-chainlint\" test also passes.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/chainlint.sed | 56 ++++++++++++++++++++++++-------------------------\n 1 file changed, 28 insertions(+), 28 deletions(-)\n\ndiff --git a/t/chainlint.sed b/t/chainlint.sed\nindex dcb4b333ed..c80d2fad7a 100644\n--- a/t/chainlint.sed\n+++ b/t/chainlint.sed\n@@ -97,11 +97,11 @@\n /<<[ \t]*[-\\\\']*[A-Za-z0-9_]/ {\n \ts/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1<]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n \t\ts/\\n.*$//\n-\t\tbhereslurp\n+\t\tbhered\n \t}\n \ts/^<[^>]*>//\n \ts/\\n.*$//\n@@ -149,7 +149,7 @@ s/.*\\n//\n \n :slurp\n # incomplete line \"...\\\"\n-/\\\\$/bincomplete\n+/\\\\$/bicmplte\n # multi-line quoted string \"...\\n...\"?\n /\"/bdqstring\n # multi-line quoted string '...\\n...'? (but not contraction in string \"it's\")\n@@ -171,7 +171,7 @@ s/.*\\n//\n \t/\"[^\"]*#[^\"]*\"/!s/[ \t]#.*$//\n }\n # one-liner \"case ... esac\"\n-/^[ \t]*case[ \t]*..*esac/bcheckchain\n+/^[ \t]*case[ \t]*..*esac/bchkchn\n # multi-line \"case ... esac\"\n /^[ \t]*case[ \t]..*[ \t]in/bcase\n # multi-line \"for ... done\" or \"while ... done\"\n@@ -200,32 +200,32 @@ s/.*\\n//\n /^[ \t]*fi[ \t]*[<>|]/bdone\n /^[ \t]*fi[ \t]*)/bdone\n # nested one-liner \"(...) &&\"\n-/^[ \t]*(.*)[ \t]*&&[ \t]*$/bcheckchain\n+/^[ \t]*(.*)[ \t]*&&[ \t]*$/bchkchn\n # nested one-liner \"(...)\"\n-/^[ \t]*(.*)[ \t]*$/bcheckchain\n+/^[ \t]*(.*)[ \t]*$/bchkchn\n # nested one-liner \"(...) >x\" (or \"2>x\" or \"|]/bcheckchain\n+/^[ \t]*(.*)[ \t]*[0-9]*[<>|]/bchkchn\n # nested multi-line \"(...\\n...)\"\n /^[ \t]*(/bnest\n # multi-line \"{...\\n...}\"\n /^[ \t]*{/bblock\n # closing \")\" on own line -- exit subshell\n-/^[ \t]*)/bclosesolo\n+/^[ \t]*)/bclssolo\n # \"$((...))\" -- arithmetic expansion; not closing \")\"\n-/\\$(([^)][^)]*))[^)]*$/bcheckchain\n+/\\$(([^)][^)]*))[^)]*$/bchkchn\n # \"$(...)\" -- command substitution; not closing \")\"\n-/\\$([^)][^)]*)[^)]*$/bcheckchain\n+/\\$([^)][^)]*)[^)]*$/bchkchn\n # multi-line \"$(...\\n...)\" -- command substitution; treat as nested subshell\n /\\$([^)]*$/bnest\n # \"=(...)\" -- Bash array assignment; not closing \")\"\n-/=(/bcheckchain\n+/=(/bchkchn\n # closing \"...) &&\"\n /)[ \t]*&&[ \t]*$/bclose\n # closing \"...)\"\n /)[ \t]*$/bclose\n # closing \"...) >x\" (or \"2>x\" or \"|]/bclose\n-:checkchain\n+:chkchn\n # mark suspect if line uses \";\" internally rather than \"&&\" (but not \";\" in a\n # string and not \";;\" in one-liner \"case...esac\")\n /;/{\n@@ -244,7 +244,7 @@ n\n bslurp\n \n # found incomplete line \"...\\\" -- slurp up next line\n-:incomplete\n+:icmplte\n N\n s/\\\\\\n//\n bslurp\n@@ -282,11 +282,11 @@ bfolded\n :heredoc\n s/^\\(.*\\)<<[ \t]*[-\\\\']*\\([A-Za-z0-9_][A-Za-z0-9_]*\\)'*/<\\2>\\1<]*\\)>.*\\n[ \t]*\\1[ \t]*$/!{\n \ts/\\n.*$//\n-\tbhereslurpsub\n+\tbheredsub\n }\n s/^<[^>]*>//\n s/\\n.*$//\n@@ -316,43 +316,43 @@ x\n # is 'done' or 'fi' cuddled with \")\" to close subshell?\n /done.*)/bclose\n /fi.*)/bclose\n-bcheckchain\n+bchkchn\n \n # found nested multi-line \"(...\\n...)\" -- pass through untouched\n :nest\n x\n-:nestslurp\n+:nstslurp\n n\n # closing \")\" on own line -- stop nested slurp\n-/^[ \t]*)/bnestclose\n+/^[ \t]*)/bnstclose\n # comment -- not closing \")\" if in comment\n-/^[ \t]*#/bnestcontinue\n+/^[ \t]*#/bnstcnt\n # \"$((...))\" -- arithmetic expansion; not closing \")\"\n-/\\$(([^)][^)]*))[^)]*$/bnestcontinue\n+/\\$(([^)][^)]*))[^)]*$/bnstcnt\n # \"$(...)\" -- command substitution; not closing \")\"\n-/\\$([^)][^)]*)[^)]*$/bnestcontinue\n+/\\$([^)][^)]*)[^)]*$/bnstcnt\n # closing \"...)\" -- stop nested slurp\n-/)/bnestclose\n-:nestcontinue\n+/)/bnstclose\n+:nstcnt\n x\n-bnestslurp\n-:nestclose\n+bnstslurp\n+:nstclose\n s/^/>>/\n # is it \"))\" which closes nested and parent subshells?\n /)[ \t]*)/bslurp\n-bcheckchain\n+bchkchn\n \n # found multi-line \"{...\\n...}\" block -- pass through untouched\n :block\n x\n n\n # closing \"}\" -- stop block slurp\n-/}/bcheckchain\n+/}/bchkchn\n bblock\n \n # found closing \")\" on own line -- drop \"suspect\" from final line of subshell\n # since that line legitimately lacks \"&&\" and exit subshell loop\n-:closesolo\n+:clssolo\n x\n s/?!AMP?!//\n p\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356467","messageId":"20180824152016.20286-6-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 5/6] tests: fix version-specific portability issue in Perl JSON","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:15Z","receivedAt":"2018-08-24T15:20:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The test guarded by PERLJSON added in 75459410ed (\"json_writer: new\nroutines to create JSON data\", 2018-07-13) assumed that a JSON boolean\nvalue like \"true\" or \"false\" would be represented as \"1\" or \"0\" in\nPerl.\n\nThis behavior can't be relied upon, e.g. with JSON.pm 2.50 and\nJSON::PP A JSON::PP::Boolean object will be represented as \"true\" or\n\"false\". To work around this let's check if we have any refs left\nafter we check for hashes and arrays, assume those are JSON objects,\nand coerce them to a known boolean value.\n\nThe behavior of this test still looks odd to me. Why implement our own\nad-hoc encoder just for some one-off test, as opposed to say Perl's\nown Data::Dumper with Sortkeys et al? But with this change it works,\nso let's leave it be.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t0019/parse_json.perl | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/t0019/parse_json.perl b/t/t0019/parse_json.perl\nindex ca4e5bfa78..fea87fb81b 100644\n--- a/t/t0019/parse_json.perl\n+++ b/t/t0019/parse_json.perl\n@@ -34,6 +34,9 @@ sub dump_item {\n } elsif (ref($value) eq 'HASH') {\n \tprint \"$label_in hash\\n\";\n \tdump_hash($label_in, $value);\n+ } elsif (ref $value) {\n+\tmy $bool = $value ? 1 : 0;\n+\tprint \"$label_in $bool\\n\";\n } elsif (defined $value) {\n \tprint \"$label_in $value\\n\";\n } else {\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356468","messageId":"20180824152016.20286-7-avarab@gmail.com","threadId":"49200","inReplyTo":"CAPig+cS4yeHNbc1Anq1DtLPEEGLpGeGEV74JHZrYft2wRmZAPQ@mail.gmail.com","subject":"[PATCH v4 6/6] tests: fix and add lint for non-portable grep --file","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-24T15:20:16Z","receivedAt":"2018-08-24T15:20:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The --file option to grep isn't in POSIX[1], but -f is[1]. Let's check\nfor that in the future, and fix the portability regression in\nf237c8b6fe (\"commit-graph: implement git-commit-graph write\",\n2018-04-02) that broke e.g. AIX.\n\n1. http://pubs.opengroup.org/onlinepubs/009695399/utilities/grep.html\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/check-non-portable-shell.pl | 1 +\n t/t5318-commit-graph.sh | 2 +-\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/t/check-non-portable-shell.pl b/t/check-non-portable-shell.pl\nindex 75f38298d7..b45bdac688 100755\n--- a/t/check-non-portable-shell.pl\n+++ b/t/check-non-portable-shell.pl\n@@ -43,6 +43,7 @@ sub err {\n \t/\\bwc -l.*\"\\s*=/ and err '`\"$(wc -l)\"` is not portable (use test_line_count)';\n \t/\\bhead\\s+-c\\b/ and err 'head -c is not portable (use test_copy_bytes BYTES out)';\n \t/(?:\\$\\(seq|^\\s*seq\\b)/ and err 'seq is not portable (use test_seq)';\n+\t/\\bgrep\\b.*--file\\b/ and err 'grep --file FILE is not portable (use grep -f FILE)';\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\"';\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex 3c1ffad491..0c500f7ca2 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -134,7 +134,7 @@ test_expect_success 'Add one more commit' '\n \tgit branch commits/8 &&\n \tls $objdir/pack | grep idx >existing-idx &&\n \tgit repack &&\n-\tls $objdir/pack| grep idx | grep -v --file=existing-idx >new-idx\n+\tls $objdir/pack| grep idx | grep -v -f existing-idx >new-idx\n '\n \n # Current graph structure:\n-- \n2.18.0.865.gffc8e1a3cd6\n\n"},{"id":"356488","messageId":"CAPig+cTGPaHTecvjqBtFOrTsb9+X=ifQYcqss634Bt2cAFNdeg@mail.gmail.com","threadId":"49200","inReplyTo":"20180824152016.20286-6-avarab@gmail.com","subject":"Re: [PATCH v4 5/6] tests: fix version-specific portability issue in Perl JSON","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-24T20:41:32Z","receivedAt":"2018-08-24T20:41:45Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 24, 2018 at 11:20 AM Ævar Arnfjörð Bjarmason\n wrote:\n> The test guarded by PERLJSON added in 75459410ed (\"json_writer: new\n> routines to create JSON data\", 2018-07-13) assumed that a JSON boolean\n> value like \"true\" or \"false\" would be represented as \"1\" or \"0\" in\n> Perl.\n>\n> This behavior can't be relied upon, e.g. with JSON.pm 2.50 and\n> JSON::PP A JSON::PP::Boolean object will be represented as \"true\" or\n\ns/PP A/PP. A/\n\n(Not worth a re-roll.)\n\n> \"false\". To work around this let's check if we have any refs left\n> after we check for hashes and arrays, assume those are JSON objects,\n> and coerce them to a known boolean value.\n>\n> The behavior of this test still looks odd to me. Why implement our own\n> ad-hoc encoder just for some one-off test, as opposed to say Perl's\n> own Data::Dumper with Sortkeys et al? But with this change it works,\n> so let's leave it be.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n"},{"id":"356489","messageId":"CAPig+cTONP22UFdHKBfef_qbzHXX9PBH4h01_0_wPF6TV9xrWg@mail.gmail.com","threadId":"49200","inReplyTo":"20180824152016.20286-4-avarab@gmail.com","subject":"Re: [PATCH v4 3/6] tests: fix comment syntax in chainlint.sed for AIX sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-24T20:52:41Z","receivedAt":"2018-08-24T20:52:54Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 24, 2018 at 11:20 AM Ævar Arnfjörð Bjarmason\n wrote:\n> Change a comment in chainlint.sed to appease AIX sed, which would\n> previously print this error:\n>\n> sed: # stash for later printing is not a recognized function\n>\n> 1. https://public-inbox.org/git/CAPig+cTTbU5HFMKgNyrxTp3+kcK46-Fn=4ZH6zDt1oQChAc3KA@mail.gmail.com/\n\nIt would have been nice for the commit message to explain that the\nproblem was caused by the comment being indented, since it's otherwise\nnot otherwise obvious to the reader without chasing the link to the\nemail exchange, but probably not worth a re-roll.\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n\nFWIW,\nAcked-by: Eric Sunshine \n"},{"id":"356492","messageId":"CAPig+cQxUdr0g+i6AztcH4Q51X-W4MtdhKGv=wau+b6SFsniAQ@mail.gmail.com","threadId":"49200","inReplyTo":"20180824152016.20286-5-avarab@gmail.com","subject":"Re: [PATCH v4 4/6] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-24T21:29:59Z","receivedAt":"2018-08-24T21:30:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Aug 24, 2018 at 11:20 AM Ævar Arnfjörð Bjarmason\n wrote:\n> Improve the portability of chainlint by using shorter here-docs. On\n> AIX sed will complain about:\n>\n> sed: 0602-417 The label :hereslurp is greater than eight\n> characters\n\nShortening the names makes them ugly and often unreadable. That's not\na complaint with this patch; just a general observation regarding\n8-byte limitation with this platform's \"sed\" (and POSIX). See a few\nsuggested improvements below, but probably not worth a re-roll.\n\n> This, in combination with the previous fix to this file makes\n> GIT_TEST_CHAIN_LINT=1 (which is the default) working again on AIX\n> without issues, and the \"gmake check-chainlint\" test also passes.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n\nFWIW,\nAcked-by: Eric Sunshine \n\n> diff --git a/t/chainlint.sed b/t/chainlint.sed\n> -:checkchain\n> +:chkchn\n\n\":chkchain\"\n\n> # found incomplete line \"...\\\" -- slurp up next line\n> -:incomplete\n> +:icmplte\n\n\":fold\" (for \"fold out NL\")\n\n> # found nested multi-line \"(...\\n...)\" -- pass through untouched\n> -:nestslurp\n> +:nstslurp\n\n\":nesteat\"\n\n> -:nestcontinue\n> +:nstcnt\n\n\":nestcont\"\n\n> -:nestclose\n> +:nstclose\n\n\":nestend\" or \":endnest\"\n\n> # found closing \")\" on own line -- drop \"suspect\" from final line of subshell\n> # since that line legitimately lacks \"&&\" and exit subshell loop\n> -:closesolo\n> +:clssolo\n\n\":endsolo\"\n"},{"id":"356606","messageId":"xmqqpny3iou7.fsf@gitster-ct.c.googlers.com","threadId":"49200","inReplyTo":"20180824152016.20286-5-avarab@gmail.com","subject":"Re: [PATCH v4 4/6] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-27T19:36:32Z","receivedAt":"2018-08-27T19:36:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Improve the portability of chainlint by using shorter here-docs. On\n> AIX sed will complain about:\n>\n> sed: 0602-417 The label :hereslurp is greater than eight\n> characters\n\nRemind me again not to forget doing s/here-doc/label/ on this patch\nbefore queueing. Other than that, looks good, together with 3/6 for\nthe the indented comment issue.\n\n"},{"id":"356732","messageId":"87a7p6dza3.fsf@evledraar.gmail.com","threadId":"49200","inReplyTo":"CAPig+cQxUdr0g+i6AztcH4Q51X-W4MtdhKGv=wau+b6SFsniAQ@mail.gmail.com","subject":"Re: [PATCH v4 4/6] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-28T20:14:28Z","receivedAt":"2018-08-28T20:14:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 24 2018, Eric Sunshine wrote:\n\n> On Fri, Aug 24, 2018 at 11:20 AM Ævar Arnfjörð Bjarmason\n> wrote:\n>> Improve the portability of chainlint by using shorter here-docs. On\n>> AIX sed will complain about:\n>>\n>> sed: 0602-417 The label :hereslurp is greater than eight\n>> characters\n>\n> Shortening the names makes them ugly and often unreadable. That's not\n> a complaint with this patch; just a general observation regarding\n> 8-byte limitation with this platform's \"sed\" (and POSIX). See a few\n> suggested improvements below, but probably not worth a re-roll.\n>\n>> This, in combination with the previous fix to this file makes\n>> GIT_TEST_CHAIN_LINT=1 (which is the default) working again on AIX\n>> without issues, and the \"gmake check-chainlint\" test also passes.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>\n> FWIW,\n> Acked-by: Eric Sunshine \n>\n>> diff --git a/t/chainlint.sed b/t/chainlint.sed\n>> -:checkchain\n>> +:chkchn\n>\n> \":chkchain\"\n>\n>> # found incomplete line \"...\\\" -- slurp up next line\n>> -:incomplete\n>> +:icmplte\n>\n> \":fold\" (for \"fold out NL\")\n>\n>> # found nested multi-line \"(...\\n...)\" -- pass through untouched\n>> -:nestslurp\n>> +:nstslurp\n>\n> \":nesteat\"\n>\n>> -:nestcontinue\n>> +:nstcnt\n>\n> \":nestcont\"\n>\n>> -:nestclose\n>> +:nstclose\n>\n> \":nestend\" or \":endnest\"\n>\n>> # found closing \")\" on own line -- drop \"suspect\" from final line of subshell\n>> # since that line legitimately lacks \"&&\" and exit subshell loop\n>> -:closesolo\n>> +:clssolo\n>\n> \":endsolo\"\n\nI was meaning to get to this with a re-roll, but since this is already\nin next & these label renames seem cosmetic, it seems better to do this\nafter 2.19, i.e. the compiler doesn't care about the specific names, but\nshortening them to <=8 fixes the bug.\n"},{"id":"356733","messageId":"CAPig+cTo402TVFpdrz5JPOrX7OhNNQCs6vGCP4LdQ8=a3dtgww@mail.gmail.com","threadId":"49200","inReplyTo":"87a7p6dza3.fsf@evledraar.gmail.com","subject":"Re: [PATCH v4 4/6] tests: use shorter here-docs in chainlint.sed for AIX sed","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-08-28T20:17:00Z","receivedAt":"2018-08-28T20:17:14Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Aug 28, 2018 at 4:14 PM Ævar Arnfjörð Bjarmason\n wrote:\n> On Fri, Aug 24 2018, Eric Sunshine wrote:\n> > Shortening the names makes them ugly and often unreadable. That's not\n> > a complaint with this patch; just a general observation regarding\n> > 8-byte limitation with this platform's \"sed\" (and POSIX). See a few\n> > suggested improvements below, but probably not worth a re-roll.\n> >> +:clssolo\n> > \":endsolo\"\n>\n> I was meaning to get to this with a re-roll, but since this is already\n> in next & these label renames seem cosmetic, it seems better to do this\n> after 2.19, i.e. the compiler doesn't care about the specific names, but\n> shortening them to <=8 fixes the bug.\n\nSounds fine. There's no rush. Thanks.\n"}]}