From patchwork Wed Jun 30 17:29:58 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12352607 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3522FC11F69 for ; Wed, 30 Jun 2021 17:30:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 14B3261474 for ; Wed, 30 Jun 2021 17:30:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232712AbhF3Rce (ORCPT ); Wed, 30 Jun 2021 13:32:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39108 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232222AbhF3Rcd (ORCPT ); Wed, 30 Jun 2021 13:32:33 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0AFBDC0617A8 for ; Wed, 30 Jun 2021 10:30:04 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id i94so4634666wri.4 for ; Wed, 30 Jun 2021 10:30:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=SHGMIZnAoter2Y2BJLFA2yhFPLjw2VrycsoaR8FA+Fw=; b=Ngksl4gvaKFxvB3jQOmBipYuFF9Cz5yizVvMCzZMq8saG3Leh+/1+tUX9b9qTqW/fZ Zqmf7iDLC8goXu997FeANwo67/vkyltP618LeJz1W7Xqp+1h+MUT0/RQK3K8NqKiqSR3 j55IC6AZ95wiaYQQ87cVOVX/rdqrprGTZtgijC3rM8zPsekirApwmbAdnasdJQH9EFyz G4Fz84LcAivfD17qkACxbAi/aO2NMsK8wfG7Vj7VozI0VSo+E246swU9kUF37HtKWqEm Q4sv3r562QAM0eZngWY4st1BgNeBdMGO9uoJWy0Yx1a8T6Q80u0EOVnZhQkQXG/9+nnG 8aZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=SHGMIZnAoter2Y2BJLFA2yhFPLjw2VrycsoaR8FA+Fw=; b=XOQ9evNGuev65smragHd2J1AO7v1k5XIsuJFs6Qgv8u2sxgd0whIxeP/hgSWVKCEHI Op91TCXfJVZ2G9ydapSQhfCH0pHJvTDLsBjv0hzf8bq7+dXz7XjGCCmGCjC0VXvcwuRS hfSaPXk1RK6q0yrBMsivUeh4wD/f8SF4U+nHQ332d6k9qkjiVklnGimafzv8Cep/LR14 nWK96iXAHaiLv1xnq0mmHX8qnpN6e+goER3EHABpuepk9U21HhI37XejqfnDGsTTOfs6 /OuHrQwi8fT5KBNCES5u+eIYV0HAlcG2XKErGGRojpKpiKWAV28OH0DpQYN5EklRO4Db ZmmA== X-Gm-Message-State: AOAM5333ng1/nndEuL5H7hRzYhhW7ghHkw6SwF2HJVwUb5QtWzjvCCjs 1TDzbz7XlJf47//HUNwBWYBZYGTL/mY= X-Google-Smtp-Source: ABdhPJwmlIEcOIKbrcwPCImOJN2GsBRZ0AJqmH+BFj3M/HbVLetqQKL7gH2Dq5LEaIIxTJJ6a6j34A== X-Received: by 2002:a05:6000:2c4:: with SMTP id o4mr39558014wry.79.1625074202662; Wed, 30 Jun 2021 10:30:02 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id r4sm23367753wre.84.2021.06.30.10.30.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 10:30:02 -0700 (PDT) Message-Id: <86b2eaea75e5e23626c41bf61bf882591331bbef.1625074200.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 30 Jun 2021 17:29:58 +0000 Subject: [PATCH v2 1/3] t6423: test directory renames causing rename-to-self Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Derrick Stolee , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Directory rename detection can cause transitive renames, e.g. if the two different sides of history each do one half of: A/file -> B/file B/ -> C/ then directory rename detection transitively renames to give us C/file. Since the default for merge.directoryRenames is conflict, this results in an error message saying it is unclear whether the file should be placed at B/file or C/file. What if C/ is A/, though? In such a case, the transitive rename would give us A/file, the original name we started with. Logically, having an error message with B/file vs. A/file should be fine, as should leaving the file where it started. But the logic in both merge-recursive and merge-ort did not handle a case of a filename being renamed to itself correctly; merge-recursive had two bugs, and merge-ort had one. Add some testcases covering such a scenario. Based-on-testcase-by: Anders Kaseorg Signed-off-by: Elijah Newren --- t/t6423-merge-rename-directories.sh | 175 ++++++++++++++++++++++++++++ 1 file changed, 175 insertions(+) diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index be84d22419d..89fe6778b94 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5024,6 +5024,181 @@ test_expect_failure '12h: renaming a file within a renamed directory' ' ) ' +# Testcase 12i, Directory rename causes rename-to-self +# Commit O: source/{subdir/foo, bar, baz_1} +# Commit A: source/{foo, bar, baz_1} +# Commit B: source/{subdir/{foo, bar}, baz_2} +# Expected: source/{foo, bar, baz_2}, with conflicts on +# source/bar vs. source/subdir/bar + +test_setup_12i () { + test_create_repo 12i && + ( + cd 12i && + + mkdir -p source/subdir && + echo foo >source/subdir/foo && + echo bar >source/bar && + echo baz >source/baz && + git add source && + git commit -m orig && + + git branch O && + git branch A && + git branch B && + + git switch A && + git mv source/subdir/foo source/foo && + git commit -m A && + + git switch B && + git mv source/bar source/subdir/bar && + echo more baz >>source/baz && + git commit -m B + ) +} + +test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' ' + test_setup_12i && + ( + cd 12i && + + git checkout A^0 && + + test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 && + + test_path_is_missing source/subdir && + test_path_is_file source/bar && + test_path_is_file source/baz && + + git ls-files | uniq >tracked && + test_line_count = 3 tracked && + + git status --porcelain -uno >actual && + cat >expect <<-\EOF && + UU source/bar + M source/baz + EOF + test_cmp expect actual + ) +' + +# Testcase 12j, Directory rename to root causes rename-to-self +# Commit O: {subdir/foo, bar, baz_1} +# Commit A: {foo, bar, baz_1} +# Commit B: {subdir/{foo, bar}, baz_2} +# Expected: {foo, bar, baz_2}, with conflicts on bar vs. subdir/bar + +test_setup_12j () { + test_create_repo 12j && + ( + cd 12j && + + mkdir -p subdir && + echo foo >subdir/foo && + echo bar >bar && + echo baz >baz && + git add . && + git commit -m orig && + + git branch O && + git branch A && + git branch B && + + git switch A && + git mv subdir/foo foo && + git commit -m A && + + git switch B && + git mv bar subdir/bar && + echo more baz >>baz && + git commit -m B + ) +} + +test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' ' + test_setup_12j && + ( + cd 12j && + + git checkout A^0 && + + test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 && + + test_path_is_missing subdir && + test_path_is_file bar && + test_path_is_file baz && + + git ls-files | uniq >tracked && + test_line_count = 3 tracked && + + git status --porcelain -uno >actual && + cat >expect <<-\EOF && + UU bar + M baz + EOF + test_cmp expect actual + ) +' + +# Testcase 12k, Directory rename with sibling causes rename-to-self +# Commit O: dirB/foo, dirA/{bar, baz_1} +# Commit A: dirA/{foo, bar, baz_1} +# Commit B: dirB/{foo, bar}, dirA/baz_2 +# Expected: dirA/{foo, bar, baz_2}, with conflicts on dirA/bar vs. dirB/bar + +test_setup_12k () { + test_create_repo 12k && + ( + cd 12k && + + mkdir dirA dirB && + echo foo >dirB/foo && + echo bar >dirA/bar && + echo baz >dirA/baz && + git add . && + git commit -m orig && + + git branch O && + git branch A && + git branch B && + + git switch A && + git mv dirB/* dirA/ && + git commit -m A && + + git switch B && + git mv dirA/bar dirB/bar && + echo more baz >>dirA/baz && + git commit -m B + ) +} + +test_expect_merge_algorithm failure failure '12k: Directory rename with sibling causes rename-to-self' ' + test_setup_12k && + ( + cd 12k && + + git checkout A^0 && + + test_must_fail git -c merge.directoryRenames=conflict merge -s recursive B^0 && + + test_path_is_missing dirB && + test_path_is_file dirA/bar && + test_path_is_file dirA/baz && + + git ls-files | uniq >tracked && + test_line_count = 3 tracked && + + git status --porcelain -uno >actual && + cat >expect <<-\EOF && + UU dirA/bar + M dirA/baz + EOF + test_cmp expect actual + ) +' + ########################################################################### # SECTION 13: Checking informational and conflict messages # From patchwork Wed Jun 30 17:29:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12352609 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4B573C11F65 for ; Wed, 30 Jun 2021 17:30:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 237986147D for ; Wed, 30 Jun 2021 17:30:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232745AbhF3Rcg (ORCPT ); Wed, 30 Jun 2021 13:32:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232692AbhF3Rcd (ORCPT ); Wed, 30 Jun 2021 13:32:33 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 94623C061756 for ; Wed, 30 Jun 2021 10:30:04 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id u8so4600437wrq.8 for ; Wed, 30 Jun 2021 10:30:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=ziiVNYbu5Wtne6H/gEqGzugVUt2JJLkNpP7bgcYm64c=; b=FmTyJTLf0BRv6D4cgHQkwJDzYaLl3ZXdkMNiiui10Vmr+q92zGOVeZGVsVZR0bl5DX uP+BvKrwyn5jq7TGKjmDVTrnirbpcP32CpH5AOx2IS3NKEmfzuw+9GIHjEiYZiP64Bhw sZgO+J6MtIDQT15CZPiTDq/LGKtxl3koWrxIokjL0RSpexKJT/QSN2v+vCi4p77rtlqa lfpa5UJUX71i8qBzsfBWYhwQDJ6e8pOTUSfr/nNhiMWZlwPMxohRidmzKhXO+mt+X5aJ iftr2xhH1xWPklXkdrzHaW79p+yG4G42xjadf9SVItnodFifMlsmug4A3hLFjAz5qGgr IEIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=ziiVNYbu5Wtne6H/gEqGzugVUt2JJLkNpP7bgcYm64c=; b=kqgoIe1dPBxjURqZ03p4mi2axRtzMKmbKQ/z7vS63knSMjA52Pr7oHnU5AEl2V9RMo uoCFeb0B+LpuxWGh0OLw8u2XMhkMGJoARnxczbHydX3/mqpd7aOTanSeiSxQAyft095V PZ8fKNPRxEWnj6vqVt3l/uebtoweJBPTSB4MnlX91RYhFKkRBgoyJrWF7zHaP3Cnn0+D ++xgHf2ktsmmt8YprWCeYITw3hB1soIWNFukS8lOqokcpnd2xM8Y7+8B5QxdxnH4OaRZ 7/YAQ+SwIxQv4HMf56xvM7Qoz7n+YuVa5EZ+6xIJAIODqtR/seLMgzEgkWikZa1Z0It6 waiw== X-Gm-Message-State: AOAM531lEbLj3icqOHTYM2yKOkwwZVypaXp5/JmLiqNvwFxVGAXfozJO Wt+DOksdOq6e6+2wVczBNixvU7ccB6k= X-Google-Smtp-Source: ABdhPJx93k7k7YRghXl6RNWl2q5Lb52w5DRlw/XIMs7hwy7I3CNYHXq8GcXYvdOjL88K85y9CZBazQ== X-Received: by 2002:a05:6000:1a41:: with SMTP id t1mr40026061wry.175.1625074203263; Wed, 30 Jun 2021 10:30:03 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id t83sm5393808wmf.36.2021.06.30.10.30.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 10:30:02 -0700 (PDT) Message-Id: <6e7ef18bf53a579e360e4970a6fbcc0a68a821f6.1625074200.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 30 Jun 2021 17:29:59 +0000 Subject: [PATCH v2 2/3] merge-ort: ensure we consult df_conflict and path_conflicts Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Derrick Stolee , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Path conflicts (typically rename path conflicts, e.g. rename/rename(1to2) or rename/add/delete), and directory/file conflicts should obviously result in files not being marked as clean in the merge. We had a codepath where we missed consulting the path_conflict and df_conflict flags, based on match_mask. Granted, it requires an unusual setup to trigger this codepath (directory rename causing rename-to-self is the only case I can think of), but we still need to handle it. To make it clear that we have audited the other codepaths that do not explicitly mention these flags, add some assertions that the flags are not set. Reported-by: Anders Kaseorg Signed-off-by: Elijah Newren --- merge-ort.c | 6 +++++- t/t6423-merge-rename-directories.sh | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/merge-ort.c b/merge-ort.c index b954f7184a5..373dbac5079 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -3237,7 +3237,7 @@ static void process_entry(struct merge_options *opt, * above. */ if (ci->match_mask) { - ci->merged.clean = 1; + ci->merged.clean = !ci->df_conflict && !ci->path_conflict; if (ci->match_mask == 6) { /* stages[1] == stages[2] */ ci->merged.result.mode = ci->stages[1].mode; @@ -3249,6 +3249,8 @@ static void process_entry(struct merge_options *opt, ci->merged.result.mode = ci->stages[side].mode; ci->merged.is_null = !ci->merged.result.mode; + if (ci->merged.is_null) + ci->merged.clean = 1; oidcpy(&ci->merged.result.oid, &ci->stages[side].oid); assert(othermask == 2 || othermask == 4); @@ -3421,6 +3423,7 @@ static void process_entry(struct merge_options *opt, path)) { ci->merged.is_null = 1; ci->merged.clean = 1; + assert(!ci->df_conflict && !ci->path_conflict); } else if (ci->path_conflict && oideq(&ci->stages[0].oid, &ci->stages[side].oid)) { /* @@ -3447,6 +3450,7 @@ static void process_entry(struct merge_options *opt, ci->merged.is_null = 1; ci->merged.result.mode = 0; oidcpy(&ci->merged.result.oid, null_oid()); + assert(!ci->df_conflict); ci->merged.clean = !ci->path_conflict; } diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index 89fe6778b94..c3968dc3642 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5058,7 +5058,7 @@ test_setup_12i () { ) } -test_expect_merge_algorithm failure failure '12i: Directory rename causes rename-to-self' ' +test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' ' test_setup_12i && ( cd 12i && @@ -5116,7 +5116,7 @@ test_setup_12j () { ) } -test_expect_merge_algorithm failure failure '12j: Directory rename to root causes rename-to-self' ' +test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' ' test_setup_12j && ( cd 12j && @@ -5174,7 +5174,7 @@ test_setup_12k () { ) } -test_expect_merge_algorithm failure failure '12k: Directory rename with sibling causes rename-to-self' ' +test_expect_merge_algorithm failure success '12k: Directory rename with sibling causes rename-to-self' ' test_setup_12k && ( cd 12k && From patchwork Wed Jun 30 17:30:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 12352611 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0060C11F65 for ; Wed, 30 Jun 2021 17:30:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C6ADD6146E for ; Wed, 30 Jun 2021 17:30:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232775AbhF3Rci (ORCPT ); Wed, 30 Jun 2021 13:32:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39118 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232544AbhF3Rce (ORCPT ); Wed, 30 Jun 2021 13:32:34 -0400 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73248C061756 for ; Wed, 30 Jun 2021 10:30:05 -0700 (PDT) Received: by mail-wm1-x336.google.com with SMTP id a5-20020a7bc1c50000b02901e3bbe0939bso2219010wmj.0 for ; Wed, 30 Jun 2021 10:30:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:in-reply-to:references:from:date:subject:fcc :content-transfer-encoding:mime-version:to:cc; bh=Kp55Syi8wt9xGzMqLjkASrKSnJTvUnh5ifMJ9TPXpHs=; b=oAMc5k4Otv4lgTOIxZdzmB0MPWWUv3PWpF+4gxrRxtB2IYvlgjKFl1vqrMYLp5m2bd eKwX6XWmDOhYXTm0Uwd/ghMdVKDk389qdi8PNfZEXALSHbHxiA41Oq/s2YPnBo8jqPWw uKv+w5s9PAY26ZZKuDaOLU+L6NY0Uy3/YXN7KJ0e1tuWoK0Y2RaBVpABScUdk/woiIVQ ofNH/nfsZZOWN7DduP0OpN1pmC6kkyln74QDBmrtYTW8/mCCvZH57fDuYy7D6Dz2nao9 ZOFZM92DIp/kSzSRpGHsBdl7cC0seeJ0EZLLF/x8StyIMWaODqEjHE32rfJ+K2w0Wc1y 6j1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:in-reply-to:references:from:date :subject:fcc:content-transfer-encoding:mime-version:to:cc; bh=Kp55Syi8wt9xGzMqLjkASrKSnJTvUnh5ifMJ9TPXpHs=; b=aaatC9O6oW/DlYH5EHPPlNmbeQLVIeVYwe5fEX7Qh5mx/9U2s/lRo11Vcmo/4RniGs hMFSbI6Br//a+E7Jp6EGaxkvhYP3rbCxVacqfZkIahq4j4JFj9ogTxC5LrngHYX16PIg pOtgm+lVh0dhF6qC8rHEQ0LVVycQdHh8SmymmjLSG6+0+AXK9dEFTusmpO/a5t8W4hUM wbgPSi4xg6neNLce5LPJq25o7Q8tcDc1Ac47EquS8zPR9uKIV4a86lS5QxN9NQkiO/6J oYlWr5sDmoRnirrv59BoOHIQD67bpVXZnHpPVdb6huksXqj2YMhnHLtb0+H8fc4wf/++ X0kg== X-Gm-Message-State: AOAM533jeVQhfe8LRMyZeLkhfLdyDV+AQBfD0efEDp7zLivdQ1GhbLcc i57hB33c8aIqlX2sQVaQ6Zn/d8PkDY0= X-Google-Smtp-Source: ABdhPJxqu6hwelJjwDvdFBVufdtv+0q48L1cZ8QzcaiZNthWH2F0q+2g9GBJ24sd1NKbSGa5jYfu4g== X-Received: by 2002:a7b:c097:: with SMTP id r23mr38967043wmh.63.1625074203994; Wed, 30 Jun 2021 10:30:03 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id n7sm6506437wmq.37.2021.06.30.10.30.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 30 Jun 2021 10:30:03 -0700 (PDT) Message-Id: <7c96d6c7a0a4fd25794856e556e071f048771fd1.1625074200.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Wed, 30 Jun 2021 17:30:00 +0000 Subject: [PATCH v2 3/3] merge-recursive: handle rename-to-self case Fcc: Sent MIME-Version: 1.0 To: git@vger.kernel.org Cc: Derrick Stolee , Elijah Newren , Elijah Newren , Elijah Newren Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org From: Elijah Newren From: Elijah Newren Directory rename detection can cause transitive renames, e.g. if the two different sides of history each do one half of: A/file -> B/file B/ -> C/ then directory rename detection transitively renames to give us A/file -> C/file However, when C/ == A/, note that this gives us A/file -> A/file. merge-recursive assumed that any rename D -> E would have D != E. While that is almost always true, the above is a special case where it is not. So we cannot do things like delete the rename source, we cannot assume that a file existing at path E implies a rename/add conflict and we have to be careful about what stages end up in the output. This change feels a bit hackish. It took me surprisingly many hours to find, and given merge-recursive's design causing it to attempt to enumerate all combinations of edge and corner cases with special code for each combination, I'm worried there are other similar fixes needed elsewhere if we can just come up with the right special testcase. Perhaps an audit would rule it out, but I have not the energy. merge-recursive deserves to die, and since it is on its way out anyway, fixing this particular bug narrowly will have to be good enough. Reported-by: Anders Kaseorg Signed-off-by: Elijah Newren --- merge-recursive.c | 19 +++++++++++++------ t/t6423-merge-rename-directories.sh | 6 +++--- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/merge-recursive.c b/merge-recursive.c index d146bb116f7..c895145a8f5 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -2804,12 +2804,19 @@ static int process_renames(struct merge_options *opt, int renamed_stage = a_renames == renames1 ? 2 : 3; int other_stage = a_renames == renames1 ? 3 : 2; + /* + * Directory renames have a funny corner case... + */ + int renamed_to_self = !strcmp(ren1_src, ren1_dst); + /* BUG: We should only remove ren1_src in the base * stage and in other_stage (think of rename + * add-source case). */ - remove_file(opt, 1, ren1_src, - renamed_stage == 2 || !was_tracked(opt, ren1_src)); + if (!renamed_to_self) + remove_file(opt, 1, ren1_src, + renamed_stage == 2 || + !was_tracked(opt, ren1_src)); oidcpy(&src_other.oid, &ren1->src_entry->stages[other_stage].oid); @@ -2823,6 +2830,9 @@ static int process_renames(struct merge_options *opt, ren1->dir_rename_original_type == 'A') { setup_rename_conflict_info(RENAME_VIA_DIR, opt, ren1, NULL); + } else if (renamed_to_self) { + setup_rename_conflict_info(RENAME_NORMAL, + opt, ren1, NULL); } else if (oideq(&src_other.oid, null_oid())) { setup_rename_conflict_info(RENAME_DELETE, opt, ren1, NULL); @@ -3180,7 +3190,6 @@ static int handle_rename_normal(struct merge_options *opt, struct rename *ren = ci->ren1; struct merge_file_info mfi; int clean; - int side = (ren->branch == opt->branch1 ? 2 : 3); /* Merge the content and write it out */ clean = handle_content_merge(&mfi, opt, path, was_dirty(opt, path), @@ -3190,9 +3199,7 @@ static int handle_rename_normal(struct merge_options *opt, opt->detect_directory_renames == MERGE_DIRECTORY_RENAMES_CONFLICT && ren->dir_rename_original_dest) { if (update_stages(opt, path, - NULL, - side == 2 ? &mfi.blob : NULL, - side == 2 ? NULL : &mfi.blob)) + &mfi.blob, &mfi.blob, &mfi.blob)) return -1; clean = 0; /* not clean, but conflicted */ } diff --git a/t/t6423-merge-rename-directories.sh b/t/t6423-merge-rename-directories.sh index c3968dc3642..01d27b66399 100755 --- a/t/t6423-merge-rename-directories.sh +++ b/t/t6423-merge-rename-directories.sh @@ -5058,7 +5058,7 @@ test_setup_12i () { ) } -test_expect_merge_algorithm failure success '12i: Directory rename causes rename-to-self' ' +test_expect_success '12i: Directory rename causes rename-to-self' ' test_setup_12i && ( cd 12i && @@ -5116,7 +5116,7 @@ test_setup_12j () { ) } -test_expect_merge_algorithm failure success '12j: Directory rename to root causes rename-to-self' ' +test_expect_success '12j: Directory rename to root causes rename-to-self' ' test_setup_12j && ( cd 12j && @@ -5174,7 +5174,7 @@ test_setup_12k () { ) } -test_expect_merge_algorithm failure success '12k: Directory rename with sibling causes rename-to-self' ' +test_expect_success '12k: Directory rename with sibling causes rename-to-self' ' test_setup_12k && ( cd 12k &&