From fa87b813853b12fee2a931c6084134fe1eea3b55 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 23 Oct 2019 16:32:28 -0700 Subject: [PATCH 1/5] t4108: replace create_file with test_write_lines Since the locally defined create_file() duplicates the functionality of the test_write_lines() helper function, remove create_file() and replace all instances with test_write_lines(). While we're at it, move redirection operators to the end of the command which is the more conventional place to put it. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t4108-apply-threeway.sh | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index fa5d4efb89..b109ecbd9f 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -4,13 +4,6 @@ test_description='git apply --3way' . ./test-lib.sh -create_file () { - for i - do - echo "$i" - done -} - sanitize_conflicted_diff () { sed -e ' /^index /d @@ -20,7 +13,7 @@ sanitize_conflicted_diff () { test_expect_success setup ' test_tick && - create_file >one 1 2 3 4 5 6 7 && + test_write_lines 1 2 3 4 5 6 7 >one && cat one >two && git add one two && git commit -m initial && @@ -28,13 +21,13 @@ test_expect_success setup ' git branch side && test_tick && - create_file >one 1 two 3 4 5 six 7 && - create_file >two 1 two 3 4 5 6 7 && + test_write_lines 1 two 3 4 5 six 7 >one && + test_write_lines 1 two 3 4 5 6 7 >two && git commit -a -m master && git checkout side && - create_file >one 1 2 3 4 five 6 7 && - create_file >two 1 2 3 4 five 6 7 && + test_write_lines 1 2 3 4 five 6 7 >one && + test_write_lines 1 2 3 4 five 6 7 >two && git commit -a -m side && git checkout master @@ -87,7 +80,7 @@ test_expect_success 'apply with --3way with rerere enabled' ' test_must_fail git merge --no-commit side && # Manually resolve and record the resolution - create_file 1 two 3 4 five six 7 >one && + test_write_lines 1 two 3 4 five six 7 >one && git rerere && cat one >expect && @@ -104,14 +97,14 @@ test_expect_success 'apply -3 with add/add conflict setup' ' git reset --hard && git checkout -b adder && - create_file 1 2 3 4 5 6 7 >three && - create_file 1 2 3 4 5 6 7 >four && + test_write_lines 1 2 3 4 5 6 7 >three && + test_write_lines 1 2 3 4 5 6 7 >four && git add three four && git commit -m "add three and four" && git checkout -b another adder^ && - create_file 1 2 3 4 5 6 7 >three && - create_file 1 2 3 four 5 6 7 >four && + test_write_lines 1 2 3 4 5 6 7 >three && + test_write_lines 1 2 3 four 5 6 7 >four && git add three four && git commit -m "add three and four" && From b0069684d430ea67fd3fbe019c71a18d4ef87fa3 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 23 Oct 2019 16:32:31 -0700 Subject: [PATCH 2/5] t4108: remove git command upstream of pipe Before, the output of `git diff HEAD` would always be piped to sanitize_conflicted_diff(). However, since the Git command was upstream of the pipe, in case the Git command fails, the return code would be lost. Rewrite into separate statements so that the return code is no longer lost. Since only the command `git diff HEAD` was being piped to sanitize_conflicted_diff(), move the command into the function and rename it to print_sanitized_conflicted_diff(). Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t4108-apply-threeway.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index b109ecbd9f..3c0ddacddf 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -4,11 +4,12 @@ test_description='git apply --3way' . ./test-lib.sh -sanitize_conflicted_diff () { +print_sanitized_conflicted_diff () { + git diff HEAD >diff.raw && sed -e ' /^index /d s/^\(+[<>][<>][<>][<>]*\) .*/\1/ - ' + ' diff.raw } test_expect_success setup ' @@ -54,14 +55,14 @@ test_expect_success 'apply with --3way' ' git checkout master^0 && test_must_fail git merge --no-commit side && git ls-files -s >expect.ls && - git diff HEAD | sanitize_conflicted_diff >expect.diff && + print_sanitized_conflicted_diff >expect.diff && # should fail to apply git reset --hard && git checkout master^0 && test_must_fail git apply --index --3way P.diff && git ls-files -s >actual.ls && - git diff HEAD | sanitize_conflicted_diff >actual.diff && + print_sanitized_conflicted_diff >actual.diff && # The result should resemble the corresponding merge test_cmp expect.ls actual.ls && @@ -114,7 +115,7 @@ test_expect_success 'apply -3 with add/add conflict setup' ' git checkout adder^0 && test_must_fail git merge --no-commit another && git ls-files -s >expect.ls && - git diff HEAD | sanitize_conflicted_diff >expect.diff + print_sanitized_conflicted_diff >expect.diff ' test_expect_success 'apply -3 with add/add conflict' ' @@ -124,7 +125,7 @@ test_expect_success 'apply -3 with add/add conflict' ' test_must_fail git apply --index --3way P.diff && # ... and leave conflicts in the index and in the working tree git ls-files -s >actual.ls && - git diff HEAD | sanitize_conflicted_diff >actual.diff && + print_sanitized_conflicted_diff >actual.diff && # The result should resemble the corresponding merge test_cmp expect.ls actual.ls && From 95806205cd7013f524b6a7f10afc8542ab0c1929 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 23 Oct 2019 16:32:33 -0700 Subject: [PATCH 3/5] t4108: use `test_config` instead of `git config` Since `git config` leaves the configurations set even after the test case completes, use `test_config` instead so that the configurations are reset once the test case finishes. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t4108-apply-threeway.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index 3c0ddacddf..7f96ae9101 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -70,7 +70,7 @@ test_expect_success 'apply with --3way' ' ' test_expect_success 'apply with --3way with rerere enabled' ' - git config rerere.enabled true && + test_config rerere.enabled true && # Merging side should be similar to applying this patch git diff ...side >P.diff && From aa76ae4905c28e264f0affc58e36c1db692fa881 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 23 Oct 2019 16:32:36 -0700 Subject: [PATCH 4/5] t4108: demonstrate bug in apply Currently, apply does not respect the merge.conflictStyle setting. Demonstrate this by making the 'apply with --3way' test case generic and extending it to show that the configuration of merge.conflictStyle = diff3 causes a breakage. Change print_sanitized_conflicted_diff() to also sanitize `|||||||` conflict markers. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- t/t4108-apply-threeway.sh | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index 7f96ae9101..bffe37f1ba 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -8,7 +8,7 @@ print_sanitized_conflicted_diff () { git diff HEAD >diff.raw && sed -e ' /^index /d - s/^\(+[<>][<>][<>][<>]*\) .*/\1/ + s/^\(+[<>|][<>|][<>|][<>|]*\) .*/\1/ ' diff.raw } @@ -46,7 +46,7 @@ test_expect_success 'apply without --3way' ' git diff-index --exit-code --cached HEAD ' -test_expect_success 'apply with --3way' ' +test_apply_with_3way () { # Merging side should be similar to applying this patch git diff ...side >P.diff && @@ -67,6 +67,15 @@ test_expect_success 'apply with --3way' ' # The result should resemble the corresponding merge test_cmp expect.ls actual.ls && test_cmp expect.diff actual.diff +} + +test_expect_success 'apply with --3way' ' + test_apply_with_3way +' + +test_expect_failure 'apply with --3way with merge.conflictStyle = diff3' ' + test_config merge.conflictStyle diff3 && + test_apply_with_3way ' test_expect_success 'apply with --3way with rerere enabled' ' From 091489d068e0e812123f5114149bdb0e7d74c257 Mon Sep 17 00:00:00 2001 From: Denton Liu Date: Wed, 23 Oct 2019 16:32:38 -0700 Subject: [PATCH 5/5] apply: respect merge.conflictStyle in --3way Before, when doing a 3-way merge, the merge.conflictStyle option was not respected and the "merge" style was always used, even if "diff3" was specified. Call git_xmerge_config() at the end of git_apply_config() so that the merge.conflictStyle config is read. Signed-off-by: Denton Liu Signed-off-by: Junio C Hamano --- apply.c | 2 +- t/t4108-apply-threeway.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apply.c b/apply.c index cde95369bb..84704572ca 100644 --- a/apply.c +++ b/apply.c @@ -32,7 +32,7 @@ static void git_apply_config(void) { git_config_get_string_const("apply.whitespace", &apply_default_whitespace); git_config_get_string_const("apply.ignorewhitespace", &apply_default_ignorewhitespace); - git_config(git_default_config, NULL); + git_config(git_xmerge_config, NULL); } static int parse_whitespace_option(struct apply_state *state, const char *option) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index bffe37f1ba..d7349ced6b 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -73,7 +73,7 @@ test_expect_success 'apply with --3way' ' test_apply_with_3way ' -test_expect_failure 'apply with --3way with merge.conflictStyle = diff3' ' +test_expect_success 'apply with --3way with merge.conflictStyle = diff3' ' test_config merge.conflictStyle diff3 && test_apply_with_3way '