From patchwork Thu Nov 8 04:40:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673451 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 6FE8718FD for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5EA732D7EA for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 534F72D7F5; Thu, 8 Nov 2018 04:40:44 +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 BB9142D7EA for ; Thu, 8 Nov 2018 04:40:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728827AbeKHOON (ORCPT ); Thu, 8 Nov 2018 09:14:13 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44912 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726145AbeKHOOM (ORCPT ); Thu, 8 Nov 2018 09:14:12 -0500 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84dbBD024774; Wed, 7 Nov 2018 20:40:35 -0800 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm96drdsf-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:35 -0800 Received: from sj-prod-exch-01.YOJOE.local (10.129.18.26) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 20:40:33 -0800 Received: from smtp-transport.yojoe.local (10.129.56.124) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 20:40:33 -0800 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id B4CDC2101E7C; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 01/10] Add testcases for consistency in file collision conflict handling Date: Wed, 7 Nov 2018 20:40:22 -0800 Message-ID: <20181108044031.25885-2-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 Thu Nov 8 04:40:23 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673465 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 E0A35109C for ; Thu, 8 Nov 2018 04:40:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CEB782D7EF for ; Thu, 8 Nov 2018 04:40:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C0BC02D7F4; Thu, 8 Nov 2018 04:40:55 +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 EE2D32D7EF for ; Thu, 8 Nov 2018 04:40:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728824AbeKHOOM (ORCPT ); Thu, 8 Nov 2018 09:14:12 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44910 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728539AbeKHOOM (ORCPT ); Thu, 8 Nov 2018 09:14:12 -0500 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84d3Up024298; Wed, 7 Nov 2018 20:40:35 -0800 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm96drdsg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:35 -0800 Received: from sj-prod-exch-01.YOJOE.local (10.129.18.26) 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; Wed, 7 Nov 2018 20:40:33 -0800 Received: from smtp-transport.yojoe.local (10.129.56.124) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 20:40:33 -0800 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id BB15B2101E7D; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 02/10] t6036, t6042: testcases for rename collision of already conflicting files Date: Wed, 7 Nov 2018 20:40:23 -0800 Message-ID: <20181108044031.25885-3-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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..f229d7e47b 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..23c3b6dffa 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 Thu Nov 8 04:40:24 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673469 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 C73D013AD for ; Thu, 8 Nov 2018 04:40:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B528A2D7EA for ; Thu, 8 Nov 2018 04:40:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A95642D7F3; Thu, 8 Nov 2018 04:40:57 +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 DC0692D7EA for ; Thu, 8 Nov 2018 04:40:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728903AbeKHOO2 (ORCPT ); Thu, 8 Nov 2018 09:14:28 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44916 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728752AbeKHOOM (ORCPT ); Thu, 8 Nov 2018 09:14:12 -0500 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84dbBE024774; Wed, 7 Nov 2018 20:40:36 -0800 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm96drdsf-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:36 -0800 Received: from sj-prod-exch-01.YOJOE.local (10.129.18.26) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 20:40:33 -0800 Received: from smtp-transport.yojoe.local (10.129.56.124) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 20:40:33 -0800 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id C2AC42101E7E; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 03/10] merge-recursive: increase marker length with depth of recursion Date: Wed, 7 Nov 2018 20:40:24 -0800 Message-ID: <20181108044031.25885-4-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 3c8fb917e9..5b8d46aede 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 6c6e22e40d..b9e2af1c88 100644 --- a/ll-merge.h +++ b/ll-merge.h @@ -13,6 +13,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 acc2f64a4e..f35e3b5f95 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) { @@ -1301,6 +1303,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) { @@ -1311,7 +1314,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; @@ -1352,7 +1356,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")); @@ -1641,7 +1646,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; /* @@ -1708,9 +1714,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); @@ -2756,7 +2764,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; } @@ -3053,7 +3061,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 f229d7e47b..3091cbc06a 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 Thu Nov 8 04:40:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673461 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 3020D109C for ; Thu, 8 Nov 2018 04:40:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1D9392D7EA for ; Thu, 8 Nov 2018 04:40:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 11EEF2D7EF; Thu, 8 Nov 2018 04:40:46 +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 2127C2D7F2 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728843AbeKHOOO (ORCPT ); Thu, 8 Nov 2018 09:14:14 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47292 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728801AbeKHOOO (ORCPT ); Thu, 8 Nov 2018 09:14:14 -0500 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84bwu6026337; Wed, 7 Nov 2018 20:40:37 -0800 Received: from mail.palantir.com ([198.97.14.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm9610eb8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:36 -0800 Received: from dc-prod-exch-01.YOJOE.local (10.193.18.14) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 23:40:34 -0500 Received: from smtp-transport.yojoe.local (10.129.56.124) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 23:40:34 -0500 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id D36A32101E7F; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 04/10] merge-recursive: new function for better colliding conflict resolutions Date: Wed, 7 Nov 2018 20:40:25 -0800 Message-ID: <20181108044031.25885-5-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 f35e3b5f95..16ba425c2d 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1560,6 +1560,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 Thu Nov 8 04:40:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673455 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 D958915E9 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C0C502D7EA for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B57E12D7F3; Thu, 8 Nov 2018 04:40:44 +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 EA8ED2D7EF for ; Thu, 8 Nov 2018 04:40:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728833AbeKHOON (ORCPT ); Thu, 8 Nov 2018 09:14:13 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44918 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728761AbeKHOON (ORCPT ); Thu, 8 Nov 2018 09:14:13 -0500 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84eI9x025054; Wed, 7 Nov 2018 20:40:36 -0800 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm96drdsh-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:36 -0800 Received: from sj-prod-exch-01.YOJOE.local (10.129.18.26) 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; Wed, 7 Nov 2018 20:40:34 -0800 Received: from smtp-transport.yojoe.local (10.129.56.124) by sj-prod-exch-01.YOJOE.local (10.129.18.26) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 20:40:34 -0800 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id DA5692101E93; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 05/10] merge-recursive: fix rename/add conflict handling Date: Wed, 7 Nov 2018 20:40:26 -0800 Message-ID: <20181108044031.25885-6-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 16ba425c2d..c0aa93e3df 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); @@ -1560,7 +1566,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, @@ -1576,6 +1581,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 */ @@ -1679,7 +1698,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, @@ -2861,47 +2911,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; @@ -3313,6 +3339,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 3091cbc06a..d6b7e568f5 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 23c3b6dffa..3e934ae89e 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 Thu Nov 8 04:40:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673463 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 892A2109C for ; Thu, 8 Nov 2018 04:40:47 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7769B2D7EA for ; Thu, 8 Nov 2018 04:40:47 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6B7032D7F2; Thu, 8 Nov 2018 04:40:47 +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 59C042D7EA for ; Thu, 8 Nov 2018 04:40:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728862AbeKHOOR (ORCPT ); Thu, 8 Nov 2018 09:14:17 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47306 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728761AbeKHOOP (ORCPT ); Thu, 8 Nov 2018 09:14:15 -0500 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84bwuA026337; Wed, 7 Nov 2018 20:40:39 -0800 Received: from mail.palantir.com ([198.97.14.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm9610eb8-5 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:38 -0800 Received: from dc-prod-exch-01.YOJOE.local (10.193.18.14) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 23:40:34 -0500 Received: from smtp-transport.yojoe.local (10.129.56.124) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 23:40:34 -0500 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id EBBD02101E94; Wed, 7 Nov 2018 20:40:33 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 06/10] merge-recursive: improve handling for rename/rename(2to1) conflicts Date: Wed, 7 Nov 2018 20:40:27 -0800 Message-ID: <20181108044031.25885-7-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 c0aa93e3df..62eab9e4cc 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, @@ -1871,7 +1850,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. " @@ -1879,81 +1857,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); } /* @@ -3362,9 +3281,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 d6b7e568f5..99cddb05af 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 3e934ae89e..b93139f628 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 Thu Nov 8 04:40:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673453 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 B4EAA1923 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A2EFC2D7F4 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 976C42D7F3; Thu, 8 Nov 2018 04:40:44 +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 414732D7F4 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728858AbeKHOOP (ORCPT ); Thu, 8 Nov 2018 09:14:15 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47298 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726145AbeKHOOO (ORCPT ); Thu, 8 Nov 2018 09:14:14 -0500 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84bwu7026337; Wed, 7 Nov 2018 20:40:37 -0800 Received: from mail.palantir.com ([198.97.14.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm9610eb8-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:37 -0800 Received: from dc-prod-exch-01.YOJOE.local (10.193.18.14) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 23:40:34 -0500 Received: from smtp-transport.yojoe.local (10.129.56.124) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 23:40:34 -0500 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 0A2DF2101E96; Wed, 7 Nov 2018 20:40:34 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 07/10] merge-recursive: use handle_file_collision for add/add conflicts Date: Wed, 7 Nov 2018 20:40:28 -0800 Message-ID: <20181108044031.25885-8-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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=820 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1811080038 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 62eab9e4cc..88e9e1166a 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -3356,14 +3356,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 Thu Nov 8 04:40:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673467 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 7567A15E9 for ; Thu, 8 Nov 2018 04:40:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 622932D7EA for ; Thu, 8 Nov 2018 04:40:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 568442D7F3; Thu, 8 Nov 2018 04:40:56 +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 906CC2D7EA for ; Thu, 8 Nov 2018 04:40:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728906AbeKHOO2 (ORCPT ); Thu, 8 Nov 2018 09:14:28 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:44920 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728499AbeKHOON (ORCPT ); Thu, 8 Nov 2018 09:14:13 -0500 Received: from pps.filterd (m0131697.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84eIA0025054; Wed, 7 Nov 2018 20:40:36 -0800 Received: from mail.palantir.com ([8.4.231.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm96drdsh-2 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:36 -0800 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; Wed, 7 Nov 2018 20:40:34 -0800 Received: from smtp-transport.yojoe.local (10.129.56.124) by sj-prod-exch-02.YOJOE.local (10.129.18.29) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 20:40:34 -0800 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 186F22101E97; Wed, 7 Nov 2018 20:40:34 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 08/10] merge-recursive: improve rename/rename(1to2)/add[/add] handling Date: Wed, 7 Nov 2018 20:40:29 -0800 Message-ID: <20181108044031.25885-9-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 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 88e9e1166a..3e2a63d094 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1710,80 +1710,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\" " @@ -1791,15 +1728,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 @@ -1831,8 +1769,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 b93139f628..7cc34e7579 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 ) ' From patchwork Thu Nov 8 04:40:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673457 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 059B31932 for ; Thu, 8 Nov 2018 04:40:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E83292D7EF for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DCC042D7EA; Thu, 8 Nov 2018 04:40:44 +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 619872D7F6 for ; Thu, 8 Nov 2018 04:40:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728856AbeKHOOP (ORCPT ); Thu, 8 Nov 2018 09:14:15 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47302 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728829AbeKHOOO (ORCPT ); Thu, 8 Nov 2018 09:14:14 -0500 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84bwu8026337; Wed, 7 Nov 2018 20:40:38 -0800 Received: from mail.palantir.com ([198.97.14.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm9610eb8-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:37 -0800 Received: from dc-prod-exch-01.YOJOE.local (10.193.18.14) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 23:40:35 -0500 Received: from smtp-transport.yojoe.local (10.129.56.124) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 23:40:34 -0500 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 2A30B2101E98; Wed, 7 Nov 2018 20:40:34 -0800 (PST) From: Elijah Newren To: CC: , , Elijah Newren Subject: [PATCH v5 09/10] t6036, t6043: increase code coverage for file collision handling Date: Wed, 7 Nov 2018 20:40:30 -0800 Message-ID: <20181108044031.25885-10-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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-1811080038 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Stolee's coverage reports found a few code blocks for file collision conflicts that had not previously been covered by testcases; add a few more testcases to cover those too. Signed-off-by: Elijah Newren --- t/t6036-recursive-corner-cases.sh | 51 +++++++++++++++++++++++++++++ t/t6043-merge-rename-directories.sh | 37 +++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh index 99cddb05af..b7488b00c0 100755 --- a/t/t6036-recursive-corner-cases.sh +++ b/t/t6036-recursive-corner-cases.sh @@ -240,6 +240,57 @@ test_expect_success 'git detects differently handled merges conflict' ' ) ' +# Repeat the above testcase with precisely the same setup, other than with +# the two merge bases having different orderings of commit timestamps so +# that they are reversed in the order they are provided to merge-recursive, +# so that we can improve code coverage. +test_expect_success 'git detects differently handled merges conflict, swapped' ' + ( + cd rename-add && + + # Difference #1: Do cleanup from previous testrun + git reset --hard && + git clean -fdqx && + + # Difference #2: Change commit timestamps + btime=$(git log --no-walk --date=raw --format=%cd B | awk "{print \$1}") && + ctime=$(git log --no-walk --date=raw --format=%cd C | awk "{print \$1}") && + newctime=$(($btime+1)) && + git fast-export --no-data --all | sed -e s/$ctime/$newctime/ | git fast-import --force --quiet && + # End of differences; rest is copy-paste of last test + + git checkout D^0 && + test_must_fail git merge -s recursive E^0 && + + git ls-files -s >out && + test_line_count = 3 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 1 out && + + git rev-parse >expect \ + C:new_a D:new_a E:new_a && + git rev-parse >actual \ + :1:new_a :2:new_a :3:new_a && + test_cmp expect actual && + + # 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 "HEAD" \ + -L "" \ + -L "E^0" \ + ours empty theirs && + sed -e "s/^\([<=>]\)/\1\1\1/" ours >expect && + git hash-object new_a >actual && + git hash-object ours >expect && + test_cmp expect actual + ) +' + # # criss-cross + modify/delete: # diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh index 5c01a0c14a..62c564707b 100755 --- a/t/t6043-merge-rename-directories.sh +++ b/t/t6043-merge-rename-directories.sh @@ -3163,6 +3163,43 @@ test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2) ) ' +test_expect_success '10c-check: Overwrite untracked with dir rename/rename(1to2), other direction' ' + ( + cd 10c && + + git reset --hard && + git clean -fdqx && + + git checkout B^0 && + mkdir y && + echo important >y/c && + + test_must_fail git merge -s recursive A^0 >out 2>err && + test_i18ngrep "CONFLICT (rename/rename)" out && + test_i18ngrep "Refusing to lose untracked file at y/c; adding as y/c~HEAD instead" out && + + git ls-files -s >out && + test_line_count = 6 out && + git ls-files -u >out && + test_line_count = 3 out && + git ls-files -o >out && + test_line_count = 3 out && + + git rev-parse >actual \ + :0:y/a :0:y/b :0:x/d :1:x/c :3:w/c :2:y/c && + git rev-parse >expect \ + O:z/a O:z/b O:x/d O:x/c O:x/c O:x/c && + test_cmp expect actual && + + git hash-object y/c~HEAD >actual && + git rev-parse O:x/c >expect && + test_cmp expect actual && + + echo important >expect && + test_cmp expect y/c + ) +' + # Testcase 10d, Delete untracked w/ dir rename/rename(2to1) # 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 From patchwork Thu Nov 8 04:40:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 10673459 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 E028117D4 for ; Thu, 8 Nov 2018 04:40:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CC97D2D7EA for ; Thu, 8 Nov 2018 04:40:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C0E122D7F3; Thu, 8 Nov 2018 04:40:45 +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 5AFFF2D7EA for ; Thu, 8 Nov 2018 04:40:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728870AbeKHOOR (ORCPT ); Thu, 8 Nov 2018 09:14:17 -0500 Received: from mx0a-00153501.pphosted.com ([67.231.148.48]:47310 "EHLO mx0a-00153501.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726145AbeKHOOR (ORCPT ); Thu, 8 Nov 2018 09:14:17 -0500 Received: from pps.filterd (m0096528.ppops.net [127.0.0.1]) by mx0a-00153501.pphosted.com (8.16.0.27/8.16.0.27) with SMTP id wA84bwu9026337; Wed, 7 Nov 2018 20:40:38 -0800 Received: from mail.palantir.com ([198.97.14.70]) by mx0a-00153501.pphosted.com with ESMTP id 2nm9610eb8-4 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Wed, 07 Nov 2018 20:40:38 -0800 Received: from dc-prod-exch-01.YOJOE.local (10.193.18.14) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1531.3; Wed, 7 Nov 2018 23:40:35 -0500 Received: from smtp-transport.yojoe.local (10.129.56.124) by dc-prod-exch-01.YOJOE.local (10.193.18.14) with Microsoft SMTP Server id 15.1.1531.3 via Frontend Transport; Wed, 7 Nov 2018 23:40:34 -0500 Received: from newren2-linux.yojoe.local (newren2-linux.pa.palantir.tech [10.100.71.66]) by smtp-transport.yojoe.local (Postfix) with ESMTPS id 394DF2101E99; Wed, 7 Nov 2018 20:40:34 -0800 (PST) From: Elijah Newren To: CC: , , Derrick Stolee , Elijah Newren Subject: [PATCH v5 10/10] merge-recursive: combine error handling Date: Wed, 7 Nov 2018 20:40:31 -0800 Message-ID: <20181108044031.25885-11-newren@gmail.com> X-Mailer: git-send-email 2.19.1.858.g526e8fe740.dirty In-Reply-To: <20181108044031.25885-1-newren@gmail.com> References: <20181108044031.25885-1-newren@gmail.com> MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-11-08_02:,, 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=819 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1811080038 Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Derrick Stolee In handle_rename_rename_1to2(), we have duplicated error handling around colliding paths. Specifically, when we want to write out the file and there is a directory or untracked file in the way, we need to create a temporary file to hold the contents. This has some special output to alert the user, and this output is duplicated for each side of the conflict. Simplify the call by generating this new path in a helper function. Signed-off-by: Derrick Stolee Signed-off-by: Elijah Newren --- merge-recursive.c | 53 ++++++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index 3e2a63d094..ecf8db0b71 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1710,6 +1710,27 @@ static int handle_rename_add(struct merge_options *o, ci->dst_entry1->stages[other_stage].mode); } +static char *find_path_for_conflict(struct merge_options *o, + const char *path, + const char *branch1, + const char *branch2) +{ + char *new_path = NULL; + if (dir_in_way(path, !o->call_depth, 0)) { + new_path = unique_path(o, path, branch1); + output(o, 1, _("%s is a directory in %s adding " + "as %s instead"), + path, branch2, new_path); + } else if (would_lose_untracked(path)) { + new_path = unique_path(o, path, branch1); + output(o, 1, _("Refusing to lose untracked file" + " at %s; adding as %s instead"), + path, new_path); + } + + return new_path; +} + static int handle_rename_rename_1to2(struct merge_options *o, struct rename_conflict_info *ci) { @@ -1784,19 +1805,9 @@ static int handle_rename_rename_1to2(struct merge_options *o, &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); - } - + char *new_path = find_path_for_conflict(o, a->path, + ci->branch1, + ci->branch2); if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : a->path)) return -1; free(new_path); @@ -1813,19 +1824,9 @@ static int handle_rename_rename_1to2(struct merge_options *o, &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); - } - + char *new_path = find_path_for_conflict(o, b->path, + ci->branch2, + ci->branch1); if (update_file(o, 0, &mfi.oid, mfi.mode, new_path ? new_path : b->path)) return -1; free(new_path);