# [PATCH v3 0/8] fix test failure with busybox 9 messages from 2020-03-25 to 2020-03-25. Participants: Đoàn Trần Công Danh. Thread: https://gitlist.dev/t/53095 ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 0/8] fix test failure with busybox Message-ID: URL: https://gitlist.dev/e/cover.1585148327.git.congdanhqx%40gmail.com ``` Despite non-compiance from busybox's sh(1), grep(1), diff(1), find(1) Alpine Linux is still a popular choice for container these days. Fix false-positive failure in testsuite when run in Alpine Linux. Đoàn Trần Công Danh (8): t4061: use POSIX compliant regex(7) test-lib-functions: test_cmp: eval $GIT_TEST_CMP t5003: drop the subshell in test_lazy_prereq t5003: skip conversion test if unzip -a is unavailable t5616: use rev-parse instead to get HEAD's object_id t7063: drop non-POSIX argument "-ls" from find(1) t4124: fix test for non-compliant diff(1) t5703: feed raw data into test-tool unpack-sideband t/helper/test-pkt-line.c | 2 +- t/t4061-diff-indent.sh | 2 +- t/t4124-apply-ws-rule.sh | 6 ++++++ t/t5003-archive-zip.sh | 24 ++++++++++++------------ t/t5616-partial-clone.sh | 2 +- t/t5703-upload-pack-ref-in-want.sh | 5 +---- t/t7063-status-untracked-cache.sh | 2 +- t/test-lib-functions.sh | 2 +- 8 files changed, 24 insertions(+), 21 deletions(-) Range-diff against v2: 1: 34f96548de = 1: 088905ab6f t4061: use POSIX compliant regex(7) 2: 50f46986a6 = 2: 691d9d47ba test-lib-functions: test_cmp: eval $GIT_TEST_CMP 3: 8719a07753 = 3: 759a589b83 t5003: drop the subshell in test_lazy_prereq 4: 457eecaf9b = 4: e5b09c290c t5003: skip conversion test if unzip -a is unavailable 5: d3bc855e17 = 5: 1b8740018c t5616: use rev-parse instead to get HEAD's object_id 6: 64472ec3bc ! 6: abb21b9e51 t7063: drop non-POSIX argument "-ls" from find(1) @@ Commit message However, `-ls` flag isn't required by POSIX's find(1), and busybox(1) doesn't implement it. - From the original conversation, it seems like find(1) with "-type d" - could trigger enough "lstat(2)" to ask FreeBSD update mtime. - - Use only filter "-type d" for now. + Use "-exec ls -ld {} +" instead. Signed-off-by: Đoàn Trần Công Danh @@ t/t7063-status-untracked-cache.sh: GIT_FORCE_UNTRACKED_CACHE=true sync_mtime () { - find . -type d -ls >/dev/null -+ find . -type d >/dev/null ++ find . -type d -exec ls -ld {} + >/dev/null } avoid_racy() { 7: 51df6dd12d ! 7: 0502171f0f t4124: fix test for non-compliant diff(1) @@ Commit message POSIX's diff(1) requires output in normal diff format. However, busybox's diff's output is written in unified format. - POSIX requires no option for normal-diff format. + HP-UX's diff(1) doesn't understand "-u" as of now. - A hint in test-lib-functions::test_cmp said `diff -u` isn't available - everywhere. - - Workaround this problem by assuming `diff(1)` output is unified + Workaround this problem by checking "diff -u" output, if we couldn't make anything from normal-diff format. Signed-off-by: Đoàn Trần Công Danh @@ t/t4124-apply-ws-rule.sh: test_fix () { # find touched lines $DIFF file target | sed -n -e "s/^> //p" >fixed -+ # busybox's diff(1) output unified format ++ # busybox's diff(1) doesn't output normal format + if ! test -s fixed; then -+ $DIFF file target | ++ $DIFF -u file target | + grep -v '^+++ target' | + sed -e "/^+/s/+//" >fixed + fi 8: 843c7f66d9 = 8: 8689e03400 t5703: feed raw data into test-tool unpack-sideband -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 1/8] t4061: use POSIX compliant regex(7) Message-ID: <088905ab6fb5acffbf9a8158cf9b1ee28d0bb7b0.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/088905ab6fb5acffbf9a8158cf9b1ee28d0bb7b0.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` BRE interprets `+` literally, and `\+` is undefined for POSIX BRE, from: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_02 > The interpretation of an ordinary character preceded > by an unescaped ( '\\' ) is undefined, except for: > - The characters ')', '(', '{', and '}' > - The digits 1 to 9 inclusive > - A character inside a bracket expression This test is failing with busybox sed, the default sed of Alpine Linux We have 2 options here: - Using literal `+` because BRE will interpret it as-is, or - Using character class `[+]` to defend against a sed that expects ERE ERE-expected sed is theoretical at this point, but we haven't found it, yet. And, we may run into other problems with that sed. Let's go with first option and fix it later if that sed could be found. Signed-off-by: Đoàn Trần Công Danh --- t/t4061-diff-indent.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh index 2affd7a100..0f7a6d97a8 100755 --- a/t/t4061-diff-indent.sh +++ b/t/t4061-diff-indent.sh @@ -17,7 +17,7 @@ compare_diff () { # Compare blame output using the expectation for a diff as reference. # Only look for the lines coming from non-boundary commits. compare_blame () { - sed -n -e "1,4d" -e "s/^\+//p" <"$1" >.tmp-1 + sed -n -e "1,4d" -e "s/^+//p" <"$1" >.tmp-1 sed -ne "s/^[^^][^)]*) *//p" <"$2" >.tmp-2 test_cmp .tmp-1 .tmp-2 && rm -f .tmp-1 .tmp-2 } -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 2/8] test-lib-functions: test_cmp: eval $GIT_TEST_CMP Message-ID: <691d9d47ba1495d68c9506cd6600267b38afdd85.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/691d9d47ba1495d68c9506cd6600267b38afdd85.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` Shell recognises first non-assignment token as command name. With /bin/sh linked to either /bin/bash or /bin/dash, `cd t/perf && ./p0000-perf-lib-sanity.sh -d -i -v` reports: > test_cmp:1: command not found: diff -u Using `eval` to unquote $GIT_TEST_CMP as same as precedence in `git_editor`. Signed-off-by: Đoàn Trần Công Danh --- t/test-lib-functions.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 352c213d52..ab0e47ae17 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -905,7 +905,7 @@ test_expect_code () { # - not all diff versions understand "-u" test_cmp() { - $GIT_TEST_CMP "$@" + eval "$GIT_TEST_CMP" '"$@"' } # Check that the given config key has the expected value. -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 3/8] t5003: drop the subshell in test_lazy_prereq Message-ID: <759a589b83515cc7a676009d820e11fc36c5a0a2.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/759a589b83515cc7a676009d820e11fc36c5a0a2.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` test_lazy_prereq will be evaluated in a throw-away directory. Drop unnecessary subshell and mkdir. Signed-off-by: Đoàn Trần Công Danh --- t/t5003-archive-zip.sh | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh index 106eddbd85..df1374a312 100755 --- a/t/t5003-archive-zip.sh +++ b/t/t5003-archive-zip.sh @@ -7,12 +7,8 @@ test_description='git archive --format=zip test' SUBSTFORMAT=%H%n test_lazy_prereq UNZIP_SYMLINKS ' - ( - mkdir unzip-symlinks && - cd unzip-symlinks && - "$GIT_UNZIP" "$TEST_DIRECTORY"/t5003/infozip-symlinks.zip && - test -h symlink - ) + "$GIT_UNZIP" "$TEST_DIRECTORY"/t5003/infozip-symlinks.zip && + test -h symlink ' check_zip() { -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 4/8] t5003: skip conversion test if unzip -a is unavailable Message-ID: URL: https://gitlist.dev/e/e5b09c290c1ff0fb0d7171927194e598e46daae2.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` Alpine Linux's default unzip(1) doesn't support `-a`. Skip those tests on that platform. Signed-off-by: Đoàn Trần Công Danh --- t/t5003-archive-zip.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/t/t5003-archive-zip.sh b/t/t5003-archive-zip.sh index df1374a312..3b76d2eb65 100755 --- a/t/t5003-archive-zip.sh +++ b/t/t5003-archive-zip.sh @@ -11,6 +11,10 @@ test_lazy_prereq UNZIP_SYMLINKS ' test -h symlink ' +test_lazy_prereq UNZIP_CONVERT ' + "$GIT_UNZIP" -a "$TEST_DIRECTORY"/t5003/infozip-symlinks.zip +' + check_zip() { zipfile=$1.zip listfile=$1.lst @@ -35,33 +39,33 @@ check_zip() { extracted=${dir_with_prefix}a original=a - test_expect_success UNZIP " extract ZIP archive with EOL conversion" ' + test_expect_success UNZIP_CONVERT " extract ZIP archive with EOL conversion" ' (mkdir $dir && cd $dir && "$GIT_UNZIP" -a ../$zipfile) ' - test_expect_success UNZIP " validate that text files are converted" " + test_expect_success UNZIP_CONVERT " validate that text files are converted" " test_cmp_bin $extracted/text.cr $extracted/text.crlf && test_cmp_bin $extracted/text.cr $extracted/text.lf " - test_expect_success UNZIP " validate that binary files are unchanged" " + test_expect_success UNZIP_CONVERT " validate that binary files are unchanged" " test_cmp_bin $original/binary.cr $extracted/binary.cr && test_cmp_bin $original/binary.crlf $extracted/binary.crlf && test_cmp_bin $original/binary.lf $extracted/binary.lf " - test_expect_success UNZIP " validate that diff files are converted" " + test_expect_success UNZIP_CONVERT " validate that diff files are converted" " test_cmp_bin $extracted/diff.cr $extracted/diff.crlf && test_cmp_bin $extracted/diff.cr $extracted/diff.lf " - test_expect_success UNZIP " validate that -diff files are unchanged" " + test_expect_success UNZIP_CONVERT " validate that -diff files are unchanged" " test_cmp_bin $original/nodiff.cr $extracted/nodiff.cr && test_cmp_bin $original/nodiff.crlf $extracted/nodiff.crlf && test_cmp_bin $original/nodiff.lf $extracted/nodiff.lf " - test_expect_success UNZIP " validate that custom diff is unchanged " " + test_expect_success UNZIP_CONVERT " validate that custom diff is unchanged " " test_cmp_bin $original/custom.cr $extracted/custom.cr && test_cmp_bin $original/custom.crlf $extracted/custom.crlf && test_cmp_bin $original/custom.lf $extracted/custom.lf -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 5/8] t5616: use rev-parse instead to get HEAD's object_id Message-ID: <1b8740018c411dec2813d03c34c56dfa5d1d8845.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/1b8740018c411dec2813d03c34c56dfa5d1d8845.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` Only HEAD's object_id is necessary, rev-list is an overkill. Despite POSIX requires grep(1) treat single pattern with as multiple patterns. busybox's grep(1) (as of v1.31.1) haven't implemented it yet. Use rev-parse to simplify the test and avoid busybox unimplemented features. Signed-off-by: Đoàn Trần Công Danh --- t/t5616-partial-clone.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index 77bb91e976..09e640cae4 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -49,7 +49,7 @@ test_expect_success 'do partial clone 1' ' test_expect_success 'verify that .promisor file contains refs fetched' ' ls pc1/.git/objects/pack/pack-*.promisor >promisorlist && test_line_count = 1 promisorlist && - git -C srv.bare rev-list HEAD >headhash && + git -C srv.bare rev-parse --verify HEAD >headhash && grep "$(cat headhash) HEAD" $(cat promisorlist) && grep "$(cat headhash) refs/heads/master" $(cat promisorlist) ' -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 7/8] t4124: fix test for non-compliant diff(1) Message-ID: <0502171f0fc25a44e865b5a435906ef1096546c5.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/0502171f0fc25a44e865b5a435906ef1096546c5.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` POSIX's diff(1) requires output in normal diff format. However, busybox's diff's output is written in unified format. HP-UX's diff(1) doesn't understand "-u" as of now. Workaround this problem by checking "diff -u" output, if we couldn't make anything from normal-diff format. Signed-off-by: Đoàn Trần Công Danh --- t/t4124-apply-ws-rule.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh index 971a5a7512..691f82da9f 100755 --- a/t/t4124-apply-ws-rule.sh +++ b/t/t4124-apply-ws-rule.sh @@ -52,6 +52,12 @@ test_fix () { # find touched lines $DIFF file target | sed -n -e "s/^> //p" >fixed + # busybox's diff(1) doesn't output normal format + if ! test -s fixed; then + $DIFF -u file target | + grep -v '^+++ target' | + sed -e "/^+/s/+//" >fixed + fi # the changed lines are all expected to change fixed_cnt=$(wc -l URL: https://gitlist.dev/e/abb21b9e518079f8d5ee98e9e6d3bc2b1bae64a4.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` Since commit 6b7728db81, (t7063: work around FreeBSD's lazy mtime update feature, 2016-08-03), we started to use ls as a trick to update directory's mtime. However, `-ls` flag isn't required by POSIX's find(1), and busybox(1) doesn't implement it. Use "-exec ls -ld {} +" instead. Signed-off-by: Đoàn Trần Công Danh --- t/t7063-status-untracked-cache.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 190ae149cf..6738497ea7 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -18,7 +18,7 @@ GIT_FORCE_UNTRACKED_CACHE=true export GIT_FORCE_UNTRACKED_CACHE sync_mtime () { - find . -type d -ls >/dev/null + find . -type d -exec ls -ld {} + >/dev/null } avoid_racy() { -- 2.26.0.rc2.357.g1e1ba0441d ``` ## Đoàn Trần Công Danh, 2020-03-25 15:06 Subject: [PATCH v3 8/8] t5703: feed raw data into test-tool unpack-sideband Message-ID: <8689e03400a4a4c2a2c95cc5aac3f1d24ba7525d.1585148327.git.congdanhqx@gmail.com> URL: https://gitlist.dev/e/8689e03400a4a4c2a2c95cc5aac3f1d24ba7525d.1585148327.git.congdanhqx%40gmail.com In-Reply-To: ``` busybox's sed isn't binary clean. Thus, triggers false-negative on this test. We could replace sed with perl on this usecase. But, we could slightly modify the helper to discard unwanted data in the beginning. Fix the false negative by updating this helper. Helped-by: Jeff King Signed-off-by: Đoàn Trần Công Danh --- t/helper/test-pkt-line.c | 2 +- t/t5703-upload-pack-ref-in-want.sh | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c index 282d536384..12ca698e17 100644 --- a/t/helper/test-pkt-line.c +++ b/t/helper/test-pkt-line.c @@ -67,7 +67,7 @@ static void unpack_sideband(void) case PACKET_READ_NORMAL: band = reader.line[0] & 0xff; if (band < 1 || band > 2) - die("unexpected side band %d", band); + continue; /* skip non-sideband packets */ fd = band; write_or_die(fd, reader.line + 1, reader.pktlen - 1); diff --git a/t/t5703-upload-pack-ref-in-want.sh b/t/t5703-upload-pack-ref-in-want.sh index 7fba3063bf..a34460f7d8 100755 --- a/t/t5703-upload-pack-ref-in-want.sh +++ b/t/t5703-upload-pack-ref-in-want.sh @@ -13,10 +13,7 @@ get_actual_refs () { } get_actual_commits () { - sed -n -e '/packfile/,/0000/{ - /packfile/d - p - }' o.pack && + test-tool pkt-line unpack-sideband o.pack && git index-pack o.pack && git verify-pack -v o.idx >objs && grep commit objs | cut -d" " -f1 | sort >actual_commits -- 2.26.0.rc2.357.g1e1ba0441d ```