From patchwork Fri Oct 19 19:31:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650101 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 62A0B13B0 for ; Fri, 19 Oct 2018 19:31:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4E67C28496 for ; Fri, 19 Oct 2018 19:31:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3F50C284A3; Fri, 19 Oct 2018 19:31:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AEFA928496 for ; Fri, 19 Oct 2018 19:31:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727867AbeJTDiu (ORCPT ); Fri, 19 Oct 2018 23:38:50 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:33236 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727772AbeJTDit (ORCPT ); Fri, 19 Oct 2018 23:38:49 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJNkqb028046; Fri, 19 Oct 2018 12:31:20 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n6h853fnm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:20 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:18 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:18 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:13 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 413BB2101E7C; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 1/8] Add testcases for consistency in file collision conflict handling Date: Fri, 19 Oct 2018 12:31:04 -0700 Message-ID: <20181019193111.12051-2-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=4 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Add testcases dealing with file collisions for the following types of conflicts: * add/add * rename/add * rename/rename(2to1) All these conflict types simplify down to two files "colliding" and should thus be handled similarly. This means that rename/add and rename/rename(2to1) conflicts need to be modified to behave the same as add/add conflicts currently do: the colliding files should be two-way merged (instead of the current behavior of writing the two colliding files out to separate temporary unique pathnames). Add testcases which check this; subsequent commits will fix the conflict handling to make these tests pass. Signed-off-by: Elijah Newren --- t/t6042-merge-rename-corner-cases.sh | 162 +++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index b97aca7fa2..b6fed2cb9a 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -937,4 +937,166 @@ test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename ) ' +test_conflicts_with_adds_and_renames() { + sideL=$1 + sideR=$2 + expect=$3 + + # Setup: + # L + # / \ + # master ? + # \ / + # R + # + # Where: + # Both L and R have files named 'three' which collide. Each of + # the colliding files could have been involved in a rename, in + # which case there was a file named 'one' or 'two' that was + # modified on the opposite side of history and renamed into the + # collision on this side of history. + # + # Questions: + # 1) The index should contain both a stage 2 and stage 3 entry + # for the colliding file. Does it? + # 2) When renames are involved, the content merges are clean, so + # the index should reflect the content merges, not merely the + # version of the colliding file from the prior commit. Does + # it? + # 3) There should be a file in the worktree named 'three' + # containing the two-way merged contents of the content-merged + # versions of 'three' from each of the two colliding + # files. Is it present? + # 4) There should not be any three~* files in the working + # tree + test_expect_success "setup simple $sideL/$sideR conflict" ' + test_create_repo simple_${sideL}_${sideR} && + ( + cd simple_${sideL}_${sideR} && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >file_v1 && + cp file_v1 file_v2 && + echo modification >>file_v2 && + + cp file_v1 file_v3 && + echo more stuff >>file_v3 && + cp file_v3 file_v4 && + echo yet more stuff >>file_v4 && + + # Use a tag to record both these files for simple + # access, and clean out these untracked files + git tag file_v1 $(git hash-object -w file_v1) && + git tag file_v2 $(git hash-object -w file_v2) && + git tag file_v3 $(git hash-object -w file_v3) && + git tag file_v4 $(git hash-object -w file_v4) && + git clean -f && + + # Setup original commit (or merge-base), consisting of + # files named "one" and "two" if renames were involved. + touch irrelevant_file && + git add irrelevant_file && + if [ $sideL = "rename" ] + then + git show file_v1 >one && + git add one + fi && + if [ $sideR = "rename" ] + then + git show file_v3 >two && + git add two + fi && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + if [ $sideL = "rename" ] + then + git mv one three + else + git show file_v2 >three && + git add three + fi && + if [ $sideR = "rename" ] + then + git show file_v4 >two && + git add two + fi && + test_tick && git commit -m L && + + # Handle the right side + git checkout R && + if [ $sideL = "rename" ] + then + git show file_v2 >one && + git add one + fi && + if [ $sideR = "rename" ] + then + git mv two three + else + git show file_v4 >three && + git add three + fi && + test_tick && git commit -m R + ) + ' + + test_expect_$expect "check simple $sideL/$sideR conflict" ' + ( + cd simple_${sideL}_${sideR} && + + git checkout L^0 && + + # Merge must fail; there is a conflict + test_must_fail git merge -s recursive R^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Nothing should have touched irrelevant_file + git rev-parse >actual \ + :0:irrelevant_file \ + :2:three \ + :3:three && + git rev-parse >expected \ + master:irrelevant_file \ + file_v2 \ + file_v4 && + test_cmp expected actual && + + # Make sure we have the correct merged contents for + # three + git show file_v1 >expected && + cat <<-\EOF >>expected && + <<<<<<< HEAD + modification + ======= + more stuff + yet more stuff + >>>>>>> R^0 + EOF + + test_cmp expected three + ) + ' +} + +test_conflicts_with_adds_and_renames rename rename failure +test_conflicts_with_adds_and_renames rename add failure +test_conflicts_with_adds_and_renames add rename failure +test_conflicts_with_adds_and_renames add add success + test_done From patchwork Fri Oct 19 19:31:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650095 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D14783B73 for ; Fri, 19 Oct 2018 19:31:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BDEDE2849D for ; Fri, 19 Oct 2018 19:31:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AFE2B28496; Fri, 19 Oct 2018 19:31:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E103928496 for ; Fri, 19 Oct 2018 19:31:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727846AbeJTDis (ORCPT ); Fri, 19 Oct 2018 23:38:48 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:33228 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727772AbeJTDir (ORCPT ); Fri, 19 Oct 2018 23:38:47 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJNg6C028015; Fri, 19 Oct 2018 12:31:18 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n6h853fnj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:18 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:16 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:16 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:13 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 46FDE2101E7D; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 2/8] t6036, t6042: testcases for rename collision of already conflicting files Date: Fri, 19 Oct 2018 12:31:05 -0700 Message-ID: <20181019193111.12051-3-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=4 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When a single file is renamed, it can also be modified, yielding the possibility of that renamed file having content conflicts. If two different such files are renamed into the same location, then two-way merging those files may result in nested conflicts. Add a testcase that makes sure we get this case correct, and uses different lengths of conflict markers to differentiate between the different nestings. Also add another case with an extra (i.e. third) level of conflict markers due to using merge.conflictstyle=diff3 and the virtual merge base also having conflicts present. Signed-off-by: Elijah Newren --- t/t6036-recursive-corner-cases.sh | 194 +++++++++++++++++++++++++++ t/t6042-merge-rename-corner-cases.sh | 118 ++++++++++++++++ 2 files changed, 312 insertions(+) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index e1cef58f2a..78138a7fb4 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -1402,4 +1402,198 @@ test_expect_failure 'check conflicting modes for regular file' ' ) ' +# Setup: +# L1---L2 +# / \ / \ +# master X ? +# \ / \ / +# R1---R2 +# +# Where: +# master has two files, named 'b' and 'a' +# branches L1 and R1 both modify each of the two files in conflicting ways +# +# L2 is a merge of R1 into L1; more on it later. +# R2 is a merge of L1 into R1; more on it later. +# +# X is an auto-generated merge-base used when merging L2 and R2. +# since X is a merge of L1 and R1, it has conflicting versions of each file +# +# More about L2 and R2: +# - both resolve the conflicts in 'b' and 'a' differently +# - L2 renames 'b' to 'm' +# - R2 renames 'a' to 'm' +# +# In the end, in file 'm' we have four different conflicting files (from +# two versions of 'b' and two of 'a'). In addition, if +# merge.conflictstyle is diff3, then the base version also has +# conflict markers of its own, leading to a total of three levels of +# conflict markers. This is a pretty weird corner case, but we just want +# to ensure that we handle it as well as practical. + +test_expect_success "setup nested conflicts" ' + test_create_repo nested_conflicts && + ( + cd nested_conflicts && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >initial && + + cp initial b_L1 && + cp initial b_R1 && + cp initial b_L2 && + cp initial b_R2 && + cp initial a_L1 && + cp initial a_R1 && + cp initial a_L2 && + cp initial a_R2 && + + test_write_lines b b_L1 >>b_L1 && + test_write_lines b b_R1 >>b_R1 && + test_write_lines b b_L2 >>b_L2 && + test_write_lines b b_R2 >>b_R2 && + test_write_lines a a_L1 >>a_L1 && + test_write_lines a a_R1 >>a_R1 && + test_write_lines a a_L2 >>a_L2 && + test_write_lines a a_R2 >>a_R2 && + + # Setup original commit (or merge-base), consisting of + # files named "b" and "a" + cp initial b && + cp initial a && + echo b >>b && + echo a >>a && + git add b a && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + mv -f b_L1 b && + mv -f a_L1 a && + git add b a && + test_tick && git commit -m "version L1 of files" && + git tag L1 && + + # Handle the right side + git checkout R && + mv -f b_R1 b && + mv -f a_R1 a && + git add b a && + test_tick && git commit -m "verson R1 of files" && + git tag R1 && + + # Create first merge on left side + git checkout L && + test_must_fail git merge R1 && + mv -f b_L2 b && + mv -f a_L2 a && + git add b a && + git mv b m && + test_tick && git commit -m "left merge, rename b->m" && + git tag L2 && + + # Create first merge on right side + git checkout R && + test_must_fail git merge L1 && + mv -f b_R2 b && + mv -f a_R2 a && + git add b a && + git mv a m && + test_tick && git commit -m "right merge, rename a->m" && + git tag R2 + ) +' + +test_expect_failure "check nested conflicts" ' + ( + cd nested_conflicts && + + git clean -f && + git checkout L2^0 && + + # Merge must fail; there is a conflict + test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R2^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Create a and b from virtual merge base X + git cat-file -p master:a >base && + git cat-file -p L1:a >ours && + git cat-file -p R1:a >theirs && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + ours \ + base \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >vmb_a && + + git cat-file -p master:b >base && + git cat-file -p L1:b >ours && + git cat-file -p R1:b >theirs && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + ours \ + base \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >vmb_b && + + # Compare :2:m to expected values + git cat-file -p L2:m >ours && + git cat-file -p R2:b >theirs && + test_must_fail git merge-file --diff3 \ + -L "HEAD:m" \ + -L "merged common ancestors:b" \ + -L "R2^0:b" \ + ours \ + vmb_b \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >m_stage_2 && + git cat-file -p :2:m >actual && + test_cmp m_stage_2 actual && + + # Compare :3:m to expected values + git cat-file -p L2:a >ours && + git cat-file -p R2:m >theirs && + test_must_fail git merge-file --diff3 \ + -L "HEAD:a" \ + -L "merged common ancestors:a" \ + -L "R2^0:m" \ + ours \ + vmb_a \ + theirs && + sed -e "s/^\([<|=>]\)/\1\1/" ours >m_stage_3 && + git cat-file -p :3:m >actual && + test_cmp m_stage_3 actual && + + # Compare m to expected contents + >empty && + cp -a m_stage_2 expected_final_m && + test_must_fail git merge-file --diff3 \ + -L "HEAD" \ + -L "merged common ancestors" \ + -L "R2^0" \ + expected_final_m \ + empty \ + m_stage_3 && + test_cmp expected_final_m m + ) +' + test_done diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index b6fed2cb9a..5e69e60b48 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -1099,4 +1099,122 @@ test_conflicts_with_adds_and_renames rename add failure test_conflicts_with_adds_and_renames add rename failure test_conflicts_with_adds_and_renames add add success +# Setup: +# L +# / \ +# master ? +# \ / +# R +# +# Where: +# master has two files, named 'one' and 'two'. +# branches L and R both modify 'one', in conflicting ways. +# branches L and R both modify 'two', in conflicting ways. +# branch L also renames 'one' to 'three'. +# branch R also renames 'two' to 'three'. +# +# So, we have four different conflicting files that all end up at path +# 'three'. +test_expect_success "setup nested conflicts from rename/rename(2to1)" ' + test_create_repo nested_conflicts_from_rename_rename && + ( + cd nested_conflicts_from_rename_rename && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >file_v1 && + + cp file_v1 file_v2 && + cp file_v1 file_v3 && + cp file_v1 file_v4 && + cp file_v1 file_v5 && + cp file_v1 file_v6 && + + echo one >>file_v1 && + echo uno >>file_v2 && + echo eins >>file_v3 && + + echo two >>file_v4 && + echo dos >>file_v5 && + echo zwei >>file_v6 && + + # Setup original commit (or merge-base), consisting of + # files named "one" and "two". + mv file_v1 one && + mv file_v4 two && + git add one two && + test_tick && git commit -m english && + + git branch L && + git branch R && + + # Handle the left side + git checkout L && + git mv one three && + mv -f file_v2 three && + mv -f file_v5 two && + git add two three && + test_tick && git commit -m spanish && + + # Handle the right side + git checkout R && + git mv two three && + mv -f file_v3 one && + mv -f file_v6 three && + git add one three && + test_tick && git commit -m german + ) +' + +test_expect_failure "check nested conflicts from rename/rename(2to1)" ' + ( + cd nested_conflicts_from_rename_rename && + + git checkout L^0 && + + # Merge must fail; there is a conflict + test_must_fail git merge -s recursive R^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 2 out && + git ls-files -u >out && + test_line_count = 2 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Compare :2:three to expected values + git cat-file -p master:one >base && + git cat-file -p L:three >ours && + git cat-file -p R:one >theirs && + test_must_fail git merge-file \ + -L "HEAD:three" -L "" -L "R^0:one" \ + ours base theirs && + sed -e "s/^\([<=>]\)/\1\1/" ours >L-three && + git cat-file -p :2:three >expect && + test_cmp expect L-three && + + # Compare :2:three to expected values + git cat-file -p master:two >base && + git cat-file -p L:two >ours && + git cat-file -p R:three >theirs && + test_must_fail git merge-file \ + -L "HEAD:two" -L "" -L "R^0:three" \ + ours base theirs && + sed -e "s/^\([<=>]\)/\1\1/" ours >R-three && + git cat-file -p :3:three >expect && + test_cmp expect R-three && + + # Compare three to expected contents + >empty && + test_must_fail git merge-file \ + -L "HEAD" -L "" -L "R^0" \ + L-three empty R-three && + test_cmp three L-three + ) +' + test_done From patchwork Fri Oct 19 19:31:06 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650093 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 920C913B0 for ; Fri, 19 Oct 2018 19:31:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7C82828496 for ; Fri, 19 Oct 2018 19:31:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6BA8C284AF; Fri, 19 Oct 2018 19:31:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9049328496 for ; Fri, 19 Oct 2018 19:31:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727825AbeJTDiq (ORCPT ); Fri, 19 Oct 2018 23:38:46 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:33216 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727754AbeJTDiq (ORCPT ); Fri, 19 Oct 2018 23:38:46 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJNkqX028046; Fri, 19 Oct 2018 12:31:17 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n6h853fnh-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:16 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:15 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:15 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:13 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 4DFA72101E7E; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 3/8] merge-recursive: increase marker length with depth of recursion Date: Fri, 19 Oct 2018 12:31:06 -0700 Message-ID: <20181019193111.12051-4-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=13 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Later patches in this series will modify file collision conflict handling (e.g. from rename/add and rename/rename(2to1) conflicts) so that multiply nested conflict markers can arise even before considering conflicts in the virtual merge base. Including the virtual merge base will provide a way to get triply (or higher) nested conflict markers. This new way to get nested conflict markers will force the need for a more general mechanism to extend the length of conflict markers in order to differentiate between different nestings. Along with this change to conflict marker length handling, we want to make sure that we don't regress handling for other types of conflicts with nested conflict markers. Add a more involved testcase using merge.conflictstyle=diff3, where not only does the virtual merge base contain conflicts, but its virtual merge base does as well (i.e. a case with triply nested conflict markers). While there are multiple reasonable ways to handle nested conflict markers in the virtual merge base for this type of situation, the easiest approach that dovetails well with the new needs for the file collision conflict handling is to require that the length of the conflict markers increase with each subsequent nesting. Subsequent patches which change the rename/add and rename/rename(2to1) conflict handling will modify the extra_marker_size flag appropriately for their new needs. Signed-off-by: Elijah Newren --- ll-merge.c | 4 +- ll-merge.h | 1 + merge-recursive.c | 25 +++-- t/t6036-recursive-corner-cases.sh | 151 ++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 9 deletions(-) diff --git a/ll-merge.c b/ll-merge.c index 0e2800f7bb..aabc1b5c2e 100644 --- a/ll-merge.c +++ b/ll-merge.c @@ -384,7 +384,9 @@ int ll_merge(mmbuffer_t *result_buf, if (opts->virtual_ancestor) { if (driver->recursive) driver = find_ll_merge_driver(driver->recursive); - marker_size += 2; + } + if (opts->extra_marker_size) { + marker_size += opts->extra_marker_size; } return driver->fn(driver, result_buf, path, ancestor, ancestor_label, ours, our_label, theirs, their_label, diff --git a/ll-merge.h b/ll-merge.h index b72b19921e..5b4e158502 100644 --- a/ll-merge.h +++ b/ll-merge.h @@ -11,6 +11,7 @@ struct ll_merge_options { unsigned virtual_ancestor : 1; unsigned variant : 2; /* favor ours, favor theirs, or union merge */ unsigned renormalize : 1; + unsigned extra_marker_size; long xdl_opts; }; diff --git a/merge-recursive.c b/merge-recursive.c index 73b5710386..f795c92a69 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1058,7 +1058,8 @@ static int merge_3way(struct merge_options *o, const struct diff_filespec *a, const struct diff_filespec *b, const char *branch1, - const char *branch2) + const char *branch2, + const int extra_marker_size) { mmfile_t orig, src1, src2; struct ll_merge_options ll_opts = {0}; @@ -1066,6 +1067,7 @@ static int merge_3way(struct merge_options *o, int merge_status; ll_opts.renormalize = o->renormalize; + ll_opts.extra_marker_size = extra_marker_size; ll_opts.xdl_opts = o->xdl_opts; if (o->call_depth) { @@ -1300,6 +1302,7 @@ static int merge_mode_and_contents(struct merge_options *o, const char *filename, const char *branch1, const char *branch2, + const int extra_marker_size, struct merge_file_info *result) { if (o->branch1 != branch1) { @@ -1310,7 +1313,8 @@ static int merge_mode_and_contents(struct merge_options *o, */ return merge_mode_and_contents(o, one, b, a, filename, - branch2, branch1, result); + branch2, branch1, + extra_marker_size, result); } result->merge = 0; @@ -1351,7 +1355,8 @@ static int merge_mode_and_contents(struct merge_options *o, int ret = 0, merge_status; merge_status = merge_3way(o, &result_buf, one, a, b, - branch1, branch2); + branch1, branch2, + extra_marker_size); if ((merge_status < 0) || !result_buf.ptr) ret = err(o, _("Failed to execute internal merge")); @@ -1640,7 +1645,8 @@ static int handle_rename_rename_1to2(struct merge_options *o, struct diff_filespec other; struct diff_filespec *add; if (merge_mode_and_contents(o, one, a, b, one->path, - ci->branch1, ci->branch2, &mfi)) + ci->branch1, ci->branch2, + o->call_depth * 2, &mfi)) return -1; /* @@ -1707,9 +1713,11 @@ static int handle_rename_rename_2to1(struct merge_options *o, path_side_1_desc = xstrfmt("version of %s from %s", path, a->path); path_side_2_desc = xstrfmt("version of %s from %s", path, b->path); if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc, - o->branch1, o->branch2, &mfi_c1) || + o->branch1, o->branch2, + o->call_depth * 2, &mfi_c1) || merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc, - o->branch1, o->branch2, &mfi_c2)) + o->branch1, o->branch2, + o->call_depth * 2, &mfi_c2)) return -1; free(path_side_1_desc); free(path_side_2_desc); @@ -2755,7 +2763,7 @@ static int process_renames(struct merge_options *o, if (merge_mode_and_contents(o, &one, &a, &b, ren1_dst, branch1, branch2, - &mfi)) { + o->call_depth * 2, &mfi)) { clean_merge = -1; goto cleanup_and_return; } @@ -3052,7 +3060,8 @@ static int handle_content_merge(struct merge_options *o, df_conflict_remains = 1; } if (merge_mode_and_contents(o, &one, &a, &b, path, - o->branch1, o->branch2, &mfi)) + o->branch1, o->branch2, + o->call_depth * 2, &mfi)) return -1; /* diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 78138a7fb4..32fd0a2995 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -1596,4 +1596,155 @@ test_expect_failure "check nested conflicts" ' ) ' +# Setup: +# L1---L2---L3 +# / \ / \ / \ +# master X1 X2 ? +# \ / \ / \ / +# R1---R2---R3 +# +# Where: +# master has one file named 'content' +# branches L1 and R1 both modify each of the two files in conflicting ways +# +# L (n>1) is a merge of R into L +# R (n>1) is a merge of L into R +# L and R resolve the conflicts differently. +# +# X is an auto-generated merge-base used when merging L and R. +# By construction, X1 has conflict markers due to conflicting versions. +# X2, due to using merge.conflictstyle=3, has nested conflict markers. +# +# So, merging R3 into L3 using merge.conflictstyle=3 should show the +# nested conflict markers from X2 in the base version -- that means we +# have three levels of conflict markers. Can we distinguish all three? + +test_expect_success "setup virtual merge base with nested conflicts" ' + test_create_repo virtual_merge_base_has_nested_conflicts && + ( + cd virtual_merge_base_has_nested_conflicts && + + # Create some related files now + for i in $(test_seq 1 10) + do + echo Random base content line $i + done >content && + + # Setup original commit + git add content && + test_tick && git commit -m initial && + + git branch L && + git branch R && + + # Create L1 + git checkout L && + echo left >>content && + git add content && + test_tick && git commit -m "version L1 of content" && + git tag L1 && + + # Create R1 + git checkout R && + echo right >>content && + git add content && + test_tick && git commit -m "verson R1 of content" && + git tag R1 && + + # Create L2 + git checkout L && + test_must_fail git -c merge.conflictstyle=diff3 merge R1 && + git checkout L1 content && + test_tick && git commit -m "version L2 of content" && + git tag L2 && + + # Create R2 + git checkout R && + test_must_fail git -c merge.conflictstyle=diff3 merge L1 && + git checkout R1 content && + test_tick && git commit -m "version R2 of content" && + git tag R2 && + + # Create L3 + git checkout L && + test_must_fail git -c merge.conflictstyle=diff3 merge R2 && + git checkout L1 content && + test_tick && git commit -m "version L3 of content" && + git tag L3 && + + # Create R3 + git checkout R && + test_must_fail git -c merge.conflictstyle=diff3 merge L2 && + git checkout R1 content && + test_tick && git commit -m "version R3 of content" && + git tag R3 + ) +' + +test_expect_success "check virtual merge base with nested conflicts" ' + ( + cd virtual_merge_base_has_nested_conflicts && + + git checkout L3^0 && + + # Merge must fail; there is a conflict + test_must_fail git -c merge.conflictstyle=diff3 merge -s recursive R3^0 && + + # Make sure the index has the right number of entries + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 3 out && + # Ensure we have the correct number of untracked files + git ls-files -o >out && + test_line_count = 1 out && + + # Compare :[23]:content to expected values + git rev-parse L1:content R1:content >expect && + git rev-parse :2:content :3:content >actual && + test_cmp expect actual && + + # Imitate X1 merge base, except without long enough conflict + # markers because a subsequent sed will modify them. Put + # result into vmb. + git cat-file -p master:content >base && + git cat-file -p L:content >left && + git cat-file -p R:content >right && + cp left merged-once && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + merged-once \ + base \ + right && + sed -e "s/^\([<|=>]\)/\1\1\1/" merged-once >vmb && + + # Imitate X2 merge base, overwriting vmb. Note that we + # extend both sets of conflict markers to make them longer + # with the sed command. + cp left merged-twice && + test_must_fail git merge-file --diff3 \ + -L "Temporary merge branch 1" \ + -L "merged common ancestors" \ + -L "Temporary merge branch 2" \ + merged-twice \ + vmb \ + right && + sed -e "s/^\([<|=>]\)/\1\1\1/" merged-twice >vmb && + + # Compare :1:content to expected value + git cat-file -p :1:content >actual && + test_cmp vmb actual && + + # Determine expected content in final outer merge, compare to + # what the merge generated. + cp -f left expect && + test_must_fail git merge-file --diff3 \ + -L "HEAD" -L "merged common ancestors" -L "R3^0" \ + expect vmb right && + test_cmp expect content + ) +' + test_done From patchwork Fri Oct 19 19:31:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650099 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8C40E13B0 for ; Fri, 19 Oct 2018 19:31:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 762E628496 for ; Fri, 19 Oct 2018 19:31:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 66DE42849D; Fri, 19 Oct 2018 19:31:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 96A1E284AF for ; Fri, 19 Oct 2018 19:31:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727857AbeJTDit (ORCPT ); Fri, 19 Oct 2018 23:38:49 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47538 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727754AbeJTDis (ORCPT ); Fri, 19 Oct 2018 23:38:48 -0400 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJS8md010737; Fri, 19 Oct 2018 12:31:19 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n65vthh6b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:19 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:17 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:17 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:13 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 54A262101E7F; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 4/8] merge-recursive: new function for better colliding conflict resolutions Date: Fri, 19 Oct 2018 12:31:07 -0700 Message-ID: <20181019193111.12051-5-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=13 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP There are three conflict types that represent two (possibly entirely unrelated) files colliding at the same location: * add/add * rename/add * rename/rename(2to1) These three conflict types already share more similarity than might be immediately apparent from their description: (1) the handling of the rename variants already involves removing any entries from the index corresponding to the original file names[*], thus only leaving entries in the index for the colliding path; (2) likewise, any trace of the original file name in the working tree is also removed. So, in all three cases we're left with how to represent two colliding files in both the index and the working copy. [*] Technically, this isn't quite true because rename/rename(2to1) conflicts in the recursive (o->call_depth > 0) case do an "unrename" since about seven years ago. But even in that case, Junio felt compelled to explain that my decision to "unrename" wasn't necessarily the only or right answer -- search for "Comment from Junio" in t6036 for details. My initial motivation for looking at these three conflict types was that if the handling of these three conflict types is the same, at least in the limited set of cases where a renamed file is unmodified on the side of history where the file is not renamed, then a significant performance improvement for rename detection during merges is possible. However, while that served as motivation to look at these three types of conflicts, the actual goal of this new function is to try to improve the handling for all three cases, not to merely make them the same as each other in that special circumstance. === Handling the working tree === The previous behavior for these conflict types in regards to the working tree (assuming the file collision occurs at 'foo') was: * add/add does a two-way merge of the two files and records it as 'foo'. * rename/rename(2to1) records the two different files into two new uniquely named files (foo~HEAD and foo~$MERGE), while removing 'foo' from the working tree. * rename/add records the two different files into two different locations, recording the add at foo~$SIDE and, oddly, recording the rename at foo (why is the rename more important than the add?) So, the question for what to write to the working tree boils down to whether the two colliding files should be two-way merged and recorded in place, or recorded into separate files. As per discussion on the git mailing lit, two-way merging was deemed to always be preferred, as that makes these cases all more like content conflicts that users can handle from within their favorite editor, IDE, or merge tool. Note that since renames already involve a content merge, rename/add and rename/rename(2to1) conflicts could result in nested conflict markers. === Handling of the index === For a typical rename, unpack_trees() would set up the index in the following fashion: old_path new_path stage1: 5ca1ab1e 00000000 stage2: f005ba11 00000000 stage3: 00000000 b0a710ad And merge-recursive would rewrite this to new_path stage1: 5ca1ab1e stage2: f005ba11 stage3: b0a710ad Removing old_path from the index means the user won't have to `git rm old_path` manually every time a renamed path has a content conflict. It also means they can use `git checkout [--ours|--theirs|--conflict|-m] new_path`, `git diff [--ours|--theirs]` and various other commands that would be difficult otherwise. This strategy becomes a problem when we have a rename/add or rename/rename(2to1) conflict, however, because then we have only three slots to store blob sha1s and we need either four or six. Previously, this was handled by continuing to delete old_path from the index, and just outright ignoring any blob shas from old_path. That had the downside of deleting any trace of changes made to old_path on the other side of history. This function instead does a three-way content merge of the renamed file, and stores the blob sha1 for that at either stage2 or stage3 for new_path (depending on which side the rename came from). That has the advantage of bringing information about changes on both sides and still allows for easy resolution (no need to git rm old_path, etc.), but does have the downside that if the content merge had conflict markers, then what we store in the index is the sha1 of a blob with conflict markers. While that is a downside, it seems less problematic than the downsides of any obvious alternatives, and certainly makes more sense than the previous handling. Further, it has a precedent in that when we do recursive merges, we may accept a file with conflict markers as the resolution for the merge of the merge-bases, which will then show up in the index of the outer merge at stage 1 if a conflict exists at the outer level. Signed-off-by: Elijah Newren --- merge-recursive.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/merge-recursive.c b/merge-recursive.c index f795c92a69..24d979022e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1559,6 +1559,127 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, return target; } +#if 0 // #if-0-ing avoids unused function warning; will make live in next commit +static int handle_file_collision(struct merge_options *o, + const char *collide_path, + const char *prev_path1, + const char *prev_path2, + const char *branch1, const char *branch2, + const struct object_id *a_oid, + unsigned int a_mode, + const struct object_id *b_oid, + unsigned int b_mode) +{ + struct merge_file_info mfi; + struct diff_filespec null, a, b; + char *alt_path = NULL; + const char *update_path = collide_path; + + /* + * In the recursive case, we just opt to undo renames + */ + if (o->call_depth && (prev_path1 || prev_path2)) { + /* Put first file (a_oid, a_mode) in its original spot */ + if (prev_path1) { + if (update_file(o, 1, a_oid, a_mode, prev_path1)) + return -1; + } else { + if (update_file(o, 1, a_oid, a_mode, collide_path)) + return -1; + } + + /* Put second file (b_oid, b_mode) in its original spot */ + if (prev_path2) { + if (update_file(o, 1, b_oid, b_mode, prev_path2)) + return -1; + } else { + if (update_file(o, 1, b_oid, b_mode, collide_path)) + return -1; + } + + /* Don't leave something at collision path if unrenaming both */ + if (prev_path1 && prev_path2) + remove_file(o, 1, collide_path, 0); + + return 0; + } + + /* Remove rename sources if rename/add or rename/rename(2to1) */ + if (prev_path1) + remove_file(o, 1, prev_path1, + o->call_depth || would_lose_untracked(prev_path1)); + if (prev_path2) + remove_file(o, 1, prev_path2, + o->call_depth || would_lose_untracked(prev_path2)); + + /* + * Remove the collision path, if it wouldn't cause dirty contents + * or an untracked file to get lost. We'll either overwrite with + * merged contents, or just write out to differently named files. + */ + if (was_dirty(o, collide_path)) { + output(o, 1, _("Refusing to lose dirty file at %s"), + collide_path); + update_path = alt_path = unique_path(o, collide_path, "merged"); + } else if (would_lose_untracked(collide_path)) { + /* + * Only way we get here is if both renames were from + * a directory rename AND user had an untracked file + * at the location where both files end up after the + * two directory renames. See testcase 10d of t6043. + */ + output(o, 1, _("Refusing to lose untracked file at " + "%s, even though it's in the way."), + collide_path); + update_path = alt_path = unique_path(o, collide_path, "merged"); + } else { + /* + * FIXME: It's possible that the two files are identical + * and that the current working copy happens to match, in + * which case we are unnecessarily touching the working + * tree file. It's not a likely enough scenario that I + * want to code up the checks for it and a better fix is + * available if we restructure how unpack_trees() and + * merge-recursive interoperate anyway, so punting for + * now... + */ + remove_file(o, 0, collide_path, 0); + } + + /* Store things in diff_filespecs for functions that need it */ + memset(&a, 0, sizeof(struct diff_filespec)); + memset(&b, 0, sizeof(struct diff_filespec)); + null.path = a.path = b.path = (char *)collide_path; + oidcpy(&null.oid, &null_oid); + null.mode = 0; + oidcpy(&a.oid, a_oid); + a.mode = a_mode; + a.oid_valid = 1; + oidcpy(&b.oid, b_oid); + b.mode = b_mode; + b.oid_valid = 1; + + if (merge_mode_and_contents(o, &null, &a, &b, collide_path, + branch1, branch2, o->call_depth * 2, &mfi)) + return -1; + mfi.clean &= !alt_path; + if (update_file(o, mfi.clean, &mfi.oid, mfi.mode, update_path)) + return -1; + if (!mfi.clean && !o->call_depth && + update_stages(o, collide_path, NULL, &a, &b)) + return -1; + free(alt_path); + /* + * FIXME: If both a & b both started with conflicts (only possible + * if they came from a rename/rename(2to1)), but had IDENTICAL + * contents including those conflicts, then in the next line we claim + * it was clean. If someone cares about this case, we should have the + * caller notify us if we started with conflicts. + */ + return mfi.clean; +} +#endif + static int handle_file(struct merge_options *o, struct diff_filespec *rename, int stage, From patchwork Fri Oct 19 19:31:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650105 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9E742112B for ; Fri, 19 Oct 2018 19:31:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8C6D128496 for ; Fri, 19 Oct 2018 19:31:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 80F8B284AF; Fri, 19 Oct 2018 19:31:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BC5AC28496 for ; Fri, 19 Oct 2018 19:31:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727882AbeJTDiw (ORCPT ); Fri, 19 Oct 2018 23:38:52 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:33252 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727772AbeJTDiv (ORCPT ); Fri, 19 Oct 2018 23:38:51 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJNg6F028015; Fri, 19 Oct 2018 12:31:22 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n6h853fnr-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:22 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:20 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:20 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:15 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 5A2602101E93; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 5/8] merge-recursive: fix rename/add conflict handling Date: Fri, 19 Oct 2018 12:31:08 -0700 Message-ID: <20181019193111.12051-6-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=13 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This makes the rename/add conflict handling make use of the new handle_file_collision() function, which fixes several bugs and improves things for the rename/add case significantly. Previously, rename/add would: * Not leave any higher order stage entries in the index, making it appear as if there were no conflict. * Would place the rename file at the colliding path, and move the added file elsewhere, which combined with the lack of higher order stage entries felt really odd. It's not clear to me why the rename should take precedence over the add; if one should be moved out of the way, they both probably should. * In the recursive case, it would do a two way merge of the added file and the version of the renamed file on the renamed side, completely excluding modifications to the renamed file on the unrenamed side of history. Use the new handle_file_collision() to fix all of these issues. Signed-off-by: Elijah Newren --- merge-recursive.c | 137 +++++++++++++++++---------- t/t6036-recursive-corner-cases.sh | 24 ++--- t/t6042-merge-rename-corner-cases.sh | 4 +- 3 files changed, 101 insertions(+), 64 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 24d979022e..0805095168 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -186,6 +186,7 @@ static int oid_eq(const struct object_id *a, const struct object_id *b) enum rename_type { RENAME_NORMAL = 0, RENAME_VIA_DIR, + RENAME_ADD, RENAME_DELETE, RENAME_ONE_FILE_TO_ONE, RENAME_ONE_FILE_TO_TWO, @@ -228,6 +229,7 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, struct stage_data *src_entry1, struct stage_data *src_entry2) { + int ostage1 = 0, ostage2; struct rename_conflict_info *ci; /* @@ -264,18 +266,22 @@ static inline void setup_rename_conflict_info(enum rename_type rename_type, dst_entry2->rename_conflict_info = ci; } - if (rename_type == RENAME_TWO_FILES_TO_ONE) { - /* - * For each rename, there could have been - * modifications on the side of history where that - * file was not renamed. - */ - int ostage1 = o->branch1 == branch1 ? 3 : 2; - int ostage2 = ostage1 ^ 1; + /* + * For each rename, there could have been + * modifications on the side of history where that + * file was not renamed. + */ + if (rename_type == RENAME_ADD || + rename_type == RENAME_TWO_FILES_TO_ONE) { + ostage1 = o->branch1 == branch1 ? 3 : 2; ci->ren1_other.path = pair1->one->path; oidcpy(&ci->ren1_other.oid, &src_entry1->stages[ostage1].oid); ci->ren1_other.mode = src_entry1->stages[ostage1].mode; + } + + if (rename_type == RENAME_TWO_FILES_TO_ONE) { + ostage2 = ostage1 ^ 1; ci->ren2_other.path = pair2->one->path; oidcpy(&ci->ren2_other.oid, &src_entry2->stages[ostage2].oid); @@ -1559,7 +1565,6 @@ static struct diff_filespec *filespec_from_entry(struct diff_filespec *target, return target; } -#if 0 // #if-0-ing avoids unused function warning; will make live in next commit static int handle_file_collision(struct merge_options *o, const char *collide_path, const char *prev_path1, @@ -1575,6 +1580,20 @@ static int handle_file_collision(struct merge_options *o, char *alt_path = NULL; const char *update_path = collide_path; + /* + * It's easiest to get the correct things into stage 2 and 3, and + * to make sure that the content merge puts HEAD before the other + * branch if we just ensure that branch1 == o->branch1. So, simply + * flip arguments around if we don't have that. + */ + if (branch1 != o->branch1) { + return handle_file_collision(o, collide_path, + prev_path2, prev_path1, + branch2, branch1, + b_oid, b_mode, + a_oid, a_mode); + } + /* * In the recursive case, we just opt to undo renames */ @@ -1678,7 +1697,38 @@ static int handle_file_collision(struct merge_options *o, */ return mfi.clean; } -#endif + +static int handle_rename_add(struct merge_options *o, + struct rename_conflict_info *ci) +{ + /* a was renamed to c, and a separate c was added. */ + struct diff_filespec *a = ci->pair1->one; + struct diff_filespec *c = ci->pair1->two; + char *path = c->path; + char *prev_path_desc; + struct merge_file_info mfi; + + int other_stage = (ci->branch1 == o->branch1 ? 3 : 2); + + output(o, 1, _("CONFLICT (rename/add): " + "Rename %s->%s in %s. Added %s in %s"), + a->path, c->path, ci->branch1, + c->path, ci->branch2); + + prev_path_desc = xstrfmt("version of %s from %s", path, a->path); + if (merge_mode_and_contents(o, a, c, &ci->ren1_other, prev_path_desc, + o->branch1, o->branch2, + 1 + o->call_depth * 2, &mfi)) + return -1; + free(prev_path_desc); + + return handle_file_collision(o, + c->path, a->path, NULL, + ci->branch1, ci->branch2, + &mfi.oid, mfi.mode, + &ci->dst_entry1->stages[other_stage].oid, + ci->dst_entry1->stages[other_stage].mode); +} static int handle_file(struct merge_options *o, struct diff_filespec *rename, @@ -2860,47 +2910,23 @@ static int process_renames(struct merge_options *o, 0 /* update_wd */)) clean_merge = -1; } else if (!oid_eq(&dst_other.oid, &null_oid)) { - clean_merge = 0; - try_merge = 1; - output(o, 1, _("CONFLICT (rename/add): Rename %s->%s in %s. " - "%s added in %s"), - ren1_src, ren1_dst, branch1, - ren1_dst, branch2); - if (o->call_depth) { - struct merge_file_info mfi; - struct diff_filespec one, a, b; - - oidcpy(&one.oid, &null_oid); - one.mode = 0; - one.path = ren1->pair->two->path; - - oidcpy(&a.oid, &ren1->pair->two->oid); - a.mode = ren1->pair->two->mode; - a.path = one.path; - - oidcpy(&b.oid, &dst_other.oid); - b.mode = dst_other.mode; - b.path = one.path; - - if (merge_mode_and_contents(o, &one, &a, &b, ren1_dst, - branch1, branch2, - o->call_depth * 2, &mfi)) { - clean_merge = -1; - goto cleanup_and_return; - } - output(o, 1, _("Adding merged %s"), ren1_dst); - if (update_file(o, 0, &mfi.oid, - mfi.mode, ren1_dst)) - clean_merge = -1; - try_merge = 0; - } else { - char *new_path = unique_path(o, ren1_dst, branch2); - output(o, 1, _("Adding as %s instead"), new_path); - if (update_file(o, 0, &dst_other.oid, - dst_other.mode, new_path)) - clean_merge = -1; - free(new_path); - } + /* + * Probably not a clean merge, but it's + * premature to set clean_merge to 0 here, + * because if the rename merges cleanly and + * the merge exactly matches the newly added + * file, then the merge will be clean. + */ + setup_rename_conflict_info(RENAME_ADD, + ren1->pair, + NULL, + branch1, + branch2, + ren1->dst_entry, + NULL, + o, + ren1->src_entry, + NULL); } else try_merge = 1; @@ -3312,6 +3338,15 @@ static int process_entry(struct merge_options *o, conflict_info->branch2)) clean_merge = -1; break; + case RENAME_ADD: + /* + * Probably unclean merge, but if the renamed file + * merges cleanly and the result can then be + * two-way merged cleanly with the added file, I + * guess it's a clean merge? + */ + clean_merge = handle_rename_add(o, conflict_info); + break; case RENAME_DELETE: clean_merge = 0; if (handle_rename_delete(o, diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 32fd0a2995..44807603e3 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -185,7 +185,7 @@ test_expect_success 'setup differently handled merges of rename/add conflict' ' git branch B && git checkout -b C && echo 10 >>a && - echo "other content" >>new_a && + test_write_lines 0 1 2 3 4 5 6 7 foobar >new_a && git add a new_a && test_tick && git commit -m C && @@ -195,14 +195,14 @@ test_expect_success 'setup differently handled merges of rename/add conflict' ' git checkout B^0 && test_must_fail git merge C && - git clean -f && + git show :2:new_a >new_a && + git add new_a && test_tick && git commit -m D && git tag D && git checkout C^0 && test_must_fail git merge B && - rm new_a~HEAD new_a && - printf "Incorrectly merged content" >>new_a && + test_write_lines 0 1 2 3 4 5 6 7 bad_merge >new_a && git add -u && test_tick && git commit -m E && git tag E @@ -225,21 +225,23 @@ test_expect_success 'git detects differently handled merges conflict' ' test_line_count = 1 out && git rev-parse >expect \ - D:new_a E:new_a && + C:new_a D:new_a E:new_a && git rev-parse >actual \ - :2:new_a :3:new_a && + :1:new_a :2:new_a :3:new_a && test_cmp expect actual && - git cat-file -p C:new_a >ours && - git cat-file -p B:new_a >theirs && + # Test that the two-way merge in new_a is as expected + git cat-file -p D:new_a >ours && + git cat-file -p E:new_a >theirs && >empty && test_must_fail git merge-file \ - -L "Temporary merge branch 1" \ + -L "HEAD" \ -L "" \ - -L "Temporary merge branch 2" \ + -L "E^0" \ ours empty theirs && sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect && - git cat-file -p :1:new_a >actual && + git hash-object new_a >actual && + git hash-object ours >expect && test_cmp expect actual ) ' diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index 5e69e60b48..f1c4a06dd6 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -1095,8 +1095,8 @@ test_conflicts_with_adds_and_renames() { } test_conflicts_with_adds_and_renames rename rename failure -test_conflicts_with_adds_and_renames rename add failure -test_conflicts_with_adds_and_renames add rename failure +test_conflicts_with_adds_and_renames rename add success +test_conflicts_with_adds_and_renames add rename success test_conflicts_with_adds_and_renames add add success # Setup: From patchwork Fri Oct 19 19:31:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650103 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E45FA13B0 for ; Fri, 19 Oct 2018 19:31:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D08522849D for ; Fri, 19 Oct 2018 19:31:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C3333284A7; Fri, 19 Oct 2018 19:31:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B52722849D for ; Fri, 19 Oct 2018 19:31:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727875AbeJTDiv (ORCPT ); Fri, 19 Oct 2018 23:38:51 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:33248 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727754AbeJTDiu (ORCPT ); Fri, 19 Oct 2018 23:38:50 -0400 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJNkqc028046; Fri, 19 Oct 2018 12:31:21 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n6h853fnn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:21 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:19 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:19 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:15 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 602FD2101E94; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 6/8] merge-recursive: improve handling for rename/rename(2to1) conflicts Date: Fri, 19 Oct 2018 12:31:09 -0700 Message-ID: <20181019193111.12051-7-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=13 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=979 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This makes the rename/rename(2to1) conflicts use the new handle_file_collision() function. Since that function was based originally on the rename/rename(2to1) handling code, the main differences here are in what was added. In particular: * Instead of storing files at collide_path~HEAD and collide_path~MERGE, the files are two-way merged and recorded at collide_path. * Instead of recording the version of the renamed file that existed on the renamed side in the index (thus ignoring any changes that were made to the file on the side of history without the rename), we do a three-way content merge on the renamed path, then store that at either stage 2 or stage 3. * Note that since the content merge for each rename may have conflicts, and then we have to merge the two renamed files, we can end up with nested conflict markers. Signed-off-by: Elijah Newren --- merge-recursive.c | 104 ++++----------------------- t/t6036-recursive-corner-cases.sh | 12 +--- t/t6042-merge-rename-corner-cases.sh | 38 ++++++---- t/t6043-merge-rename-directories.sh | 83 ++++++++++++--------- 4 files changed, 89 insertions(+), 148 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 0805095168..ead6054a75 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -696,27 +696,6 @@ static int update_stages(struct merge_options *opt, const char *path, return 0; } -static int update_stages_for_stage_data(struct merge_options *opt, - const char *path, - const struct stage_data *stage_data) -{ - struct diff_filespec o, a, b; - - o.mode = stage_data->stages[1].mode; - oidcpy(&o.oid, &stage_data->stages[1].oid); - - a.mode = stage_data->stages[2].mode; - oidcpy(&a.oid, &stage_data->stages[2].oid); - - b.mode = stage_data->stages[3].mode; - oidcpy(&b.oid, &stage_data->stages[3].oid); - - return update_stages(opt, path, - is_null_oid(&o.oid) ? NULL : &o, - is_null_oid(&a.oid) ? NULL : &a, - is_null_oid(&b.oid) ? NULL : &b); -} - static void update_entry(struct stage_data *entry, struct diff_filespec *o, struct diff_filespec *a, @@ -1870,7 +1849,6 @@ static int handle_rename_rename_2to1(struct merge_options *o, char *path_side_2_desc; struct merge_file_info mfi_c1; struct merge_file_info mfi_c2; - int ret; output(o, 1, _("CONFLICT (rename/rename): " "Rename %s->%s in %s. " @@ -1878,81 +1856,22 @@ static int handle_rename_rename_2to1(struct merge_options *o, a->path, c1->path, ci->branch1, b->path, c2->path, ci->branch2); - remove_file(o, 1, a->path, o->call_depth || would_lose_untracked(a->path)); - remove_file(o, 1, b->path, o->call_depth || would_lose_untracked(b->path)); - path_side_1_desc = xstrfmt("version of %s from %s", path, a->path); path_side_2_desc = xstrfmt("version of %s from %s", path, b->path); if (merge_mode_and_contents(o, a, c1, &ci->ren1_other, path_side_1_desc, o->branch1, o->branch2, - o->call_depth * 2, &mfi_c1) || + 1 + o->call_depth * 2, &mfi_c1) || merge_mode_and_contents(o, b, &ci->ren2_other, c2, path_side_2_desc, o->branch1, o->branch2, - o->call_depth * 2, &mfi_c2)) + 1 + o->call_depth * 2, &mfi_c2)) return -1; free(path_side_1_desc); free(path_side_2_desc); - if (o->call_depth) { - /* - * If mfi_c1.clean && mfi_c2.clean, then it might make - * sense to do a two-way merge of those results. But, I - * think in all cases, it makes sense to have the virtual - * merge base just undo the renames; they can be detected - * again later for the non-recursive merge. - */ - remove_file(o, 0, path, 0); - ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, a->path); - if (!ret) - ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, - b->path); - } else { - char *new_path1 = unique_path(o, path, ci->branch1); - char *new_path2 = unique_path(o, path, ci->branch2); - output(o, 1, _("Renaming %s to %s and %s to %s instead"), - a->path, new_path1, b->path, new_path2); - if (was_dirty(o, path)) - output(o, 1, _("Refusing to lose dirty file at %s"), - path); - else if (would_lose_untracked(path)) - /* - * Only way we get here is if both renames were from - * a directory rename AND user had an untracked file - * at the location where both files end up after the - * two directory renames. See testcase 10d of t6043. - */ - output(o, 1, _("Refusing to lose untracked file at " - "%s, even though it's in the way."), - path); - else - remove_file(o, 0, path, 0); - ret = update_file(o, 0, &mfi_c1.oid, mfi_c1.mode, new_path1); - if (!ret) - ret = update_file(o, 0, &mfi_c2.oid, mfi_c2.mode, - new_path2); - /* - * unpack_trees() actually populates the index for us for - * "normal" rename/rename(2to1) situtations so that the - * correct entries are at the higher stages, which would - * make the call below to update_stages_for_stage_data - * unnecessary. However, if either of the renames came - * from a directory rename, then unpack_trees() will not - * have gotten the right data loaded into the index, so we - * need to do so now. (While it'd be tempting to move this - * call to update_stages_for_stage_data() to - * apply_directory_rename_modifications(), that would break - * our intermediate calls to would_lose_untracked() since - * those rely on the current in-memory index. See also the - * big "NOTE" in update_stages()). - */ - if (update_stages_for_stage_data(o, path, ci->dst_entry1)) - ret = -1; - - free(new_path2); - free(new_path1); - } - - return ret; + return handle_file_collision(o, path, a->path, b->path, + ci->branch1, ci->branch2, + &mfi_c1.oid, mfi_c1.mode, + &mfi_c2.oid, mfi_c2.mode); } /* @@ -3361,9 +3280,14 @@ static int process_entry(struct merge_options *o, clean_merge = -1; break; case RENAME_TWO_FILES_TO_ONE: - clean_merge = 0; - if (handle_rename_rename_2to1(o, conflict_info)) - clean_merge = -1; + /* + * Probably unclean merge, but if the two renamed + * files merge cleanly and the two resulting files + * can then be two-way merged cleanly, I guess it's + * a clean merge? + */ + clean_merge = handle_rename_rename_2to1(o, + conflict_info); break; default: entry->processed = 0; diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 44807603e3..a4a78b6cef 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -64,15 +64,12 @@ test_expect_success 'merge simple rename+criss-cross with no modifications' ' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >expect \ - L2:three R2:three \ L2:three R2:three && git rev-parse >actual \ :2:three :3:three && - git hash-object >>actual \ - three~HEAD three~R2^0 && test_cmp expect actual ) ' @@ -140,15 +137,12 @@ test_expect_success 'merge criss-cross + rename merges with basic modification' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >expect \ - L2:three R2:three \ L2:three R2:three && git rev-parse >actual \ :2:three :3:three && - git hash-object >>actual \ - three~HEAD three~R2^0 && test_cmp expect actual ) ' @@ -1512,7 +1506,7 @@ test_expect_success "setup nested conflicts" ' ) ' -test_expect_failure "check nested conflicts" ' +test_expect_success "check nested conflicts" ' ( cd nested_conflicts && diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index f1c4a06dd6..4cfcde3082 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -464,17 +464,28 @@ test_expect_success 'handle rename/rename (2to1) conflict correctly' ' git ls-files -u c >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && test_path_is_missing a && test_path_is_missing b && - test_path_is_file c~HEAD && - test_path_is_file c~C^0 && - git rev-parse >expect \ - C:a B:b && - git hash-object >actual \ - c~HEAD c~C^0 && + git rev-parse >expect \ + C:a B:b && + git rev-parse >actual \ + :2:c :3:c && + test_cmp expect actual && + + # Test that the two-way merge in new_a is as expected + git cat-file -p :2:c >>ours && + git cat-file -p :3:c >>theirs && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "C^0" \ + ours empty theirs && + git hash-object c >actual && + git hash-object ours >expect && test_cmp expect actual ) ' @@ -940,7 +951,6 @@ test_expect_failure 'mod6-check: chains of rename/rename(1to2) and rename/rename test_conflicts_with_adds_and_renames() { sideL=$1 sideR=$2 - expect=$3 # Setup: # L @@ -1048,7 +1058,7 @@ test_conflicts_with_adds_and_renames() { ) ' - test_expect_$expect "check simple $sideL/$sideR conflict" ' + test_expect_success "check simple $sideL/$sideR conflict" ' ( cd simple_${sideL}_${sideR} && @@ -1094,10 +1104,10 @@ test_conflicts_with_adds_and_renames() { ' } -test_conflicts_with_adds_and_renames rename rename failure -test_conflicts_with_adds_and_renames rename add success -test_conflicts_with_adds_and_renames add rename success -test_conflicts_with_adds_and_renames add add success +test_conflicts_with_adds_and_renames rename rename +test_conflicts_with_adds_and_renames rename add +test_conflicts_with_adds_and_renames add rename +test_conflicts_with_adds_and_renames add add # Setup: # L @@ -1168,7 +1178,7 @@ test_expect_success "setup nested conflicts from rename/rename(2to1)" ' ) ' -test_expect_failure "check nested conflicts from rename/rename(2to1)" ' +test_expect_success "check nested conflicts from rename/rename(2to1)" ' ( cd nested_conflicts_from_rename_rename && diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index 4a71f17edd..fedaeafc55 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -278,7 +278,7 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:x/b :0:x/c :0:x/d :0:x/e :0:x/m :0:x/n && @@ -293,15 +293,16 @@ test_expect_success '1d-check: Directory renames cause a rename/rename(2to1) con A:y/wham B:z/wham && test_cmp expect actual && - test_path_is_missing x/wham && - test_path_is_file x/wham~HEAD && - test_path_is_file x/wham~B^0 && - - git hash-object >actual \ - x/wham~HEAD x/wham~B^0 && - git rev-parse >expect \ - A:y/wham B:z/wham && - test_cmp expect actual + # Test that the two-way merge in x/wham is as expected + git cat-file -p :2:x/wham >expect && + git cat-file -p :3:x/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect x/wham ) ' @@ -1670,7 +1671,7 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:y/b :0:y/c :2:y/d :3:y/d && @@ -1678,15 +1679,16 @@ test_expect_success '7b-check: rename/rename(2to1), but only due to transitive r O:z/b O:z/c O:w/d O:x/d && test_cmp expect actual && - test_path_is_missing y/d && - test_path_is_file y/d~HEAD && - test_path_is_file y/d~B^0 && - - git hash-object >actual \ - y/d~HEAD y/d~B^0 && - git rev-parse >expect \ - O:w/d O:x/d && - test_cmp expect actual + # Test that the two-way merge in y/d is as expected + git cat-file -p :2:y/d >expect && + git cat-file -p :3:y/d >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/d ) ' @@ -3165,7 +3167,7 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2) # Commit O: z/{a,b,c_1}, x/{d,e,f_2} # Commit A: y/{a,b}, x/{d,e,f_2,wham_1} + untracked y/wham # Commit B: z/{a,b,c_1,wham_2}, y/{d,e} -# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~B^0,wham~HEAD}+ +# Expected: Failed Merge; y/{a,b,d,e} + untracked y/{wham,wham~merged}+ # CONFLICT(rename/rename) z/c_1 vs x/f_2 -> y/wham # ERROR_MSG(Refusing to lose untracked file at y/wham) @@ -3219,7 +3221,7 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' ' git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && git rev-parse >actual \ :0:y/a :0:y/b :0:y/d :0:y/e :2:y/wham :3:y/wham && @@ -3232,11 +3234,16 @@ test_expect_success '10d-check: Delete untracked with dir rename/rename(2to1)' ' echo important >expect && test_cmp expect y/wham && - git hash-object >actual \ - y/wham~B^0 y/wham~HEAD && - git rev-parse >expect \ - O:x/f O:z/c && - test_cmp expect actual + # Test that the two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged ) ' @@ -3689,7 +3696,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena # Commit O: z/{a,b}, x/{c_1,d_2} # Commit A: y/{a,b,wham_1}, x/d_2, except y/wham has uncommitted mods # Commit B: z/{a,b,wham_2}, x/c_1 -# Expected: Failed Merge; y/{a,b} + untracked y/{wham~B^0,wham~B^HEAD} + +# Expected: Failed Merge; y/{a,b} + untracked y/{wham~merged} + # y/wham with dirty changes from before merge + # CONFLICT(rename/rename) x/c vs x/d -> y/wham # ERROR_MSG(Refusing to lose dirty file at y/wham) @@ -3741,24 +3748,30 @@ test_expect_success '11f-check: Avoid deleting not-uptodate with dir rename/rena git ls-files -u >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && test_seq 1 10 >expected && echo important >>expected && test_cmp expected y/wham && test_must_fail git rev-parse :1:y/wham && - git hash-object >actual \ - y/wham~B^0 y/wham~HEAD && - git rev-parse >expect \ - O:x/d O:x/c && - test_cmp expect actual && git rev-parse >actual \ :0:y/a :0:y/b :2:y/wham :3:y/wham && git rev-parse >expect \ O:z/a O:z/b O:x/c O:x/d && - test_cmp expect actual + test_cmp expect actual && + + # Test that the two-way merge in y/wham~merged is as expected + git cat-file -p :2:y/wham >expect && + git cat-file -p :3:y/wham >other && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + expect empty other && + test_cmp expect y/wham~merged ) ' From patchwork Fri Oct 19 19:31:10 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650107 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BCFC2112B for ; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id ACC79284A3 for ; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A0FC1284A7; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 337C42849D for ; Fri, 19 Oct 2018 19:31:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727943AbeJTDiw (ORCPT ); Fri, 19 Oct 2018 23:38:52 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47558 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727754AbeJTDiw (ORCPT ); Fri, 19 Oct 2018 23:38:52 -0400 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJRv7s010654; Fri, 19 Oct 2018 12:31:23 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n65vthh6k-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:23 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:21 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:21 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:16 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 6728C2101E96; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 7/8] merge-recursive: use handle_file_collision for add/add conflicts Date: Fri, 19 Oct 2018 12:31:10 -0700 Message-ID: <20181019193111.12051-8-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=4 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=757 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This results in no-net change of behavior, it simply ensures that all file-collision conflict handling types are being handled the same by calling the same function. Signed-off-by: Elijah Newren --- merge-recursive.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index ead6054a75..c78b347112 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -3355,14 +3355,27 @@ static int process_entry(struct merge_options *o, clean_merge = -1; } } else if (a_oid && b_oid) { - /* Case C: Added in both (check for same permissions) and */ - /* case D: Modified in both, but differently. */ - int is_dirty = 0; /* unpack_trees would have bailed if dirty */ - clean_merge = handle_content_merge(o, path, is_dirty, - o_oid, o_mode, - a_oid, a_mode, - b_oid, b_mode, - NULL); + if (!o_oid) { + /* Case C: Added in both (check for same permissions) */ + output(o, 1, + _("CONFLICT (add/add): Merge conflict in %s"), + path); + clean_merge = handle_file_collision(o, + path, NULL, NULL, + o->branch1, + o->branch2, + a_oid, a_mode, + b_oid, b_mode); + } else { + /* case D: Modified in both, but differently. */ + int is_dirty = 0; /* unpack_trees would have bailed if dirty */ + clean_merge = handle_content_merge(o, path, + is_dirty, + o_oid, o_mode, + a_oid, a_mode, + b_oid, b_mode, + NULL); + } } else if (!o_oid && !a_oid && !b_oid) { /* * this entry was deleted altogether. a_mode == 0 means From patchwork Fri Oct 19 19:31:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10650109 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id D6E8D3B73 for ; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C5C3428496 for ; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id BA110284B1; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0445328496 for ; Fri, 19 Oct 2018 19:31:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727954AbeJTDiy (ORCPT ); Fri, 19 Oct 2018 23:38:54 -0400 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47562 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727772AbeJTDix (ORCPT ); Fri, 19 Oct 2018 23:38:53 -0400 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.23/8.16.0.23) with SMTP id w9JJRvvk010651; Fri, 19 Oct 2018 12:31:24 -0700 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2n65vthh6n-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 19 Oct 2018 12:31:24 -0700 Received: from sj-prod-exch-02.YOJOE.local (10.129.18.29) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Fri, 19 Oct 2018 12:31:22 -0700 Received: from EX02-WEST.YOJOE.local (10.160.10.131) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server (version=TLS1_0, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA) id 15.1.1531.3 via Frontend Transport; Fri, 19 Oct 2018 12:31:22 -0700 Received: from smtp-transport.yojoe.local (10.129.56.124) by EX02-WEST.YOJOE.local (10.160.10.131) with Microsoft SMTP Server id 14.3.319.2; Fri, 19 Oct 2018 12:31:16 -0700 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 6B6772101E97; Fri, 19 Oct 2018 12:31:14 -0700 (PDT) From: Elijah Newren To: CC: , Elijah Newren Subject: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling Date: Fri, 19 Oct 2018 12:31:11 -0700 Message-ID: <20181019193111.12051-9-newren@gmail.com> X-Mailer: git-send-email 2.19.0.230.g45940724d5 In-Reply-To: <20181019193111.12051-1-newren@gmail.com> References: <20181014020537.17991-1-newren@gmail.com> <20181019193111.12051-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-10-19_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=13 phishscore=0 bulkscore=0 spamscore=0 clxscore=1034 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1810190172 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP When we have a rename/rename(1to2) conflict, each of the renames can collide with a file addition. Each of these rename/add conflicts suffered from the same kinds of problems that normal rename/add suffered from. Make the code use handle_file_conflicts() as well so that we get all the same fixes and consistent behavior between the different conflict types. Signed-off-by: Elijah Newren --- merge-recursive.c | 154 +++++++++++++-------------- t/t6042-merge-rename-corner-cases.sh | 29 +++-- t/t6043-merge-rename-directories.sh | 24 +++-- 3 files changed, 113 insertions(+), 94 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index c78b347112..59811116b6 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1709,80 +1709,17 @@ static int handle_rename_add(struct merge_options *o, ci->dst_entry1->stages[other_stage].mode); } -static int handle_file(struct merge_options *o, - struct diff_filespec *rename, - int stage, - struct rename_conflict_info *ci) -{ - char *dst_name = rename->path; - struct stage_data *dst_entry; - const char *cur_branch, *other_branch; - struct diff_filespec other; - struct diff_filespec *add; - int ret; - - if (stage == 2) { - dst_entry = ci->dst_entry1; - cur_branch = ci->branch1; - other_branch = ci->branch2; - } else { - dst_entry = ci->dst_entry2; - cur_branch = ci->branch2; - other_branch = ci->branch1; - } - - add = filespec_from_entry(&other, dst_entry, stage ^ 1); - if (add) { - int ren_src_was_dirty = was_dirty(o, rename->path); - char *add_name = unique_path(o, rename->path, other_branch); - if (update_file(o, 0, &add->oid, add->mode, add_name)) - return -1; - - if (ren_src_was_dirty) { - output(o, 1, _("Refusing to lose dirty file at %s"), - rename->path); - } - /* - * Because the double negatives somehow keep confusing me... - * 1) update_wd iff !ren_src_was_dirty. - * 2) no_wd iff !update_wd - * 3) so, no_wd == !!ren_src_was_dirty == ren_src_was_dirty - */ - remove_file(o, 0, rename->path, ren_src_was_dirty); - dst_name = unique_path(o, rename->path, cur_branch); - } else { - if (dir_in_way(rename->path, !o->call_depth, 0)) { - dst_name = unique_path(o, rename->path, cur_branch); - output(o, 1, _("%s is a directory in %s adding as %s instead"), - rename->path, other_branch, dst_name); - } else if (!o->call_depth && - would_lose_untracked(rename->path)) { - dst_name = unique_path(o, rename->path, cur_branch); - output(o, 1, _("Refusing to lose untracked file at %s; " - "adding as %s instead"), - rename->path, dst_name); - } - } - if ((ret = update_file(o, 0, &rename->oid, rename->mode, dst_name))) - ; /* fall through, do allow dst_name to be released */ - else if (stage == 2) - ret = update_stages(o, rename->path, NULL, rename, add); - else - ret = update_stages(o, rename->path, NULL, add, rename); - - if (dst_name != rename->path) - free(dst_name); - - return ret; -} - static int handle_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { /* One file was renamed in both branches, but to different names. */ + struct merge_file_info mfi; + struct diff_filespec other; + struct diff_filespec *add; struct diff_filespec *one = ci->pair1->one; struct diff_filespec *a = ci->pair1->two; struct diff_filespec *b = ci->pair2->two; + char *path_desc; output(o, 1, _("CONFLICT (rename/rename): " "Rename \"%s\"->\"%s\" in branch \"%s\" " @@ -1790,15 +1727,16 @@ static int handle_rename_rename_1to2(struct merge_options *o, one->path, a->path, ci->branch1, one->path, b->path, ci->branch2, o->call_depth ? _(" (left unresolved)") : ""); - if (o->call_depth) { - struct merge_file_info mfi; - struct diff_filespec other; - struct diff_filespec *add; - if (merge_mode_and_contents(o, one, a, b, one->path, - ci->branch1, ci->branch2, - o->call_depth * 2, &mfi)) - return -1; + path_desc = xstrfmt("%s and %s, both renamed from %s", + a->path, b->path, one->path); + if (merge_mode_and_contents(o, one, a, b, path_desc, + ci->branch1, ci->branch2, + o->call_depth * 2, &mfi)) + return -1; + free(path_desc); + + if (o->call_depth) { /* * FIXME: For rename/add-source conflicts (if we could detect * such), this is wrong. We should instead find a unique @@ -1830,8 +1768,70 @@ static int handle_rename_rename_1to2(struct merge_options *o, } else remove_file_from_cache(b->path); - } else if (handle_file(o, a, 2, ci) || handle_file(o, b, 3, ci)) - return -1; + } else { + /* + * For each destination path, we need to see if there is a + * rename/add collision. If not, we can write the file out + * to the specified location. + */ + add = filespec_from_entry(&other, ci->dst_entry1, 2 ^ 1); + if (add) { + if (handle_file_collision(o, a->path, + NULL, NULL, + ci->branch1, ci->branch2, + &mfi.oid, mfi.mode, + &add->oid, add->mode) < 0) + return -1; + } else { + char *new_path = NULL; + if (dir_in_way(a->path, !o->call_depth, 0)) { + new_path = unique_path(o, a->path, ci->branch1); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + a->path, ci->branch2, new_path); + } else if (would_lose_untracked(a->path)) { + new_path = unique_path(o, a->path, ci->branch1); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + a->path, new_path); + } + + if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path)) + return -1; + free(new_path); + if (update_stages(o, a->path, NULL, a, NULL)) + return -1; + } + + add = filespec_from_entry(&other, ci->dst_entry2, 3 ^ 1); + if (add) { + if (handle_file_collision(o, b->path, + NULL, NULL, + ci->branch1, ci->branch2, + &add->oid, add->mode, + &mfi.oid, mfi.mode) < 0) + return -1; + } else { + char *new_path = NULL; + if (dir_in_way(b->path, !o->call_depth, 0)) { + new_path = unique_path(o, b->path, ci->branch2); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + b->path, ci->branch1, new_path); + } else if (would_lose_untracked(b->path)) { + new_path = unique_path(o, b->path, ci->branch2); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + b->path, new_path); + } + + if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path)) + return -1; + free(new_path); + if (update_stages(o, b->path, NULL, NULL, b)) + return -1; + } + } return 0; } diff --git a/t/t6042-merge-rename-corner-cases.sh b/t/t6042-merge-rename-corner-cases.sh index 4cfcde3082..534f7b55a1 100755 --- a/t/t6042-merge-rename-corner-cases.sh +++ b/t/t6042-merge-rename-corner-cases.sh @@ -684,7 +684,7 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting git ls-files -u c >out && test_line_count = 2 out && git ls-files -o >out && - test_line_count = 5 out && + test_line_count = 1 out && git rev-parse >expect \ A:a C:b B:b C:c B:c && @@ -692,14 +692,27 @@ test_expect_success 'rename/rename/add-dest merge still knows about conflicting :1:a :2:b :3:b :2:c :3:c && test_cmp expect actual && - git rev-parse >expect \ - C:c B:c C:b B:b && - git hash-object >actual \ - c~HEAD c~B\^0 b~HEAD b~B\^0 && - test_cmp expect actual && + # Record some contents for re-doing merges + git cat-file -p A:a >stuff && + git cat-file -p C:b >important_info && + git cat-file -p B:c >precious_data && + >empty && - test_path_is_missing b && - test_path_is_missing c + # Test the merge in b + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + important_info empty stuff && + test_cmp important_info b && + + # Test the merge in c + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + stuff empty precious_data && + test_cmp stuff c ) ' diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index fedaeafc55..5c01a0c14a 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -1078,7 +1078,7 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam git ls-files -u >out && test_line_count = 6 out && git ls-files -o >out && - test_line_count = 3 out && + test_line_count = 1 out && git rev-parse >actual \ :0:y/b :0:y/c :0:y/e && @@ -1094,9 +1094,9 @@ test_expect_success '5c-check: Transitive rename would cause rename/rename/renam test_cmp expect actual && git hash-object >actual \ - w/d~HEAD w/d~B^0 z/d && + z/d && git rev-parse >expect \ - O:x/d B:w/d O:x/d && + O:x/d && test_cmp expect actual && test_path_is_missing x/d && test_path_is_file y/d && @@ -3672,7 +3672,7 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena git ls-files -u >out && test_line_count = 4 out && git ls-files -o >out && - test_line_count = 4 out && + test_line_count = 3 out && echo different >expected && echo mods >>expected && @@ -3684,11 +3684,17 @@ test_expect_success '11e-check: Avoid deleting not-uptodate with dir rename/rena O:z/a O:z/b O:x/d O:x/c O:x/c A:y/c O:x/c && test_cmp expect actual && - git hash-object >actual \ - y/c~B^0 y/c~HEAD && - git rev-parse >expect \ - O:x/c A:y/c && - test_cmp expect actual + # See if y/c~merged has expected contents; requires manually + # doing the expected file merge + git cat-file -p A:y/c >c1 && + git cat-file -p B:z/c >c2 && + >empty && + test_must_fail git merge-file \ + -L "HEAD" \ + -L "" \ + -L "B^0" \ + c1 empty c2 && + test_cmp c1 y/c~merged ) '