From bcf325ae775c4e5dab39cd30b16564b7b3d66b80 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Sun, 28 Aug 2022 05:17:57 +0000 Subject: [PATCH 1/3] t4301: account for behavior differences between sed implementations It is a common pattern in this script to write the result of `merge-tree -z` (NUL-termination mode) to an "actual" file and then manually append a newline to that file so that it can be diff'd easily with a hand-crafted "expect" file which itself ends with a newline since it has been created by standard Unix tools which terminate lines by default. For instance: git merge-tree --write-tree -z ... >out && printf "\\n" >>out anonymize_hash out >actual && q_to_nul <<-EOF >expect && ... EOF test_cmp expect actual However, one test gets this backward: git merge-tree --write-tree -z ... >out && anonymize_hash out >actual && printf "\\n" >>actual which means that, unlike all other cases, when anonymize_hash() is called, the file being anonymized does not end with a newline. As a result, this test fails on some platforms. anonymize_hash() is implemented like this: anonymize_hash() { sed -e "s/[0-9a-f]\{40,\}/HASH/g" "$@" } The problem arises due to differences in behavior of various `sed` implementations when fed an incomplete line (lacking a newline). Although most modern `sed` implementations output such a line unmolested (i.e. without a newline), some older `sed` implementations forcibly add a newline to the incomplete line (giving the output an extra unexpected newline), while other very old implementations simply swallow an incomplete line and don't emit it at all (making the output shorter than expected). Fix this test by manually adding the newline before passing it through `sed`, thus ensuring identical behavior with all `sed` implementation, and bringing the test in line with other tests in this script. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t4301-merge-tree-write-tree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh index c5fd56df28..d44c7767f3 100755 --- a/t/t4301-merge-tree-write-tree.sh +++ b/t/t4301-merge-tree-write-tree.sh @@ -760,8 +760,8 @@ test_expect_success 'NUL terminated conflicted file "lines"' ' git commit -m "Renamed numbers" && test_expect_code 1 git merge-tree --write-tree -z tweak1 side2 >out && + printf "\\n" >>out && anonymize_hash out >actual && - printf "\\n" >>actual && # Expected results: # "greeting" should merge with conflicts From 87ed97167a1b1bce41e0380ec6a9507876015e89 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Sun, 28 Aug 2022 05:17:58 +0000 Subject: [PATCH 2/3] t4301: fix broken &&-chains and add missing loop termination Fix &&-chain breaks in a couple tests which went unnoticed due to blind spots in the &&-chain linters. In particular, the "magic exit code 117" &&-chain checker built into test-lib.sh only recognizes broken &&-chains at the top-level; it does not work within `{...}` groups, `(...)` subshells, `$(...)` substitutions, or within bodies of compound statements, such as `if`, `for`, `while`, `case`, etc. Furthermore, `chainlint.sed`, which detects broken &&-chains only in `(...)` subshells, missed these cases (which are in subshells) because it (surprisingly) neglects to check for intact &&-chain on single-line `for` loops. While at it, explicitly signal failure of commands within the `for` loops (which might arise due to the filesystem being full or "inode" exhaustion). This is important since failures within `for` and `while` loops can go unnoticed if not detected and signaled manually since the loop itself does not abort when a contained command fails, nor will a failure necessarily be detected when the loop finishes since the loop returns the exit code of the last command it ran on the final iteration, which may not be the command which failed. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t4301-merge-tree-write-tree.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh index d44c7767f3..82a104bcbc 100755 --- a/t/t4301-merge-tree-write-tree.sh +++ b/t/t4301-merge-tree-write-tree.sh @@ -150,7 +150,7 @@ test_expect_success 'directory rename + content conflict' ' cd dir-rename-and-content && test_write_lines 1 2 3 4 5 >foo && mkdir olddir && - for i in a b c; do echo $i >olddir/$i; done + for i in a b c; do echo $i >olddir/$i || exit 1; done && git add foo olddir && git commit -m "original" && @@ -662,7 +662,7 @@ test_expect_success 'directory rename + rename/delete + modify/delete + director cd 4-stacked-conflict && test_write_lines 1 2 3 4 5 >foo && mkdir olddir && - for i in a b c; do echo $i >olddir/$i; done + for i in a b c; do echo $i >olddir/$i || exit 1; done && git add foo olddir && git commit -m "original" && From 3871bdb7e40cc9a91d2cce58e2f5c49313521dc4 Mon Sep 17 00:00:00 2001 From: Eric Sunshine Date: Sun, 28 Aug 2022 05:17:59 +0000 Subject: [PATCH 3/3] t4301: emit blank line in more idiomatic fashion The unusual use of: printf "\\n" >>file && may give readers pause, making them wonder why this form was chosen over the more typical: printf "\n" >>file && However, even that may give pause since it is a somewhat unusual and long-winded way of saying: echo >>file && Therefore, replace `printf` with the more idiomatic `echo`, with the hope of eliminating a possible stumbling block for those reading the code. Signed-off-by: Eric Sunshine Signed-off-by: Junio C Hamano --- t/t4301-merge-tree-write-tree.sh | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/t/t4301-merge-tree-write-tree.sh b/t/t4301-merge-tree-write-tree.sh index 82a104bcbc..28ca5c38bb 100755 --- a/t/t4301-merge-tree-write-tree.sh +++ b/t/t4301-merge-tree-write-tree.sh @@ -176,7 +176,7 @@ test_expect_success 'directory rename + content conflict' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && HASH @@ -230,7 +230,7 @@ test_expect_success 'rename/delete handling' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && HASH @@ -284,7 +284,7 @@ test_expect_success 'rename/add handling' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && # # First, check that the bar that appears at stage 3 does not @@ -351,7 +351,7 @@ test_expect_success SYMLINKS 'rename/add, where add is a mode conflict' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && # # First, check that the bar that appears at stage 3 does not @@ -417,7 +417,7 @@ test_expect_success 'rename/rename + content conflict' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && HASH @@ -471,7 +471,7 @@ test_expect_success 'rename/add/delete conflict' ' test_expect_code 1 \ git merge-tree -z B^0 A^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && @@ -528,7 +528,7 @@ test_expect_success 'rename/rename(2to1)/delete/delete conflict' ' test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && @@ -600,7 +600,7 @@ test_expect_success 'mod6: chains of rename/rename(1to2) and add/add via collidi test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && # # First, check that some of the hashes that appear as stage @@ -690,7 +690,7 @@ test_expect_success 'directory rename + rename/delete + modify/delete + director test_expect_code 1 \ git merge-tree -z A^0 B^0 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && q_to_tab <<-\EOF | lf_to_nul >expect && @@ -760,7 +760,7 @@ test_expect_success 'NUL terminated conflicted file "lines"' ' git commit -m "Renamed numbers" && test_expect_code 1 git merge-tree --write-tree -z tweak1 side2 >out && - printf "\\n" >>out && + echo >>out && anonymize_hash out >actual && # Expected results: